bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov <dmitry@gutov.dev>]

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 introduced
> changes 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.