fix:prevent JS hint leak on Ctrl+Space and show allowed root hints #6776
Conversation
Abhinandan-pixel
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughReplaces Ctrl/Cmd+Space autocomplete trigger with a new exported Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CodeEditor
participant Autocomplete
participant CodeMirror
participant STATIC_API_HINTS
User->>CodeEditor: Press Ctrl/Cmd+Space
CodeEditor->>Autocomplete: showRootHints(cm, showHintsFor)
Autocomplete->>CodeMirror: read cursor & current word
alt No current word
Autocomplete->>STATIC_API_HINTS: filter hints by showHintsFor
Autocomplete->>CodeMirror: showHint(completions)
CodeMirror-->>User: display API root hints popup
else Word exists
Autocomplete-->>CodeEditor: return null (defer to existing flow)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (1)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-12-17T21:41:24.730ZApplied to files:
📚 Learning: 2026-01-09T18:25:14.640ZApplied to files:
🧬 Code graph analysis (1)packages/bruno-app/src/components/CodeEditor/index.js (1)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-app/src/utils/codemirror/autocomplete.js (1)
521-540: Return value consistency.Line 527 returns
nullwhile line 531 returnsundefined. Consider returningnullconsistently for all early-exit paths.♻️ Consistent return values
const hints = Object.keys(STATIC_API_HINTS).filter((rootHint) => showHintsFor.includes(rootHint)); - if (hints.length === 0) return; + if (hints.length === 0) return null;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/bruno-app/src/components/CodeEditor/index.jspackages/bruno-app/src/utils/codemirror/autocomplete.jspackages/bruno-app/src/utils/codemirror/autocomplete.spec.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/utils/codemirror/autocomplete.spec.jspackages/bruno-app/src/components/CodeEditor/index.jspackages/bruno-app/src/utils/codemirror/autocomplete.js
🧠 Learnings (2)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/utils/codemirror/autocomplete.spec.jspackages/bruno-app/src/components/CodeEditor/index.jspackages/bruno-app/src/utils/codemirror/autocomplete.js
📚 Learning: 2026-01-09T18:25:14.640Z
Learnt from: kanakkholwal
Repo: usebruno/bruno PR: 6767
File: packages/bruno-app/src/components/ResponseExample/index.js:221-226
Timestamp: 2026-01-09T18:25:14.640Z
Learning: In the Bruno Electron renderer code (packages/bruno-app), assume window.ipcRenderer is always available and skip existence checks. Do not guard for ipcRenderer in this Electron context; use window.ipcRenderer directly (e.g., window.ipcRenderer.send(...), window.ipcRenderer.on(...)). If there are non-Electron contexts (such as test environments or non-Electron builds), add guards or mocks to avoid runtime errors there, but for the intended Electron renderer files, this pattern should be applied broadly within packages/bruno-app.
Applied to files:
packages/bruno-app/src/utils/codemirror/autocomplete.spec.jspackages/bruno-app/src/components/CodeEditor/index.jspackages/bruno-app/src/utils/codemirror/autocomplete.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/CodeEditor/index.js (1)
packages/bruno-app/src/utils/codemirror/autocomplete.js (2)
showRootHints(521-540)showRootHints(521-540)
🔇 Additional comments (5)
packages/bruno-app/src/utils/codemirror/autocomplete.js (2)
297-306: Improved progressive API hint discovery.The bidirectional prefix matching enables users to discover API hints progressively (e.g.,
r→re→req), which aligns with the PR's goal of fixing the Ctrl+Space autocomplete behavior.
653-657: Prevents premature completion closure.This check ensures the completion popup only closes when the user is actively typing a word with no hints, rather than closing on any keyup when no hints are present. This is essential for the Ctrl+Space fix.
packages/bruno-app/src/components/CodeEditor/index.js (2)
11-11: LGTM!Import correctly references the new
showRootHintsexport.
114-119: Ctrl+Space and Cmd+Space now trigger root hints.The implementation correctly delegates to
showRootHintswith the component'sshowHintsForconfiguration. SinceshowRootHintshas a default parameter of[], undefined values are handled safely.packages/bruno-app/src/utils/codemirror/autocomplete.spec.js (1)
485-485: Test updated to match new closing behavior.Changed from
' '(spaces) to'req.bodyy'to test the new logic where the completion popup only closes when a word is being typed and no hints are available. With the previous empty line,getCurrentWordWithContextwould returnnull, and the new logic wouldn't close the popup (which is correct for the Ctrl+Space fix).
Abhinandan-pixel
force-pushed
the
bugfix/issue-5871
branch
from
58f1743 to
db57e11
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bruno-app/src/utils/codemirror/autocomplete.js (2)
297-306: Consider renaming variables for clarity.The variable
hintin the callback represents an API root key ('req', 'res', 'bru'), not a hint. Renaming toapiRootorrootKeywould improve readability.♻️ Proposed refactor
const determineWordContext = (word) => { - const isApiHint = Object.keys(STATIC_API_HINTS).some( - (hint) => hint.toLowerCase().startsWith(word.toLowerCase()) || word.toLowerCase().startsWith(hint.toLowerCase()) + const isApiHint = Object.keys(STATIC_API_HINTS).some( + (apiRoot) => apiRoot.toLowerCase().startsWith(word.toLowerCase()) || word.toLowerCase().startsWith(apiRoot.toLowerCase()) ); if (isApiHint) { return 'api'; } return 'anyword'; };
521-540: Add JSDoc and ensure consistent return values.As a public export, this function should have JSDoc documentation. Additionally, the function returns
nullexplicitly in some paths but implicitly returnsundefinedafter callingcm.showHint(), which is inconsistent.♻️ Proposed improvements
+/** + * Show root-level API hints when the editor is empty + * @param {Object} cm - CodeMirror instance + * @param {string[]} showHintsFor - Array of hint types to show (e.g., ['req', 'res', 'bru']) + * @returns {boolean} True if hints were shown, false otherwise + */ export const showRootHints = (cm, showHintsFor = []) => { const wordInfo = getCurrentWordWithContext(cm); // If user is currently typing a word, let handleKeyupForAutocomplete // handle it instead of showing root hints. if (wordInfo) { - return null; + return false; } const hints = Object.keys(STATIC_API_HINTS).filter((rootHint) => showHintsFor.includes(rootHint)); - if (hints.length === 0) return null; + if (hints.length === 0) return false; const cursor = cm.getCursor(); const hintList = createStandardHintList(hints, cursor, cursor); cm.showHint({ hint: () => hintList, completeSingle: false }); + + return true; };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/bruno-app/src/components/CodeEditor/index.jspackages/bruno-app/src/utils/codemirror/autocomplete.jspackages/bruno-app/src/utils/codemirror/autocomplete.spec.js
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/bruno-app/src/components/CodeEditor/index.js
- packages/bruno-app/src/utils/codemirror/autocomplete.spec.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/utils/codemirror/autocomplete.js
🧠 Learnings (2)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/utils/codemirror/autocomplete.js
📚 Learning: 2026-01-09T18:25:14.640Z
Learnt from: kanakkholwal
Repo: usebruno/bruno PR: 6767
File: packages/bruno-app/src/components/ResponseExample/index.js:221-226
Timestamp: 2026-01-09T18:25:14.640Z
Learning: In the Bruno Electron renderer code (packages/bruno-app), assume window.ipcRenderer is always available and skip existence checks. Do not guard for ipcRenderer in this Electron context; use window.ipcRenderer directly (e.g., window.ipcRenderer.send(...), window.ipcRenderer.on(...)). If there are non-Electron contexts (such as test environments or non-Electron builds), add guards or mocks to avoid runtime errors there, but for the intended Electron renderer files, this pattern should be applied broadly within packages/bruno-app.
Applied to files:
packages/bruno-app/src/utils/codemirror/autocomplete.js
🔇 Additional comments (1)
packages/bruno-app/src/utils/codemirror/autocomplete.js (1)
653-657: LGTM! Correctly prevents premature popup close.The added
wordInfocheck ensures the completion popup only closes when the user is actively typing a word with no matching hints. This prevents the popup from closing when root hints are shown via Ctrl+Space with no word context, directly addressing the issue.
Abhinandan-pixel
force-pushed
the
bugfix/issue-5871
branch
from
db57e11 to
64125aa
Compare
|
|
||
| mockedCodemirror.getCursor.mockReturnValue({ line: 0, ch: 0 }); | ||
| mockedCodemirror.getLine.mockReturnValue(' '); | ||
| mockedCodemirror.getLine.mockReturnValue('req.bodyy'); |
There was a problem hiding this comment.
Seems there's a typo here, don't know how it could affect the feature (maybe the mock returns the wrong value?).
There was a problem hiding this comment.
An empty value (' ') is no longer valid because I changed the close condition to:
if (cm.state.completionActive && wordInfo) {
cm.state.completionActive.close();
}
When the editor is empty, wordInfo is null. If we allowed the popup to close in that case, the root hints shown on Ctrl + Space would immediately be closed by the keyUp handler (which runs after the Ctrl + Space handler). That’s exactly the bug this PR is fixing.
So the behavior is now:
- Empty editor + Ctrl + Space → show root hints and do not close them
- User types some text (e.g. req.bodyy) with no matching hints → wordInfo exists, so we do close the popup
The test was updated to reflect this new logic by using a non-matching word instead of an empty value, ensuring we only close the popup when the user has actually typed something.
2 participants
Description
Root cause
In the Script editor, pressing Ctrl + Space when the cursor is at an empty position triggers autocomplete without a valid word context. Since the hint provider returns null in this case, CodeMirror immediately closes the completion popup, causing it to briefly flash and disappear.
Fix
This change explicitly handles Ctrl + Space when the editor is empty by showing root-level hints (API objects), while preserving the existing keyup-based autocomplete behavior for all other cases.
Additionally, context detection was improved to allow progressive discovery of API hints (r → re → req).
Contribution Checklist:
Ctrl+Space now shows root hints without flashing
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.