GNU bug report logs - #67809
[PATCH] Add font-locking for assignments in typescript-ts-mode

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 67809 in the body.
You can then email your comments to 67809 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Noah Peart <noah.v.peart <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add font-locking for assignments in typescript-ts-mode
Date: Wed, 13 Dec 2023 00:33:16 -0800

[Message part 1 (text/plain, inline)]

Tags: patch


This patch adds fontification for the left-hand side of assignments in
typescript-ts-mode.

The object and/or property being assigned to is fontified with
`font-lock-variable-name-face` and/or `font-lock-property-name-face`,
while any other identifiers/properties on the left-hand side used in
index expressions are fontified with
`font-lock-variable-use-face`/`font-lock-property-use-face`.

For example, fontification is added in `typescript-ts-mode` for
identifiers in statements like the following:

    arr[obj.x * obj.x] = 1;
    //^ font-lock-variable-name-face
    //    ^ font-lock-variable-use-face
    //      ^ font-lock-property-use-face
    obj.x.y = 0;
    //^ font-lock-variable-name-face
    //  ^ font-lock-property-name-face
    //    ^ font-lock-property-name-face
    ++mat[x][arr[0]];
    // ^ font-lock-variable-name-face
    //        ^ font-lock-variable-use-face

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.33, cairo version 1.16.0) of 2023-12-12 built on noah-X580VD
Repository revision: 75fd7550ed6cede6c9e8224f1f2d62637c43fdd4
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Ubuntu 22.04.3 LTS

Configured using:
 'configure --prefix=/usr/local --with-modules --with-tree-sitter
--with-threads --with-x-toolkit=gtk3 --with-xwidgets --with-gnutls
--with-json --with-mailutils --with-jpeg --with-png --with-rsvg
--with-tiff --with-xml2 --with-xpm --with-imagemagick CC=gcc-12
CXX=gcc-12'

[Message part 2 (text/html, inline)]

[typescript-ts-fontify-assignments.patch (text/x-patch, attachment)]

Message #8 received at 67809 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Noah Peart <noah.v.peart <at> gmail.com>, 67809 <at> debbugs.gnu.org
Subject: Re: bug#67809: [PATCH] Add font-locking for assignments in
 typescript-ts-mode
Date: Wed, 13 Dec 2023 20:31:06 +0200

Hi!

On 13/12/2023 10:33, Noah Peart wrote:
>      arr[obj.x * obj.x] = 1;
>      //^ font-lock-variable-name-face
>      //    ^ font-lock-variable-use-face
>      //      ^ font-lock-property-use-face
>      obj.x.y = 0;
>      //^ font-lock-variable-name-face
>      //  ^ font-lock-property-name-face
>      //    ^ font-lock-property-name-face
>      ++mat[x][arr[0]];
>      // ^ font-lock-variable-name-face
>      //        ^ font-lock-variable-use-face

I think in all of these cases font-lock-variable-name-face should not be 
used, since arr, and obj, and mat, are all introduced (declared) at a 
different place.

font-lock-variable-use-face is more appropriate.

Message #11 received at 67809 <at> debbugs.gnu.org (full text, mbox):

From: Noah Peart <noah.v.peart <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67809 <at> debbugs.gnu.org
Subject: Re: bug#67809: [PATCH] Add font-locking for assignments in
 typescript-ts-mode
Date: Wed, 13 Dec 2023 11:26:55 -0800

[Message part 1 (text/plain, inline)]

I thought `font-lock-variable-name-face` was standard for assignments?
It's what is applied
in ruby-ts-mode, python-ts-mode, and c-ts-mode for example.  I like it
personally, cause
it allows for visual distinction between l/r values.

Either way, I think the patch can be simplified to only highlight the
variable being assigned
and not any others on the left-hand side.  It would be simpler and more
customizable
to highlight any remaining variables in a `variable` feature in a following
rule.

I noticed another issue where I forgot to handle `this.var = ` cases as
well. I could take
another stab at it unless there's no interest in this feature.

On Wed, Dec 13, 2023 at 10:31 AM Dmitry Gutov <dmitry <at> gutov.dev> wrote:

> Hi!
>
> On 13/12/2023 10:33, Noah Peart wrote:
> >      arr[obj.x * obj.x] = 1;
> >      //^ font-lock-variable-name-face
> >      //    ^ font-lock-variable-use-face
> >      //      ^ font-lock-property-use-face
> >      obj.x.y = 0;
> >      //^ font-lock-variable-name-face
> >      //  ^ font-lock-property-name-face
> >      //    ^ font-lock-property-name-face
> >      ++mat[x][arr[0]];
> >      // ^ font-lock-variable-name-face
> >      //        ^ font-lock-variable-use-face
>
> I think in all of these cases font-lock-variable-name-face should not be
> used, since arr, and obj, and mat, are all introduced (declared) at a
> different place.
>
> font-lock-variable-use-face is more appropriate.
>

[Message part 2 (text/html, inline)]

Message #14 received at 67809 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Noah Peart <noah.v.peart <at> gmail.com>
Cc: 67809 <at> debbugs.gnu.org
Subject: Re: bug#67809: [PATCH] Add font-locking for assignments in
 typescript-ts-mode
Date: Wed, 13 Dec 2023 22:45:51 +0200

On 13/12/2023 21:26, Noah Peart wrote:
> I thought `font-lock-variable-name-face` was standard for assignments?  
> It's what is applied
> in ruby-ts-mode, python-ts-mode, and c-ts-mode for example.  I like it 
> personally, cause
> it allows for visual distinction between l/r values.

ruby and python's parsers cannot distinguish between assignments that 
introduce a new variable (thus working as an implicit declaration) and 
those that reassign an existing variable. Hopefully, we'll be able to 
improve that in the future.

TypeScript, however, has explicit variable declarations.

> Either way, I think the patch can be simplified to only highlight the 
> variable being assigned
> and not any others on the left-hand side.  It would be simpler and more 
> customizable
> to highlight any remaining variables in a `variable` feature in a 
> following rule.

I'm also not sure I agree that, for example, 'arr' is the variable being 
assigned to in the first example. Its value (the reference) doesn't change.

But it's totally fine to add a 'variable' feature to 
typescript-ts-mode's font-lock to apply font-lock-variable-use-face to it.

> I noticed another issue where I forgot to handle `this.var = ` cases as 
> well. I could take
> another stab at it unless there's no interest in this feature.
> 
> On Wed, Dec 13, 2023 at 10:31 AM Dmitry Gutov <dmitry <at> gutov.dev 
> <mailto:dmitry <at> gutov.dev>> wrote:
> 
>     Hi!
> 
>     On 13/12/2023 10:33, Noah Peart wrote:
>      >      arr[obj.x * obj.x] = 1;
>      >      //^ font-lock-variable-name-face
>      >      //    ^ font-lock-variable-use-face
>      >      //      ^ font-lock-property-use-face
>      >      obj.x.y = 0;
>      >      //^ font-lock-variable-name-face
>      >      //  ^ font-lock-property-name-face
>      >      //    ^ font-lock-property-name-face
>      >      ++mat[x][arr[0]];
>      >      // ^ font-lock-variable-name-face
>      >      //        ^ font-lock-variable-use-face
> 
>     I think in all of these cases font-lock-variable-name-face should
>     not be
>     used, since arr, and obj, and mat, are all introduced (declared) at a
>     different place.
> 
>     font-lock-variable-use-face is more appropriate.
> 

Message #19 received at 67809 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: noah.v.peart <at> gmail.com, 67809 <at> debbugs.gnu.org
Subject: Re: bug#67809: [PATCH] Add font-locking for assignments in
 typescript-ts-mode
Date: Sat, 23 Dec 2023 10:45:19 +0200

> Cc: 67809 <at> debbugs.gnu.org
> Date: Wed, 13 Dec 2023 22:45:51 +0200
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> On 13/12/2023 21:26, Noah Peart wrote:
> > I thought `font-lock-variable-name-face` was standard for assignments?  
> > It's what is applied
> > in ruby-ts-mode, python-ts-mode, and c-ts-mode for example.  I like it 
> > personally, cause
> > it allows for visual distinction between l/r values.
> 
> ruby and python's parsers cannot distinguish between assignments that 
> introduce a new variable (thus working as an implicit declaration) and 
> those that reassign an existing variable. Hopefully, we'll be able to 
> improve that in the future.
> 
> TypeScript, however, has explicit variable declarations.
> 
> > Either way, I think the patch can be simplified to only highlight the 
> > variable being assigned
> > and not any others on the left-hand side.  It would be simpler and more 
> > customizable
> > to highlight any remaining variables in a `variable` feature in a 
> > following rule.
> 
> I'm also not sure I agree that, for example, 'arr' is the variable being 
> assigned to in the first example. Its value (the reference) doesn't change.
> 
> But it's totally fine to add a 'variable' feature to 
> typescript-ts-mode's font-lock to apply font-lock-variable-use-face to it.

What is the conclusion here?  Do we have an agreed-upon patch to
install?

Thanks.

Message #22 received at 67809 <at> debbugs.gnu.org (full text, mbox):

From: Noah Peart <noah.v.peart <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, 67809 <at> debbugs.gnu.org
Subject: Re: bug#67809: [PATCH] Add font-locking for assignments in
 typescript-ts-mode
Date: Sat, 23 Dec 2023 05:39:17 -0500

[Message part 1 (text/plain, inline)]

I think this should be closed for now.

On Sat, Dec 23, 2023 at 3:45 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > Cc: 67809 <at> debbugs.gnu.org
> > Date: Wed, 13 Dec 2023 22:45:51 +0200
> > From: Dmitry Gutov <dmitry <at> gutov.dev>
> >
> > On 13/12/2023 21:26, Noah Peart wrote:
> > > I thought `font-lock-variable-name-face` was standard for
> assignments?
> > > It's what is applied
> > > in ruby-ts-mode, python-ts-mode, and c-ts-mode for example.  I like it
> > > personally, cause
> > > it allows for visual distinction between l/r values.
> >
> > ruby and python's parsers cannot distinguish between assignments that
> > introduce a new variable (thus working as an implicit declaration) and
> > those that reassign an existing variable. Hopefully, we'll be able to
> > improve that in the future.
> >
> > TypeScript, however, has explicit variable declarations.
> >
> > > Either way, I think the patch can be simplified to only highlight the
> > > variable being assigned
> > > and not any others on the left-hand side.  It would be simpler and
> more
> > > customizable
> > > to highlight any remaining variables in a `variable` feature in a
> > > following rule.
> >
> > I'm also not sure I agree that, for example, 'arr' is the variable being
> > assigned to in the first example. Its value (the reference) doesn't
> change.
> >
> > But it's totally fine to add a 'variable' feature to
> > typescript-ts-mode's font-lock to apply font-lock-variable-use-face to
> it.
>
> What is the conclusion here?  Do we have an agreed-upon patch to
> install?
>
> Thanks.
>

[Message part 2 (text/html, inline)]

Message #25 received at 67809 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noah Peart <noah.v.peart <at> gmail.com>
Cc: dmitry <at> gutov.dev, 67809 <at> debbugs.gnu.org
Subject: Re: bug#67809: [PATCH] Add font-locking for assignments in
 typescript-ts-mode
Date: Sat, 23 Dec 2023 13:21:49 +0200

> From: Noah Peart <noah.v.peart <at> gmail.com>
> Date: Sat, 23 Dec 2023 05:39:17 -0500
> Cc: Dmitry Gutov <dmitry <at> gutov.dev>, 67809 <at> debbugs.gnu.org
> 
> I think this should be closed for now.

If Dmitry agrees, it's fine by me.

Message #30 received at 67809-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>, Noah Peart <noah.v.peart <at> gmail.com>
Cc: 67809-done <at> debbugs.gnu.org
Subject: Re: bug#67809: [PATCH] Add font-locking for assignments in
 typescript-ts-mode
Date: Sat, 23 Dec 2023 22:57:17 +0200

On 23/12/2023 13:21, Eli Zaretskii wrote:
>> From: Noah Peart<noah.v.peart <at> gmail.com>
>> Date: Sat, 23 Dec 2023 05:39:17 -0500
>> Cc: Dmitry Gutov<dmitry <at> gutov.dev>,67809 <at> debbugs.gnu.org
>>
>> I think this should be closed for now.
> If Dmitry agrees, it's fine by me.

Closing. I'm sorry if this was a disappointing response.

Previous Next

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