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

Previous Next

Full log

View this message in rfc822 format

From: Yuan Fu <casouri <at> gmail.com>
To: Denis Zubarev <dvzubarev <at> yandex.ru>, Dmitry Gutov <dmitry <at> gutov.dev>, 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: Tue, 12 Dec 2023 00:24:41 -0800

On 12/11/23 5:18 PM, Denis Zubarev wrote:
> Yuan and Dmitry, thank you for review and suggestions.
> > Can we do this instead: in python--treesit-fontify-string, we check if
>   string-interpolation feature is enabled, if it is, fontify string_start,
>   string_content and string_end only; if not, fontify the whole string.
> Done.
> Enable interpolation highlighting only if 'string-interpolation is 
> presented on the third level of treesit-font-lock-feature-list.
> Personally, If I saw a f-string with an interpolation fontified as 
> string, I would assume that it is bug.
> Clearly it is not a string, so it should be highlighted distinctly.
> But if it is a convention across all languages, we should follow it.

I  encourage everyone to also think in terms of fontification levels, in 
addition to features.

For many Emacs users, they want a quieter or even minimal fontification. 
Some people only want comment and function names highlighted, and they 
can get it by setting the fontification level to 1, because 
python-ts-mode only activates the comment and definition feature at 
level 1. The string feature is at level 2, this level is still 
relatively simplistic. And full string interpolation probably don't 
belong at that level. That's why I separated it out into another 
feature, and placed string-interpolation at level 3.

> > I think "for var in range(3)" should be part of the "definition" feature
>   because a variable is defined there. Alongside parameters.
> I added it to definitions.

Again, if we think of fontification levels, the definition feature is 
about fontifying the function names of definitions, and it's at a low 
level (level 1). Non-essential fontification like "var" shouldn't be 
activated at that level. So I suggest to put it in the variable feature, 
along with many other non-essential fontifications. (Variable feature is 
placed at level 4.)

> My thoughts about parameters. I started to extend rules for them since 
> they are very limited now.
> But I'm not sure what face to use for them.
> I would like to not use the same face as for assignments, because I'd 
> want to highlight them differently.
> It seems that there is no appropriate face in font-lock.el, so I ended 
> up creating my own face in my config.
> Does it make sense to add new face for parameters in font-lock.el?
> Or it is too small feature for its own face?
> I also apply this face for keyword argument in function calls.
To be honest, I don't have any good ideas. Perhaps we can add a 
parameter face that inherits from variable name face by default, Dmitry, 
WDYT?
> Summary for all changes in the patch.
> definition feature:
> `for var in range(3)` highlight var as font-lock-variable-name-face
> assignment feature:
> var := 3 (named_expression)
> var *= 3 (augmented_assignment)
> Highlight var as font-lock-variable-name-face.
> Make list_splat_pattern query more precise.
> list_splat_pattern may appear not only in assignments: var, *rest = 
> call(),
> but in the parameter list too: def f(*args).
> Highlight args only for the first case in assignment feature.
> type feature:
> Fontify built-ins (dict,list,etc.) as types when they are used in type 
> hints.
> support nested union types, for example `Lvl1 | Lvl2[Lvl3[Lvl3], Lvl2]`.
> This structure is represented via nesting binary_operator and 
> subscript nodes in the grammar.
> Function python--treesit-fontify-union-types iterates over all 
> children and highlight identifier nodes.
> Fontify base class names in the class definition: class Temp(Base1, 
> pack0.Base2):
> Fontify class patterns in case statement: case [TempC() | bytes(b)]:
> Highlight the second argument as a type in isinstance/issubclass call:
> isinstance(var2, (str, dict, Type1)); issubclass(var1, int|str)
> For all dotted names of a type highlight only the last part of the name,
> e.g. collections.abc.Iterator.
> decorator feature:
> Highlight dotted names: @pytest.mark.skip
> Function python--treesit-fontify-dotted-decorator iterates over all 
> nested attribute nodes and highlight identifier nodes.
> When font-lock-level is set 4, `skip` had function-call face in: 
> @pytest.mark.skip(reason='t')
> Add `:override t` to decorator feature to override function-call face.
> string feature:
> Enable interpolation highlighting only if string-interpolation is 
> presented on the third level of treesit-font-lock-feature-list.
> Fix fontification of strings inside of f-strings interpolation,
> e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
Instead of the third level, the check should use the value 
treesit-font-lock-level. And it should check for each level smaller than 
or equal to treesit-font-lock-level.
> function feature:
> Do not override the face of builtin functions (all, bytes etc.) with
> the function call face
> keyword feature:
> Add "is not"  to the `python--treesit-keywords` list.
>
Thanks, they look good. The patch is getting rather large, let's focus 
on getting the existing changes merged rather than adding new stuff to 
it. Though I think your copyright assignment hasn't completed, right?

Yuan

Previous Next

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