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 spurious external change warnings #1354

Merged
merged 4 commits into from
Jul 24, 2023
Merged

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented Jul 20, 2023

Since recent changes to external change handling it was noticed that spurious warnings of external changes were produced while editing. This was traced to file status checking not completing before autosaving commenced because source loading is asynchronous.

This PR makes sure file status checking completes before saving starts when autosaving.

@jeremypw jeremypw marked this pull request as ready for review July 20, 2023 15:01
@jeremypw jeremypw requested a review from a team July 20, 2023 15:01
@jeremypw
Copy link
Collaborator Author

As this issue depends on semi-random events order and timing, it was tested by using it for intensive coding for a while without spurious warnings.

@jeremypw jeremypw marked this pull request as draft July 20, 2023 18:03
@jeremypw
Copy link
Collaborator Author

Converting to draft as started getting unexplained closing of Code. Investigating.

@jeremypw jeremypw marked this pull request as ready for review July 20, 2023 18:08
@jeremypw
Copy link
Collaborator Author

OK, its to do with a certain file causing problems with the Symbol Pane - nothing to do with this PR.

@jeremypw jeremypw marked this pull request as draft July 20, 2023 18:33
@jeremypw
Copy link
Collaborator Author

It could be something to do with this although it only seems to affect a certain file - need to investigate effect of changes in timing of autosave on the outline plugin. Seems to sometimes only partially parse the file - or not at all.

@jeremypw jeremypw marked this pull request as ready for review July 21, 2023 08:39
@jeremypw
Copy link
Collaborator Author

Now confident that the outline issue is unrelated to the PR.

@jeremypw jeremypw mentioned this pull request Jul 14, 2023
10 tasks
Copy link
Contributor

@zeebok zeebok left a comment

Choose a reason for hiding this comment

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

Tried to repro for like 5-10 minutes but it wasn't happening. The code seems to make sense though. If you really want me to try and see if it works, I can keep testing but for now I approve.

@jeremypw
Copy link
Collaborator Author

@zeebok Thanks for testing. Its one of those annoying timing issues that is not always reproducible. I'd guess it might be more likely to happen with a large document that takes a longer time to status check.

@jeremypw
Copy link
Collaborator Author

I think I'll merge this but wait a while for further dog-fooding before finalizing the release.

@jeremypw jeremypw merged commit 077ed4b into master Jul 24, 2023
6 checks passed
@jeremypw jeremypw deleted the fix-spurious-external-changes branch July 24, 2023 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants