Skip to content

Add cancellation from query history view#1105

Merged
aeisenberg merged 2 commits intomainfrom
aeisenberg/history-cancel
Feb 2, 2022
Merged

Add cancellation from query history view#1105
aeisenberg merged 2 commits intomainfrom
aeisenberg/history-cancel

Conversation

@aeisenberg
Copy link
Contributor

@aeisenberg aeisenberg commented Jan 28, 2022 •
edited
Loading

And tweak the commands visible from the view.

No changelog entry needed since it is still part of the query history feature.

Checklist

  • [n/a] 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.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@aeisenberg aeisenberg requested a review from a team as a code owner January 28, 2022 02:22
@aeisenberg aeisenberg force-pushed the aeisenberg/query-history branch from 3b62f0f to dd0a21a Compare January 28, 2022 04:30
@aeisenberg aeisenberg force-pushed the aeisenberg/history-cancel branch 3 times, most recently from 2d1c9a0 to b79c80b Compare January 28, 2022 23:35
@aeisenberg
Copy link
Contributor Author

aeisenberg commented Jan 29, 2022

Another piece of #400

Base automatically changed from aeisenberg/query-history to main February 1, 2022 14:34
shati-patel
shati-patel approved these changes Feb 1, 2022
View reviewed changes
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.

This looks good as well 🎉 The diff has changed a bit since merging #1094, but I think I've checked the right changes!

@aeisenberg
Copy link
Contributor Author

aeisenberg commented Feb 1, 2022

Oh... a bunch of conflicts now. I will fix.

@aeisenberg
Add cancellation from query history view
6f64d4f 
And tweak the commands visible from the view.
@aeisenberg aeisenberg force-pushed the aeisenberg/history-cancel branch from b79c80b to 6f64d4f Compare February 2, 2022 00:37
shati-patel
shati-patel reviewed Feb 2, 2022
View reviewed changes
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.

Oh... a bunch of conflicts now. I will fix.

Much nicer, thanks! 🧹

Just a minor bug in the package.json conditions that I hadn't spotted before!

extensions/ql-vscode/package.json Outdated
"command": "codeQLQueryHistory.compareWith",
"group": "9_qlCommands",
"when": "view == codeQLQueryHistory"
"when": "view == codeQLQueryHistory && (viewItem == rawResultsItem || viewItem == interpretedResultsItem)"
Copy link
Contributor

@shati-patel shati-patel Feb 2, 2022

Choose a reason for hiding this comment

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

"Compare With" wasn't visible at all (ditto for "Show Query Log" and "View DIL" below). This works locally:

Suggested change
"when": "view == codeQLQueryHistory && (viewItem == rawResultsItem || viewItem == interpretedResultsItem)"
"when": "view == codeQLQueryHistory && viewItem == rawResultsItem || view == codeQLQueryHistory && viewItem == interpretedResultsItem"

Equivalent, but VS Code just doesn't seem to support parentheses in when clauses 🙄

extensions/ql-vscode/package.json Outdated
"command": "codeQLQueryHistory.showQueryLog",
"group": "9_qlCommands",
"when": "view == codeQLQueryHistory"
"when": "view == codeQLQueryHistory && (viewItem == rawResultsItem || viewItem == interpretedResultsItem)"
Copy link
Contributor

@shati-patel shati-patel Feb 2, 2022

Choose a reason for hiding this comment

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

as above! 😄

extensions/ql-vscode/package.json Outdated
"command": "codeQLQueryHistory.viewDil",
"group": "9_qlCommands",
"when": "view == codeQLQueryHistory"
"when": "view == codeQLQueryHistory && (viewItem == rawResultsItem || viewItem == interpretedResultsItem)"
Copy link
Contributor

@shati-patel shati-patel Feb 2, 2022

Choose a reason for hiding this comment

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

And here! 👆🏽

@aeisenberg
Copy link
Contributor Author

aeisenberg commented Feb 2, 2022

Darn...thought I tested that. But maybe not enough. I'll need to make sure the expressions are grouped correctly with the correct && || precedence.

@aeisenberg
Simplify command expressions
e1aecc2 
Use only `||` and clearly specify when each item should be visible.
@aeisenberg
Copy link
Contributor Author

aeisenberg commented Feb 2, 2022

Fixed. I simplified the expressions and now there is no need for both && and ||.

@aeisenberg aeisenberg merged commit c084e31 into main Feb 2, 2022
@aeisenberg aeisenberg deleted the aeisenberg/history-cancel branch February 2, 2022 21:39
@aeisenberg
Copy link
Contributor Author

aeisenberg commented Feb 9, 2022

Fixes #1101

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shati-patel shati-patel shati-patel approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aeisenberg @shati-patel