GNU bug report logs -
#67061
[PATCH] Improve syntax highlighting for python-ts-mode
Previous Next
Reported by: Denis Zubarev <dvzubarev <at> yandex.ru>
Date: Sat, 11 Nov 2023 02:23:02 UTC
Severity: normal
Tags: patch
Fixed in version 29.2
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 67061 in the body.
You can then email your comments to 67061 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 11 Nov 2023 02:23:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Denis Zubarev <dvzubarev <at> yandex.ru>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Sat, 11 Nov 2023 02:23:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Tags: patch
Improve syntax highlighting for python-ts-mode.
- Fontify compound keywords "not in" and "is not" (fixes bug#67015)
- Fix fontification of strings inside of f-strings interpolation,
e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
- Do not override the face of builtin functions (all, bytes etc.) with
the function call face
- Add missing assignment expressions
- Fontify built-ins (dict,list,etc.) as types when they are used in type hints
I have added tests to treesit-tests.el, but I'm not sure that it is the
right place for them. I tried to create new file
python-ts-mode-tests.el, but when I run tests using make, I got
an error:
*** No rule to make target '../lisp/progmodes/python-ts-mode.el', needed by 'lisp/progmodes/python-ts-mode-tests.log'. Stop.
It seems that python-ts-mode is required to be in its own file, but it
is defined in python.el.
Adding tests to python-tests.el seems like not good idea too, because
some CI code is searching for *-ts-mode-tests.el or treesit-tests.el files.
In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
3.24.33, cairo version 1.16.0) of 2023-11-11 built on NUC-here
Repository revision: 400a71b8f2c5a49dce4f542adfd2fdb59eb34243
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Ubuntu 22.04.3 LTS
Configured using:
'configure --with-modules --with-native-compilation=aot
--with-imagemagick --with-json --with-tree-sitter --with-xft'
[0001-Improve-syntax-highlighting-for-python-ts-mode.patch (text/patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 11 Nov 2023 07:34:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> From: Denis Zubarev <dvzubarev <at> yandex.ru>
> Date: Sat, 11 Nov 2023 05:21:25 +0300
>
> Improve syntax highlighting for python-ts-mode.
>
> - Fontify compound keywords "not in" and "is not" (fixes bug#67015)
> - Fix fontification of strings inside of f-strings interpolation,
> e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
> - Do not override the face of builtin functions (all, bytes etc.) with
> the function call face
> - Add missing assignment expressions
> - Fontify built-ins (dict,list,etc.) as types when they are used in type hints
Yuan, any comments?
My only comment is that we should decide in which fontification
level(s) should the additional fontifications be.
> I have added tests to treesit-tests.el, but I'm not sure that it is the
> right place for them. I tried to create new file
> python-ts-mode-tests.el, but when I run tests using make, I got
> an error:
> *** No rule to make target '../lisp/progmodes/python-ts-mode.el', needed by 'lisp/progmodes/python-ts-mode-tests.log'. Stop.
>
> It seems that python-ts-mode is required to be in its own file, but it
> is defined in python.el.
> Adding tests to python-tests.el seems like not good idea too, because
> some CI code is searching for *-ts-mode-tests.el or treesit-tests.el files.
The tests should be in python-tests.el, and should be skipped if
(treesit-ready-p 'python t) returns nil.
What problems with CI did you see? I don't think I understand your
last sentence above.
> >From 0b3165eb90bdc1b85688d3c9a1902c1f293972d1 Mon Sep 17 00:00:00 2001
> From: Denis Zubarev <dvzubarev <at> yandex.ru>
> Date: Sat, 11 Nov 2023 04:55:44 +0300
> Subject: [PATCH] Improve syntax highlighting for python-ts-mode
This submission is too large for us to accept without a copyright
assignment. Would you like to start your assignment copyright
paperwork rolling at this time? If yes, I will send you the form to
fill and the instructions to email it.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 11 Nov 2023 10:54:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 11 Nov 2023 11:02:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> From: Denis Zubarev <dvzubarev <at> yandex.ru>
> Cc: "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
> Date: Sat, 11 Nov 2023 13:52:45 +0300
>
> My only comment is that we should decide in which fontification
> level(s) should the additional fontifications be.
>
> I didn't add any new features, just tweak the old ones to be more correct.
I thought at least some of these are new:
> > - Fontify compound keywords "not in" and "is not" (fixes bug#67015)
> > - Fix fontification of strings inside of f-strings interpolation,
> > e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
> > - Do not override the face of builtin functions (all, bytes etc.) with
> > the function call face
> > - Add missing assignment expressions
> > - Fontify built-ins (dict,list,etc.) as types when they are used in type hints
For example, isn't the first item an addition? And what about
"missing assignment expressions"?
> What problems with CI did you see? I don't think I understand your
> last sentence above.
>
> I've grepped the codebase and found those lines:
>
> TREE-SITTER-FILES ?= $(shell cd .. ; \
> find lisp src \( -name "*-ts-mode-tests.el" -o -name "treesit-tests.el" \) | \
> sort | sed s/\\.el/.log/)
>
> in test/infra/Makefile.in
> I thought that tree-sitter tests should go to either *-ts-mode-tests.el or treesit-tests.el files.
No, I don't think so. If having them in python-tests.el triggers some
problems, please show them, and let's take it from there.
> Would you like to start your assignment copyright
> paperwork rolling at this time? If yes, I will send you the form to
> fill and the instructions to email it.
>
> Yes, please.
Thanks, form sent off-list.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 11 Nov 2023 12:11:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Wed, 15 Nov 2023 16:08:03 GMT)
Full text and
rfc822 format available.
Message #20 received at 67061 <at> debbugs.gnu.org (full text, mbox):
Ping!
> Cc: 67061 <at> debbugs.gnu.org
> Date: Sat, 11 Nov 2023 09:32:48 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
>
> > From: Denis Zubarev <dvzubarev <at> yandex.ru>
> > Date: Sat, 11 Nov 2023 05:21:25 +0300
> >
> > Improve syntax highlighting for python-ts-mode.
> >
> > - Fontify compound keywords "not in" and "is not" (fixes bug#67015)
> > - Fix fontification of strings inside of f-strings interpolation,
> > e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
> > - Do not override the face of builtin functions (all, bytes etc.) with
> > the function call face
> > - Add missing assignment expressions
> > - Fontify built-ins (dict,list,etc.) as types when they are used in type hints
>
> Yuan, any comments?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 25 Nov 2023 09:36:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 67061 <at> debbugs.gnu.org (full text, mbox):
Ping! Ping! Yuan, could you please chime in? We are waiting for
Denis's legal paperwork to come through, but meanwhile I'd like your
opinions on the changes he proposed, and also on whether this should
be installed on the emacs-29 branch or on master.
> Cc: dvzubarev <at> yandex.ru, 67061 <at> debbugs.gnu.org
> Date: Wed, 15 Nov 2023 15:28:33 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
>
> Ping!
>
> > Cc: 67061 <at> debbugs.gnu.org
> > Date: Sat, 11 Nov 2023 09:32:48 +0200
> > From: Eli Zaretskii <eliz <at> gnu.org>
> >
> > > From: Denis Zubarev <dvzubarev <at> yandex.ru>
> > > Date: Sat, 11 Nov 2023 05:21:25 +0300
> > >
> > > Improve syntax highlighting for python-ts-mode.
> > >
> > > - Fontify compound keywords "not in" and "is not" (fixes bug#67015)
> > > - Fix fontification of strings inside of f-strings interpolation,
> > > e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
> > > - Do not override the face of builtin functions (all, bytes etc.) with
> > > the function call face
> > > - Add missing assignment expressions
> > > - Fontify built-ins (dict,list,etc.) as types when they are used in type hints
> >
> > Yuan, any comments?
>
>
>
>
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 26 Nov 2023 02:13:01 GMT)
Full text and
rfc822 format available.
Message #26 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 11/11/2023 13:00, Eli Zaretskii wrote:
>> What problems with CI did you see? I don't think I understand your
>> last sentence above.
>>
>> I've grepped the codebase and found those lines:
>>
>> TREE-SITTER-FILES ?= $(shell cd .. ; \
>> find lisp src \( -name "*-ts-mode-tests.el" -o -name "treesit-tests.el" \) | \
>> sort | sed s/\\.el/.log/)
>>
>> in test/infra/Makefile.in
>> I thought that tree-sitter tests should go to either *-ts-mode-tests.el or treesit-tests.el files.
> No, I don't think so. If having them in python-tests.el triggers some
> problems, please show them, and let's take it from there.
I'm guessing the main problem with that is that the test-tree-sitter job
on EMBA won't be picking those tests up.
Which is unfortunate, but not at all a deal-breaker (but if we could
make the build of python-ts-mode-tests.log load python.el, that would be
even better).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 26 Nov 2023 02:19:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 25/11/2023 11:35, Eli Zaretskii wrote:
> Ping! Ping! Yuan, could you please chime in? We are waiting for
> Denis's legal paperwork to come through, but meanwhile I'd like your
> opinions on the changes he proposed, and also on whether this should
> be installed on the emacs-29 branch or on master.
FWIW, the changes and the explanations look reasonable to me. The
presence of tests is a good sign too (though they'll probably need to
move to another file).
The fixes are relevant to Emacs 29 for sure.
I don't know at what stage we're going to start worrying when adding new
elements to the queries, though, in fear of breaking compatibility with
some potential older version of the grammar.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Wed, 29 Nov 2023 14:06:02 GMT)
Full text and
rfc822 format available.
Message #32 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 26 Nov 2023 04:17:59 +0200
> Cc: dvzubarev <at> yandex.ru, 67061 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> gutov.dev>
>
> On 25/11/2023 11:35, Eli Zaretskii wrote:
> > Ping! Ping! Yuan, could you please chime in? We are waiting for
> > Denis's legal paperwork to come through, but meanwhile I'd like your
> > opinions on the changes he proposed, and also on whether this should
> > be installed on the emacs-29 branch or on master.
>
> FWIW, the changes and the explanations look reasonable to me. The
> presence of tests is a good sign too (though they'll probably need to
> move to another file).
>
> The fixes are relevant to Emacs 29 for sure.
>
> I don't know at what stage we're going to start worrying when adding new
> elements to the queries, though, in fear of breaking compatibility with
> some potential older version of the grammar.
Thanks.
Yuan, please chime in, I'm waiting for your response.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 09 Dec 2023 00:41:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
[0002-Improve-syntax-highlighting-for-python-ts-mode.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 09 Dec 2023 07:33:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> From: Denis Zubarev <dvzubarev <at> yandex.ru>
> Cc: "casouri <at> gmail.com" <casouri <at> gmail.com>,
> "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
> Date: Sat, 09 Dec 2023 03:39:39 +0300
>
> I've moved tests to python-tests.el and added another fixes:
>
> assignment feature:
> `for var in range(3)` highlight var as font-lock-variable-name-face
> `var1[ii] = 1; t.var2[jj] = 2` highlight var1, var2 as font-lock-variable-name-face
>
> type feature:
> support nested optional types up to 3 level deep, for example `tuple[tuple, list[Lvl1 | Lvl2[Lvl3[Lvl3],
> Lvl2]]]`
>
>
> Summary of all changes in the patch:
>
> keyword feature:
> Add "is not" to the `python--treesit-keywords` list.
>
> assignment feature:
> For all examples,
> `for var in range(3)`
> `var1[ii] = 1; t.var2[jj] = 2`
> `var := 3`
> `var *= 3`
> highlight var as font-lock-variable-name-face
>
> string feature:
> Fix fontification of strings inside of f-strings interpolation,
> e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
>
> function feature:
> Do not override the face of builtin functions (all, bytes etc.) with
> the function call face
>
> type feature:
> Fontify built-ins (dict,list,etc.) as types when they are used in type hints.
> E.g. def func(v:dict[ list[ tuple[str] ], int | None] | None):
Thanks.
Yuan, would you please chime in and provide your comments, if any?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 09 Dec 2023 18:19:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 09/12/2023 02:39, Denis Zubarev wrote:
> assignment feature:
> For all examples,
> `for var in range(3)`
> `var1[ii] = 1; t.var2[jj] = 2`
> `var := 3`
> `var *= 3`
> highlight var as font-lock-variable-name-face
Arguably, the last 2 lines are "variable references" rather than
definitions (so font-lock-variable-use-face might make more sense),
since such operators imply that a variable has already been defined
previously.
And python--treesit-fontify-variable (in the 'variable' feature) already
applies that face.
The first example, however, should indeed use font-lock-variable-name-face.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 10 Dec 2023 10:18:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 12/8/23 11:32 PM, Eli Zaretskii wrote:
>> From: Denis Zubarev <dvzubarev <at> yandex.ru>
>> Cc: "casouri <at> gmail.com" <casouri <at> gmail.com>,
>> "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
>> Date: Sat, 09 Dec 2023 03:39:39 +0300
>>
>> I've moved tests to python-tests.el and added another fixes:
>>
>> assignment feature:
>> `for var in range(3)` highlight var as font-lock-variable-name-face
>> `var1[ii] = 1; t.var2[jj] = 2` highlight var1, var2 as font-lock-variable-name-face
>>
>> type feature:
>> support nested optional types up to 3 level deep, for example `tuple[tuple, list[Lvl1 | Lvl2[Lvl3[Lvl3],
>> Lvl2]]]`
>>
>>
>> Summary of all changes in the patch:
>>
>> keyword feature:
>> Add "is not" to the `python--treesit-keywords` list.
>>
>> assignment feature:
>> For all examples,
>> `for var in range(3)`
>> `var1[ii] = 1; t.var2[jj] = 2`
>> `var := 3`
>> `var *= 3`
>> highlight var as font-lock-variable-name-face
>>
>> string feature:
>> Fix fontification of strings inside of f-strings interpolation,
>> e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
>>
>> function feature:
>> Do not override the face of builtin functions (all, bytes etc.) with
>> the function call face
>>
>> type feature:
>> Fontify built-ins (dict,list,etc.) as types when they are used in type hints.
>> E.g. def func(v:dict[ list[ tuple[str] ], int | None] | None):
> Thanks.
>
> Yuan, would you please chime in and provide your comments, if any?
This should be the last bug report that I missed. I'll look at this
tomorrow, promise promise.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 10 Dec 2023 12:06:01 GMT)
Full text and
rfc822 format available.
Message #47 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Mon, 11 Dec 2023 00:02:01 GMT)
Full text and
rfc822 format available.
Message #50 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 10/12/2023 14:04, Denis Zubarev wrote:
> > Arguably, the last 2 lines are "variable references" rather than
> definitions
> `var := 3` is assignment expressions. It allows variable assignments to
> occur inside of larger expressions. For example `if (match :=
> pattern.search(data)) is not None:`.
> It mostly used to define new variables and act on them if some condition
> is met.
Ah, thanks! This feature is newer than my working experience with Python.
> My rationale for `var *= 3` was that it is shorthand for `var = var * 3`
> and currently the `var` on the left hand side is fontified with
> `font-lock-variable-name-face`.
I think ideally, in cases when "var =" is not the first occurrence for
the same var in a given scope, we wouldn't highlight it as "definition"
either.
Speaking of font-lock-variable-use-face, I think it would be most useful
if it helped with noticing typos (meaning, it would only be used for
references to variables that have been defined previously, according to
the rules of the language). But for that we still need to implement the
scope tracking. And before that, well, my personal preference is not to
highlight the references at all, but opinions differ.
> I wanted shorthand form to be consistent with the full form.
> Your point makes sense too, I don't have strong opinion about this.
> Also I'm not sure now about `var[ii] = 1`, since it is actually
> accessing the list or dictionary element and
> `font-lock-variable-use-face` may suit better here.
Yep. To sum up, I would add highlighting to your examples `for var in
range(3)` and `var := 3` but not others.
> Question about new changes.
> Should I push them to this patch and provide description of new changes,
> or it would be better to wait for review and send them as new patch?
I suggest sending an updated patch for review in this case, but you can
also wait for Yuan's comments first.
Also, please double-check that the tests pass. It might be something in
the way I applied your latest patch, but in the example that you gave
regarding the "type" feature,
def func(v:dict[ list[ tuple[str] ], int | None] | None):
, only the last "None" gets highlighted for me with font-lock-type-face.
There are corresponding test failures too. Again, could be something on
my side, but it would help to verify.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Mon, 11 Dec 2023 06:55:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 67061 <at> debbugs.gnu.org (full text, mbox):
Here's my long due review:
> Improve syntax highlighting for python-ts-mode.
>
> - Fontify compound keywords "not in" and "is not" (fixes bug#67015)
This is great, though you'll need to rebase your changes on top of
latest emacs-29 now, because I added one of the keywords when fixing
bug#67015. Sorry for the trouble.
> - Fix fontification of strings inside of f-strings interpolation,
> e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
I see that you removed string-interpolation feature, I don't know how I
feel about it. For one, if this fix is to be applied to emacs-29, we
cannot remove that feature—that's a breaking change IMO.
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.
> - Do not override the face of builtin functions (all, bytes etc.) with
> the function call face
> - Fontify built-ins (dict,list,etc.) as types when they are used in type hints
LGTM
> - Add missing assignment expressions
Do you have examples on the type of assignments you are adding
fontification for?
Some general tips: for comments, we always write complete sentences,
capitalize the first character and ends with a period. Also I see double
empty lines and missing empty lines at places.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Mon, 11 Dec 2023 07:12:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 12/10/23 4:00 PM, Dmitry Gutov wrote:
> On 10/12/2023 14:04, Denis Zubarev wrote:
>> > Arguably, the last 2 lines are "variable references" rather than
>> definitions
>> `var := 3` is assignment expressions. It allows variable assignments
>> to occur inside of larger expressions. For example `if (match :=
>> pattern.search(data)) is not None:`.
>> It mostly used to define new variables and act on them if some
>> condition is met.
>
> Ah, thanks! This feature is newer than my working experience with Python.
>
>> My rationale for `var *= 3` was that it is shorthand for `var = var *
>> 3` and currently the `var` on the left hand side is fontified with
>> `font-lock-variable-name-face`.
>
> I think ideally, in cases when "var =" is not the first occurrence for
> the same var in a given scope, we wouldn't highlight it as
> "definition" either.
>
> Speaking of font-lock-variable-use-face, I think it would be most
> useful if it helped with noticing typos (meaning, it would only be
> used for references to variables that have been defined previously,
> according to the rules of the language). But for that we still need to
> implement the scope tracking. And before that, well, my personal
> preference is not to highlight the references at all, but opinions
> differ.
Personally I regard the "assignment" feature to mean "any assignment",
rather than definition. But that's just me. For my personal taste, I
would make *= always highlight its LHS.
>> I wanted shorthand form to be consistent with the full form.
>> Your point makes sense too, I don't have strong opinion about this.
>> Also I'm not sure now about `var[ii] = 1`, since it is actually
>> accessing the list or dictionary element and
>> `font-lock-variable-use-face` may suit better here.
>
> Yep. To sum up, I would add highlighting to your examples `for var in
> range(3)` and `var := 3` but not others.
IMHO, for the assignment feature, we should stick to the narrow
definition of assignments, ie, anything that looks like "a = b". Things
like "for var in range(3)" could be highlighted by variable feature, I
think.
And for var[i] = 1, I don't know either. I think I decided to not
fontify it back then, but it wasn't based on any strong reasoning.
>> Question about new changes.
>> Should I push them to this patch and provide description of new changes,
>> or it would be better to wait for review and send them as new patch?
>
> I suggest sending an updated patch for review in this case, but you
> can also wait for Yuan's comments first.
>
If you can consolidate everything into a single patch, and pair it with
a summary, that'll be a great aid to me. Also, in case you don't know
yet, we follow certain format for commit messages, you can check it out
in the CONTRIBUTE file, "Commit messages" section.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Mon, 11 Dec 2023 12:03:02 GMT)
Full text and
rfc822 format available.
Message #59 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 11/12/2023 09:10, Yuan Fu wrote:
>>> I wanted shorthand form to be consistent with the full form.
>>> Your point makes sense too, I don't have strong opinion about this.
>>> Also I'm not sure now about `var[ii] = 1`, since it is actually
>>> accessing the list or dictionary element and
>>> `font-lock-variable-use-face` may suit better here.
>>
>> Yep. To sum up, I would add highlighting to your examples `for var in
>> range(3)` and `var := 3` but not others.
> IMHO, for the assignment feature, we should stick to the narrow
> definition of assignments, ie, anything that looks like "a = b". Things
> like "for var in range(3)" could be highlighted by variable feature, I
> think.
I think "for var in range(3)" should be part of the "definition" feature
because a variable is defined there. Alongside parameters.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Tue, 12 Dec 2023 01:19:01 GMT)
Full text and
rfc822 format available.
Message #62 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
[0003-Improve-syntax-highlighting-for-python-ts-mode.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Tue, 12 Dec 2023 08:26:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 67061 <at> debbugs.gnu.org (full text, mbox):
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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Wed, 13 Dec 2023 00:45:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 12/12/2023 10:24, Yuan Fu wrote:
>> > 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.)
I disagree: 'var' in this example is not much different from a function
parameter. It's a definite place where a variable's name introduced in
the current scope.
Python doesn't have special keywords for variable declarations (unlike
'let' in JavaScript or typed declaration in C), so the first time a
variable is introduced serves as its declaration. For assignments, we
can't easily determine which is the first time for a given scope, but
examples like 'for var in ...' or 'except ZeroDivisionError as e:' or
'[... for var in ...]' are all unambiguously variable definitions.
So I think that:
a) All variable definitions (functions parameters or not) should use
font-lock-variable-name-face -- to make it easier to find where a given
symbol is introduced.
b) No font-lock-variable-name-face highlights should be put into the
'variable' feature, which is disabled by default. All of the examples
above should either go into 'definition', or if somebody does like that
approach, into some new 'variable-declaration' feature (enabled by
default). And maybe some into 'assignment', which is on feature level 3.
c) The 'variable' feature should, at this point, only contain the
relatively useless highlights, since we don't track variable lifetimes
yet. That's why it's disabled by default.
The current situation across ts modes is that js-ts-mode has variable
declarations in the 'definition' feature (and not by my hand, FWIW);
ruby-ts-mode has a separate 'parameter-definition' feature that
encompasses both parameters and other variables; in c-ts-mode
highlighting for 'int i = 4' is split between 'definition' and
'assignment' (the latter seemingly redundant); typescript-ts-mode and
rust-ts-mode also follow the principle, more or less.
>> 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?
As per above, parameters don't seem too different from any other
variable declarations from my POV. They are similarly useful, so I'd
highlight them the same way.
Do we want to have a common face which would inherit from
font-lock-variable-name-face and would be used solely for
function/methods parameters and nothing else? I don't object, but I
don't quite see the point either.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Wed, 13 Dec 2023 03:51:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 12/12/23 4:44 PM, Dmitry Gutov wrote:
> On 12/12/2023 10:24, Yuan Fu wrote:
>>> > 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.)
>
> I disagree: 'var' in this example is not much different from a
> function parameter. It's a definite place where a variable's name
> introduced in the current scope.
>
> Python doesn't have special keywords for variable declarations (unlike
> 'let' in JavaScript or typed declaration in C), so the first time a
> variable is introduced serves as its declaration. For assignments, we
> can't easily determine which is the first time for a given scope, but
> examples like 'for var in ...' or 'except ZeroDivisionError as e:' or
> '[... for var in ...]' are all unambiguously variable definitions.
Sure, I don't really care too much about which feature should a rule be
in; what I do care about is to keep first and second fontification level
relatively quite and minimal, and keep level 3 reasonably conservative.
And people that want a lot of highlight can turn on level 4.
>
> So I think that:
>
> a) All variable definitions (functions parameters or not) should use
> font-lock-variable-name-face -- to make it easier to find where a
> given symbol is introduced.
> b) No font-lock-variable-name-face highlights should be put into the
> 'variable' feature, which is disabled by default. All of the examples
> above should either go into 'definition', or if somebody does like
> that approach, into some new 'variable-declaration' feature (enabled
> by default). And maybe some into 'assignment', which is on feature
> level 3.
> c) The 'variable' feature should, at this point, only contain the
> relatively useless highlights, since we don't track variable lifetimes
> yet. That's why it's disabled by default.
>
> The current situation across ts modes is that js-ts-mode has variable
> declarations in the 'definition' feature (and not by my hand, FWIW);
Gah!
> ruby-ts-mode has a separate 'parameter-definition' feature that
> encompasses both parameters and other variables;
> in c-ts-mode highlighting for 'int i = 4' is split between
> 'definition' and 'assignment' (the latter seemingly redundant);
Should've been in assignment IMO. I probably overlooked it.
> typescript-ts-mode and rust-ts-mode also follow the principle, more or
> less.
Well, the only ts-mode that I actually wrote is python-ts-mode. For
other major modes, I can only suggest. Even for python-ts-mode, I don't
want to exert my personal opinion onto it too much, except for keeping
font-lock level 1 and 2 quiet.
>>> 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?
>
> As per above, parameters don't seem too different from any other
> variable declarations from my POV. They are similarly useful, so I'd
> highlight them the same way.
>
> Do we want to have a common face which would inherit from
> font-lock-variable-name-face and would be used solely for
> function/methods parameters and nothing else? I don't object, but I
> don't quite see the point either.
I agree.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Wed, 13 Dec 2023 11:54:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> Date: Tue, 12 Dec 2023 00:24:41 -0800
> Cc: "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
> From: Yuan Fu <casouri <at> gmail.com>
>
> 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?
Denis's assignment is on file, so we are good to go with his
contributions.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Wed, 13 Dec 2023 18:30:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 13/12/2023 05:49, Yuan Fu wrote:
>> Python doesn't have special keywords for variable declarations (unlike
>> 'let' in JavaScript or typed declaration in C), so the first time a
>> variable is introduced serves as its declaration. For assignments, we
>> can't easily determine which is the first time for a given scope, but
>> examples like 'for var in ...' or 'except ZeroDivisionError as e:' or
>> '[... for var in ...]' are all unambiguously variable definitions.
>
> Sure, I don't really care too much about which feature should a rule be
> in; what I do care about is to keep first and second fontification level
> relatively quite and minimal, and keep level 3 reasonably conservative.
> And people that want a lot of highlight can turn on level 4.
I don't mind if assignments in python-ts-mode go to level 3, that's what
ruby-ts-mode does anyway. But '[... for var in ...]' really should use
variable-name-face and it should be in the default config (level 3 at
most). I think the 'definition' feature is good for it (going by the
name, since it's an implicit variable declaration), but it could be
split off into a separate feature too.
>> in c-ts-mode highlighting for 'int i = 4' is split between
>> 'definition' and 'assignment' (the latter seemingly redundant);
>
> Should've been in assignment IMO. I probably overlooked it.
The current state is that the query in 'definition' can highlight both
'int i;' and 'int i = 4;'. The query in 'assignment' in c-ts-mode only
highlights 'int i = 4;'.
If you just keep the latter query, 'int i;' would stay unfontified. If
you move the corresponding query from 'definition' to 'assignment', it
would start matching non-assignment declarations too. Might seem odd.
>> typescript-ts-mode and rust-ts-mode also follow the principle, more or
>> less.
>
> Well, the only ts-mode that I actually wrote is python-ts-mode. For
> other major modes, I can only suggest. Even for python-ts-mode, I don't
> want to exert my personal opinion onto it too much, except for keeping
> font-lock level 1 and 2 quiet.
For my part, I mostly care about keeping the level 3 feature-rich
enough, but precise at the same time. And without frivolous highlights
(only a little more fruit-salady than the pre-treesit modes).
>>>> 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?
>>
>> As per above, parameters don't seem too different from any other
>> variable declarations from my POV. They are similarly useful, so I'd
>> highlight them the same way.
>>
>> Do we want to have a common face which would inherit from
>> font-lock-variable-name-face and would be used solely for
>> function/methods parameters and nothing else? I don't object, but I
>> don't quite see the point either.
>
> I agree.
Then I suppose we should clarify whether Denis wants a face that only
matches function parameters, or implicit variable declarations as well.
Or maybe instead a face that is only used for assignments (only first
assignments?) -- which would separate them from the two semantic units
above.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Wed, 13 Dec 2023 21:18:01 GMT)
Full text and
rfc822 format available.
Message #80 received at 67061 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> gutov.dev> writes:
> I don't know at what stage we're going to start worrying when adding new
> elements to the queries, though, in fear of breaking compatibility with
> some potential older version of the grammar.
Right. But can we really stop installing support for new language
features that users expect?
I'm not super close to the tree-sitter stuff to be honest, so apologies
if I misunderstood something. But IIUC, the current situation means
that we can't depend on concrete versions of grammars, which means we
can't depend on that to make adaptions. But that situation is not
really caused by us, right?
So maybe at the point when we find problems in practice, we should just
throw up our hands and urge users to upgrade. Perhaps it'll encourage
more work on improving the situation with grammar versioning.
The biggest problems will be with faster moving languages, of course.
And who knows how common they will be even then - only experience will
tell.
Just my two cents.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Thu, 14 Dec 2023 01:32:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 13/12/2023 23:16, Stefan Kangas wrote:
> Dmitry Gutov <dmitry <at> gutov.dev> writes:
>
>> I don't know at what stage we're going to start worrying when adding new
>> elements to the queries, though, in fear of breaking compatibility with
>> some potential older version of the grammar.
>
> Right. But can we really stop installing support for new language
> features that users expect?
Some less important ones -- maybe. E.g. syntax highlighting is less
essential than indentation. Though it's usually easier to implement
(with tree-sitter, at least).
> I'm not super close to the tree-sitter stuff to be honest, so apologies
> if I misunderstood something. But IIUC, the current situation means
> that we can't depend on concrete versions of grammars, which means we
> can't depend on that to make adaptions. But that situation is not
> really caused by us, right?
We made our choices here too:
- Trying to support different versions of grammars, not just the latest
ones. Or a "pinned" revision.
- Adding treesit modes to the core, rather than publishing them to ELPA.
> So maybe at the point when we find problems in practice, we should just
> throw up our hands and urge users to upgrade. Perhaps it'll encourage
> more work on improving the situation with grammar versioning.
Simply asking our users to upgrade to the latest grammar won't work if
the major mode they are using is only compatible with some older grammar
version. And it's not upgradable because the major mode is not in ELPA.
We might solve this in the future with a two-step: "throwing up our
hands" and publishing major modes to "ELPA core". treesit.el will
probably need to stabilize a bit more before that, though.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Thu, 14 Dec 2023 05:55:02 GMT)
Full text and
rfc822 format available.
Message #86 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> On Dec 13, 2023, at 10:28 AM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
>
> On 13/12/2023 05:49, Yuan Fu wrote:
>
>>> Python doesn't have special keywords for variable declarations (unlike 'let' in JavaScript or typed declaration in C), so the first time a variable is introduced serves as its declaration. For assignments, we can't easily determine which is the first time for a given scope, but examples like 'for var in ...' or 'except ZeroDivisionError as e:' or '[... for var in ...]' are all unambiguously variable definitions.
>> Sure, I don't really care too much about which feature should a rule be in; what I do care about is to keep first and second fontification level relatively quite and minimal, and keep level 3 reasonably conservative. And people that want a lot of highlight can turn on level 4.
>
> I don't mind if assignments in python-ts-mode go to level 3, that's what ruby-ts-mode does anyway.
Assignment is in level 3 for python-ts-mode.
> But '[... for var in ...]' really should use variable-name-face and it should be in the default config (level 3 at most).
I’m fine with that.
> I think the 'definition' feature is good for it (going by the name, since it's an implicit variable declaration), but it could be split off into a separate feature too.
As long as it’s not added to the definition feature, because, again, definition is at level 1 and I don’t want to keep level 1 minimal.
Maybe we can use local-definition, or something similar, to signify that this feature highlights scoped definitions.
>
>>> in c-ts-mode highlighting for 'int i = 4' is split between 'definition' and 'assignment' (the latter seemingly redundant);
>> Should've been in assignment IMO. I probably overlooked it.
>
> The current state is that the query in 'definition' can highlight both 'int i;' and 'int i = 4;'. The query in 'assignment' in c-ts-mode only highlights 'int i = 4;'.
>
> If you just keep the latter query, 'int i;' would stay unfontified. If you move the corresponding query from 'definition' to 'assignment', it would start matching non-assignment declarations too. Might seem odd.
Right… hmm… This one is hard to decide...
>
>>> typescript-ts-mode and rust-ts-mode also follow the principle, more or less.
>> Well, the only ts-mode that I actually wrote is python-ts-mode. For other major modes, I can only suggest. Even for python-ts-mode, I don't want to exert my personal opinion onto it too much, except for keeping font-lock level 1 and 2 quiet.
>
> For my part, I mostly care about keeping the level 3 feature-rich enough, but precise at the same time. And without frivolous highlights (only a little more fruit-salady than the pre-treesit modes).
Sounds good to me :-)
>>>>> 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?
>>>
>>> As per above, parameters don't seem too different from any other variable declarations from my POV. They are similarly useful, so I'd highlight them the same way.
>>>
>>> Do we want to have a common face which would inherit from font-lock-variable-name-face and would be used solely for function/methods parameters and nothing else? I don't object, but I don't quite see the point either.
>> I agree.
>
> Then I suppose we should clarify whether Denis wants a face that only matches function parameters, or implicit variable declarations as well. Or maybe instead a face that is only used for assignments (only first assignments?) -- which would separate them from the two semantic units above.
I’m ok with either. And I’ll leave it to you guys to decide, like I did other faces we added in Emacs 29 ;-)
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Thu, 14 Dec 2023 12:12:02 GMT)
Full text and
rfc822 format available.
Message #89 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 14/12/2023 07:54, Yuan Fu wrote:
>> I think the 'definition' feature is good for it (going by the name, since it's an implicit variable declaration), but it could be split off into a separate feature too.
> As long as it’s not added to the definition feature, because, again, definition is at level 1 and I don’t want to keep level 1 minimal.
>
> Maybe we can use local-definition, or something similar, to signify that this feature highlights scoped definitions.
But you think function parameters should be in 'definition'? They are
also "scoped", I would say.
If not, we could have a separate feature 'variable-definition' which
would include parameters as well.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Thu, 14 Dec 2023 22:50:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 67061 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> gutov.dev> writes:
> We might solve this in the future with a two-step: "throwing up our
> hands" and publishing major modes to "ELPA core". treesit.el will
> probably need to stabilize a bit more before that, though.
Yeah, that's what I had in mind. It's not hard to make packages into
:core packages, we just have to be careful not to use
backwards-incompatible stuff in them.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Fri, 15 Dec 2023 07:16:02 GMT)
Full text and
rfc822 format available.
Message #95 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> On Dec 14, 2023, at 2:49 PM, Stefan Kangas <stefankangas <at> gmail.com> wrote:
>
> Dmitry Gutov <dmitry <at> gutov.dev> writes:
>
>> We might solve this in the future with a two-step: "throwing up our
>> hands" and publishing major modes to "ELPA core". treesit.el will
>> probably need to stabilize a bit more before that, though.
>
> Yeah, that's what I had in mind. It's not hard to make packages into
> :core packages, we just have to be careful not to use
> backwards-incompatible stuff in them.
Maybe we can think of that before Emacs 30 releases? By then treesit.el should be more or less stable and feature-complete.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 16 Dec 2023 13:05:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Wed, 13 Dec 2023 21:54:16 -0800
> Cc: Denis Zubarev <dvzubarev <at> yandex.ru>,
> Eli Zaretskii <eliz <at> gnu.org>,
> "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
>
> > Then I suppose we should clarify whether Denis wants a face that only matches function parameters, or implicit variable declarations as well. Or maybe instead a face that is only used for assignments (only first assignments?) -- which would separate them from the two semantic units above.
>
> I’m ok with either. And I’ll leave it to you guys to decide, like I did other faces we added in Emacs 29 ;-)
I'm not sure how to move forward here. The copyright assignment is on
file now, and Denis posted a rebased patch, but I'm not sure we all
agree that it should be installed. is there anything else that needs
to be done before Denis's patch can be installed on the emacs-29
branch?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 17 Dec 2023 00:27:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
[0004-Improve-syntax-highlighting-for-python-ts-mode.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 17 Dec 2023 01:08:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> On Dec 14, 2023, at 3:51 AM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
>
> On 14/12/2023 07:54, Yuan Fu wrote:
>>> I think the 'definition' feature is good for it (going by the name, since it's an implicit variable declaration), but it could be split off into a separate feature too.
>> As long as it’s not added to the definition feature, because, again, definition is at level 1 and I don’t want to keep level 1 minimal.
>> Maybe we can use local-definition, or something similar, to signify that this feature highlights scoped definitions.
>
> But you think function parameters should be in 'definition'? They are also "scoped", I would say.
I don’t think function parameters should be in ‘definition’. In fact, in my head, only the variable/function/class name of top-level constructs should be in ‘definition’. Now I can see that the name ‘definition’ is too vague for my original intent, and most ts modes probably don’t share the same interpretation as I do...
Maybe we can allow definition to include more things and move it to level 3, and add a more restricted ’top-level-definition’ to level 1 to take it’s current role. WDYT?
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 17 Dec 2023 01:11:01 GMT)
Full text and
rfc822 format available.
Message #107 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> On Dec 16, 2023, at 4:26 PM, Denis Zubarev <dvzubarev <at> yandex.ru> wrote:
>
> Sorry for the delayed response.
> > For many Emacs users, they want a quieter or even minimal fontification.
> I'm not against it. I just think that highlighting of an interpolation
> as a string is wrong. Is it possible to set quiet fontification in
> emacs-lisp mode, in such a way that `keywords' in doc-strings were
> fontified as a doc-string itself? I think it is similar to
> interpolation, it serves the purpose of separating different semantic
> elements from each other. IMHO, users who like quiet levels will benefit
> from interpolation highlighted differently.
I definitely can see your point, and it makes a lot of sense. I don’t really know people who want quieter fontification wants (probably both approach has their supporters) so can’t speak for them. But in general, I think it wouldn’t hurt to have the option.
> > 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.
> Done.
Thank you for your hard work! I’m just here talking and you went ahead and did all the work :-)
> > 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.)
> I added a new feature variable-definition for variables defined for local scopes and put it on the 3rd level.
> I also added rules to variable-definition feature for variables in list
> comprehension ( [var+1 for var in []] ) and as_pattern (with T as var:,
> except E as var:, case str() as var:).
> I've noticed that vars in `for var1, (var2, var3) in []:` are highlighted by the rule from the assignment feature (specifically `pattern_list`, `tuple_pattern`).
> It seems easy to fix `pattern_list`, but not so easy for
> `tuple_pattern`, since this node may occur recursively.
> I didn't touch these rules for now.
Ok, makes sense.
> 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.
> 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 enabled levels 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.
> 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. I think the only thing that’s still up to discussion is the variable-definition rules. Others can be merged to emacs-29.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 17 Dec 2023 01:57:02 GMT)
Full text and
rfc822 format available.
Message #110 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 17 Dec 2023 02:08:02 GMT)
Full text and
rfc822 format available.
Message #113 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
[0005-Improve-syntax-highlighting-for-python-ts-mode.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 17 Dec 2023 21:37:02 GMT)
Full text and
rfc822 format available.
Message #116 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 17/12/2023 03:07, Yuan Fu wrote:
>
>> On Dec 14, 2023, at 3:51 AM, Dmitry Gutov<dmitry <at> gutov.dev> wrote:
>>
>> On 14/12/2023 07:54, Yuan Fu wrote:
>>>> I think the 'definition' feature is good for it (going by the name, since it's an implicit variable declaration), but it could be split off into a separate feature too.
>>> As long as it’s not added to the definition feature, because, again, definition is at level 1 and I don’t want to keep level 1 minimal.
>>> Maybe we can use local-definition, or something similar, to signify that this feature highlights scoped definitions.
>> But you think function parameters should be in 'definition'? They are also "scoped", I would say.
> I don’t think function parameters should be in ‘definition’. In fact, in my head, only the variable/function/class name of top-level constructs should be in ‘definition’. Now I can see that the name ‘definition’ is too vague for my original intent, and most ts modes probably don’t share the same interpretation as I do...
>
> Maybe we can allow definition to include more things and move it to level 3, and add a more restricted ’top-level-definition’ to level 1 to take it’s current role. WDYT?
What about a split like function-definition/variable-definition? Not all
functions are top-level. But there are also classes...
Or the features could just be called 'definition' and
'variable-definition', and that the former only contains functions and
classes might simply be implied by the existence of the latter.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sun, 17 Dec 2023 23:40:02 GMT)
Full text and
rfc822 format available.
Message #119 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 17/12/2023 03:56, Denis Zubarev wrote:
> > Do we want to have a common face which would inherit from
> > font-lock-variable-name-face and would be used solely for
> > function/methods parameters and nothing else? I don't object, but I
> > don't quite see the point either.
> The point is to make it easy for users to customize faces of features
> independently from each other.
> It is not only about variables/parameters.
Granularity of faces can be increased, but one should also consider
which nodes go together better with which others.
E.g. even if variable-assignment is a separate face, we would need to
make it inherit from one of the more basic faces.
> For example, if I want to change a face for decorators, I have to change
> font-lock-type-face, which will change also all type faces.
> I like approach from the helix editor. They introduce many captures with
> different levels of specificity, for example @variable for (identifier),
> @variable.parameter for function parameters, @variable.builtin for
> self|cls etc. I guess by default the default face defined for a @variable
> is used. But one can customize variable.parameter to their liking
> without touching any
> other variables.
> > Then I suppose we should clarify whether Denis wants a face that only
> > matches function parameters, or implicit variable declarations as well.
> > Or maybe instead a face that is only used for assignments (only first
> > assignments?) -- which would separate them from the two semantic units
> > above.
> I think ideally, there should be a face for a feature (or even multiple
> faces).
> For example, faces for variables in helix notation:
> - @variable
> - @variable.definition
> - @variable.definition.parameter
> - @variable.assignment
> - @variable.use
I think this is fairly similar to our faces hierarchy, where children
inherit attributes from the parent. Just using a shorter notation.
Going back to what is a good thing for highlighting assignments, I would
separate "first assignments" from the rest, and either inherit their
face from "variable definition", or simply used the same face. Only in
languages like Python or Ruby, or course, where any first assignment is
an implicit variable declaration.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Mon, 18 Dec 2023 00:26:01 GMT)
Full text and
rfc822 format available.
Message #122 received at 67061 <at> debbugs.gnu.org (full text, mbox):
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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Tue, 19 Dec 2023 00:15:02 GMT)
Full text and
rfc822 format available.
Message #125 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
[0006-Improve-syntax-highlighting-for-python-ts-mode.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Wed, 20 Dec 2023 23:35:02 GMT)
Full text and
rfc822 format available.
Message #128 received at 67061 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Thu, 21 Dec 2023 07:05:02 GMT)
Full text and
rfc822 format available.
Message #131 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> On Dec 20, 2023, at 3:34 PM, Dmitry Gutov <dmitry <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 23 Dec 2023 09:44:01 GMT)
Full text and
rfc822 format available.
Message #134 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> From: Denis Zubarev <dvzubarev <at> yandex.ru>
> Cc: Dmitry Gutov <dmitry <at> gutov.dev>,
> Eli Zaretskii <eliz <at> gnu.org>,
> "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
> Date: Sun, 17 Dec 2023 05:07:01 +0300
>
> > Thanks. I think the only thing that’s still up to discussion is the variable-definition rules. Others can
> be merged to emacs-29.
>
> I can extract part with variable-definition into the next patch.
> In case it is ok, I attached patch without new variable-definition feature.
Thanks, but it doesn't apply cleanly to the current emacs-29 branch.
Would you mind please rebasing the patch on the emacs-29 branch and
resubmitting it?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 23 Dec 2023 21:47:01 GMT)
Full text and
rfc822 format available.
Message #137 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 23 Dec 2023 21:47:02 GMT)
Full text and
rfc822 format available.
Message #140 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 30 Dec 2023 10:54:02 GMT)
Full text and
rfc822 format available.
Message #143 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
[0008-Improve-syntax-highlighting-for-python-ts-mode.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 30 Dec 2023 11:20:02 GMT)
Full text and
rfc822 format available.
Message #146 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> From: Denis Zubarev <dvzubarev <at> yandex.ru>
> Cc: "casouri <at> gmail.com" <casouri <at> gmail.com>,
> "dmitry <at> gutov.dev" <dmitry <at> gutov.dev>,
> "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
> Date: Sat, 30 Dec 2023 13:53:38 +0300
>
> It seems, there is no consensus about new variable-definition feature. So I removed it from this
> patch.
> Later I will send a new patch for discussing it.
>
> I rebased on the latest emacs-29 and also fixed a test that failed on this branch.
> Please find the patch in the attachment.
Thanks, now installed on the emacs-29 branch.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Mon, 01 Jan 2024 17:43:02 GMT)
Full text and
rfc822 format available.
Message #149 received at 67061 <at> debbugs.gnu.org (full text, mbox):
On 23/12/2023 23:45, Denis Zubarev wrote:
> Just adding a rule for highlighting CamelCase identifiers as types would
> lead to many false positives. For example, global variables or an object
> instantiation.
It seems like the convention is to use ALL_CAPITAL for constants and
CamelCase for classes/constructors. Those can be distinguished with a
regexp, with single-char names being sorted into constants, which they
usually are.
I suppose some code could be violating that, but perhaps we should
remind such authors about that with highlighting as well.
Regarding object instantiation, I'd be happy to see the class name in
Class(...) instantiation calls highlighted with font-lock-type-face.
that's more useful that telling the user that they are seeing a function
call by the means of font-lock-function-call-face.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Tue, 09 Jan 2024 20:05:01 GMT)
Full text and
rfc822 format available.
Message #152 received at 67061 <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 1 Jan 2024 19:42:16 +0200
> Cc: "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
> From: Dmitry Gutov <dmitry <at> gutov.dev>
>
> On 23/12/2023 23:45, Denis Zubarev wrote:
> > Just adding a rule for highlighting CamelCase identifiers as types would
> > lead to many false positives. For example, global variables or an object
> > instantiation.
>
> It seems like the convention is to use ALL_CAPITAL for constants and
> CamelCase for classes/constructors. Those can be distinguished with a
> regexp, with single-char names being sorted into constants, which they
> usually are.
>
> I suppose some code could be violating that, but perhaps we should
> remind such authors about that with highlighting as well.
>
> Regarding object instantiation, I'd be happy to see the class name in
> Class(...) instantiation calls highlighted with font-lock-type-face.
> that's more useful that telling the user that they are seeing a function
> call by the means of font-lock-function-call-face.
Is there anything else left to do here, or should I close this bug?
bug Marked as fixed in versions 29.2.
Request was from
Stefan Kangas <stefankangas <at> gmail.com>
to
control <at> debbugs.gnu.org.
(Sun, 14 Jan 2024 06:17:01 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 20 Jan 2024 09:09:02 GMT)
Full text and
rfc822 format available.
Message #157 received at 67061 <at> debbugs.gnu.org (full text, mbox):
Ping! Should this bug be closed now?
> Cc: casouri <at> gmail.com, dvzubarev <at> yandex.ru, 67061 <at> debbugs.gnu.org
> Date: Tue, 09 Jan 2024 22:03:33 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
>
> > Date: Mon, 1 Jan 2024 19:42:16 +0200
> > Cc: "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
> > From: Dmitry Gutov <dmitry <at> gutov.dev>
> >
> > On 23/12/2023 23:45, Denis Zubarev wrote:
> > > Just adding a rule for highlighting CamelCase identifiers as types would
> > > lead to many false positives. For example, global variables or an object
> > > instantiation.
> >
> > It seems like the convention is to use ALL_CAPITAL for constants and
> > CamelCase for classes/constructors. Those can be distinguished with a
> > regexp, with single-char names being sorted into constants, which they
> > usually are.
> >
> > I suppose some code could be violating that, but perhaps we should
> > remind such authors about that with highlighting as well.
> >
> > Regarding object instantiation, I'd be happy to see the class name in
> > Class(...) instantiation calls highlighted with font-lock-type-face.
> > that's more useful that telling the user that they are seeing a function
> > call by the means of font-lock-function-call-face.
>
> Is there anything else left to do here, or should I close this bug?
>
>
>
>
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 27 Jan 2024 09:50:01 GMT)
Full text and
rfc822 format available.
Message #160 received at 67061 <at> debbugs.gnu.org (full text, mbox):
Ping! Ping! Can this bug be closed now, or do we have anything left to
do?
> Cc: dvzubarev <at> yandex.ru, 67061 <at> debbugs.gnu.org
> Date: Sat, 20 Jan 2024 11:08:18 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
>
> Ping! Should this bug be closed now?
>
> > Cc: casouri <at> gmail.com, dvzubarev <at> yandex.ru, 67061 <at> debbugs.gnu.org
> > Date: Tue, 09 Jan 2024 22:03:33 +0200
> > From: Eli Zaretskii <eliz <at> gnu.org>
> >
> > > Date: Mon, 1 Jan 2024 19:42:16 +0200
> > > Cc: "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
> > > From: Dmitry Gutov <dmitry <at> gutov.dev>
> > >
> > > On 23/12/2023 23:45, Denis Zubarev wrote:
> > > > Just adding a rule for highlighting CamelCase identifiers as types would
> > > > lead to many false positives. For example, global variables or an object
> > > > instantiation.
> > >
> > > It seems like the convention is to use ALL_CAPITAL for constants and
> > > CamelCase for classes/constructors. Those can be distinguished with a
> > > regexp, with single-char names being sorted into constants, which they
> > > usually are.
> > >
> > > I suppose some code could be violating that, but perhaps we should
> > > remind such authors about that with highlighting as well.
> > >
> > > Regarding object instantiation, I'd be happy to see the class name in
> > > Class(...) instantiation calls highlighted with font-lock-type-face.
> > > that's more useful that telling the user that they are seeing a function
> > > call by the means of font-lock-function-call-face.
> >
> > Is there anything else left to do here, or should I close this bug?
> >
> >
> >
> >
>
>
>
>
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#67061; Package
emacs.
(Sat, 27 Jan 2024 10:48:02 GMT)
Full text and
rfc822 format available.
Message #163 received at 67061 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/html, inline)]
Reply sent
to
Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility.
(Sat, 27 Jan 2024 11:32:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Denis Zubarev <dvzubarev <at> yandex.ru>:
bug acknowledged by developer.
(Sat, 27 Jan 2024 11:32:01 GMT)
Full text and
rfc822 format available.
Message #168 received at 67061-done <at> debbugs.gnu.org (full text, mbox):
> From: Denis Zubarev <dvzubarev <at> yandex.ru>
> Cc: "67061 <at> debbugs.gnu.org" <67061 <at> debbugs.gnu.org>
> Date: Sat, 27 Jan 2024 13:47:08 +0300
>
> I think, It can be closed since the patch was merged. I may send a new patch if I continue work on
> this.
Thanks, closing.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sat, 24 Feb 2024 12:24:06 GMT)
Full text and
rfc822 format available.
This bug report was last modified 1 year and 76 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.