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

Wait for document to be saved before running query #947

Merged
merged 2 commits into from
Sep 7, 2021

Conversation

hmac
Copy link
Contributor

@hmac hmac commented Sep 7, 2021

If AUTOSAVE_SETTING is true and I trigger the Run Query command, I occasionally see race conditions where the query is run before the file is saved, and I see results corresponding to the previous version of the query. I'm not sure how to test this and I haven't been able to reliably reproduce the problem (though it occurs often).

Intuitively, it seems like this change should fix it, since editor.document.save returns a Thenable, which I think means it returns a promise or the function is async.

My javascript is very rusty, sorry if this is inaccurate!

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • @github/docs-content-codeql has been cc'd in all issues for UI or other user-facing changes made by this pull request.

This prevents a race condition where the query runs before the editor has saved the file.
@hmac hmac requested a review from a team as a code owner September 7, 2021 13:43
Copy link
Contributor

@shati-patel shati-patel left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @hmac! This looks good 🎉

It's technically a user-facing change, so you could add an entry to the CHANGELOG (just under the "unreleased" header). Maybe something like

- Fix bug where a query is sometimes run before the file is saved. [#947](https://github.com/github/vscode-codeql/pull/947)

@hmac
Copy link
Contributor Author

hmac commented Sep 7, 2021

@shati-patel done - thanks for the quick review!

@shati-patel shati-patel enabled auto-merge (rebase) September 7, 2021 14:56
@shati-patel shati-patel merged commit db529d5 into main Sep 7, 2021
@shati-patel shati-patel deleted the hmac-wait-for-save branch September 7, 2021 14: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.

2 participants