Skip to content
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

Fix tree view selection #9673

Merged
merged 1 commit into from
Jun 30, 2021

Conversation

xcariba
Copy link
Contributor

@xcariba xcariba commented Jun 30, 2021

Signed-off-by: Alexander Kozinko [email protected]

What it does

Fixes tree view selection #9672. Original implementation always checked old selection state, I think it was a typo.

How to test

  1. add the vscode-file-explorer-selection plugin to the example application
  2. start the application, open the explorer and perform a selection in the tree-view file-explorer
  3. execute the command FileExplorer: Echo Selection
  4. a notification should appear with the selection path - on master no selection is present

Review checklist

Reminder for reviewers

Signed-off-by: Alexander Kozinko [email protected]
@msujew
Copy link
Member

msujew commented Jun 30, 2021

Thank you for the contribution. I imagine it works as expected, but could you please provide a working example in the form of a minimal git repo or in a .vsix file (wrapped in a .zip file)? It would make reviewing this PR a lot easier.

@vince-fugnitto vince-fugnitto added tree issues related to the tree (ex: tree widget) vscode issues related to VSCode compatibility labels Jun 30, 2021
@vince-fugnitto
Copy link
Member

We can use the following plugin for test-purposes (modified version of vscode-tree-view-sample:

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed that the changes successfully set the selection using the example plugin:

image

On master no selection was present.

@vince-fugnitto
Copy link
Member

@msujew do you mind performing a quick review as well? I believe it can fit for today's release.

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

Looks good to me. I confirm the bug on master and its absence with this code, and the diagnosis of the fault as a typo seems eminently plausible :-).

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

I can also confirm that the fix is working.

@vince-fugnitto vince-fugnitto merged commit 0a87508 into eclipse-theia:master Jun 30, 2021
@vince-fugnitto vince-fugnitto added this to the 1.15.0 milestone Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tree issues related to the tree (ex: tree widget) vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants