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
fix(lsp): do not rename in strings and comments #11041
fix(lsp): do not rename in strings and comments #11041
Conversation
cli/lsp/language_server.rs
Outdated
true, | ||
true, | ||
false, | ||
false, | ||
false, | ||
)); |
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.
By looking at this code alone, it's hard to tell what this means. It would be nice if we didn't use tuples here.
These are the arguments for find_in_strings
and find_in_comments
.
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'd be fine using q struct with the more complicated APIs. It gets unwieldy for all of them though.
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.
Done in 8940d22.
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.
LGTM, but would be glad to have a struct here.
cli/lsp/language_server.rs
Outdated
true, | ||
true, | ||
false, | ||
false, | ||
false, | ||
)); |
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'd be fine using q struct with the more complicated APIs. It gets unwieldy for all of them though.
Actually, so you think it warrents a test of some sort? |
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.
LGTM
Fixes denoland/vscode_deno#450
This functionality is not the default with TypeScript so I believe this is a bug and we shouldn't do it.