-
Notifications
You must be signed in to change notification settings - Fork 191
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
Add feature to jump to the .ql file referenced by a .qlref #815
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Let's get the related PR in semmle-code merged and then we can merge this one.
b9745d9
to
d07b4ef
Compare
Updated to account for the minor CLI changes in the internal PR, which has now been merged. This is now ready for merging, although doing so will mean we should not release a new version of the extension until the new CLI is released. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good and can is ready to be merged, but I think you're right that we should hold off for now until cli v2.5.1 is released. It would be annoying if you got this message and had no way of upgrading to a sufficient version.
helpers.showAndLogErrorMessage( | ||
'Jumping from a .qlref file to the .ql file it references is not ' | ||
+ 'supported with the CLI version you are running.\n' | ||
+ 'Please upgrade your CLI to use this feature.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to include the minimum cli version here. Something like:
+ 'Please upgrade your CLI to use this feature.'); | |
+ `Please upgrade your CLI to version ${CLI_VERSION_WITH_RESOLVE_QLREF} or later to use this feature.`); |
This PR adds an option to jump from a
.qlref
file to the.ql
file it references. Note that it should not be merged yet, as it relies on a new CLI callresolve qlref
which is not released yet.Closes #654