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

Code Folding won't be remembered after latest update. (fixes #26157) #26322

Merged
merged 1 commit into from
May 10, 2017

Conversation

bpasero
Copy link
Member

@bpasero bpasero commented May 9, 2017

This is a fix for #26157 intended to be pushed to the recovery release.

Issue (original):

  • open an editor
  • keep some view state (folding, cursors, scroll position)
  • close the editor
  • reopen the editor
    => view state is lost

Issue (variant):

  • make sure tab preview is enabled (it is by default)
  • open an editor
  • keep some view state (folding, cursors, scroll position)
  • open another editor
  • go back to the previous editor
    => view state is lost

We always store the view state of files when you switch between editors or when you close and come back to editors (persisted in local storage). The bug happened during last milestone because the way we dispose resources changed.

Previously:

  • close an editor
  • persist view state from the editor model
  • dispose editor model

Now:

  • close an editor
  • dispose editor model
  • persist view state from editor model
    => since the model is disposed, the view state is undefined

My fix introduces a new event onWillCloseEditor directly on the stacks model which the text file editor can use to persist the view state right before the model goes away. On top of that we also only persist the view state when the input is not disposed, because otherwise we still hit the current issue that we try to persist the view state of a disposed model.

@Tyriar can you check the code and let me know what you think? If you guys want to move ahead and release recovery today, feel free to merge. Otherwise I can do it tomorrow.

@bpasero bpasero requested a review from isidorn May 9, 2017 16:23
@bpasero bpasero added this to the April Recovery 2017 milestone May 9, 2017
@bpasero bpasero self-assigned this May 9, 2017
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Not sure why the breaking change to behavior was made, but the fix seems reasonable. I'll let you merge it when it's been reviewed as we're pushing the recovery build out to Monday.

@bpasero
Copy link
Member Author

bpasero commented May 10, 2017

@Tyriar no real reason, just a new way of dealing with file resources. Bottom line is that I adopted model references for file editor input and closing an editor will dispose them directly where previously someone outside would listen for editor changes and then dispose models. Looks like previously the listener outside just came a bit later than the editor itself persisting the view state and now its turned around. This was for #17073

@bpasero bpasero merged commit 52129db into release/1.12 May 10, 2017
@bpasero bpasero deleted the stable/26157 branch May 10, 2017 09:53
@egamma egamma removed this from the April Recovery 2017 milestone May 16, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants