|
From: | Dmitry Gutov |
Subject: | bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode |
Date: | Thu, 21 Dec 2023 01:34:30 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 |
On 19/12/2023 02:14, Denis Zubarev wrote:
> If you recall my earlier complaint that these highlightings didn't work> (and the tests didn't pass), this happened due to an older Python grammar.Thank you for investigating this. It seems this commit introducedchanges to type nodes hierarchy (https://github.com/tree-sitter/tree-sitter-python/commit/bcbf41589f4dc38a98bda4ca4c924eb5cae26f7b).
Could be this one, yes.
> The queries didn't lead to errors either (that's a good thing), but maybe > we'll want to revisit these highlights later to add support for the > older grammar as well. It may lead to unnecessarily complex rules. I don't know is it worth it, since users can easily update grammars.
No problem.
> I'm not sure highlighting types based on the caller method and position > is a good idea. I think that's backward, logically. If one puts a > non-type value in such argument, and we would highlight it as a type -- > that seems like the wrong message. These two functions expect a type (or tuple of types) as the second argument. To address your concerns about highlighting as a type a non-type variable, I added regexp python--treesit-type-regex. This regex matches if text is either built-in type or text starts with capital letter. I extracted built-in types from the python--treesit-builtins into its own variable python--treesit-builtin-types. python--treesit-builtins is now constructing by appending python--treesit-builtin-types and other built-ins. I hope it is ok.
Thank you. I'm actually not sure if we _have to_ check the identifier names in this context (any chance to have a false negative, miss some valid types?), but it probably doesn't hurt either.
> One of the complaints is that "User" is not highlighted as a type when > used in other, non-built-in methods, which like a reasonable question to > me. Yes, Python is dynamic, but using CamelCase for types is a fairly > regular convention, so highlighting such identifiers as types can work. It is good idea, to highlight some variables as types. But I think it should be done on the 4th level. One could split the variable feature into multiple features: variable-type, variable-argument, variable-use, etc. So for variable-type feature we can use python--treesit-type-regex and highlight matched identifiers with type face. For now I wanted to properly highlight types in places where they expected to be.
I wouldn't mind the level 4 (after all, python-mode is also conservative here and doesn't add such highlighting), but I'd rather not add the special handling for isinstance/issubclass thing for the reasons previously outlined.
Perhaps Yuan will disagree. I'm just here to say that the rest of the patch LGTM.
[Prev in Thread] | Current Thread | [Next in Thread] |