-
Notifications
You must be signed in to change notification settings - Fork 667
only show TODO code lens on lines that start with a comment token #7962
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
base: main
Are you sure you want to change the base?
Conversation
I suggest adding |
"default": [ | ||
"//", | ||
"#", | ||
"--", | ||
" * ", | ||
"///" |
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 seems suspicious. It's something that would be language specific too.
<!-- TODO: ...... -->
/*
TODO: some multi-line
comment with context
*/
<#
TODO: PowerShell block comments are weird
I know
#>
The comment syntax is declared by the language support, I feel like we should somehow leverage that if we can.
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.
we have this in core... and we have a number of commands in core that are about retrieval of symbols:
https://code.visualstudio.com/api/references/commands#commands
I wonder if this warrants some way to get the LanguageConfiguration via a Command. Or at least just the comment stuff...
@hediet @alexdima I see your name around this code, maybe you have ideas
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.
Thanks for the pointers, this looks a lot more correct.
I justified that this was ok since it's configurable via a setting and did not know we could possibly leverage core for this information.
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.
I don't think we should have two setting that both control whether the code action/codelens show.
Can we instead just remove "BUG" from the list of defaults?
While #7962 is under discussion
@alexr00 I still think it's incorrect to match outside of a comment. Do you disagree? @TylerLeonhardt thanks for the idea/pointers |
fix for #7954 being far too verbose.