GNU bug report logs - #67061
[PATCH] Improve syntax highlighting for python-ts-mode

Previous Next

Full log

View this message in rfc822 format

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Denis Zubarev <dvzubarev <at> yandex.ru>, Yuan Fu <casouri <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
Subject: bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode
Date: Thu, 21 Dec 2023 01:34:30 +0200

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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.