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

[Yuan Fu <casouri@gmail.com>]

> On Dec 20, 2023, at 3:34 PM, Dmitry Gutov <dmitry@gutov.dev> wrote:
> 
> 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.

I wouldn’t mind either, go crazy with level 4 :-) I wouldn’t even mind it in level 3, since they are indeed types. Using a separate feature is a good idea, so people who doesn’t want it can turn it off.

Yuan