fix(editor): improve external editor (Ctrl-X) UX - #21377
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience of the external editor (Ctrl-X) feature by addressing several long-standing usability issues. It provides clearer feedback to users through descriptive error messages, improves the editor selection interface, and ensures a smoother workflow with popular editors like VS Code by opening files in new windows. Additionally, it expands the range of officially supported editors and updates the configuration documentation for better clarity. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request significantly improves the user experience for the external editor (Ctrl-X) feature by introducing more descriptive error messages, fixing an infinite loop in the /editor dialog, ensuring all editors are visible in the picker, and adding necessary arguments for VS Code-family editors. Documentation and tests have also been updated. However, it introduces or preserves a high-severity command injection vulnerability on Windows due to the openFileInEditor function using shell: true with unquoted arguments. This should be resolved by properly quoting command-line arguments or avoiding shell: true. Furthermore, the new, robust logic for handling editor arguments is only applied when using the preferredEditor setting, not with $VISUAL or $EDITOR environment variables, leading to inconsistent behavior and an incomplete fix for VS Code-family editors.
|
Thanks for the PR! I need to point out that this PR is a duplicate of PR #21090, which has already received its first review. Can we merge the best from both PR? How should we proceed? |
Thanks for the heads-up! I definitely don't want to duplicate our efforts. Our PRs do overlap on adding Sublime/Emacs as supported editors and fixing the error message when opening up unsupported editors, but mine also addresses a few extra bugs I found while testing:
Let me know how best to proceed! I'm happy to add these additional fixes to your branch or trim my PR to include just these improvements. |
|
Yes, we should definitely keep your extra fixes and extra editors. I don't want work to be lost. I am not checking it how hard it is to cherry-pick your commits and replay them onto my PR and resolve conflicts. I will now make a quick assessment and report back. |
|
Hi @AnanthKini1, thanks for the great work on this! Rather than running two competing PRs, I've integrated your commits into my branch. Here's what I did:
I've also invited you as a collaborator on my fork (you would need to accept it if you agree with the proposal), so you can push directly to the extra-editors branch. That way we can iterate together without having to continuously cherry-pick between PRs. The PR to track is #21090. |
Hi @alberti42! Thank you so much for merging our work and preserving my authorship, I really appreciate it. I've accepted the invitation to your fork and will keep an eye out for any comments left by maintainers. |
|
@AnanthKini1 We have received feedback from #21090. I just wanted to give you a heads-up in case you are not tracking it. I will be very busy in the next few days. So if you have time to already take care of some of the comments, it would be great. |
Hi @alberti42 ! Yes, just saw the comments--I will address these issues tomorrow. |
|
Hi Ananth, I did reply to the reviewers' concern on the other branch. Let's see what he thinks of it. |
Hi Andrea, thank you for addressing the fixes! I apologize for not addressing them earlier, I was overwhelmed with some schoolwork as it is now finals week 🥲. I will be more diligent in fixing concerns earlier in the future. |
|
Love it. No, you were not slow at all. I was tired of preparing exams (also here we have finals) so I needed a couple of hours thinking of something else, and I had decided to fix what was left to be done. |
|
Closing this since the #21090 is the latest one. |
Summary
Fixes three gaps in the external editor (Ctrl-X) feature: cryptic error messages, editors hidden below the fold in
/editor, and VS Code-family editors opening their start screen instead of the file.Details
editorUtilsnow throws specific errors (e.g.Editor command 'doesnotexist' was not found in PATH. Install it or use /editor to choose another editor.) instead of emitting a generic label.text-bufferdisplays the thrown message viagetErrorMessage(err). This also eliminates double error reporting./editordialog:coreEvents.emitFeedback(...)was called directly in the React render body whenpreferredEditorwas set to an unrecognized value, causing an infinite re-render loop. Removed the call; falling back to index 0 ("None") is sufficient since the user is already in the dialog to fix their config./editorpicker: With 17 entries andmaxItemsToShow=10, items 10–16 (nova through zed) were hidden below the fold with no scroll indicator, making them appear filtered. Fixed by passingmaxItemsToShow={editorItems.length}.--new-windowfor VS Code-family editors:cursor --wait file.txtwithout an active workspace opens Cursor's start screen instead of the file. Added--new-windowtoeditorExtraArgsforvscode,vscodium,cursor, andwindsurf, producingcursor --new-window --wait file.txt.docs/reference/configuration.mdto list all 16 supported editor identifiers and document the$VISUAL/$EDITORfallback behavior.Related Issues
Closes #21084
How to Validate
preferredEditor: "notvim"in~/.gemini/settings.json, press Ctrl-X → one descriptive error naming the unrecognized identifier (not a generic label)EDITOR=doesnotexist, press Ctrl-X → seeEditor command 'doesnotexist' was not found in PATH.../editor→ dialog opens cleanly with no spam or infinite loop, defaults to "None"/editor→ all 17 entries (None + 16 editors) visible at once without scrollingpreferredEditor: "cursor"(withcursorCLI installed via Cmd+Shift+P → "Install 'cursor' command in PATH"), press Ctrl-X → Cursor opens a new window directly onbuffer.txt; closing that tab returns the edited text to gemini-cliPre-Merge Checklist