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

Cancel Tree.refresh when new root set #11340

Conversation

colin-grant-work
Copy link
Contributor

What it does

On application startup, these errors are logged by the scm package:

image

This PR addresses the error by modifying the logic for handling set root on the TreeImpl so that it cancels a refresh cycle when a new root is set. Previously, refresh would continue running and eventually discover that the root with which it began no longer corresponded to the current root of the tree, producing the error.

How to test

  1. Start the application.
  2. Assert that the errors noted above no longer occur.
  3. Trees should continue to work normally: since refresh is called every time set root runs, the final refresh should still run to completion.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added the tree issues related to the tree (ex: tree widget) label Jun 24, 2022
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.

Looks good to me 👍 I can confirm that the issue exists on master and is addressed by this change.

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.

LGTM 👍

@colin-grant-work colin-grant-work merged commit 943f64b into eclipse-theia:master Jun 28, 2022
@colin-grant-work colin-grant-work deleted the bugfix/file-change-error branch June 28, 2022 16:54
@colin-grant-work colin-grant-work added this to the 1.27.0 milestone Jun 29, 2022
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants