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

Fixes source control remember tree collapsed state #89313

Merged
merged 7 commits into from Nov 4, 2020

Conversation

jeanp413
Copy link
Contributor

This PR fixes #89145

Comment on lines -52 to +56
element = children[0];

if (element.incompressible) {
let childElement: ICompressedTreeElement<T> = children[0];
if (childElement.incompressible) {
break;
}
element = childElement;
Copy link
Contributor Author

@jeanp413 jeanp413 Jan 27, 2020

Choose a reason for hiding this comment

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

I was getting an incorrect behavior when an ISCMResourceGroup contained just one element.
The issue was that if a child was incompressible, the returned compressed element was using the collapsible and collapsed attribute from said child. This changes fix that.
scmrememberstatebug

Copy link
Member

Choose a reason for hiding this comment

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

Fantastic find and fix!

@joaomoreno joaomoreno added the scm General SCM compound issues label Jan 27, 2020
@Trass3r
Copy link

Trass3r commented Oct 3, 2020

This has been rotting for more than 8 months.
Some review and reaction on this would be nice @microsoft @joaomoreno.

joaomoreno added a commit that referenced this pull request Nov 4, 2020
Copy link
Member

@joaomoreno joaomoreno left a comment

Choose a reason for hiding this comment

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

@jeanp413 Once again, fantastic work. Thanks a lot of keeping the changes up to date and for your patience! 🙏

Apart from the inline comments I found other issues:

  • When using the tree view mode, new folders appeared as collapsed by default. For that reason I've changed the state to store collapsed folders, instead of expanded ones.

I've addressed my own feedback.

const storageMode = this.storageService.get(`scm.viewMode`, StorageScope.WORKSPACE) as ViewModelMode;
let treeViewState: ITreeViewState | undefined;

const raw = this.storageService.get(`scm.viewState`, StorageScope.WORKSPACE);
Copy link
Member

Choose a reason for hiding this comment

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

This breaks the scm.viewMode key. Let's keep each state in its separate key.

Comment on lines -52 to +56
element = children[0];

if (element.incompressible) {
let childElement: ICompressedTreeElement<T> = children[0];
if (childElement.incompressible) {
break;
}
element = childElement;
Copy link
Member

Choose a reason for hiding this comment

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

Fantastic find and fix!

@@ -783,6 +798,8 @@ class ViewModel {

configurationService.onDidChangeConfiguration(this.onDidChangeConfiguration, this, this.disposables);
this.onDidChangeConfiguration();

this.disposables.add(this.tree.onDidChangeCollapseState(() => this.updateViewState()));
Copy link
Member

Choose a reason for hiding this comment

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

This might be too expensive. We really only need to snapshot the view state whenever the view goes invisible or when the workbench shuts down.

@joaomoreno joaomoreno added this to the November 2020 milestone Nov 4, 2020
@joaomoreno joaomoreno merged commit 0382313 into microsoft:master Nov 4, 2020
@jeanp413 jeanp413 deleted the fix-89145 branch November 4, 2020 15:58
@github-actions github-actions bot locked and limited conversation to collaborators Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
scm General SCM compound issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Source Control select default setting show/hide untracked changes tab
3 participants