Extension breaks highlight selected word occurences on scrollbar

[pohmelie]

Environment data

vscode

Version: 1.44.2
Commit: ff915844119ce9485abfe8aa9076ec76b5300ddd
Date: 2020-04-16T17:50:03.709Z
Electron: 7.1.11
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Linux x64 4.15.0-96-generic

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.

[pohmelie]

Laggy a bit :/

[karrtikr]

Looks like the analysis in Language server might be blocking the highlighting.

[MikhailArkhipov]

Might be side effect of #1767

[jakebailey]

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]

It's not working at all.

[jakebailey]

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]

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.

[pohmelie]

the screencast shows it working for variables

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]

[pohmelie]

With diabled plugin:

[jakebailey]

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.

But none of these are it "not working".

[pohmelie]

So, how can I bring back primitive builtin matching and have benefits from extension: autocomplete, linting, go to definition, etc.?

[jakebailey]

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]

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]

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.

[pohmelie]

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]

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.

[jakebailey]

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]

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.