Skip to content

Conversation

@cbandera
Copy link
Contributor

This PR implements a synchronization between the file opened in the editor and the workspace tree view of the bazel extension.

Note on implementation:
The implementation of reveal() in the tree view required proper parent-child relationships and package path resolution. The changes ensure that:

  1. The tree view can properly navigate and reveal items by maintaining correct parent references
  2. Package paths are consistently resolved through the parent chain
  3. The selected item in the tree view is synchronized with the currently open editor

A cache of BazelPackageTreeItems was added to improve performance by:

  • Reducing redundant package path calculations
  • Avoiding bazel queries performed by existing getChildren operator
  • Minimizing repeated string operations when resolving package paths

This PR is part of #353

@cbandera
Copy link
Contributor Author

cbandera commented Aug 1, 2025

@cameron-martin , @vogelsgesang I assumed the tests might be failing because the github runner env doesn't have bazel installed, but adding it to the workflow file didn't solve the problem. The tests run through locally (even though I got the same error once). So they might also be flaky - I have the feeling it could have something to do with timing. Maybe we need to wait longer inside the testcase, but I am unsure how and where to do it.
Any hints on how to debug this are welcome.

Update: I solved it now by explicitly calling the asynchronous sync function and awaiting it's result. I guess that's good enough for the test, but if someone knows of a more elegant way to do this feel free to comment.
Update 2: 😝 Oh man, that only fixed it locally. I also added a step in CI to build the workspace once to assure the local bazel instance is warmed up to speed up queries within the test cases. But still no success.

@cbandera cbandera force-pushed the feature/353-sync-tree-view branch 2 times, most recently from cb48761 to a4b0bb4 Compare August 2, 2025 18:26
@cbandera
Copy link
Contributor Author

@cameron-martin , @vogelsgesang I have deactivated the one problematic test case for now, so that this PR becomes green anyway.
As stated before, the feature works when manually starting the extension and performing this test case, I just can't get the automated test to work, probably due to timing/concurrency issues.
If you have the time to analyze or even fix the testcase, feel free to do so. Otherwise I am also happy for any advice you can give.

@cameron-martin
Copy link
Collaborator

Looks like the tests are failing even though you disabled the problematic test. Could you take a look at this?

@cameron-martin cameron-martin added the waiting-for-author Waiting for the PR author to address review feedback or make changes. label Nov 8, 2025
@cbandera cbandera force-pushed the feature/353-sync-tree-view branch from 0d67415 to ac3d667 Compare November 9, 2025 15:01
@cbandera
Copy link
Contributor Author

cbandera commented Nov 9, 2025

@cameron-martin I had to update the test to recent changes on master. The pipeline is now successfull again.
But I am still at a loss at why the last test is not working. There must be some things screwed up with respect to global state and/or timing or concurrency. I am trying to look into this again, but as the feature is working locally, I vote for merging this PR without the automated test and try to add that in a followup.

@cbandera cbandera force-pushed the feature/353-sync-tree-view branch 2 times, most recently from fd9422c to af53df2 Compare November 9, 2025 18:19
@cbandera
Copy link
Contributor Author

cbandera commented Nov 9, 2025

Ok found it and fixed it. The problem was that due to the way the test was setup, there were multiple instances of the treeview provider which messed things up.
I now changed it to use the one single instance of the running extension for testing. Which fixes all tests.

The implementation of reveal() in the tree view required proper parent-child
relationships and package path resolution. The changes ensure that:

1. The tree view can properly navigate and reveal items by maintaining
   correct parent references
2. Package paths are consistently resolved through the parent chain
3. The selected item in the tree view is synchronized with the currently open editor

A cache of BazelPackageTreeItems was added to improve performance by:

- Reducing redundant package path calculations
- Avoiding bazel queries performed by existing getChildren operator
- Minimizing repeated string operations when resolving package paths

Part of bazel-contrib#353
@cbandera cbandera force-pushed the feature/353-sync-tree-view branch from af53df2 to f3af823 Compare November 26, 2025 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-author Waiting for the PR author to address review feedback or make changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants