-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
voice command #25
voice command #25
Conversation
WalkthroughThe pull request introduces several changes across multiple files, primarily focusing on enhancing the Changes
Possibly related PRs
Suggested reviewers
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
( ;´ワ `;)つLGTM☆
(余談)
コードフォーマットがなかなか安定しない感
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
denops/aider/bufferOperation.ts (1)
72-81
: New function to centralize Aider buffer preparationThe new
prepareAiderBuffer
function centralizes the logic for preparing the Aider buffer, which is a good improvement for code organization and reusability. However, there's an unnecessary console.log statement that should be removed.Consider removing the console.log statement on line 78:
- console.log("Run Command again.");
denops/aider/main.ts (3)
Line range hint
26-42
: Ensure consistent language for code commentsThe code includes comments in both English and Japanese. For consistency and maintainability, consider using a single language for all code comments throughout the codebase. This will make it easier for all contributors to understand and maintain the code.
Line range hint
190-199
: Add error handling for 'git' command execution in 'openIgnore' commandThe
openIgnore
command assumes that Git is available and that the current directory is within a Git repository. If Git is not installed or the command fails (e.g., not in a Git repository), the code may throw an unhandled exception.Consider adding error handling to manage cases where
git rev-parse --show-toplevel
fails, and provide a user-friendly message or fallback behavior.Apply this diff to add error handling:
const gitRoot = (await fn.system(denops, "git rev-parse --show-toplevel")).trim(); +if (!gitRoot) { + console.error("Not a git repository or Git not available."); + return; +}
Line range hint
201-212
: Use 'path.relative' to compute the relative path and handle potential errorsIn the
addIgnoreCurrentFile
command, calculatingrelativePath
usingcurrentFile.replace(gitRoot, "")
may not always produce the correct relative path, especially ifcurrentFile
is not a subpath ofgitRoot
or if the paths share common prefixes.Consider using
path.relative
from Deno's standard library to accurately compute the relative path betweengitRoot
andcurrentFile
. Additionally, add error handling in case Git is not available or the command fails.Apply this diff to use
path.relative
and add error handling:+import { relative } from "https://deno.land/std/path/mod.ts"; const gitRoot = (await fn.system(denops, "git rev-parse --show-toplevel")).trim(); +if (!gitRoot) { + console.error("Not a git repository or Git not available."); + return; +} -const relativePath = currentFile.replace(gitRoot, ""); +const relativePath = relative(gitRoot, currentFile);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- biome.jsonc (1 hunks)
- denops/aider/bufferOperation.ts (19 hunks)
- denops/aider/main.ts (12 hunks)
🧰 Additional context used
🔇 Additional comments (10)
denops/aider/bufferOperation.ts (8)
23-23
: Improved readability with concise return statementThe function has been simplified by using a single-line return statement with a ternary operator. This change improves readability while maintaining the same functionality.
46-46
: Improved consistency in function signatureThe function signature has been updated to be on a single line, improving consistency with other function declarations in the file. The functionality remains unchanged.
83-83
: Improved consistency in function signatureThe function signature has been updated to be on a single line, improving consistency with other function declarations in the file. The functionality remains unchanged.
104-105
: Improved function signature and simplified return statementThe function signature has been updated to be consistent with other functions in the file. The return statement has been simplified by removing the unnecessary explicit return of undefined.
Also applies to: 114-114
Line range hint
124-153
: Enhanced floating window functionality with new keymapsThe function has been updated to include new keymap settings for the floating window. These changes improve user interaction by providing:
- A way to close the window with the 'q' key.
- A method to hide the visual select floating window with 'Q'.
- The ability to send a prompt with the enter key.
These additions enhance the usability of the floating window.
166-166
: Improved consistency in function signaturesThe function signatures for
handleBackupPrompt
andhandleNoBackupPrompt
have been updated to be on single lines, improving consistency with other function declarations in the file. The functionality of both functions remains unchanged.Also applies to: 182-183
203-204
: Improved consistency in function signatureThe function signature for
hideVisualSelectFloatingWindow
has been updated to be on a single line, improving consistency with other function declarations in the file. The functionality remains unchanged.
Line range hint
1-366
: Overall assessment: Improved code consistency and functionalityThe changes in this file have significantly improved the consistency of function signatures and introduced new functionality, particularly in the floating window handling. The new
prepareAiderBuffer
function centralizes buffer preparation logic, which is a good improvement for code organization.A minor suggestion was made to remove a console.log statement in the new function. Otherwise, all changes are approved and contribute positively to the codebase.
denops/aider/main.ts (2)
91-91
: Refactored buffer preparation into 'prepareAiderBuffer'Replacing direct buffer initialization with a call to
prepareAiderBuffer
improves code modularity and reusability by encapsulating the buffer setup logic.
215-220
: Verify the use of 'fn.feedkeys' to enter insert modeIn the
voice
command, the use ofawait fn.feedkeys(denops, "a");
to enter insert mode may have unintended side effects or may not be reliable in all contexts.Consider verifying whether this is the best approach to programmatically enter insert mode. It might be preferable to use a more controlled method, such as
await denops.cmd('startinsert')
, to ensure consistent behavior without interfering with the user's workflow.
"indentStyle": "space", | ||
"lineWidth": 120 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Lines exceed the configured line width of 120 characters.
The following lines in the codebase exceed the 120-character limit set in the formatter configuration:
denops/aider/main.ts:38
denops/aider/main.ts:58
denops/aider/main.ts:76
denops/aider/utils.ts:14
tests/testRunner.ts:17
Please refactor these lines to comply with the new line width setting.
🔗 Analysis chain
LGTM! Consider verifying the impact on the codebase.
The addition of "lineWidth": 120
to the formatter configuration aligns with the PR objectives and is a common standard in many codebases. This change will allow for more content per line while still maintaining readability.
To ensure this change doesn't negatively impact the existing codebase, consider running the following script to check the current line lengths in your JavaScript and TypeScript files:
This script will help you identify any potential formatting issues that may arise from this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check current line lengths in JS/TS files
# Find all JS and TS files, excluding node_modules
files=$(fd -e js -e ts --exclude node_modules)
# Check for lines longer than 120 characters
echo "Lines longer than 120 characters:"
for file in $files; do
awk 'length($0) > 120 {print FILENAME ":" NR ":" $0}' "$file"
done
# Show distribution of line lengths
echo -e "\nDistribution of line lengths:"
for file in $files; do
awk '{print length($0)}' "$file"
done | sort -n | uniq -c | sort -n
Length of output: 4436
commands.map((command) => [command.methodName, command.impl as (args: unknown) => Promise<void>]), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid unnecessary type assertions in dispatcher mapping
In the dispatcher mapping, the type assertion as (args: unknown) => Promise<void>
may mask potential type errors.
Consider refining the types to avoid using type assertions, ensuring type safety and better maintainability.
Apply this diff to adjust types:
denops.dispatcher = Object.fromEntries(
- commands.map((command) => [command.methodName, command.impl as (args: unknown) => Promise<void>]),
+ commands.map((command) => [command.methodName, command.impl]),
);
📝 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.
commands.map((command) => [command.methodName, command.impl as (args: unknown) => Promise<void>]), | |
); | |
commands.map((command) => [command.methodName, command.impl]), | |
); |
@@ -103,7 +103,7 @@ export async function main(denops: Denops): Promise<void> { | |||
|
|||
const currentFile = await getCurrentFilePath(denops); | |||
const prompt = `/${prefix} ${currentFile}`; | |||
await buffer.sendPromptWithInput(denops, prompt); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tsukimizake
過去の遺産(コマンドラインからプロンプトを入力していたころの名残)が残ったままだったの気になっていましたが、対応ありがとうございます。 🙇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
わかる
Summary by CodeRabbit
New Features
prepareAiderBuffer
function.Improvements
Bug Fixes