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

Semantic Highlighting for Type Hints #335

Closed
hassanselim0 opened this issue Sep 8, 2020 · 20 comments
Closed

Semantic Highlighting for Type Hints #335

hassanselim0 opened this issue Sep 8, 2020 · 20 comments
Assignees
Labels
enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@hassanselim0
Copy link

I recently updated to Pylance 2020.9.0 and I really like the Semantic Highlighting!

However it seems like type hints don't have any special semantic (usually it's a class) and I'm wanted type hints to have a different color than classes. Whether it's annotation type hints or comment type hints.

Before this update, comment type hints had special TextMate scopes like comment.typehint and meta.typehint which I've customized, and now these are overridden by the semantic colors. Is it at least possible to still enforce my colors in this case?

@github-actions github-actions bot added the triage label Sep 8, 2020
@hassanselim0
Copy link
Author

hassanselim0 commented Sep 8, 2020

This is a screenshot of how comment type hints look right now after the update:
image
Previously the classes (int, Order, datetime) had the same color as the # type: token (a semi-transparent purple), is it possible to achieve that again without completely disabling semantic highlighting?

Note: These are the color customizations I had in my settings json

{
    "scope": "comment.typehint",
    "settings": {
        "foreground": "#C586C0AA",
    }
},
{
    "scope": "meta.typehint",
    "settings": {
        "foreground": "#C586C0AA",
    }
},

@erictraut
Copy link
Contributor

Out of curiosity, why are you still using type comments? Are you stuck on an old version of Python? Since Python 3.6, variable type annotations can and should be provided using the new syntax in PEP 526.

@hassanselim0
Copy link
Author

I used the type comments mainly because I was able to give them a different color based on TextMate Scopes. Not being able to customize the color for annotations made Django Model Definitions quite unreadable. I use annotations everywhere else though.

@huguesv
Copy link
Contributor

huguesv commented Sep 9, 2020

Thanks. I'll investigate if we can make this customizable.

@huguesv huguesv added enhancement New feature or request needs investigation Could be an issue - needs investigation labels Sep 9, 2020
@github-actions github-actions bot removed the triage label Sep 9, 2020
@huguesv huguesv added fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed needs investigation Could be an issue - needs investigation labels Sep 17, 2020
@huguesv
Copy link
Contributor

huguesv commented Sep 17, 2020

This has been fixed for the next release.

image

@huguesv
Copy link
Contributor

huguesv commented Sep 17, 2020

You'll also be able to do something like this:
image

Given settings like these:

  "editor.semanticTokenColorCustomizations": {
    "[One Dark Pro]": {
      "rules": {
        "*.typeHint": "#00ff00"
      }
    }
  },
  "editor.tokenColorCustomizations": {
    "[One Dark Pro]": {
      "textMateRules": [
        {
          "scope": "meta.typehint",
          "settings": {
            "foreground": "#a32f9aaa"
          }
        },
        {
          "scope": "comment.typehint",
          "settings": {
            "foreground": "#a32f9aaa"
          }
        }
      ]
    }
  },

@hassanselim0
Copy link
Author

@huguesv great 😁
Does modifier also apply to annotation-based type hints?

self._identifier: int = MY_CONSTANT

@huguesv
Copy link
Contributor

huguesv commented Sep 18, 2020

@hassanselim0 it does not, from what I saw it didn't seem that textmate was setting a special scope for it either.

image

But it's relatively easy to add a different modifier for this case (so that you'd be able to distinguish from the ones in comments).

@hassanselim0
Copy link
Author

@huguesv It would be great to have a modifier for annotation type hints (same or different), the reason I went with comment hints in the first place was because TestMate didn't have a scope for annotation hints.
I think if I can give annotation hints a different color it would make certain bits of code more readable, like the Django Model in the screenshot I uploaded earlier, to visually separate the type from the field class beside it. That's my main goal here.

Side Note: The reason I'm even putting type hints in Django Models is because even though the class attributes have types that are a subclass of orm Field, the instance attributes have normal python types. So name = CharField() when later accessed via self.name gives an str type. So I write the class attr as name: str = CharField() or name = CharField() # type: str so that VS Code would better help me.
If there is a way to write a pyi file and define CharField in such a way that the the Language Server would automatically show me all instances of CharField as having a type of str without using any type hints, I'd be glad to know.

@erictraut
Copy link
Contributor

@hassanselim0, the best way to declare CharField in a stub file is to define a __get__ method that returns str. That's how CharField is implemented in django, so defining it that way in the type stub will produce the most accurate results.

Incidentally, there is a stub package available for django. That might save you a bunch of time. https://pypi.org/project/django-stubs/

@jakebailey
Copy link
Member

We ship those stubs with pylance, so I would have expected for this to have worked (if the stub was defined in this way).

@hassanselim0
Copy link
Author

hassanselim0 commented Sep 18, 2020

I do have django stubs installed and working, but it doesn't do that behavior in my code.
I checked to see the stub file here: https://github.com/typeddjango/django-stubs/blob/master/django-stubs/db/models/fields/__init__.pyi#L107
They have overloaded __get__, it can return a Field type or a "get return type", but I don't understand how the type checker is supposed to figure out which overload to pick. The end result is that Pylance tells me the instance attr is of type Any. I'm not sure if this is a bug in the stub or in Pylance, if it's a Pylance bug then maybe create a separate issue for this?

@jakebailey
Copy link
Member

This would warrant another issue, yes, as it doesn't really fit within the bounds of the original semantic highlighting issue.

@erictraut
Copy link
Contributor

Yeah, if you think that the type checker is not correctly handling __get__ in this case, please file a separate issue — preferably in the pyright repo, since that's where the type checking logic lives. Thanks!

@hassanselim0
Copy link
Author

Done, I've actually commented on a closed ticket about a very similar issue, so it can be reopened in case it's a regression.

Getting back to the original issue, I still think it would be useful to have a semantic modifier for annotation type hints, so you can easily customize the colors for all type hints whether they are in an annotation or a comment.

@huguesv
Copy link
Contributor

huguesv commented Sep 18, 2020

I've just checked in a change that allows customization of colors for type hints in annotation or comment. It will be in the next release of Pylance.

Default colors (One Dark Pro):
image

Customized colors (One Dark Pro):

  "editor.semanticTokenColorCustomizations": {
    "[One Dark Pro]": {
      "rules": {
        "*.typeHint": "#286605",
        "*.typeHintComment": "#6ed3d3"
      }
    }
  }

image

@hassanselim0
Copy link
Author

@huguesv thank you 😁

@jakebailey
Copy link
Member

This issue has been fixed in version 2020.9.6, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/master/CHANGELOG.md#202096-23-september-2020

@hassanselim0
Copy link
Author

hassanselim0 commented Oct 5, 2020

@jakebailey @huguesv Thank you for your efforts, I just got around to testing this today and stumbled upon two issues. I set *.typeHint and *.typeHintComment to #ff0000 to test the new modifiers.

This is the first one:
image
The square brackets used for generic types doesn't have the new typeHint modifier.

This is the second one:
image
In function argument annotations that have generic types, the generic type doesn't have the typeHint modifier.

@jakebailey
Copy link
Member

That's likely a bug; we'd appreciate a new issue for tracking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

4 participants