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.

Sign up for GitHub

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

Jump to bottom

Fix 9914: vscode builtin GIT showing incorrect tree state #9915

Merged
merged 1 commit into from
Aug 18, 2021
Merged

Fix 9914: vscode builtin GIT showing incorrect tree state #9915

merged 1 commit into from
Aug 18, 2021

Conversation

EstherPerelman
Copy link
Contributor

@EstherPerelman EstherPerelman commented Aug 17, 2021 •
edited
Loading

Signed-off-by: Esther Perelman esther.perelman@sap.com

What it does

Fixes #9914 by adding a listener to git resources changes

How to test

Follow #9914 Steps to Reproduce

Review checklist

Reminder for reviewers

@EstherPerelman EstherPerelman mentioned this pull request Aug 17, 2021
Closed
@msujew msujew self-requested a review August 17, 2021 09:19
msujew
msujew approved these changes Aug 17, 2021
View reviewed changes
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 confirm that the issue exists on master and is addressed by this change nicely:

  • On master installing git 1.50.0 or 1.52.1 (version linked in the issue), both exhibit the issue ❌
  • With your changes in place, both extension versions behave as expected ✔️

Clean solution, code looks good to me 👍

@msujew msujew added scm issues related to the source control manager vscode issues related to VSCode compatibility labels Aug 17, 2021
vince-fugnitto
vince-fugnitto reviewed Aug 17, 2021
View reviewed changes
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.

@EstherPerelman I confirmed that the bug exists on master and the pull-request resolves the problem 👍

My only nitpick would be to update the commit message to provide additional details as to why the changes are necessary, it was not immediately obvious looking at the code.

@EstherPerelman EstherPerelman force-pushed the fix-9914 branch 2 times, most recently from 302b526 to bb18526 Compare August 18, 2021 08:42
@EstherPerelman
Fix 9914: vscode builtin git showing incorrect tree state,
b698540 
Due to incorrect `scm` tree view after executing file actions (like `stage`/`unstage`),
Added `onDidChangeResources` listener to the `scm` provider which recreates `scm` tree on every resource change.

Signed-off-by: Esther Perelman <esther.perelman@sap.com>
@EstherPerelman
Copy link
Contributor Author

@EstherPerelman I confirmed that the bug exists on master and the pull-request resolves the problem 👍

My only nitpick would be to update the commit message to provide additional details as to why the changes are necessary, it was not immediately obvious looking at the code.

@vince-fugnitto I have updated the commit, Is that clearer now? (if it is - please merge it), thanks.

@vince-fugnitto vince-fugnitto merged commit 2ee5bb7 into eclipse-theia:master Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vince-fugnitto vince-fugnitto vince-fugnitto left review comments

@msujew msujew msujew approved these changes

Assignees
No one assigned
Labels
scm issues related to the source control manager vscode issues related to VSCode compatibility
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vscode builtin GIT showing incorrect tree state
3 participants
@EstherPerelman @msujew @vince-fugnitto