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

From:

Denis Zubarev

Subject:

Date:

Tue, 19 Dec 2023 03:14:09 +0300

> 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).

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

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

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

> Could we just move it above the 'function' feature, so that the override

> is not needed?

Done.

I attach a patch with new changes.

18.12.2023, 03:25, "Dmitry Gutov" <dmitry@gutov.dev>:

On 17/12/2023 02:26, Denis Zubarev wrote:

Summary for all changes in the patch.
New feature variable-definition:
`for var in range(3)`
`[var+1 for var in []]`
`with T as var:`
`except E as var:`
`case str() as var:`
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.

I still think variable-name-face is not the best fit for
augmented_assignment, but admittedly it's a minor thing.

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.

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.

More specifically, these highlights, and the type-related face tests,
don't work with the Python ts grammar I had from March 7th 2023. 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.

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)

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.

OTOH, see this reddit thread and this screenshot:

https://www.reddit.com/r/emacs/comments/18kr1gl/how_can_i_configure_pythontsmode_to_fontify_more/

https://preview.redd.it/y8l3k8tt4x6c1.png?width=3840&format=png&auto=webp&s=0a6882e66d4b334c07e856934ce847e63aa2db2c

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.
You can see rust-ts-mode for an example of this approach.

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:

Could we just move it above the 'function' feature, so that the override
is not needed?