feat: 一時ファイルに選択したテキストを保存 #38
Conversation
WalkthroughThe pull request introduces new functionality to the Aider plugin for saving selected text from a buffer to a temporary file. A new function Changes
Sequence DiagramsequenceDiagram
participant User
participant Vim
participant Plugin
participant TempFile
User->>Vim: Select line range
User->>Vim: Trigger addPartialReadonlyContext
Vim->>Plugin: Call with start/end lines
Plugin->>Plugin: Get current file path
Plugin->>TempFile: Create temp file with context
Plugin->>Vim: Return temp file path
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
denops/aider/bufferOperation.ts(2 hunks)denops/aider/main.ts(1 hunks)
🔇 Additional comments (2)
denops/aider/main.ts (1)
247-257: Ensure argument validation and consider user inputs carefully.
The new addPartialReadonlyContext command accepts two string parameters, start and end, which control line ranges in a buffer. Since these values are derived from user inputs or Vim's range, please confirm they are properly validated or sanitized to prevent potential off-by-one errors or malicious injection attempts. In addition, handle potential edge cases (like negative or zero-length ranges) to ensure robust behavior in all scenarios.
Would you like a script to scan the codebase for all usage sites or references to addPartialReadonlyContext to confirm no unhandled edge cases?
denops/aider/bufferOperation.ts (1)
8-8: Dependency import looks good.
Importing getCurrentFilePath ensures that the current file path can be retrieved for constructing the annotation. This addition appears consistent with the new logic. Just be mindful to handle exceptions if getCurrentFilePath fails or returns an unexpected result.
|
|
||
| /** | ||
| * 選択したテキストを一時ファイルに保存し、ファイルのパスを返します。 | ||
| * | ||
| * @param {Denops} denops - Denopsインスタンス | ||
| * @param {string} start - 選択範囲の開始行 | ||
| * @param {string} end - 選択範囲の終了行 | ||
| * @retuns {Promise<string>} 一時ファイルのパス | ||
| */ | ||
| export async function getPartialContextFilePath(denops: Denops, start: string, end: string): Promise<string> { | ||
| const context = ensure(await denops.call("getline", start, end), is.ArrayOf(is.String)); | ||
| const filePath = ensure(await getCurrentFilePath(denops), is.String); | ||
|
|
||
| const annotation = ensure([`// Path: ${filePath}`], is.ArrayOf(is.String)); | ||
|
|
||
| annotation.push(`// Line: ${start}-${end}`); | ||
|
|
||
| context.unshift(...annotation); | ||
|
|
||
| // 一時ファイルに選択範囲を書き込む | ||
| const tempFile = ensure(await denops.call("tempname"), is.String); | ||
| await Deno.writeTextFile(tempFile, context.join("\n")); | ||
|
|
||
| // set filetype | ||
| const fileType = ensure(await fn.getbufvar(denops, "%", "&filetype"), is.String); | ||
| await denops.cmd(`setlocal filetype=${fileType}`); | ||
|
|
||
| return tempFile; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against potential file I/O issues and correct the documented tag.
-
The function correctly writes the selected text and annotation to a temporary file, enabling partial read-only context. Consider:
- Adding try/catch blocks around file I/O to handle unexpected disk failures or permission issues.
- Verifying the line range is valid (e.g., non-empty result from
getline).
-
The JSDoc tag currently says
@retuns. Consider updating it to@returnsfor clarity and consistency.
Here’s a proposed diff to address the JSDoc tag:
- * @retuns {Promise<string>} 一時ファイルのパス
+ * @returns {Promise<string>} 一時ファイルのパスCommittable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
135-135: Documentation added for the new command.The new command
:AiderAddPartialReadonlyContextis properly documented in the Usage section.Consider enhancing the documentation with:
- A more detailed description explaining that the selected text is saved to a temporary file
- An example key mapping in both the vimscript and lua configuration sections
- A usage example showing how to use the command in visual mode
Here's a suggested expansion of the documentation:
- - `:AiderAddPartialReadonlyContext` - Adds the selected text in visual mode as read-only context to Aider. + - `:AiderAddPartialReadonlyContext` - Adds the selected text in visual mode as read-only context to Aider by saving it to a temporary file. This is useful when you want to provide specific parts of a file as context without including the entire file.And add these key mappings to the configuration sections:
" In vimscript section " Add selected text as read-only context vmap <leader>ac :AiderAddPartialReadonlyContext<CR>-- In lua section vim.api.nvim_set_keymap('v', '<leader>ac', ':AiderAddPartialReadonlyContext<CR>', { noremap = true, silent = true })
134-136: Consider reordering commands for better organization.The new command
:AiderAddPartialReadonlyContextis related to visual mode operations. Consider moving it next to the existing visual mode command:AiderVisualTextWithPromptto maintain logical grouping of commands.Suggested order:
- `:AiderVisualTextWithPrompt` - Edit the selected text in visual mode in a floating window and send it to aider. In the floating window, send to aider with `<CR>` in normal mode, and close the floating window with `Q`. You can also backup prompt with `q`. - `:AiderAddPartialReadonlyContext` - Adds the selected text in visual mode as read-only context to Aider. - `:AiderAddWeb` - Displays a prompt for the specified URL and adds it to the aider context.🧰 Tools
🪛 LanguageTool
[grammar] ~134-~134: The word “backup” is a noun. The verb is spelled with a space.
Context: ... floating window withQ. You can also backup prompt withq. - `:AiderAddPartialRea...(NOUN_VERB_CONFUSION)
Summary by CodeRabbit
New Features
addPartialReadonlyContextto the Aider plugin.:AiderAddPartialReadonlyContextfor users to add selected text in visual mode.Improvements