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

Make error messages clearer for some common problems #702

Merged
merged 1 commit into from
Dec 9, 2020

Conversation

aeisenberg
Copy link
Contributor

@aeisenberg aeisenberg commented Dec 9, 2020

  1. Clicking on query history menu items when nothing is selected. Error
    message is clearer. It would be better to disable when nothing is
    selected, but waiting on
    Enablement using listDoubleSelection and listMultiSelection not working in contributed view microsoft/vscode#99767 to be released.
  2. Trying to run query with a missing or invalid qlpack has better
    message.
  3. Better hover text for "Open query".

Checklist

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

@aeisenberg aeisenberg force-pushed the aeisenberg/message-cleanup branch from 4cf7f0b to 69a4cac Compare December 9, 2020 19:41
extensions/ql-vscode/CHANGELOG.md Outdated Show resolved Hide resolved
extensions/ql-vscode/package.json Outdated Show resolved Hide resolved
@@ -310,6 +310,10 @@ export class QueryHistoryManager extends DisposableObject {
return;
}

if (!finalSingleItem) {
throw new Error('No query selected. Select a query history item you have already run and try again.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Factor out to constant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

@@ -345,7 +345,7 @@ async function promptUserToSaveChanges(document: TextDocument): Promise<boolean>
else {
const yesItem = { title: 'Yes', isCloseAffordance: false };
const alwaysItem = { title: 'Always Save', isCloseAffordance: false };
const noItem = { title: 'No (run anyway)', isCloseAffordance: false };
const noItem = { title: 'No (run version on disk)', isCloseAffordance: false };
Copy link
Contributor

Choose a reason for hiding this comment

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

Does 'the last saved version on disk' make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When would it ever not be the last saved version (unless you have a funky filesystem that automatically saves backup versions)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I expect never: what you've written is correct, I'm just wondering whether we can make the message even more obvious.

extensions/ql-vscode/src/run-queries.ts Outdated Show resolved Hide resolved
1. Clicking on query history menu items when nothing is selected. Error
   message is clearer. It would be better to disable when nothing is
   selected, but waiting on
   microsoft/vscode#99767 to be released.
2. Trying to run query with a missing or invalid qlpack has better
   message.
3. Better hover text for "Open query".

Co-authored-by: Aditya Sharad <[email protected]>
@aeisenberg aeisenberg merged commit b470e41 into github:main Dec 9, 2020
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