You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Apr 14, 2022. It is now read-only.
Repository navigation
This repository was archived by the owner on Apr 14, 2022. It is now read-only.
Extension breaks highlight selected word occurences on scrollbar #2005
Extension version: 2020.4.74986
OS and version: ubuntu 18.04
Steps to reproduce:
Enable extension, some clicks and scrolls and thing stop working. Disable extension and highlight works just fine.
More info and discussion about reasons is here microsoft/vscode#16042
Looks like many years ago some other lang extensions did have problems with this.
More to say, if I open, lets say a .yml file, then scrollbar highlight works just fine, even when python extension enabled.
I guess I'm confused; the screencast shows it working for variables. Are you specifically referring to keywords? Those aren't expected to to work, and only worked before as it was VS Code "guessing" what to highlight without understanding the context.
If that's it, then we can see if the call is returning empty or none; if it's empty than maybe returning a null result when doing an invalid hover will cause VS Code to fallback to its old behavior.
I can film new screencast with restart of vscode after plugin enabled. And nothing will work. I guess it is something like an artifact from working time before.
Note that the lower screencast is just VS Code's builtin matching which doesn't care about context at all and will highlight any matching word. We declare that we support the feature, so it disables itself.
In the first screencast:
The keywords are not highlighted, as they are not variables. This is what I referenced before that may have a simple change to tweak.
The duplicate functions are arguably an edge case; I don't know what the "right" thing to do here is when we are internally merging function definitions as only one of them is "real". Most people are not declaring the same function over and over.
self is being scoped to each function; perhaps it should be highlighted for the entire class's references to self.
I don't think there's any simple way that's not a code change in the extension or the LS. Feasibly you can disable the extension's LS downloader and manually use an old build, but that is a pretty significant workaround.
To confirm, it's the keywords not being highlighted which is the main issue?
No, the main issue that is nothing highlighted. Take a look at screencasts: async, def, self, pass, start, stop, nothing highlights. I hope there will be a toggle to choose who provides highlights. So I can bring builtin behavior back.
async, def, and pass are all keywords. This is what I said before. They are only highlighted in VS Code because VS Code does not understand Python. It's not clear to me that highlighting a keyword is a good idea.
self, start, and stop are not keywords.
self may be able to be fixed to highlight the same self in all functions, but we will likely want to look at other highlight implementations to see if they also do something similar (like with this) for consistency, but it is being highlighted in each specific function.
Your example of start and stop are special because you've declared the same function multiple times. Arguably we should be highlighting the correct region and not the prior declaration.
I got your point, no need to explain each case exactly. My point is I want behavior as it was before (builtin mode) and I want all other features of extension. Is it possible to make some toggle in options to disable extension highlight rules?
Is it possible to make some toggle in options to disable extension highlight rules?
Not in the language server itself. We can only declare at startup that we do or don't support this feature, we cannot do so dynamically. Any other changes would be a hack in the client (the VS Code extension) to skip certain calls, which is a bad idea.
It's better to improve the feature than to add toggles to disable it.
Hm, I didn't see it in the spec initially, but there is way to dynamically declare support for this. Regardless, we should try and address the issues first, rather than adding toggles we don't plan to keep.
This will need #1960 since it handles stub references better. It does not seem to be possible to return null or something to get default behavior, so for non-references we'd need to get down to tokens.
Activity
pohmelie commentedon Apr 24, 2020
Laggy a bit :/
karrtikr commentedon Apr 24, 2020
Looks like the analysis in Language server might be blocking the highlighting.
MikhailArkhipov commentedon Apr 24, 2020
Might be side effect of #1767
jakebailey commentedon Apr 27, 2020
Is the issue here that it's slow, or that it's not working at all? I can understand slow, given this now uses the analysis.
pohmelie commentedon Apr 27, 2020
It's not working at all.
jakebailey commentedon Apr 27, 2020
I guess I'm confused; the screencast shows it working for variables. Are you specifically referring to keywords? Those aren't expected to to work, and only worked before as it was VS Code "guessing" what to highlight without understanding the context.
jakebailey commentedon Apr 27, 2020
If that's it, then we can see if the call is returning empty or none; if it's empty than maybe returning a
nullresult when doing an invalid hover will cause VS Code to fallback to its old behavior.pohmelie commentedon Apr 27, 2020
I can film new screencast with restart of vscode after plugin enabled. And nothing will work. I guess it is something like an artifact from working time before.
pohmelie commentedon Apr 27, 2020
pohmelie commentedon Apr 27, 2020
With diabled plugin:
jakebailey commentedon Apr 27, 2020
Note that the lower screencast is just VS Code's builtin matching which doesn't care about context at all and will highlight any matching word. We declare that we support the feature, so it disables itself.
In the first screencast:
selfis being scoped to each function; perhaps it should be highlighted for the entire class's references to self.But none of these are it "not working".
pohmelie commentedon Apr 27, 2020
So, how can I bring back primitive builtin matching and have benefits from extension: autocomplete, linting, go to definition, etc.?
jakebailey commentedon Apr 27, 2020
I don't think there's any simple way that's not a code change in the extension or the LS. Feasibly you can disable the extension's LS downloader and manually use an old build, but that is a pretty significant workaround.
To confirm, it's the keywords not being highlighted which is the main issue?
pohmelie commentedon Apr 27, 2020
No, the main issue that is nothing highlighted. Take a look at screencasts:
async,def,self,pass,start,stop, nothing highlights. I hope there will be a toggle to choose who provides highlights. So I can bring builtin behavior back.jakebailey commentedon Apr 27, 2020
async,def, andpassare all keywords. This is what I said before. They are only highlighted in VS Code because VS Code does not understand Python. It's not clear to me that highlighting a keyword is a good idea.self,start, andstopare not keywords.selfmay be able to be fixed to highlight the sameselfin all functions, but we will likely want to look at other highlight implementations to see if they also do something similar (like withthis) for consistency, but it is being highlighted in each specific function.Your example of
startandstopare special because you've declared the same function multiple times. Arguably we should be highlighting the correct region and not the prior declaration.pohmelie commentedon Apr 27, 2020
I got your point, no need to explain each case exactly. My point is I want behavior as it was before (builtin mode) and I want all other features of extension. Is it possible to make some toggle in options to disable extension highlight rules?
jakebailey commentedon Apr 27, 2020
Not in the language server itself. We can only declare at startup that we do or don't support this feature, we cannot do so dynamically. Any other changes would be a hack in the client (the VS Code extension) to skip certain calls, which is a bad idea.
It's better to improve the feature than to add toggles to disable it.
jakebailey commentedon Apr 27, 2020
Hm, I didn't see it in the spec initially, but there is way to dynamically declare support for this. Regardless, we should try and address the issues first, rather than adding toggles we don't plan to keep.
MikhailArkhipov commentedon May 1, 2020
This will need #1960 since it handles stub references better. It does not seem to be possible to return null or something to get default behavior, so for non-references we'd need to get down to tokens.