Skip to content

Conversation

@gigara
Copy link
Contributor

@gigara gigara commented Dec 1, 2025

Purpose

$subject
Resolves #1028

Goals

Describe the solutions that this feature/fix will introduce to resolve the problems described above

Approach

Describe how you are implementing the solutions. Include an animated GIF or screenshot if the change affects the UI (email [email protected] to review all UI text). Include a link to a Markdown file or Google doc if the feature write-up is too long to paste here.

UI Component Development

Specify the reason if following are not followed.

  • Added reusable UI components to the ui-toolkit. Follow the intructions when adding the componenent.
  • Use ui-toolkit components wherever possible. Run npm run storybook from the root directory to view current components.
  • Matches with the native VSCode look and feel.

Manage Icons

Specify the reason if following are not followed.

  • Added Icons to the font-wso2-vscode. Follow the instructions.

User stories

Summary of user stories addressed by this change>

Release note

Brief description of the new feature or bug fix as it will appear in the release notes

Documentation

Link(s) to product documentation that addresses the changes of this PR. If no doc impact, enter “N/A” plus brief explanation of why there’s no doc impact

Training

Link to the PR for changes to the training content in https://github.com/wso2/WSO2-Training, if applicable

Certification

Type “Sent” when you have provided new/updated certification questions, plus four answers for each question (correct answer highlighted in bold), based on this change. Certification questions/answers should be sent to [email protected] and NOT pasted in this PR. If there is no impact on certification exams, type “N/A” and explain why.

Marketing

Link to drafts of marketing content that will describe and promote this feature, including product page changes, technical articles, blog posts, videos, etc., if applicable

Automation tests

  • Unit tests

    Code coverage information

  • Integration tests

    Details about the test cases and coverage

Security checks

Samples

Provide high-level details about the samples related to this feature

Related PRs

List any other related PRs

Migrations (if applicable)

Describe migration steps and platforms on which migration has been tested

Test environment

List all JDK versions, operating systems, databases, and browser/versions on which this feature/fix was tested

Learning

Describe the research phase and any blog posts, patterns, libraries, or add-ons you used to solve the problem.

Summary by CodeRabbit

  • Refactor

    • Reorganized refresh command registration for streamlined runtime behavior.
    • Enhanced event handling for text document changes with improved type safety.
    • Cleaned up debug logging.
  • Bug Fixes

    • Extended refresh support to include XML files alongside other document types.

✏️ Tip: You can customize this high-level summary in your review settings.

@gigara gigara requested a review from hevayo as a code owner December 1, 2025 09:34
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 1, 2025

Walkthrough

The PR refactors refresh command registration in both BI and MI extensions from conditional compile-time wiring to runtime branching. Commands are always registered under a unified name, but execution checks the WI context at runtime to delegate appropriately. Minor logging cleanup and event parameter typing fixes are included.

Changes

Cohort / File(s) Summary
Refresh command refactoring
workspaces/bi/bi-extension/src/project-explorer/activate.ts, workspaces/mi/mi-extension/src/project-explorer/activate.ts
Moved conditional command registration from compile-time to runtime. Both files now unconditionally register REFRESH_COMMAND but branch at execution: if isInWI is true, delegate to WI refresh; otherwise call provider's refresh method.
Constants and command registration
workspaces/mi/mi-extension/src/constants/index.ts
Added WI_PROJECT_EXPLORER_VIEW_REFRESH to COMMANDS object with value 'wso2-integrator.explorer.refresh'. Removed WI_PROJECT_EXPLORER_VIEW_REFRESH_COMMAND export. Prepended "xml" to REFRESH_ENABLED_DOCUMENTS array.
Event handling
workspaces/mi/mi-extension/src/visualizer/activate.ts
Updated onDidChangeTextDocument handler to receive typed event parameter (vscode.TextDocumentChangeEvent) and access document via event object properties (e.document) instead of direct parameter.
Code cleanup
workspaces/mi/mi-extension/src/stateMachine.ts
Removed debug logging statement that output all VSCode extension IDs during state machine initialization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Changes follow a consistent refactoring pattern across multiple files with straightforward conditional logic
  • Event parameter typing fix is a standard improvement with no logic changes
  • Logging removal is trivial
  • Limited scope focused on command registration and minor fixes
  • Attention points:
    • Verify runtime isInWI branching logic works correctly for both WI and non-WI contexts
    • Confirm COMMANDS.REFRESH_COMMAND constant is properly defined and wired in both extensions
    • Test refresh functionality in both BI and MI with WI context enabled/disabled

Possibly related PRs

  • #870: Overlapping changes to MI extension's stateMachine and visualizer code paths (logging/event handling modifications)
  • #901: Related refresh command registration pattern changes in project-explorer with WI context branching using isInWI logic

Suggested reviewers

  • hevayo
  • ChinthakaJ98
  • kanushka

Poem

🐰 Refresh commands now branch with grace,
Runtime checks find their place,
WI whispers its context true,
Trees refresh both old and new!
Events typed, logs cleaned away—
The explorer works today!

Pre-merge checks and finishing touches

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description contains only placeholder text ($subject, template sections with > markers) and lacks concrete implementation details, approach explanation, test results, and specific fixes. Replace placeholder text with actual implementation details, explain the approach to fix tree view refresh in WI mode, and provide concrete information for Goals, Approach, and other relevant sections.
Out of Scope Changes check ⚠️ Warning Most changes are in-scope for fixing tree view refresh, but removal of debug logging in stateMachine.ts and event handler parameter typing in visualizer/activate.ts appear unrelated to the tree view refresh bug. Clarify the necessity of stateMachine.ts debug logging removal and visualizer event handler changes, or move them to a separate PR if they are unrelated improvements.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix treeview in WI mode' directly aligns with the main change objective to fix tree view refresh functionality in WI mode, matching the linked issue #1028.
Linked Issues check ✅ Passed The code changes address the bug where tree view refresh was not working in WI mode by reworking the refresh command registration to conditionally execute WI_PROJECT_EXPLORER_VIEW_REFRESH when in WI mode [#1028].
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch wso2-integrator

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
workspaces/mi/mi-extension/src/visualizer/activate.ts (1)

211-255: Use e.document is correct; add a projectUri guard in the pom.xml branch

Typing the onDidChangeTextDocument handler as e: vscode.TextDocumentChangeEvent and switching all usage to e.document is the right way to consume this event and aligns the logic with VS Code’s API.

In the pom.xml branch, though:

if (e.document.uri.fsPath.endsWith('pom.xml')) {
    const projectUri = vscode.workspace.getWorkspaceFolder(e.document.uri)?.uri.fsPath;
    const langClient = getStateMachine(projectUri!).context().langClient;
    
}

If the user edits a pom.xml that is not in any workspace folder, workspace.getWorkspaceFolder(...) will be undefined, projectUri will be undefined, and projectUri! passed to getStateMachine will throw.

A small guard avoids this crash:

-        if (e.document.uri.fsPath.endsWith('pom.xml')) {
-            const projectUri = vscode.workspace.getWorkspaceFolder(e.document.uri)?.uri.fsPath;
-            const langClient = getStateMachine(projectUri!).context().langClient;
+        if (e.document.uri.fsPath.endsWith('pom.xml')) {
+            const projectUri = vscode.workspace.getWorkspaceFolder(e.document.uri)?.uri.fsPath;
+            if (!projectUri) {
+                return;
+            }
+            const langClient = getStateMachine(projectUri).context().langClient;
🧹 Nitpick comments (1)
workspaces/mi/mi-extension/src/stateMachine.ts (1)

695-710: WI mode detection in state machine context looks consistent

Initializing isInWI via vscode.extensions.getExtension(WI_EXTENSION_ID) and threading it through the state machine context lines up with how waitForLS, activateOtherFeatures, and focusProjectExplorer choose the tree view ID. This should allow MI logic to correctly distinguish WI vs standalone at runtime.

If you later need more nuanced detection (e.g., WI extension installed but not actually hosting MI), consider centralizing this check behind a helper, but the current approach is fine for now.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dfcd6c4 and b2e63d0.

📒 Files selected for processing (5)
  • workspaces/bi/bi-extension/src/project-explorer/activate.ts (1 hunks)
  • workspaces/mi/mi-extension/src/constants/index.ts (2 hunks)
  • workspaces/mi/mi-extension/src/project-explorer/activate.ts (2 hunks)
  • workspaces/mi/mi-extension/src/stateMachine.ts (1 hunks)
  • workspaces/mi/mi-extension/src/visualizer/activate.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-26T06:35:19.217Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:173-178
Timestamp: 2025-11-26T06:35:19.217Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the commented-out debugging block in the verifyFileContent function (lines 172-177 containing console.log, page.pause, and updateDataFileSync) is intentionally kept as a developer utility for updating test data files when needed. It should not be removed.

Applied to files:

  • workspaces/mi/mi-extension/src/stateMachine.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.{ts,tsx} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values

Applied to files:

  • workspaces/mi/mi-extension/src/constants/index.ts
📚 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/mi/mi-extension/src/constants/index.ts
🧬 Code graph analysis (2)
workspaces/mi/mi-extension/src/project-explorer/activate.ts (1)
workspaces/mi/mi-extension/src/constants/index.ts (1)
  • COMMANDS (19-118)
workspaces/bi/bi-extension/src/project-explorer/activate.ts (2)
workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
  • BI_COMMANDS (31-58)
workspaces/bi/bi-extension/src/constants/index.ts (1)
  • WI_PROJECT_EXPLORER_VIEW_REFRESH_COMMAND (24-24)
🔇 Additional comments (3)
workspaces/bi/bi-extension/src/project-explorer/activate.ts (1)

72-94: BI refresh delegation is aligned with WI mode; consider returning the underlying promises and verify WI doesn't loop back

The new registerCoreCommands wiring for BI_COMMANDS.REFRESH_COMMAND:

  • In WI mode: commands.executeCommand(WI_PROJECT_EXPLORER_VIEW_REFRESH_COMMAND);
  • Otherwise: dataProvider.refresh();

Functionally this matches the MI pattern and should make BI refresh behave correctly under WI.

Two follow‑ups worth considering:

  1. Avoid potential recursion with WI
    Ensure that WI's wso2-integrator.explorer.refresh handler does not call BI_COMMANDS.REFRESH_COMMAND again; otherwise, the two commands could recurse.

  2. Optionally return the underlying promise
    If any callers await commands.executeCommand(BI_COMMANDS.REFRESH_COMMAND), you may want to:

    commands.registerCommand(BI_COMMANDS.REFRESH_COMMAND, () => {
        if (isInWI) {
            return commands.executeCommand(WI_PROJECT_EXPLORER_VIEW_REFRESH_COMMAND);
        }
        return dataProvider.refresh();
    });

    This mirrors the MI implementation style and propagates completion.

workspaces/mi/mi-extension/src/constants/index.ts (1)

55-57: Command and refresh-document constants are wired consistently

  • Adding COMMANDS.WI_PROJECT_EXPLORER_VIEW_REFRESH = 'wso2-integrator.explorer.refresh' centralizes the WI refresh command string and is correctly consumed in the MI project-explorer refresh handler.
  • Updating REFRESH_ENABLED_DOCUMENTS to ["xml", "SynapseXml", "typescript", "markdown", "json"] correctly aligns with the visualizer's languageId filtering in both onDidChangeTextDocument (line 223) and onDidSaveTextDocument (line 324) handlers in visualizer/activate.ts.
workspaces/mi/mi-extension/src/project-explorer/activate.ts (1)

23-61: Unified refresh command wiring for MI is correct and safe

The new COMMANDS.REFRESH_COMMAND handler correctly branches on isInWI:

  • In WI mode: delegates to COMMANDS.WI_PROJECT_EXPLORER_VIEW_REFRESH (wso2-integrator.explorer.refresh) with early return.
  • Otherwise: calls and returns projectExplorerDataProvider.refresh().

Verification confirms this pattern is sound. The BI extension uses the identical delegation approach (BI.project-explorer.refresh → wso2-integrator.explorer.refresh), indicating this is a tested and intentional design. The early return in WI mode prevents double-refresh, and there is no evidence of cross-extension recursion within the MI codebase. The one-way delegation structure is secure.

@gigara gigara merged commit 6735fb8 into main Dec 1, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Tree view refresh not working in WI

3 participants