-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat/add send prompt by commandline #35
Conversation
WalkthroughThis pull request introduces significant updates to the Changes
Possibly related PRs
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
tests/aider_test.ts (1)
38-49
: LGTM! Consider extracting the sleep duration.The test structure is clear and follows good testing practices. However, the hardcoded sleep duration could be made more maintainable.
Consider using the
SLEEP_BEFORE_ASSERT
constant that's already defined:- await sleep(SLEEP_BEFORE_ASSERT); + await sleep(SLEEP_BEFORE_ASSERT);README.md (3)
138-139
: Enhance command descriptions for clarity.The new command descriptions could be more detailed to better explain their functionality and use cases.
Consider updating the descriptions as follows:
-`:AiderSendPromptByCommandline <prompt>` - Sends a prompt from the command line and displays the Aider window. -`:AiderSilentSendPromptByCommandline <prompt>` - Sends a prompt from the command line and refreshes the buffer. +`:AiderSendPromptByCommandline <prompt>` - Sends a custom prompt from the command line and displays the Aider window for interactive feedback. +`:AiderSilentSendPromptByCommandline <prompt>` - Sends a custom prompt from the command line and refreshes the buffer without displaying the Aider window.
146-156
: Expand Advanced Usage documentation.While the examples are helpful, this section could be more comprehensive.
Consider enhancing the section with:
- Brief explanations of what the chat modes do (architect vs code)
- Additional examples of common use cases
- Notes about prompt syntax requirements
Here's a suggested expansion:
### Advanced Usage If you want to send a custom prompt to Aider, use `AiderSendPromptByCommandline`. Set it up as follows: ```vim " Send a prompt to Aider and display the Aider window :AiderSendPromptByCommandline "/chat-mode architect" " Send a prompt to Aider but do not display the Aider window :AiderSilentSendPromptByCommandline "/chat-mode code"
+#### Chat Modes
+-/chat-mode architect
: Switches to architectural discussion mode for high-level design guidance
+-/chat-mode code
: Switches to code-focused mode for implementation details
+
+#### Common Use Cases
+vim +" Request code review +:AiderSendPromptByCommandline "Please review this code for potential issues" + +" Generate unit tests +:AiderSendPromptByCommandline "Generate unit tests for the current file" +
+
+#### Notes
+- Prompts containing spaces must be enclosed in quotes
+- Special commands like/chat-mode
must start with a forward slash--- Line range hint `1-1`: **Document key mapping changes for floating window.** The PR includes fixes for key mapping issues when opening selected code in a floating window, but these changes are not documented. Please add documentation about: 1. The fixed key mapping behavior in floating windows 2. Any new or modified key bindings users should be aware of 3. How these changes affect the user experience when working with floating windows <details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary><blockquote> [uncategorized] ~136-~136: Possible missing comma found. Context: ...the `.aiderignore` file in the git root directory if it exists. - `:AiderAddIgnoreCurrent... (AI_HYDRA_LEO_MISSING_COMMA) </blockquote></details> </details> </blockquote></details> <details> <summary>denops/aider/main.ts (1)</summary><blockquote> Line range hint `246-252`: **Consider enhancing error messages for better user experience.** While the error handling is good, consider making the error messages more user-friendly and actionable. For example: ```diff - console.error("Failed to add file to .aiderignore:", error); + console.error("Failed to add file to .aiderignore. Please ensure you're in a git repository and have write permissions:", error); - console.error("Failed to execute voice command:", error); + console.error("Failed to start voice command. Please check if your system meets the voice requirements:", error);
Also applies to: 254-261
denops/aider/bufferOperation.ts (1)
160-162
: Update documentation to reflect command name changes.Please ensure the README.md and any other documentation are updated to reflect this command name change from
AiderSendPrompt
toAiderSendPromptByBuffer
. This will help users understand the new command structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- README.md (1 hunks)
- denops/aider/bufferOperation.ts (1 hunks)
- denops/aider/main.ts (1 hunks)
- tests/aider_test.ts (1 hunks)
🔇 Additional comments (4)
tests/aider_test.ts (1)
Line range hint
1-68
: Add tests for floating window key mapping.The PR objectives mention fixing key mapping issues in the floating window, but there are no tests covering this functionality.
Let's check if there are any existing tests for floating window functionality:
Would you like help creating test cases for the floating window key mapping functionality?
denops/aider/main.ts (2)
108-108
: LGTM: Improved function naming clarity.The renaming from
sendPrompt
tosendPromptByBuffer
better reflects the source of the prompt, making the code more maintainable.
112-119
: LGTM: Well-structured command-line prompt implementation.The new commands follow the established patterns and correctly handle command-line arguments. The silent version appropriately includes console logging for debugging purposes.
Let's verify the command registration:
Also applies to: 121-129
✅ Verification successful
Let me try a different approach to verify the command registration by looking for the command registration patterns in the codebase.
This should help us verify:
- The command registration in the TypeScript code
- Any potential command definitions in vim plugin files
- Ensure there are no duplicate registrations
Let me try one final verification to check the command registration pattern using ast-grep, which might give us better context about the command structure.
This should help us see:
- The complete command registration structure
- Any other command registrations that might provide context about the pattern consistency
Command registration is properly implemented and consistent with codebase patterns
Based on the ast-grep results, I can confirm that:
- The
sendPromptByCommandline
command is correctly registered with the expected pattern[<f-args>]
and takes 1 argument- The implementation follows the same pattern as other similar commands in the codebase (like
addFile
,addWeb
,ask
, etc.)- The command registration is consistent with the codebase's command handling patterns
- There are no duplicate registrations
The implementation is well-structured and maintains consistency with the established command registration patterns throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify command registration and usage # Test 1: Check if the commands are properly registered with correct patterns rg -A 2 'command!\s+-nargs=1.*AiderSendPromptByCommandline' rg -A 2 'command!\s+-nargs=1.*AiderSilentSendPromptByCommandline' # Test 2: Verify no duplicate command registrations rg 'command!.*Aider(Silent)?SendPromptByCommandline' | sort | uniq -dLength of output: 201
Script:
#!/bin/bash # Search for command registration patterns in the codebase # Looking for both the command function and any vim command registrations rg -A 5 'sendPromptByCommandline' # Also check the plugin directory for any vim command definitions fd -e vim plugin/ -x cat {}Length of output: 379
Script:
#!/bin/bash # Use ast-grep to find the complete command registration structure ast-grep --pattern 'await command( "sendPromptByCommandline", $$$ )' # Also check for any other command registrations in the file ast-grep --pattern 'await command($$$)'Length of output: 9955
denops/aider/bufferOperation.ts (1)
160-162
: Command renaming improves clarity but introduces a breaking change.The key mapping change from
AiderSendPrompt
toAiderSendPromptByBuffer
better reflects the command's purpose, but this is a breaking change that affects existing users.Let's verify if this command is properly documented:
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- tests/aider_test.ts (1 hunks)
🔇 Additional comments (1)
tests/aider_test.ts (1)
51-59
: LGTM with previous suggestions.The test logic is correct, but please refer to the previous review comment about reducing test flakiness through a more reliable waiting mechanism.
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
Release Notes
New Features
:AiderSendPromptByCommandline <prompt>
and:AiderSilentSendPromptByCommandline <prompt>
, enhancing user interaction with the plugin.Bug Fixes
Tests