Skip to content
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

Variable names consume the underline #371

Closed
marimeireles opened this issue Sep 21, 2020 · 9 comments
Closed

Variable names consume the underline #371

marimeireles opened this issue Sep 21, 2020 · 9 comments

Comments

@marimeireles
Copy link
Contributor

marimeireles commented Sep 21, 2020

Hey Jupyterlab-lsp folks!

Description

Using pyls-memestra I found a bug and I believe it's happening because of something wrong with juplab-lsb. Variable names are consuming the underline on the deprecated class/function.

Reproduce

You can see it happening here: https://mybinder.org/v2/gh/QuantStack/pyls-memestra/master?urlpath=/lab/tree/binder/default_decorator_example.ipynb

But here is a screenshot:

image

Any opinions on this? Have you seen this before?

Thanks a lot!

Related issue: QuantStack/pyls-memestra#25

@krassowski
Copy link
Member

krassowski commented Sep 21, 2020

Thanks for reporting. Indeed I can reproduce this (on firefox for now). I will look into this.

The binder had some hiccup on on Chrome:

Screenshot from 2020-09-21 14-01-08

@krassowski

This comment has been minimized.

@marimeireles
Copy link
Contributor Author

marimeireles commented Sep 21, 2020

Thanks for this @krassowski, didn't know we had a chrome issue. Will investigate!

@krassowski
Copy link
Member

The binder on Chrome started working after a third trial - just a hiccup apparently.

@krassowski
Copy link
Member

krassowski commented Sep 21, 2020

Hi @marimeireles, I had a quick look and I think that this might be on your side:

Screenshot from 2020-09-21 14-22-10

SomeOldClass has 12 characters but the diagnostic (as shown above) specifies a range between 6th and 12th character - and we correctly only underline the first 6 characters.

Edit: In earlier version of this comment, I incorrectly mentioned SomeOldFunction where it should have been SomeOldClass

@marimeireles
Copy link
Contributor Author

Ah yeah! It does look like something on my side.
That's right. I'm sorry for this!

@krassowski
Copy link
Member

No worries, glad I could help! It looks that the err_range defionition, which currently is:

err_range = {
    'start': {'line': lineno - 1, 'character': colno},
    'end': {'line': lineno - 1, 'character': len(fname)},
}

should have been:

err_range = {
    'start': {'line': lineno - 1, 'character': colno},
    'end': {'line': lineno - 1, 'character': colno + len(fname)},  # <- change here
}

Hope it helps. Thanks for the mention of jupyterlab-lsp in the blog post btw!

@marimeireles
Copy link
Contributor Author

You're welcome @krassowski, you're doing an amazing job here :)
Sorry for opening the issue without investigating it properly and thanks!

@bollwyvl
Copy link
Collaborator

awesome debugging, folks!

one of the things that will hopefully come out of the language server kernel proxy #278 is that we would be able to offer that part of the stack headless with a client for testing these kinds of things... some other "language server construction kits" have some utilities like this, but it might be useful for us to have in-package... it may make sense to land that in pygls and take that as an optional dependency... unless there's a lot of value in using it directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants