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

Incorrect highlighting of a call with preceding __getitem__ #197

Open
elprans opened this issue Feb 21, 2020 · 7 comments
Open

Incorrect highlighting of a call with preceding __getitem__ #197

elprans opened this issue Feb 21, 2020 · 7 comments
Assignees

Comments

@elprans
Copy link
Member

elprans commented Feb 21, 2020

Observe the differences in highlighting of the keyword argument names below:

image

@elprans
Copy link
Member Author

elprans commented Mar 16, 2020

The class token is also misclassified:

image

vpetrovykh added a commit that referenced this issue May 1, 2020
Correctly identify calls of the form "foo[123](bar=2)" to highlight the
contents of the parentheses as call arguments.

Issue: #197
@vpetrovykh
Copy link
Member

I have a solution for better kwargs detection.

Regarding the class definition I need a better idea of what's the expectation here. Technically everything that appears in the parentheses after the class name is part of the class inheritance. It could be a simple identifier (SomeClass), but also an array element (FooObject[Bar]) or even a function call (FooFactory(Bar) - this is a bit odd, but not forbidden). It could be all of the above: Foo(Bar)[Baz]. Should all of these be highlighted as entity.other.inherited-class.python? Essentially making that the default, like source is for normal code? Or should it only be some prefix?

@elprans
Copy link
Member Author

elprans commented May 1, 2020

Can we just handle the Foo[Bar] case specifically? I don't think we should veer too much into supporting other weird expressions there.

@vpetrovykh
Copy link
Member

Perhaps you misunderstand me. This is not a question of how technically hard the solution is - it's comparable complexity, regardless of what we choose here. This is a question of what kind of consistency we want to show.

So by default I would think that every argument in class definition is definitely another class semantically, regardless of how obscure the expression. So I would tend to suggest that we can then just wrap all arguments in entity.other.inherited-class.python at the base level. This way all the code that was normally white would now be green (for our color scheme) and the argument entire expression would have the inherited-class scope. This seems semantically consistent and would have the net effect of accounting for the particular special case that we currently have.

We can special-case only this one, but it's not saving me any work and I'd prefer to not make an exception. So unless you have strong opposition to calling the entire argument as entity.other.inherited-class.python let's just go with the first option.

@elprans
Copy link
Member Author

elprans commented May 1, 2020

How would the blanket approach highlight this:

class FooMeta(type(Bar)):
    pass

?

@vpetrovykh
Copy link
Member

The entire type(Bar) would have the entity.other.inherited-class.python scope (making (Bar) green) and the type would also have the support.type.python scope (the thing that makes it blue in our color scheme).

This is similar to how strings are highlighted where the entire string has a common "string" scope and then parts of it may have additional scopes (like escape sequences).

@elprans
Copy link
Member Author

elprans commented May 2, 2020

OK that seems fine to me.

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

2 participants