fix(cli): detect available editors lazily to avoid slow startup#28144
fix(cli): detect available editors lazily to avoid slow startup#28144abhay-codes07 wants to merge 1 commit into
Conversation
EditorSettingsManager was instantiated at module scope and its constructor probed every known editor via hasValidEditorCommand, which shells out with a synchronous execSync per editor. On systems where process creation is expensive (notably Windows with endpoint security intercepting each spawn) this blocked the interactive UI from drawing for tens of seconds during startup, even though the editor list is only needed when the editor settings dialog is opened. Defer the probing: compute the available editors on the first call to getAvailableEditorDisplays() and cache the result. The constructor now does no work, so importing the module no longer spawns any processes. Output is unchanged. The class is exported so the behavior can be unit tested. Fixes google-gemini#28106
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 addresses a performance regression where the application startup was blocked by synchronous editor probing. By refactoring the EditorSettingsManager to use lazy initialization, the expensive process-spawning logic is now only executed when the editor settings are actually requested, significantly improving the responsiveness of the CLI on affected platforms. Highlights
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 the 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 counterproductive. 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. Footnotes
|
|
📊 PR Size: size/M
|
There was a problem hiding this comment.
Code Review
This pull request refactors EditorSettingsManager to lazily compute the list of available editors on first access rather than during construction, preventing synchronous shell commands from blocking CLI startup. A new test file editorSettingsManager.test.ts has been added to verify this lazy loading, caching, and editor status behavior. No review comments were provided, and the implementation is clean and well-tested.
Summary
On startup,
EditorSettingsManageris instantiated at module scope and its constructor probes every known editor throughhasValidEditorCommand, which runs a synchronousexecSync(e.g.where.exe <editor>) per editor. On systems where process creation is expensive — notably Windows with enterprise endpoint/antivirus software intercepting each spawn — this blocks the interactive UI from drawing its first frame for tens of seconds (reporter measured 50s–90s).The probed list is only ever consumed by the Editor Settings dialog (
EditorSettingsDialog.tsx), so doing this work eagerly at import time is pure startup cost for everyone who never opens that dialog.Fix
Defer the probing: compute the available editors on the first call to
getAvailableEditorDisplays()and cache the result. The constructor now does no work, so importing the module (and constructing the module-scope singleton) no longer spawns any processes. The produced list is identical to before — only the timing changes (import time → first dialog open).Testing
Added
editorSettingsManager.test.ts(the class is now exported for testability) covering:"None"is always listed first; uninstalled editors are labeled(Not installed)and disabled(Not available in sandbox)and disabledAll editor tests pass (4 new + the existing
EditorSettingsDialogsuite);eslintandtsc --noEmitare clean for the package.Fixes #28106