-
Notifications
You must be signed in to change notification settings - Fork 59
Automatic Sync: Merge changes from release-ballerina to main #993
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
Conversation
…value to be reset
Fix `Add to integration` for /`typecreator` cmd
…refresh requests and improve error logging
Re-enable auto-reveal in tree view based on the active webview
…of the expression
fix completions not displayed for field access
Fix expression cursor on popup
Fix when removing all the values in the expression editor causes the …
…able fix invalid form status update in new variable creation of addConnect…
fix create config not saving the values
…s are always available, including notify and refresh commands for the Ballerina extension
…proper project type before creating test profiles
fix helperpane search fields are not focusable
Remove 'Show Examples' command and related functionality
Add force replace option for language server download
Fix incomplete source generation of union types in initial render
…packages using tree view
Update Record Configuration toggle name from Guided to Record
Enable visualizing constructs using code lenses in multi-package directory setups
…d listeners, ensuring proper positioning of unconnected listeners below other nodes.
Co-authored-by: Copilot <[email protected]>
Fix listener overlap and component menu button click issue in Architecture Diagram
Merge "ballerina-integrator-1.5.2" into "release-ballerina"
|
|
WalkthroughAdds a BI notify command and tree-view reveal/refresh APIs, removes the "Show Examples" command/activation, expands state machine updates to include ProjectInfo, converts expression completions to async with scheduling, defaults record expression mode to RECORD, and applies various UI event and layout adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VS as VS Code
participant Ext as Ballerina Extension
participant SM as StateMachine
participant BI as BI Extension
participant Tree as Project Explorer
User->>VS: open project/file
VS->>Ext: activate extension
Ext->>SM: updateProjectRootAndInfo(path, projectInfo)
SM->>BI: execute BI_COMMANDS.NOTIFY_PROJECT_EXPLORER
BI->>BI: revealInTreeView(documentUri, projectPath, position, view)
BI->>Tree: treeView.reveal(item)
Tree->>User: reveal tree item
sequenceDiagram
participant User
participant Chip as ChipExpressionEditor
participant Completer as CompletionProvider
User->>Chip: type / change
Chip->>Chip: completionsFetchScheduled = true
Chip->>Completer: getCompletions() (async)
Completer-->>Chip: Promise<CompletionItem[]>
Chip->>Chip: waitForStateChange() -> resolve when fetch complete
Chip->>User: show completions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
… sync/main-release-ballerina-1763738719
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts (1)
394-427: Possible runtime crash when last non-space char is.due towordbeingnullIn the new async
buildCompletionSource,wordcan still benullwhenlastNonSpaceChar === '.', but it’s now used unguarded (word.text,word.from), whereas the old logic always early-returned on!word.
CompletionContext.matchBeforeis typed to returnnull, so this can throw at runtime in some edge cases (e.g., certain positions after a.).You can keep the special
.behavior and avoid the crash by safely handling the no-word case and falling back tocontext.pos/ empty prefix:-export const buildCompletionSource = (getCompletions: () => Promise<CompletionItem[]>) => { - return async (context: CompletionContext): Promise<CompletionResult | null> => { - const textBeforeCursor = context.state.doc.toString().slice(0, context.pos); - const lastNonSpaceChar = textBeforeCursor.trimEnd().slice(-1); - - const word = context.matchBefore(/\w*/); - if (lastNonSpaceChar !== '.' && ( - !word || (word.from === word.to && !context.explicit) - )) { - return null; - } +export const buildCompletionSource = (getCompletions: () => Promise<CompletionItem[]>) => { + return async (context: CompletionContext): Promise<CompletionResult | null> => { + const textBeforeCursor = context.state.doc.toString().slice(0, context.pos); + const lastNonSpaceChar = textBeforeCursor.trimEnd().slice(-1); + + const word = context.matchBefore(/\w*/); + // For non-dot contexts, preserve the old guard. + if (lastNonSpaceChar !== '.' && ( + !word || (word.from === word.to && !context.explicit) + )) { + return null; + } @@ - const completions = await getCompletions(); - const prefix = word.text; + const completions = await getCompletions(); + const from = word ? word.from : context.pos; + const prefix = word?.text ?? ''; @@ - return { - from: word.from, + return { + from, options: filteredCompletions.map(item => ({This keeps the desired “always offer completions after
.” behavior, but avoids dereferencing a possibly-nullword.workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (1)
82-115: UnboundedrequestAnimationFramepolling inwaitForStateChangecan spin indefinitelyThe async completion plumbing is nicely decoupled, but there’s a reliability/perf edge case:
completionsFetchScheduledRef.currentis set totrueon every change (handleChangeListner).waitForStateChangeusesrequestAnimationFramein a loop until that flag becomesfalse.- The flag is only reset in the
useEffectthat runs whenprops.completionschanges.If, for any reason,
props.completionsdoes not change after a change (e.g., upstreamretrieveCompletionsshort-circuits, errors, or the parent unmounts quickly), the polling loop can run forever on the animation frame queue.Two small changes would make this safer without changing behavior:
@@ - const waitForStateChange = (): Promise<CompletionItem[]> => { - return new Promise((resolve) => { - const checkState = () => { - if (!completionsFetchScheduledRef.current) { - resolve(completionsRef.current); - } else { - requestAnimationFrame(checkState); - } - }; - checkState(); - }); - }; + const waitForStateChange = (): Promise<CompletionItem[]> => { + return new Promise((resolve) => { + let cancelled = false; + const checkState = () => { + if (cancelled || !completionsFetchScheduledRef.current) { + resolve(completionsRef.current); + return; + } + requestAnimationFrame(checkState); + }; + checkState(); + + // Best-effort cancellation on unmount + return () => { + cancelled = true; + }; + }); + }; @@ - const completionSource = useMemo(() => { - return buildCompletionSource(waitForStateChange); - }, [props.completions]); + const completionSource = useMemo( + () => buildCompletionSource(waitForStateChange), + [] + ); @@ - useEffect(() => { + useEffect(() => { if (!editorRef.current) return; @@ - const view = new EditorView({ + const view = new EditorView({ state: startState, parent: editorRef.current }); viewRef.current = view; return () => { + completionsFetchScheduledRef.current = false; view.destroy(); }; }, []); @@ - useEffect(() => { - completionsRef.current = props.completions; - completionsFetchScheduledRef.current = false; - }, [props.completions]); + useEffect(() => { + completionsRef.current = props.completions; + completionsFetchScheduledRef.current = false; + }, [props.completions]);The key ideas are:
- Ensure the polling loop can terminate when the component unmounts.
- Avoid tying the memoization of
completionSourcetoprops.completionssince theEditorViewis only created once andwaitForStateChangealready reads the live ref.Also applies to: 128-144, 218-266, 298-305
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (1)
71-75: Ensure updateProjectRootAndInfo promises always resolve (use try/finally)
updateProjectRootAndInfoexposes a Promise that is resolved by the async action in theUPDATE_PROJECT_ROOT_AND_INFOtransition. Right now the resolver is only called afterbuildProjectsStructure,notifyCurrentWebview, andnotifyTreeViewcomplete successfully:async (context, event) => { await buildProjectsStructure(...); notifyCurrentWebview(); notifyTreeView(...); pendingProjectRootUpdateResolvers.shift()?.(); }If any of those throw, the resolver is never invoked, and any caller awaiting
StateMachine.updateProjectRootAndInfo(...)(e.g.SHOW_VISUALIZER) can hang indefinitely.Wrapping the resolver shift in a
finallyguarantees that the Promise settles even on errors:- async (context, event) => { - await buildProjectsStructure(event.projectInfo, StateMachine.langClient(), true); - notifyCurrentWebview(); - notifyTreeView(event.projectPath, context.documentUri, context.position, context.view); - // Resolve the next pending promise waiting for project root update completion - pendingProjectRootUpdateResolvers.shift()?.(); - } + async (context, event) => { + try { + await buildProjectsStructure(event.projectInfo, StateMachine.langClient(), true); + notifyCurrentWebview(); + notifyTreeView(event.projectPath, context.documentUri, context.position, context.view); + } finally { + // Always resolve the next pending promise, even if the update fails + pendingProjectRootUpdateResolvers.shift()?.(); + } + }Callers can still handle thrown errors from
updateProjectRootAndInfo, but they will no longer hang.Also applies to: 111-125, 807-812
workspaces/ballerina/ballerina-extension/src/views/visualizer/activate.ts (1)
46-55: Remove unused 4th argument from SHOW_VISUALIZER call siteThe handler in
ballerina-extension/src/views/visualizer/activate.ts(lines 50-53) accepts(pathOrItem, position, resetHistory = false)— three parameters. However, atbi-extension/src/project-explorer/project-explorer-provider.tsline 536, the command is invoked with four arguments:[comp.path, comp.position, resetHistory, projectPath]. The 4thprojectPathargument is ignored at runtime since the new handler logic derivesprojectRootviafindBallerinaPackageRoot()instead.Remove the trailing
projectPathargument:- "arguments": [comp.path, comp.position, resetHistory, projectPath] + "arguments": [comp.path, comp.position, resetHistory]
♻️ Duplicate comments (4)
workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/AIServiceWidget.tsx (1)
84-90: LGTM! Event propagation control implemented correctly.workspaces/ballerina/component-diagram/src/components/nodes/ConnectionNode/ConnectionNodeWidget.tsx (1)
167-173: LGTM! Event propagation control implemented correctly.workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GraphQLServiceWidget.tsx (1)
194-200: LGTM! Event propagation control implemented correctly.workspaces/ballerina/component-diagram/src/components/nodes/ListenerNode/ListenerNodeWidget.tsx (1)
177-183: LGTM! Event propagation control implemented correctly.
🧹 Nitpick comments (6)
workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GeneralWidget.tsx (1)
200-206: Event propagation control implemented correctly.The handlers properly prevent menu button interactions from triggering parent drag handlers.
Consider extracting these handlers into a shared hook since identical implementations appear in ConnectionNodeWidget, AIServiceWidget, GraphQLServiceWidget, and ListenerNodeWidget:
// hooks/useMenuEventHandlers.ts export function useMenuEventHandlers() { const handleMenuMouseDown = (event: React.MouseEvent) => { event.stopPropagation(); }; const handleMenuMouseUp = (event: React.MouseEvent) => { event.stopPropagation(); }; return { handleMenuMouseDown, handleMenuMouseUp }; }workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx (1)
344-347: Record-type fields now default to RECORD mode and open config page on focusThe changes to:
- initialize
inputModeasrecordTypeField ? InputMode.RECORD : InputMode.EXP, and- force
InputMode.RECORDin theuseEffectwhenrecordTypeFieldis present, and- gate
handleFocusto openonOpenRecordConfigPagewheninputMode === InputMode.RECORDform a coherent story with
ModeSwitcherandExpressionField(which now renders RECORD viaTextModeEditor). This makes record fields consistently enter the record configuration flow instead of the chip-based expression mode.One small nit: the comment above the effect still says “always use GUIDED mode” – it’d be clearer to update it to “RECORD mode” to reflect the new behavior.
Also applies to: 403-451
workspaces/ballerina/ballerina-extension/src/views/bbe/activator.ts (1)
106-109: BBE activation appropriately disabled pending samples availability.The activation logic has been properly disabled with a clear TODO and issue reference. This aligns with the broader PR changes removing the Show Examples feature.
Consider removing or commenting out the now-unused
showExamplesfunction (lines 61-104) andexampleMaps(lines 29-59) to reduce code clutter until samples are available. This would make the temporary state more explicit.-let examplesPanel: WebviewPanel | undefined; -const exampleMaps = new Map([ - ['distributed-transactions', 'initiator.bal'], - ['basic-https-listener-client', 'basic_https_listener.bal'], - ... -]); - -function showExamples(context: ExtensionContext, langClient: ExtendedLangClient): void { - ... -} +// let examplesPanel: WebviewPanel | undefined; +// const exampleMaps = new Map([...]); +// function showExamples(...) { ... } +// Commented out until samples are available - see issue #1967workspaces/ballerina/ballerina-visualizer/src/components/TopNavigationBar/index.tsx (1)
160-161: Adding "welcome" to breadcrumb skip list is consistent with existing pattern.The addition follows the established hackToSkipForms pattern. However, as noted in the comment at line 150, this should eventually be fixed at the state machine level rather than maintaining a hardcoded skip list.
Consider creating a GitHub issue to track moving this breadcrumb filtering logic to the state machine, where navigation state can be managed more systematically rather than maintaining string-based skip lists in the UI component.
workspaces/bi/bi-extension/src/project-explorer/activate.ts (1)
18-18: Core BI commands are wired correctly; consider registering disposables with contextThe new
setTreeView(projectTree)plusregisterCoreCommandssetup givesProjectExplorerEntryProvideraccess to theTreeViewand exposes a cleanBI_COMMANDS.NOTIFY_PROJECT_EXPLORER→revealInTreeView(...)andBI_COMMANDS.REFRESH_COMMAND→refresh()flow, which looks logically sound.One improvement:
registerCoreCommandscurrently ignores theDisposables returned bycommands.registerCommand. To align with typical VS Code patterns and ensure proper cleanup, you could passcontextin and push these intocontext.subscriptions, for example:-export function activateProjectExplorer(config: ExplorerActivationConfig) { - const { context, isBI, isBallerinaPackage, isBallerinaWorkspace, isEmptyWorkspace } = config; +export function activateProjectExplorer(config: ExplorerActivationConfig) { + const { context, isBI, isBallerinaPackage, isBallerinaWorkspace, isEmptyWorkspace } = config; @@ - projectExplorerDataProvider.setTreeView(projectTree); - // Always register core commands so they're available to the Ballerina extension - registerCoreCommands(projectExplorerDataProvider); + projectExplorerDataProvider.setTreeView(projectTree); + // Always register core commands so they're available to the Ballerina extension + registerCoreCommands(context, projectExplorerDataProvider); @@ -function registerCoreCommands(dataProvider: ProjectExplorerEntryProvider) { - // Register the notify command that's called by the Ballerina extension - commands.registerCommand( - BI_COMMANDS.NOTIFY_PROJECT_EXPLORER, - (event: { projectPath: string, documentUri: string, position: NodePosition, view: MACHINE_VIEW }) => { - dataProvider.revealInTreeView(event.documentUri, event.projectPath, event.position, event.view); - } - ); - - // Register the refresh command - commands.registerCommand(BI_COMMANDS.REFRESH_COMMAND, () => dataProvider.refresh()); +function registerCoreCommands( + context: ExtensionContext, + dataProvider: ProjectExplorerEntryProvider +) { + context.subscriptions.push( + commands.registerCommand( + BI_COMMANDS.NOTIFY_PROJECT_EXPLORER, + (event: { + projectPath: string; + documentUri: string; + position: NodePosition; + view: MACHINE_VIEW; + }) => { + dataProvider.revealInTreeView(event.documentUri, event.projectPath, event.position, event.view); + } + ), + commands.registerCommand(BI_COMMANDS.REFRESH_COMMAND, () => dataProvider.refresh()) + ); }Also applies to: 42-46, 69-85
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (1)
1028-1049: Guard NOTIFY_PROJECT_EXPLORER when BI extension is missing
notifyTreeViewcurrently avoids notifying the BI project explorer only when the BI extension exists and is inactive:const biExtension = extensions.getExtension('wso2.ballerina-integrator'); if (biExtension && !biExtension.isActive) { return; } commands.executeCommand(BI_COMMANDS.NOTIFY_PROJECT_EXPLORER, { ... });If the BI extension is not installed,
biExtensionisundefinedand you still attempt to executeNOTIFY_PROJECT_EXPLORER, which will just throw and be logged by the catch block. Functionally it’s harmless but produces noisy errors in non-BI setups.Consider short-circuiting when the extension is absent as well:
- const biExtension = extensions.getExtension('wso2.ballerina-integrator'); - if (biExtension && !biExtension.isActive) { - return; - } + const biExtension = extensions.getExtension('wso2.ballerina-integrator'); + if (!biExtension || !biExtension.isActive) { + return; + }This keeps the cross-extension coupling purely opt‑in and avoids unnecessary error logs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
workspaces/ballerina/ballerina-core/src/interfaces/constants.ts(1 hunks)workspaces/ballerina/ballerina-extension/package.json(2 hunks)workspaces/ballerina/ballerina-extension/scripts/download-ls.js(2 hunks)workspaces/ballerina/ballerina-extension/src/extension.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts(2 hunks)workspaces/ballerina/ballerina-extension/src/features/bi/activator.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/features/project/cmds/cmd-runner.ts(0 hunks)workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/stateMachine.ts(9 hunks)workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts(0 hunks)workspaces/ballerina/ballerina-extension/src/views/bbe/activator.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/views/visualizer/activate.ts(2 hunks)workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/batch-processing.ts(0 hunks)workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/constants.ts(0 hunks)workspaces/ballerina/ballerina-extension/test/ai/integration_tests/libs/setup.ts(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/ModeSwitcher/index.tsx(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx(3 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx(5 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/HelperPane.tsx(0 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/types.ts(1 hunks)workspaces/ballerina/ballerina-visualizer/src/components/TopNavigationBar/index.tsx(1 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/Configurables/AddConfigurableVariables/index.tsx(1 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/AddConnectionWizard/index.tsx(0 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx(4 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx(1 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/Configurables.tsx(1 hunks)workspaces/ballerina/component-diagram/src/components/nodes/ConnectionNode/ConnectionNodeWidget.tsx(2 hunks)workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/AIServiceWidget.tsx(2 hunks)workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GeneralWidget.tsx(2 hunks)workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GraphQLServiceWidget.tsx(2 hunks)workspaces/ballerina/component-diagram/src/components/nodes/ListenerNode/ListenerNodeWidget.tsx(2 hunks)workspaces/ballerina/component-diagram/src/utils/diagram.ts(3 hunks)workspaces/bi/bi-extension/package.json(1 hunks)workspaces/bi/bi-extension/src/project-explorer/activate.ts(3 hunks)workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts(5 hunks)workspaces/common-libs/ui-toolkit/src/components/Button/Button.tsx(1 hunks)
💤 Files with no reviewable changes (6)
- workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/constants.ts
- workspaces/ballerina/ballerina-extension/src/features/project/cmds/cmd-runner.ts
- workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts
- workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/AddConnectionWizard/index.tsx
- workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/batch-processing.ts
- workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/HelperPane.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/bi/bi-extension/package.jsonworkspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/ballerina/ballerina-extension/test/ai/integration_tests/libs/setup.tsworkspaces/ballerina/ballerina-extension/src/extension.tsworkspaces/ballerina/ballerina-extension/src/features/bi/activator.tsworkspaces/ballerina/ballerina-visualizer/src/components/TopNavigationBar/index.tsxworkspaces/ballerina/ballerina-extension/package.jsonworkspaces/ballerina/ballerina-extension/src/stateMachine.tsworkspaces/ballerina/ballerina-extension/src/views/bbe/activator.tsworkspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.tsworkspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.tsworkspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
📚 Learning: 2025-11-10T15:04:50.474Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Applied to files:
workspaces/bi/bi-extension/package.jsonworkspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/ballerina/ballerina-extension/test/ai/integration_tests/libs/setup.tsworkspaces/ballerina/ballerina-extension/src/extension.tsworkspaces/ballerina/ballerina-extension/package.jsonworkspaces/ballerina/ballerina-extension/src/stateMachine.tsworkspaces/ballerina/ballerina-extension/src/views/bbe/activator.tsworkspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.tsworkspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
📚 Learning: 2025-11-05T10:31:47.583Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 830
File: workspaces/ballerina/ballerina-extension/test/ai/post_proccess/post.test.ts:0-0
Timestamp: 2025-11-05T10:31:47.583Z
Learning: In the workspaces/ballerina/ballerina-extension project, the tsconfig.json uses "rootDirs": ["src", "test"], which allows test files to import from src using shorter relative paths. For example, from test/ai/post_proccess/, the import '../../stateMachine' correctly resolves to src/stateMachine.ts due to this configuration.
Applied to files:
workspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/ballerina/ballerina-extension/test/ai/integration_tests/libs/setup.tsworkspaces/ballerina/ballerina-extension/src/features/bi/activator.tsworkspaces/ballerina/ballerina-extension/src/views/visualizer/activate.tsworkspaces/ballerina/ballerina-extension/src/stateMachine.tsworkspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.tsworkspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.tsworkspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
🧬 Code graph analysis (8)
workspaces/bi/bi-extension/src/project-explorer/activate.ts (3)
workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts (1)
ProjectExplorerEntryProvider(72-274)workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
BI_COMMANDS(31-57)workspaces/ballerina/ballerina-core/src/interfaces/bi.ts (1)
NodePosition(23-23)
workspaces/ballerina/ballerina-extension/src/views/visualizer/activate.ts (2)
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (2)
openView(821-833)StateMachine(793-819)workspaces/ballerina/ballerina-extension/src/rpc-managers/visualizer/rpc-manager.ts (1)
openView(46-59)
workspaces/ballerina/ballerina-side-panel/src/components/ModeSwitcher/index.tsx (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (1)
getDefaultExpressionMode(49-52)
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (4)
workspaces/ballerina/ballerina-extension/src/utils/project-artifacts.ts (1)
buildProjectsStructure(26-59)workspaces/ballerina/ballerina-extension/src/RPCLayer.ts (1)
notifyCurrentWebview(175-177)workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (1)
ProjectInfo(1926-1935)workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
BI_COMMANDS(31-57)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts (1)
buildCompletionSource(394-429)
workspaces/ballerina/component-diagram/src/utils/diagram.ts (1)
workspaces/ballerina/bi-diagram/src/resources/constants.ts (1)
NODE_GAP_Y(53-53)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx (1)
updateFieldsSelection(67-112)
workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts (2)
workspaces/ballerina/ballerina-core/src/interfaces/bi.ts (1)
NodePosition(23-23)workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
BI_COMMANDS(31-57)
🔇 Additional comments (24)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (1)
803-803: LGTM! Workspace-aware path handling implemented correctly.The changes properly compute workspace-relative paths for generated type files, ensuring the displayed filename in the assistant response aligns with the workspace/project layout. The logic correctly handles both absolute and relative paths, and the defensive check for both
workspacePathandprojectRootis actually more robust than similar implementations elsewhere in the file.Also applies to: 813-822, 828-828
workspaces/bi/bi-extension/package.json (1)
5-5: Version bump is appropriate; no issues found.The patch version bump from 1.5.1 to 1.5.2 is justified. The actual changes are internal refactoring and bug fixes: ensuring core commands remain reliably available and improving project refresh logic with better error handling. These do not constitute new public API features that would warrant a minor version bump per semantic versioning guidelines.
workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GeneralWidget.tsx (1)
262-267: LGTM! Menu button event handlers wired correctly.workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/AIServiceWidget.tsx (1)
112-117: LGTM! Menu button event handlers wired correctly.workspaces/ballerina/component-diagram/src/components/nodes/ConnectionNode/ConnectionNodeWidget.tsx (1)
210-215: LGTM! Menu button event handlers wired correctly.workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/GraphQLServiceWidget.tsx (1)
256-261: LGTM! Menu button event handlers wired correctly.workspaces/ballerina/component-diagram/src/components/nodes/ListenerNode/ListenerNodeWidget.tsx (1)
207-212: LGTM! Menu button event handlers wired correctly.workspaces/common-libs/ui-toolkit/src/components/Button/Button.tsx (1)
35-36: LGTM—VSCodeButton supports these props.The new mouse event handlers are correctly declared in the interface and will be passed through to VSCodeButton via the spread operator. VSCodeButton is a web component that receives standard DOM mouse events and supports React mouse props like onMouseDown and onMouseUp when using the React wrapper.
workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts (1)
20-20: LGTM! Imports are appropriate for workspace validation.The added imports support the new workspace-aware activation logic.
Also applies to: 25-27
workspaces/ballerina/ballerina-side-panel/src/components/ModeSwitcher/index.tsx (1)
34-37: Default mode RECORD for record fields is consistent with ExpressionEditorUsing
InputMode.RECORDas the primary/default mode whenisRecordTypeFieldis true aligns withExpressionEditor’s initialinputModestate and makes the ModeSwitcher behavior consistent across the stack.workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/types.ts (1)
19-28: Check for any persisted or external uses of the old"Guided"mode string
InputModenow usesRECORD = "Record"in place of the previousGUIDED = "Guided". This is fine internally, but ifInputModevalues are ever:
- stored in user settings / workspace state,
- sent over RPC / APIs, or
- embedded in configuration files,
then existing
"Guided"values will no longer map to a valid mode.Please confirm there are no such usages, or add a small migration/compat layer (e.g., mapping
"Guided"→InputMode.RECORDon load).workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx (1)
128-150: Routing RECORD throughTextModeEditormatches the new record-mode semanticsTreating
InputMode.RECORDthe same asTEXThere (renderingTextModeEditor) is consistent withExpressionEditor’s behavior where RECORD is a “primary” non-expression mode and EXP stays on the chip editor. This keeps the rendering paths aligned with the mode semantics.workspaces/ballerina/ballerina-extension/src/extension.ts (1)
157-159: BBE feature deactivation is properly implemented.The change consistently disables the BBE activation across the extension, with a clear TODO and issue reference. This aligns with the related changes in
workspaces/ballerina/ballerina-extension/src/views/bbe/activator.tsand package.json.workspaces/ballerina/ballerina-extension/src/features/bi/activator.ts (1)
201-203: Good defensive programming to prevent errors.The early return when
projectPathis undefined prevents attempts to construct an invalid file path at line 204. This is a clean guard clause that improves robustness.workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/Configurables.tsx (1)
149-149: Modification tracking flag added consistently.The
modifiedflag is set on the node before saving, which appears to be part of a modification tracking feature. This pattern is consistently applied in both this file andAddConfigurableVariables/index.tsx(line 55).workspaces/ballerina/ballerina-visualizer/src/views/BI/Configurables/AddConfigurableVariables/index.tsx (1)
55-55: Consistent modification tracking implementation.This change mirrors the modification flag addition in
Configurables.tsx(line 149), ensuring consistent tracking of config variable modifications across different save paths.workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
56-56: New BI command constant added cleanly.The
NOTIFY_PROJECT_EXPLORERconstant follows the established naming convention and enables project explorer notification functionality referenced in the PR's BI integration changes.workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx (1)
76-103: Union type member initialization logic is comprehensive.The added logic ensures that when a union/enum field is selected, an appropriate member is also selected, and its nested required fields are initialized. The implementation handles multiple scenarios:
- No member selected: picks the first appropriate member
- Member already selected: ensures its nested fields are selected
- Proper recursion for nested field selection
The direct mutation of
field.membersand nested structures is consistent with the existing pattern in this utility function, which mutates fields directly throughout (e.g., line 74:field.selected = selected).workspaces/ballerina/ballerina-extension/scripts/download-ls.js (1)
13-13: Force-replace handling and cleanup logic look correctThe
forceReplaceflag cleanly bypasses the existing-JAR early exit and ensuresLS_DIRis removed and recreated before download, while preserving the previous behavior for non-force runs. This should make prerelease and manual refresh flows deterministic without impacting normal builds.Also applies to: 181-190
workspaces/ballerina/ballerina-extension/test/ai/integration_tests/libs/setup.ts (1)
36-39: Command constants align with the simplified activation flowWith
VSCODE_COMMANDSnow limited toCLOSE_ALL_EDITORSandOPEN, the setup no longer depends on the removedballerina.showExamplescommand and relies solely on timing-based activation, which matches the updated extension behavior.workspaces/ballerina/component-diagram/src/utils/diagram.ts (1)
27-27: Listener layout refinement for unconnected nodes looks soundSeparating listeners into connected/unconnected sets, aligning connected ones to the average Y of their attached services, and then stacking unconnected listeners below all other nodes using
NODE_GAP_YandLISTENER_NODE_HEIGHTgives a predictable, non-overlapping layout. The use ofmaxYover all relevant nodes makes the “unconnected” band robust even as the diagram grows.Also applies to: 67-70, 75-86, 101-119
workspaces/ballerina/ballerina-extension/package.json (1)
5-5: Version bump and prerelease LS script are consistent with downloader changesIncrementing the extension
versionto5.6.2and addingdownload-ls:prereleaseto callscripts/download-ls.jswith--prerelease --replacealigns with the newforceReplacehandling in the downloader. The existingpostbuildbehavior remains compatible via theisPreReleaseenv flag, while the new script simplifies explicit prerelease LS refresh scenarios.Also applies to: 1183-1183
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (1)
470-480: fetchProjectInfo: behavior with missing projectPath looks saneThe updated
fetchProjectInfoservice now safely handles cases where neitherworkspacePathnorprojectPathis set by resolving with{ projectInfo: undefined }. Downstream,registerProjectArtifactsStructurealready checkscontext.projectInfobefore building structures, so this degrades gracefully when no BI-enabled project is present.workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts (1)
72-122: Tree refresh coalescing and position plumbing look good
- Refresh now coalesces concurrent calls via
_isRefreshing/_pendingRefresh, which should prevent overlappingwithProgressexecutions while still honoring queued refreshes.getProjectStructureDatadeduplicates projects byprojectPathbefore tree construction, which avoids duplicate roots when the underlying structure has repeated entries.getComponentscorrectly passescomp.positionintoProjectExplorerEntryand threads that through torevealInTreeView’s matching logic.Overall this wiring is coherent with the new NOTIFY_PROJECT_EXPLORER contract.
Also applies to: 276-336, 514-537
| const workspaceRoot = getWorkspaceRoot(); | ||
|
|
||
| const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot)); | ||
| const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot)); | ||
| const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject(); | ||
| const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE; | ||
|
|
||
| if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) { | ||
| return; | ||
| } |
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.
🧩 Analysis chain
Good defensive activation gating, but consider error handling.
The workspace validation logic correctly prevents test explorer activation in non-Ballerina contexts. However, if any of the async checks (getWorkspaceRoot, checkIsBallerinaWorkspace, checkIsBallerinaPackage, getCurrentBallerinaProject) throw an error, the entire activation will fail silently.
Consider wrapping the validation logic in a try-catch block:
+ try {
const workspaceRoot = getWorkspaceRoot();
const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot));
const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot));
const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject();
const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE;
if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) {
return;
}
+ } catch (error) {
+ // Log and return early if workspace detection fails
+ console.error('Failed to detect Ballerina workspace type:', error);
+ return;
+ }Run this script to verify that the utility functions handle edge cases properly:
🏁 Script executed:
#!/bin/bash
# Verify utility function implementations and their error handling
echo "=== Checking getWorkspaceRoot implementation ==="
ast-grep --pattern 'function getWorkspaceRoot() {
$$$
}'
ast-grep --pattern 'export function getWorkspaceRoot() {
$$$
}'
echo -e "\n=== Checking checkIsBallerinaWorkspace implementation ==="
ast-grep --pattern 'function checkIsBallerinaWorkspace($$$) {
$$$
}'
ast-grep --pattern 'export function checkIsBallerinaWorkspace($$$) {
$$$
}'
echo -e "\n=== Checking checkIsBallerinaPackage implementation ==="
ast-grep --pattern 'function checkIsBallerinaPackage($$$) {
$$$
}'
ast-grep --pattern 'export function checkIsBallerinaPackage($$$) {
$$$
}'
echo -e "\n=== Checking getCurrentBallerinaProject implementation ==="
ast-grep --pattern 'function getCurrentBallerinaProject() {
$$$
}'
ast-grep --pattern 'export function getCurrentBallerinaProject() {
$$$
}'
echo -e "\n=== Checking PROJECT_TYPE.SINGLE_FILE usage ==="
rg -n 'PROJECT_TYPE\.SINGLE_FILE' -A 2 -B 2Length of output: 6829
I need to find where these utility functions are defined and imported to assess their error handling capabilities.Let me search for the utility function implementations by looking at their imports in activator.ts.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find imports and utility function definitions
echo "=== Checking imports in activator.ts ==="
head -50 workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts
echo -e "\n=== Searching for getWorkspaceRoot definition ==="
rg -n 'getWorkspaceRoot' --type=ts -B 2 -A 10 | head -60
echo -e "\n=== Searching for checkIsBallerinaWorkspace definition ==="
rg -n 'checkIsBallerinaWorkspace' --type=ts -B 2 -A 10 | head -60
echo -e "\n=== Searching for checkIsBallerinaPackage definition ==="
rg -n 'checkIsBallerinaPackage' --type=ts -B 2 -A 10 | head -60
Length of output: 20066
I need to see the full implementations of these utility functions in the ballerina-extension to understand their error handling.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full implementations of utility functions
echo "=== checkIsBallerinaWorkspace in config.ts ==="
rg -n 'export async function checkIsBallerinaWorkspace' -A 30 workspaces/ballerina/ballerina-extension/src/utils/config.ts
echo -e "\n=== checkIsBallerinaPackage in utils ==="
rg -n 'export async function checkIsBallerinaPackage' -A 30 workspaces/ballerina/ballerina-extension/src/utils/
echo -e "\n=== getWorkspaceRoot in project-utils.ts ==="
rg -n 'export function getWorkspaceRoot' -A 15 workspaces/ballerina/ballerina-extension/src/utils/project-utils.ts
echo -e "\n=== getCurrentBallerinaProject in project-utils.ts ==="
rg -n 'export async function getCurrentBallerinaProject' -A 20 workspaces/ballerina/ballerina-extension/src/utils/project-utils.ts
Length of output: 4528
I need to search more broadly for the remaining two function implementations.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search more broadly for getWorkspaceRoot
echo "=== Searching for getWorkspaceRoot definition ==="
rg -n 'function getWorkspaceRoot' workspaces/ballerina/ballerina-extension/src/utils/ -A 10
echo -e "\n=== Searching for getCurrentBallerinaProject definition ==="
rg -n 'function getCurrentBallerinaProject' workspaces/ballerina/ballerina-extension/src/utils/ -A 15
echo -e "\n=== Checking full project-utils.ts file ==="
wc -l workspaces/ballerina/ballerina-extension/src/utils/project-utils.ts
echo -e "\n=== Looking at exports from project-utils ==="
rg -n '^export' workspaces/ballerina/ballerina-extension/src/utils/project-utils.ts | head -20
echo -e "\n=== Search for getWorkspaceRoot and getCurrentBallerinaProject in entire codebase ==="
rg -n 'getWorkspaceRoot|getCurrentBallerinaProject' workspaces/ballerina/ballerina-extension/src/utils/project-utils.ts -B 2 -A 10 | head -80
Length of output: 5685
I need to see the complete getCurrentBallerinaProject function to check its error handling.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full getCurrentBallerinaProject implementation
echo "=== Full getCurrentBallerinaProject function ==="
sed -n '45,90p' workspaces/ballerina/ballerina-extension/src/utils/project-utils.ts
echo -e "\n=== Verify getWorkspaceRoot and its usages ==="
grep -n "getWorkspaceRoot()" workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts -A 3 -B 1
echo -e "\n=== Check if Uri.file handles undefined ==="
rg -n 'Uri\.file' workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts -B 1 -A 1
Length of output: 2654
I need to verify what happens when Uri.file() receives undefined and if there's any upstream validation.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check if there's validation for undefined workspaceRoot elsewhere
echo "=== Full context of activator.ts activate function ==="
sed -n '28,50p' workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts
echo -e "\n=== Search for workspaceRoot handling in related files ==="
rg -n 'workspaceRoot' workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts -B 2 -A 2
echo -e "\n=== Check VSCode Uri.file behavior documentation ==="
rg -n 'export.*Uri' workspaces/ballerina/ballerina-extension/src/ | head -10
Length of output: 3382
Based on my verification of the codebase, I can now provide a rewritten review comment:
Null check recommended for workspace root before URI creation.
While the workspace validation logic correctly gates test explorer activation, getWorkspaceRoot() can return undefined if no workspace folders are open. If you don't have any workspace open in VS Code, like an empty window or just single/random files, workspaceFolders property won't be defined. The code passes this potentially undefined value to Uri.file() on lines 36-37 without validation.
This said, the called functions (checkIsBallerinaWorkspace, checkIsBallerinaPackage) already include try-catch blocks that gracefully handle errors and return false instead of propagating exceptions. However, adding an explicit null check improves clarity and prevents unnecessary downstream processing:
const workspaceRoot = getWorkspaceRoot();
+ if (!workspaceRoot) {
+ return;
+ }
const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot));
const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot));
const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject();
const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE;
if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) {
return;
}📝 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.
| const workspaceRoot = getWorkspaceRoot(); | |
| const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot)); | |
| const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot)); | |
| const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject(); | |
| const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE; | |
| if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) { | |
| return; | |
| } | |
| const workspaceRoot = getWorkspaceRoot(); | |
| if (!workspaceRoot) { | |
| return; | |
| } | |
| const isBallerinaWorkspace = await checkIsBallerinaWorkspace(Uri.file(workspaceRoot)); | |
| const isBallerinaProject = !isBallerinaWorkspace && await checkIsBallerinaPackage(Uri.file(workspaceRoot)); | |
| const currentProject = !isBallerinaWorkspace && !isBallerinaProject && await getCurrentBallerinaProject(); | |
| const isSingleFile = currentProject && currentProject.kind === PROJECT_TYPE.SINGLE_FILE; | |
| if (!isBallerinaWorkspace && !isBallerinaProject && !isSingleFile) { | |
| return; | |
| } |
🤖 Prompt for AI Agents
In
workspaces/ballerina/ballerina-extension/src/features/test-explorer/activator.ts
around lines 34-43, add an explicit check for workspaceRoot before calling
Uri.file(): if workspaceRoot is undefined/falsy, avoid calling Uri.file() and
instead decide activation based on getCurrentBallerinaProject (to detect
single-file projects) or return early; otherwise proceed with the existing
checks (checkIsBallerinaWorkspace / checkIsBallerinaPackage) using
Uri.file(workspaceRoot). Ensure the flow returns when there is no workspaceRoot
and no single-file project to prevent passing undefined into Uri.file.
| const [paramSelected, setParamSelected] = useState(param.selected || requiredParam); | ||
| const [selectedMemberType, setSelectedMemberType] = useState(getUnionParamName(initSelectedMember)); | ||
| const [parameter, setParameter] = useState<TypeField>(initSelectedMember); | ||
| const isInitialized = useRef(false); |
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.
Fix union member initialization guard; isInitialized is set too early
The new initialization flow looks good, but isInitialized is flipped to true unconditionally in the first useEffect, which prevents later initialization paths from ever running:
useEffect(() => { ... }, [])setsisInitialized.current = truebefore checkingparamSelected.- The second
useEffect([param.selected]) and thetoggleParamCheckpath only callinitializeUnionMember()when!isInitialized.current. - For a union param that is not initially selected, checking the checkbox will not trigger
initializeUnionMember, so no member is auto-selected even though comments indicate that behavior.
A minimal fix is to mark the ref as initialized only after initializeUnionMember() actually succeeds, in both the mount effect and the checkbox path, and to remove the unconditional assignment:
- useEffect(() => {
- // Only run initialization once
- if (isInitialized.current) {
- return;
- }
- isInitialized.current = true;
-
- // If union param is selected (or required), ensure a member is selected
- if (paramSelected && param.members && param.members.length > 0) {
- initializeUnionMember();
- }
- }, []);
+ useEffect(() => {
+ if (isInitialized.current) {
+ return;
+ }
+
+ if (paramSelected && param.members && param.members.length > 0) {
+ const initialized = initializeUnionMember();
+ if (initialized) {
+ isInitialized.current = true;
+ }
+ }
+ }, []);
@@
- if (newSelectedState) {
- // Ensure member is initialized before proceeding
- if (!isInitialized.current) {
- initializeUnionMember();
- isInitialized.current = true;
- }
+ if (newSelectedState) {
+ // Ensure member is initialized before proceeding
+ if (!isInitialized.current) {
+ const initialized = initializeUnionMember();
+ if (initialized) {
+ isInitialized.current = true;
+ }
+ }This keeps initialization idempotent while ensuring that toggling a previously unselected union param behaves as intended.
Also applies to: 45-81, 83-105, 165-176
🤖 Prompt for AI Agents
In
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx
around lines 40-43 (also applies to 45-81, 83-105, 165-176): the isInitialized
ref is being set to true unconditionally which blocks later initialization
paths; change the flow so isInitialized.current is only set to true after
initializeUnionMember() actually runs and succeeds — remove the unconditional
assignment in the mount effect, and in both the param.selected effect and the
checkbox toggle path call initializeUnionMember() when !isInitialized.current
and set isInitialized.current = true only after initializeUnionMember completes
successfully; keep initialization idempotent by guarding calls with the ref so
repeated invocations do nothing.
| /** | ||
| * Recursively search for a tree item by its path and position | ||
| */ | ||
| private findItemByPathAndPosition(targetPath: string, targetPosition: NodePosition | undefined): ProjectExplorerEntry | undefined { | ||
| for (const rootItem of this._data) { | ||
| // Check if the root item matches | ||
| if (this.matchesPathAndPosition(rootItem, targetPath, targetPosition)) { | ||
| return rootItem; | ||
| } | ||
| // Recursively search children | ||
| const found = this.searchChildrenByPathAndPosition(rootItem, targetPath, targetPosition); | ||
| if (found) { | ||
| return found; | ||
| } | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Recursively search through children for a matching path and position | ||
| */ | ||
| private searchChildrenByPathAndPosition(parent: ProjectExplorerEntry, targetPath: string, targetPosition: NodePosition | undefined): ProjectExplorerEntry | undefined { | ||
| if (!parent.children) { | ||
| return undefined; | ||
| } | ||
|
|
||
| for (const child of parent.children) { | ||
| if (this.matchesPathAndPosition(child, targetPath, targetPosition)) { | ||
| return child; | ||
| } | ||
|
|
||
| const found = this.searchChildrenByPathAndPosition(child, targetPath, targetPosition); | ||
| if (found) { | ||
| return found; | ||
| } | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Check if an item matches the given path and position | ||
| */ | ||
| private matchesPathAndPosition(item: ProjectExplorerEntry, targetPath: string, targetPosition: NodePosition | undefined): boolean { | ||
| // Path must match | ||
| if (item.info !== targetPath) { | ||
| return false; | ||
| } | ||
|
|
||
| // If no target position is provided, match by path only | ||
| if (!targetPosition) { | ||
| return true; | ||
| } | ||
|
|
||
| // If target position is provided but item has no position, don't match | ||
| if (!item.position) { | ||
| return true; // Fall back to path-only matching for items without position | ||
| } | ||
|
|
||
| // Compare positions | ||
| return this.positionsMatch(item.position, targetPosition); | ||
| } | ||
|
|
||
| /** | ||
| * Check if two positions match | ||
| */ | ||
| private positionsMatch(pos1: NodePosition, pos2: NodePosition): boolean { | ||
| return pos1.startLine === pos2.startLine && | ||
| pos1.startColumn === pos2.startColumn && | ||
| pos1.endLine === pos2.endLine && | ||
| pos1.endColumn === pos2.endColumn; | ||
| } | ||
|
|
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.
Fix path/position matching so root items don’t shadow precise matches
The new reveal helpers are a nice addition, but matchesPathAndPosition has conflicting comment vs. behavior:
// If target position is provided but item has no position, don't match
if (!item.position) {
return true; // Fall back to path-only matching for items without position
}With this, when a targetPosition is provided and the project root item has info === targetPath but no position, it will match first and short‑circuit the search, preventing a more specific child item (with the same path and a matching position) from being revealed.
Given the comment and the desire for position‑aware navigation, this should not match when a position is supplied. A small change fixes it:
- // If target position is provided but item has no position, don't match
- if (!item.position) {
- return true; // Fall back to path-only matching for items without position
- }
+ // If target position is provided but item has no position, don't match
+ if (!item.position) {
+ return false;
+ }You still get path‑only matching for calls that omit targetPosition, and position‑aware selection works correctly when a position is present.
🤖 Prompt for AI Agents
In workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
around lines 155 to 227, matchesPathAndPosition currently returns true when
targetPosition is provided but item.position is missing, which lets root items
without positions shadow more precise child matches; change that branch so when
targetPosition is supplied and item.position is undefined the function returns
false (so it won’t match), while keeping the existing behavior that if
targetPosition is undefined you still match by path-only.
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/Configurables.tsx (1)
148-162: Guard against missingdefaultValuewhen marking node as modified
node.properties.defaultValue.modified = true;assumes the entire property chain is always present. Given the rest of the file frequently uses optional chaining forpropertiesaccess, this can throw at runtime ifpropertiesordefaultValueis missing or temporarily unset in the submitted node.Consider defensively guarding this mutation (or validating earlier that
defaultValueexists) before saving, e.g.:- setIsSaving(true); - node.properties.defaultValue.modified = true; + setIsSaving(true); + if (node.properties?.defaultValue) { + (node.properties.defaultValue as any).modified = true; + } + // Optionally: log or surface an error if defaultValue is unexpectedly missing.If
defaultValueis guaranteed by theFlowNode/config-variable schema, adding an assertion or type refinement would also make this assumption explicit.
🧹 Nitpick comments (2)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx (1)
711-748: Consider performance impact of removed early-return checks.According to the summary, early-return checks that prevented updates when the value hadn't changed were removed from these onChange handlers. This means diagnostics and completions are now fetched on every keystroke, even when the value is identical. While this ensures validation always runs, it may cause unnecessary network/processing overhead.
If this change is intentional (e.g., to handle edge cases where validation state needs refreshing), consider documenting the rationale. Otherwise, restoring a value-comparison guard could improve performance:
onChange={async (updatedValue: string, updatedCursorPosition: number) => { + // Skip processing if value hasn't actually changed + if (updatedValue === value) { + return; + } // clear field diagnostics setFormDiagnostics([]);Also applies to: 781-817
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (1)
158-160: Memoization dependency may be incorrect.The
useMemodepends onprops.completions, butwaitForStateChangedoesn't directly useprops.completions—it reads fromcompletionsRef. Additionally,waitForStateChangeis recreated on every render, making this memoization ineffective.Consider wrapping
waitForStateChangewithuseCallbackand adjusting the dependency:-const waitForStateChange = (): Promise<CompletionItem[]> => { +const waitForStateChange = useCallback((): Promise<CompletionItem[]> => { return new Promise((resolve) => { const checkState = () => { if (!completionsFetchScheduledRef.current) { resolve(completionsRef.current); } else { requestAnimationFrame(checkState); } }; checkState(); }); -}; +}, []); const completionSource = useMemo(() => { return buildCompletionSource(waitForStateChange); -}, [props.completions]); +}, [waitForStateChange]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx(3 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts(2 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx(5 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/types.ts(1 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/Configurables.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx
- workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/types.ts
🧰 Additional context used
🧬 Code graph analysis (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts (1)
buildCompletionSource(571-606)
🪛 Biome (2.1.2)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts
[error] 50-50: Shouldn't redeclare 'TokenType'. Consider to delete it or rename it.
'TokenType' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (8)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx (2)
436-472: LGTM: Effect correctly sets RECORD mode for recordTypeField.The useEffect properly defaults to
InputMode.RECORDwhenrecordTypeFieldis present, with an early return to prevent mode switching. This aligns with the initialization logic and ensures consistent behavior.
474-495: LGTM: Focus behavior correctly updated for RECORD mode.The focus handler now properly checks for
InputMode.RECORD(Line 478) to open the ConfigureRecordPage, which is consistent with the mode initialization changes. The disabled autoFocus on Line 706 whenrecordTypeFieldis present prevents unintended immediate opening of the config page.workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (4)
21-21: LGTM: Import addition is appropriate.The
useMemoimport is correctly added and used for memoizing the completion source.
103-105: LGTM: Ref additions support async completion handling.The typed
completionsRefand newcompletionsFetchScheduledRefare correctly initialized to support the asynchronous completion workflow.
116-116: LGTM: Completion fetch scheduling is correct.Setting
completionsFetchScheduledRef.current = trueon change appropriately signals that completion data needs to be refreshed.
379-382: LGTM: Effect correctly completes the async completion flow.The effect properly updates
completionsRefand resetscompletionsFetchScheduledRef.current = falsewhenprops.completionschanges, allowing thewaitForStateChangepolling to resolve.workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts (2)
571-588: LGTM: Async completion source implementation is correct.The signature change to accept
() => Promise<CompletionItem[]>and return an async function properly supports asynchronous completion fetching. The pre-computation oftextBeforeCursorandlastNonSpaceCharimproves code clarity.
577-586: Incorrect review comment – ':' has not been removed as a trigger character.The code shows no removal of ':' as a completion trigger. In fact:
:is still present in the officialTRIGGER_CHARACTERSconstant inballerina-core/src/interfaces/extended-lang-client.ts- In
ChipExpressionEditor.tsxline 120,:is explicitly checked as a trigger condition- In
CodeUtils.tsbuildCompletionSource(lines 583–586), only+is blocked from showing completions; there is no guard against:, meaning:will still trigger completionsThe actual change appears to be the addition of
+as a character that blocks completions (line 584), not the removal of:. If blocking:was intended, it should also be added to the guard at line 584 to be consistent with the+blocking and the helper pane UI layer.Likely an incorrect or invalid review comment.
| const key = fieldKey ?? field.key; | ||
| const [focused, setFocused] = useState<boolean>(false); | ||
| const [inputMode, setInputMode] = useState<InputMode>(recordTypeField ? InputMode.GUIDED : InputMode.EXP); | ||
| const [inputMode, setInputMode] = useState<InputMode>(recordTypeField ? InputMode.RECORD : InputMode.EXP); |
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.
🧩 Analysis chain
Verify complete migration from GUIDED to RECORD mode.
The InputMode now defaults to RECORD when recordTypeField is present. Ensure all references to InputMode.GUIDED have been updated throughout the codebase to prevent runtime errors.
🏁 Script executed:
#!/bin/bash
# Search for any remaining references to InputMode.GUIDED or just "GUIDED" in InputMode contexts
rg -nP '\bGUIDED\b' --type=ts --type=tsx -g '!node_modules' -g '!dist' -g '!build'Length of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for remaining GUIDED references in TypeScript/TSX files
rg -nP '\bGUIDED\b' -g '*.ts' -g '*.tsx' -g '!node_modules' -g '!dist' -g '!build'
Length of output: 1707
🏁 Script executed:
#!/bin/bash
# Check InputMode enum definition to see what modes exist
rg -nA 5 'enum InputMode' -g '*.ts' -g '*.tsx' -g '!node_modules'Length of output: 853
🏁 Script executed:
#!/bin/bash
# Search for InputMode.RECORD usage to verify new mode is being used
rg -nP 'InputMode\.RECORD' -g '*.ts' -g '*.tsx' -g '!node_modules' | head -20Length of output: 861
🏁 Script executed:
#!/bin/bash
# Get context around line 437 in ExpressionEditor.tsx
sed -n '435,442p' workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsxLength of output: 193
Update outdated comment on line 437 referencing old mode name.
The comment states "always use GUIDED mode" but the code correctly uses InputMode.RECORD. Update the comment to reflect the actual mode being used:
Line 437: Change "// If recordTypeField is present, always use GUIDED mode"
to: "// If recordTypeField is present, always use RECORD mode"
The migration from InputMode.GUIDED to InputMode.RECORD is otherwise complete across the codebase.
🤖 Prompt for AI Agents
In
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx
around lines 372-437, update the outdated inline comment that still refers to
GUIDED mode; change the comment that says "// If recordTypeField is present,
always use GUIDED mode" to "// If recordTypeField is present, always use RECORD
mode" so it matches the actual InputMode.RECORD usage.
| position: SelectionRange; | ||
| } | ||
|
|
||
| export type TokenType = 'variable' | 'property' | 'parameter'; |
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.
Fix TokenType redeclaration conflict.
TokenType is already imported from ./types on Line 26, creating a naming conflict. The new type definition on Line 50 shadows the imported type, which will cause confusion and potential runtime issues when both types are referenced in the same scope.
Rename this type to avoid the conflict:
-export type TokenType = 'variable' | 'property' | 'parameter';
+export type CompletionItemType = 'variable' | 'property' | 'parameter';Then update any references to use the new name. Based on learnings.
📝 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.
| export type TokenType = 'variable' | 'property' | 'parameter'; | |
| export type CompletionItemType = 'variable' | 'property' | 'parameter'; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 50-50: Shouldn't redeclare 'TokenType'. Consider to delete it or rename it.
'TokenType' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
In
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts
around line 50 (TokenType declaration) there is a naming conflict with TokenType
imported from ./types on line 26; rename the local type to avoid shadowing
(e.g., ChipTokenType or LocalTokenType), update its export name, and update
every local reference in this file to the new name so the imported TokenType
from ./types remains unambiguous.
| const waitForStateChange = (): Promise<CompletionItem[]> => { | ||
| return new Promise((resolve) => { | ||
| const checkState = () => { | ||
| if (!completionsFetchScheduledRef.current) { | ||
| resolve(completionsRef.current); | ||
| } else { | ||
| requestAnimationFrame(checkState); | ||
| } | ||
| }; | ||
| checkState(); | ||
| }); | ||
| }; |
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.
Add cleanup mechanism for requestAnimationFrame polling.
The waitForStateChange function polls using requestAnimationFrame but has no cleanup mechanism if the component unmounts during polling. This can cause memory leaks and access to stale refs.
Consider tracking the animation frame ID and canceling it when needed:
+const animationFrameRef = useRef<number | null>(null);
+
const waitForStateChange = (): Promise<CompletionItem[]> => {
return new Promise((resolve) => {
const checkState = () => {
if (!completionsFetchScheduledRef.current) {
+ animationFrameRef.current = null;
resolve(completionsRef.current);
} else {
- requestAnimationFrame(checkState);
+ animationFrameRef.current = requestAnimationFrame(checkState);
}
};
checkState();
});
};Then add cleanup in a useEffect:
useEffect(() => {
return () => {
if (animationFrameRef.current !== null) {
cancelAnimationFrame(animationFrameRef.current);
}
};
}, []);🤖 Prompt for AI Agents
In
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx
around lines 145-156, the waitForStateChange polling uses requestAnimationFrame
without any cleanup which can leak or access stale refs if the component
unmounts; add an animationFrameRef to store the requestAnimationFrame ID, update
waitForStateChange to assign each request ID to that ref and to stop scheduling
if the component is unmounted, and add a useEffect cleanup that calls
cancelAnimationFrame on animationFrameRef.current (and clears the ref) when the
component unmounts so polling is properly cancelled.
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: 1
🧹 Nitpick comments (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts (1)
570-605: Async conversion looks good; consider adding error handling.The conversion to async/await is correctly implemented. The function now properly awaits the completion items and maintains the same filtering and transformation logic.
Consider wrapping the
await getCompletions()call in a try-catch block to gracefully handle potential failures:- const completions = await getCompletions(); + let completions: CompletionItem[]; + try { + completions = await getCompletions(); + } catch (error) { + console.error('Failed to fetch completions:', error); + return null; + }This prevents unhandled promise rejections from disrupting the completion UI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts(1 hunks)
| if (lastNonSpaceChar === '+') { | ||
| return null; | ||
| } |
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.
🧩 Analysis chain
Verify removal of : trigger character support.
The colon (:) trigger character check has been removed, and only + is now blocked. Ensure this aligns with the intended completion behavior for your expression syntax.
Run the following script to check if : is used as a trigger elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Search for colon trigger references in completion-related code
# Search for colon trigger character in completion logic
rg -nP --type=ts --type=tsx "trigger.*['\"]:['\"]|['\"]:['\"].*trigger" -g '!node_modules' -g '!dist' -g '!build'
# Search for comments or docs mentioning colon as trigger
rg -nP --type=ts --type=tsx -C2 "colon.*complet|complet.*colon" -g '!node_modules' -g '!dist' -g '!build'Length of output: 55
Let me re-run the verification with corrected syntax:
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Search for colon trigger references in completion-related code (corrected)
# Search for colon trigger character in completion logic - using correct type specification
rg -nP --type=ts "trigger.*['\"]:['\"]|['\"]:['\"].*trigger" -g '!node_modules' -g '!dist' -g '!build'
# Search for comments or docs mentioning colon as trigger
rg -nP --type=ts -C2 "colon.*complet|complet.*colon" -g '!node_modules' -g '!dist' -g '!build'
# Broader search for trigger character definitions
rg -nP --type=ts "trigger" -C2 -i "character|char" -g '!node_modules' -g '!dist' -g '!build' | head -100
Length of output: 562
🏁 Script executed:
#!/bin/bash
# Description: Examine the file under review to understand context
# View the specific function and surrounding code around lines 583-585
sed -n '570,600p' workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts
# Search for function signature and any mentions of trigger/colon in this file
rg -nP --type=ts "trigger|colon|completionTrigger" workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.tsLength of output: 1221
🏁 Script executed:
#!/bin/bash
# Description: Search for other trigger character definitions and completion logic
# Search for trigger character patterns in all TypeScript files
rg -nP --type=ts "['\"]\\+['\"]|trigger" -C3 -g '!node_modules' -g '!dist' -g '!build' | grep -E "(trigger|\\+|:)" | head -150
# Look for any configuration or constants related to triggers
rg -nP --type=ts "(const|let|var).*trigger" -C2 -g '!node_modules' -g '!dist' -g '!build'Length of output: 70911
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for TRIGGER_CHARACTERS definition
rg -nP --type=ts "TRIGGER_CHARACTERS|triggerCharacters\s*=\s*\[" -B2 -A3 -g '!node_modules' -g '!dist' -g '!build'
# Search for colon in ballerina-side-panel completion-related files
rg -nP --type=ts "['\"]:['\"]" workspaces/ballerina/ballerina-side-panel/src/components/editors/ -B2 -A2
# Look for ChipExpressionEditor completion configuration
rg -nP --type=ts "ChipExpressionEditor|buildCompletionSource" workspaces/ballerina/ballerina-side-panel/src/components/editors/ -B2 -A2 | head -50
Length of output: 21167
🏁 Script executed:
#!/bin/bash
# Check the entire buildCompletionSource function in CodeUtils.ts
sed -n '560,610p' workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts
# Search for any git history or comments about colon trigger removal
rg -nP --type=ts "colon|:\\s*trigger" workspaces/ballerina/ballerina-side-panel/ -i -B1 -A1Length of output: 1598
🏁 Script executed:
#!/bin/bash
# Look for trigger character definitions in the side-panel module
fd . workspaces/ballerina/ballerina-side-panel/src/components/editors/ --type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "triggerCharacters" | head -10
# Get the content of files that define triggerCharacters
rg -nP --type=ts "triggerCharacters.*:" workspaces/ballerina/ballerina-side-panel/ -B1 -A5 | head -100Length of output: 3438
Incomplete removal of : trigger character blocking creates inconsistency.
The colon (:) check has been removed from buildCompletionSource (lines 583-585), but : is still treated as a trigger character in ChipExpressionEditor.tsx:120 (const isTrigger = lastNonSpaceChar === '+' || lastNonSpaceChar === ':';). Additionally, : is defined in the global TRIGGER_CHARACTERS constant at workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts:1086 as [':', '.', '>', '@', '/', '\\', '?'].
Update the blocking logic in buildCompletionSource to include : if it should remain a blocked trigger character, or update ChipExpressionEditor to remove : if it should not trigger completion blocking.
🤖 Prompt for AI Agents
In
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts
around lines 583-585, the conditional that blocks completion currently checks
for '+' only; restore consistency with ChipExpressionEditor and the global
TRIGGER_CHARACTERS by adding a check for ':' as well (i.e., treat
lastNonSpaceChar === ':' the same as '+'), so update the if to include the colon
check and return null when lastNonSpaceChar is ':'.
This PR syncs changes from release-ballerina to the main branch.
Automated PR created by GitHub Actions.
Summary by CodeRabbit
New Features
Bug Fixes
Removed
Changed
✏️ Tip: You can customize this high-level summary in your review settings.