Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor #18

Merged
merged 17 commits into from
Sep 30, 2024
Merged

Refactor #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
3868de1
remove sample_test.ts, rename aider.ts to aider_test.ts
tsukimizake Sep 27, 2024
5b2c69f
move addCurrentFile and addIgnoreCurrentFile to main.ts
tsukimizake Sep 27, 2024
e7812b4
move v.r.set to buffer.ts
tsukimizake Sep 29, 2024
89e2815
refactor identifyAiderBuffer
tsukimizake Sep 30, 2024
e7d94eb
sendPrompt function added to aiderCommand.ts
tsukimizake Sep 30, 2024
edfbeeb
chore: fix denops_std version of emit
tsukimizake Sep 30, 2024
818c3d3
remove TODO commeent
tsukimizake Sep 30, 2024
41e3fb4
cleanup import
tsukimizake Sep 30, 2024
a82911d
update doc
tsukimizake Sep 30, 2024
946cf66
fix sendPrompt
tsukimizake Sep 30, 2024
73df8ef
pass prompt
tsukimizake Sep 30, 2024
20ab793
remove console.log
tsukimizake Sep 30, 2024
67d2d5d
iniline openIgnore
tsukimizake Sep 30, 2024
89c0c86
chore: reorder methods in aiderCommand.ts
tsukimizake Sep 30, 2024
d40478c
remove uncessary comment
tsukimizake Sep 30, 2024
f258b59
move aiderexit
tsukimizake Sep 30, 2024
9e37445
chore: less diff
tsukimizake Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 16 additions & 45 deletions denops/aider/aiderCommand.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,13 @@
import { Denops } from "https://deno.land/x/denops_std@v5.0.1/mod.ts";
import { Denops } from "https://deno.land/x/denops_std@v6.4.0/mod.ts";
Copy link
Contributor Author

@tsukimizake tsukimizake Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

( ；´。 ｀；)なんかバージョンが違ってた

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

（ ；´ワ ｀；）ﾍｰｷﾍｰｷ

import * as v from "https://deno.land/x/denops_std@v6.4.0/variable/mod.ts";
import { ensure, is } from "https://deno.land/x/unknownutil@v3.17.0/mod.ts";
import * as fn from "https://deno.land/x/denops_std@v6.4.0/function/mod.ts";
import { getAiderBufferNr, getCurrentFilePath } from "./utils.ts";
import { buffer } from "./buffer.ts";

export const aiderCommand = {
async debug(denops: Denops): Promise<void> {
await denops.cmd("b#");
},

/**
* .aiderignoreファイルを開きます。
* ファイルが存在する場合は編集モードで開き、存在しない場合はエラーメッセージを表示します。
* @param {Denops} denops - Denopsインスタンス
* @returns {Promise<void>}
*/
async openIgnore(denops: Denops): Promise<void> {
const gitRoot = (await fn.system(denops, "git rev-parse --show-toplevel"))
.trim();
const filePathToOpen = `${gitRoot}/.aiderignore`;
if (await fn.filereadable(denops, filePathToOpen)) {
await denops.cmd(`edit ${filePathToOpen}`);
return;
}
console.log(".aiderignoreファイルが見つかりません。");
},

async run(denops: Denops): Promise<void> {
const aiderCommand = ensure(
await v.g.get(denops, "aider_command"),
Expand Down Expand Up @@ -55,35 +36,25 @@ export const aiderCommand = {
await denops.cmd("echo 'Aider is running in the background.'");
},
/**
* 現在のファイルをAiderに追加します。
* @param {Denops} denops - Denopsインスタンス
* Aiderバッファにメッセージを送信します。
* @param {Denops} denops - Denops instance
* @param {number} jobId - The job id to send the message to
* @param {string} prompt - The prompt to send
* @returns {Promise<void>}
*/
async addCurrentFile(denops: Denops): Promise<void> {
const bufnr = await fn.bufnr(denops, "%");
if (await getAiderBufferNr(denops) === undefined) {
await aiderCommand.silentRun(denops);
}
if (await buffer.checkIfTerminalBuffer(denops, bufnr)) {
return;
}
const currentFile = await getCurrentFilePath(denops);
const prompt = `/add ${currentFile}`;
async sendPrompt(
denops: Denops,
jobId: number,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

テストの実装方針によってはaiderBufferのjobIdを取るところまでaiderCommandモジュールの責務にした方がいいかもしれないがどっちが正しいかは微妙なところ

prompt: string,
): Promise<void> {
await v.r.set(denops, "q", prompt);
await buffer.sendPromptWithInput(denops);
await fn.feedkeys(denops, "G");
await fn.feedkeys(denops, '"qp');
await denops.call("chansend", jobId, "\n");
tsukimizake marked this conversation as resolved.
Show resolved Hide resolved
},
async addIgnoreCurrentFile(denops: Denops): Promise<void> {
const currentFile = await getCurrentFilePath(denops);

const gitRoot = (await fn.system(denops, "git rev-parse --show-toplevel"))
.trim();
const filePathToOpen = `${gitRoot}/.aiderignore`;
const forAiderIgnorePath = currentFile.replace(gitRoot, "");

const file = await fn.readfile(denops, filePathToOpen);
file.push(`!${forAiderIgnorePath}`);

await fn.writefile(denops, file, filePathToOpen);
console.log(`Added ${currentFile} to .aiderignore`);
async exit(denops: Denops, jobId: number, bufnr: number): Promise<void> {
await denops.call("chansend", jobId, "/exit\n");
await denops.cmd(`bdelete! ${bufnr}`);
},
};
113 changes: 57 additions & 56 deletions denops/aider/buffer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Denops } from "https://deno.land/x/denops_std@v6.4.0/mod.ts";
import * as v from "https://deno.land/x/denops_std@v6.4.0/variable/mod.ts";
import { emit } from "https://deno.land/x/denops_std@v6.5.1/autocmd/mod.ts";
import { emit } from "https://deno.land/x/denops_std@v6.4.0/autocmd/mod.ts";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

( ；´。 ｀；)なんかバージョンが違ってた2

import * as n from "https://deno.land/x/denops_std@v6.4.0/function/nvim/mod.ts";
import * as fn from "https://deno.land/x/denops_std@v6.4.0/function/mod.ts";
import {
Expand All @@ -14,6 +14,7 @@ import {
getBufferName,
} from "./utils.ts";
import { feedkeys } from "https://deno.land/x/denops_std@v6.4.0/function/mod.ts";
import { aiderCommand } from "./aiderCommand.ts";

/**
* Enum representing different buffer layout options.
Expand All @@ -35,10 +36,12 @@ export const buffer = {
) ?? "floating";
},
async exitAiderBuffer(denops: Denops): Promise<void> {
await identifyAiderBuffer(denops, async (job_id, _winnr, bufnr) => {
await denops.call("chansend", job_id, "/exit\n");
await denops.cmd(`bdelete! ${bufnr}`);
});
const buffer = await identifyAiderBuffer(denops);
if (buffer === undefined) {
return;
}
const { job_id, bufnr } = buffer;
aiderCommand.exit(denops, job_id, bufnr);
Comment on lines +39 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add user feedback when no Aider buffer is found

In the exitAiderBuffer function, if identifyAiderBuffer returns undefined, the function silently returns without notifying the user. Consider informing the user that no Aider buffer is currently open to improve usability and debugging.

Apply this diff to provide user feedback:

if (buffer === undefined) {
+  await denops.cmd("echo 'No Aider buffer is currently open.'");
   return;
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const buffer = await identifyAiderBuffer(denops);
if (buffer === undefined) {
return;
}
const { job_id, bufnr } = buffer;
aiderCommand.exit(denops, job_id, bufnr);
const buffer = await identifyAiderBuffer(denops);
if (buffer === undefined) {
await denops.cmd("echo 'No Aider buffer is currently open.'");
return;
}
const { job_id, bufnr } = buffer;
aiderCommand.exit(denops, job_id, bufnr);

},

/**
Expand Down Expand Up @@ -76,16 +79,13 @@ export const buffer = {
is.Number,
);

await openFloatingWindow(
denops,
bufnr,
);
await openFloatingWindow(denops, bufnr);

await emit(denops, "User", "AiderOpen");
return;
},

async sendPromptWithInput(denops: Denops): Promise<void> {
async sendPromptWithInput(denops: Denops, input: string): Promise<void> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

テストしやすさを考えると、確かにinputを引数にするほうがよさそう（当時はテストを1ミリも考えていなかった顔

const bufnr = await getAiderBufferNr(denops);
if (bufnr === undefined) {
await denops.cmd("echo 'Aider is not running'");
Expand All @@ -97,23 +97,28 @@ export const buffer = {

if (openBufferType === "floating") {
await buffer.openAiderBuffer(denops, openBufferType);
await sendPromptFromFloatingWindow(denops);
await sendPromptFromFloatingWindow(denops, input);
return;
}

await sendPromptFromSplitWindow(denops);
await sendPromptFromSplitWindow(denops, input);
},
async sendPrompt(
async sendPromptByBuffer(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

実装内容に合わせて微妙にリネーム

denops: Denops,
openBufferType: BufferLayout,
): Promise<void> {
// テキストを取得してプロンプト入力ウインドウを閉じる
await feedkeys(denops, 'ggVG"qy');
const bufferContent = ensure(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

普通にこれでいいですね…（当時は覚えたてのfeedkeysを使いまくっていた記憶

Copy link
Contributor Author

@tsukimizake tsukimizake Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://zenn.dev/uga_rosa/articles/200ad8013db7a8 feedkeysは非同期処理らしいので現状の使用箇所は処理落ち時とかにバグってる可能性ありそう ( ；´。 ｀；)

await denops.call("getbufline", "%", 1, "$"),
is.ArrayOf(is.String),
).join("\n");

await denops.cmd("bdelete!");

openBufferType === "floating"
? await sendPromptFromFloatingWindow(denops)
: await sendPromptFromSplitWindow(denops);
if (openBufferType === "floating") {
await sendPromptFromFloatingWindow(denops, bufferContent);
} else {
await sendPromptFromSplitWindow(denops, bufferContent);
}
Comment on lines +106 to +121
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Outdated References to sendPrompt Found

The following references to sendPrompt still exist and need to be updated to sendPromptByBuffer:

  • denops/aider/aiderCommand.ts: async sendPrompt(
  • denops/aider/buffer.ts:
    • await aiderCommand.sendPrompt(denops, jobId, prompt);
    • await aiderCommand.sendPrompt(denops, job_id, prompt);
🔗 Analysis chain

Ensure all references to sendPrompt are updated to sendPromptByBuffer

The method sendPrompt has been renamed to sendPromptByBuffer. Please verify that all references to the old method name have been updated to prevent undefined method errors.

Use this script to find any outdated references:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find any references to the old method name `sendPrompt`

# Expected Result: No occurrences of `sendPrompt` except in `sendPromptByBuffer`.

rg --type ts 'sendPrompt\('

Length of output: 237


await emit(denops, "User", "AiderOpen");
return;
Expand Down Expand Up @@ -215,19 +220,13 @@ export const buffer = {
};

/**
* 開いているウィンドウの中からターミナルバッファを識別し、そのジョブID、ウィンドウ番号、バッファ番号をコールバック関数に渡します。
* 開いているウィンドウの中からAiderバッファを識別し、そのジョブID、ウィンドウ番号、バッファ番号を返します。
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

当時は若く、覚えたてのコールバックを以下略

Copy link
Contributor Author

@tsukimizake tsukimizake Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コールバック、async/awaitとかの非同期処理をいい感じに書く機能がない言語で非同期処理やる場合に仕方なく使うものだとおもてる
あとはサーバーとかを挟んでいてその手の機能がうまく使えない場合

*
* @param {function} callback - ジョブID、ウィンドウ番号、バッファ番号を引数に取るコールバック関数
* @returns {Promise<void>}
* @returns {Promise<{ job_id: number, winnr: number, bufnr: number }>}
*/
async function identifyAiderBuffer(
denops: Denops,
callback: (
job_id: number | undefined,
winnr?: number,
bufnr?: number,
) => Promise<void>,
): Promise<void> {
): Promise<{ job_id: number; winnr: number; bufnr: number } | undefined> {
const win_count = ensure(await fn.winnr(denops, "$"), is.Number);
for (let i = 1; i <= win_count; i++) {
const bufnr = ensure(await fn.winbufnr(denops, i), is.Number);
Expand All @@ -238,7 +237,7 @@ async function identifyAiderBuffer(
is.Number,
);
if (job_id !== 0) {
await callback(job_id, i, bufnr);
return ({ job_id, winnr: i, bufnr });
}
}
}
Expand Down Expand Up @@ -285,22 +284,21 @@ async function openFloatingWindow(

await denops.cmd("set nonumber");
}
async function sendPromptFromFloatingWindow(denops: Denops): Promise<void> {
async function sendPromptFromFloatingWindow(
denops: Denops,
prompt: string,
): Promise<void> {
const bufnr = await getAiderBufferNr(denops);
if (bufnr === undefined) {
return;
}
await openFloatingWindow(denops, bufnr);

await feedkeys(denops, "G");
await feedkeys(denops, '"qp');

const jobId = ensure(
await fn.getbufvar(denops, bufnr, "&channel"),
is.Number,
);

await denops.call("chansend", jobId, "\n");
await aiderCommand.sendPrompt(denops, jobId, prompt);
}
/**
* スプリットウィンドウからプロンプトを送信する非同期関数
Expand All @@ -316,31 +314,34 @@ async function sendPromptFromFloatingWindow(denops: Denops): Promise<void> {
*
* @param {Denops} denops - Denopsインスタンス
*/
async function sendPromptFromSplitWindow(denops: Denops): Promise<void> {
await identifyAiderBuffer(denops, async (job_id, winnr, _bufnr) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

このへんから見た目複雑に差分が出てしまっているけれどidentifyAiderBufferとsendPromptのインターフェースが変わった部分以外は変えてないはず

// await denops.cmd(`bdelete!`);
if (await v.g.get(denops, "aider_buffer_open_type") !== "floating") {
await denops.cmd(`${winnr}wincmd w`);
} else {
const totalWindows = ensure<number>(
await denops.call("winnr", "$"),
is.Number,
);
async function sendPromptFromSplitWindow(
denops: Denops,
prompt: string,
): Promise<void> {
const aiderBuffer = await identifyAiderBuffer(denops);
if (aiderBuffer === undefined) {
return;
}
const { job_id, winnr } = aiderBuffer;

for (let winnr = 1; winnr <= totalWindows; winnr++) {
const bufnr = await denops.call("winbufnr", winnr);
if (await v.g.get(denops, "aider_buffer_open_type") !== "floating") {
await denops.cmd(`${winnr}wincmd w`);
} else {
const totalWindows = ensure<number>(
await denops.call("winnr", "$"),
is.Number,
);

const buftype = await denops.call("getbufvar", bufnr, "&buftype");
for (let winnr = 1; winnr <= totalWindows; winnr++) {
const bufnr = await denops.call("winbufnr", winnr);

if (buftype === "terminal") {
await denops.cmd(`${winnr}wincmd w`);
break;
}
const buftype = await denops.call("getbufvar", bufnr, "&buftype");

if (buftype === "terminal") {
await denops.cmd(`${winnr}wincmd w`);
break;
}
}
await feedkeys(denops, "G");
await feedkeys(denops, '"qp');
await denops.call("chansend", job_id, "\n");
await denops.cmd("wincmd p");
});
}
await aiderCommand.sendPrompt(denops, job_id, prompt);
Comment on lines +317 to +346
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Review logic for handling buffer types in sendPromptFromSplitWindow

In the sendPromptFromSplitWindow function, when aider_buffer_open_type is "floating", the code iterates through all windows to find a terminal buffer. This may be counterintuitive since a floating buffer type typically wouldn't require switching windows. Consider reviewing the condition to ensure the logic aligns with the intended behavior for different buffer types.

}
Loading