unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Yuan Fu <casouri@gmail.com>
To: Denis Zubarev <dvzubarev@yandex.ru>,
	Dmitry Gutov <dmitry@gutov.dev>, Eli Zaretskii <eliz@gnu.org>
Cc: "67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
Subject: bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode
Date: Tue, 12 Dec 2023 00:24:41 -0800	[thread overview]
Message-ID: <78ffdcf3-e322-49ea-a0d5-d0485ade9e73@gmail.com> (raw)
In-Reply-To: <95071702343720@mail.yandex.ru>



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







  reply	other threads:[~2023-12-12  8:24 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-11  2:21 bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode Denis Zubarev
2023-11-11  7:32 ` Eli Zaretskii
2023-11-11 10:52   ` Denis Zubarev
2023-11-11 11:00     ` Eli Zaretskii
2023-11-11 12:09       ` Denis Zubarev
2023-11-26  2:12       ` Dmitry Gutov
2023-11-15 13:28   ` Eli Zaretskii
2023-11-25  9:35     ` Eli Zaretskii
2023-11-26  2:17       ` Dmitry Gutov
2023-11-29 14:05         ` Eli Zaretskii
2023-12-09  0:39           ` Denis Zubarev
2023-12-09  7:32             ` Eli Zaretskii
2023-12-10 10:16               ` Yuan Fu
2023-12-09 18:18             ` Dmitry Gutov
2023-12-10 12:04               ` Denis Zubarev
2023-12-11  0:00                 ` Dmitry Gutov
2023-12-11  7:10                   ` Yuan Fu
2023-12-11 12:02                     ` Dmitry Gutov
2023-12-12  1:18                     ` Denis Zubarev
2023-12-12  8:24                       ` Yuan Fu [this message]
2023-12-13  0:44                         ` Dmitry Gutov
2023-12-13  3:49                           ` Yuan Fu
2023-12-13 18:28                             ` Dmitry Gutov
2023-12-14  5:54                               ` Yuan Fu
2023-12-14 11:51                                 ` Dmitry Gutov
2023-12-17  1:07                                   ` Yuan Fu
2023-12-17 21:36                                     ` Dmitry Gutov
2023-12-23 21:46                                     ` Denis Zubarev
2023-12-16 13:03                                 ` Eli Zaretskii
2023-12-17  1:56                               ` Denis Zubarev
2023-12-17 23:38                                 ` Dmitry Gutov
2023-12-13 11:52                         ` Eli Zaretskii
2023-12-17  0:26                         ` Denis Zubarev
2023-12-17  1:10                           ` Yuan Fu
2023-12-17  2:07                             ` Denis Zubarev
2023-12-23  9:42                               ` Eli Zaretskii
2023-12-30 10:53                                 ` Denis Zubarev
2023-12-30 11:19                                   ` Eli Zaretskii
2023-12-18  0:25                           ` Dmitry Gutov
2023-12-19  0:14                             ` Denis Zubarev
2023-12-20 23:34                               ` Dmitry Gutov
2023-12-21  7:04                                 ` Yuan Fu
2023-12-23 21:45                                 ` Denis Zubarev
2024-01-01 17:42                                   ` Dmitry Gutov
2024-01-09 20:03                                     ` Eli Zaretskii
2024-01-20  9:08                                       ` Eli Zaretskii
2024-01-27  9:49                                         ` Eli Zaretskii
2024-01-27 10:47                                           ` Denis Zubarev
2024-01-27 11:30                                             ` Eli Zaretskii
2023-12-13 21:16         ` Stefan Kangas
2023-12-14  1:31           ` Dmitry Gutov
2023-12-14 22:49             ` Stefan Kangas
2023-12-15  7:14               ` Yuan Fu
2023-12-11  6:53 ` Yuan Fu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=78ffdcf3-e322-49ea-a0d5-d0485ade9e73@gmail.com \
    --to=casouri@gmail.com \
    --cc=67061@debbugs.gnu.org \
    --cc=dmitry@gutov.dev \
    --cc=dvzubarev@yandex.ru \
    --cc=eliz@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).