Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.
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

Description

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.

Activity

  1. pohmelie commented on Apr 24, 2020

    @pohmelie
    Author

    Laggy a bit :/

    Peek 2020-04-24 14-26

  2. karrtikr commented on Apr 24, 2020

    @karrtikr

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

  3. removed their assignment
    on Apr 24, 2020
  4. MikhailArkhipov commented on Apr 24, 2020

    @MikhailArkhipov

    Might be side effect of #1767

  5. jakebailey commented on Apr 27, 2020

    @jakebailey
    Member

    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.

  6. pohmelie commented on Apr 27, 2020

    @pohmelie
    Author

    It's not working at all.

  7. jakebailey commented on Apr 27, 2020

    @jakebailey
    Member

    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.

  8. jakebailey commented on Apr 27, 2020

    @jakebailey
    Member

    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.

  9. pohmelie commented on Apr 27, 2020

    @pohmelie
    Author

    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.

  10. pohmelie commented on Apr 27, 2020

    @pohmelie
    Author

    Peek 2020-04-27 20-45

  11. pohmelie commented on Apr 27, 2020

    @pohmelie
    Author

    With diabled plugin:

    Peek 2020-04-27 20-47

  12. jakebailey commented on Apr 27, 2020

    @jakebailey
    Member

    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".

  13. pohmelie commented on Apr 27, 2020

    @pohmelie
    Author

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

  14. jakebailey commented on Apr 27, 2020

    @jakebailey
    Member

    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?

  15. pohmelie commented on Apr 27, 2020

    @pohmelie
    Author

    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.

  16. jakebailey commented on Apr 27, 2020

    @jakebailey
    Member

    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.

  17. pohmelie commented on Apr 27, 2020

    @pohmelie
    Author

    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?

  18. jakebailey commented on Apr 27, 2020

    @jakebailey
    Member

    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.

  19. jakebailey commented on Apr 27, 2020

    @jakebailey
    Member

    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.

  20. MikhailArkhipov commented on May 1, 2020

    @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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions