GNU bug report logs - #70361
[PATCH] Add font-locking for operators in go-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 70361 in the body.
You can then email your comments to 70361 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 operators in go-ts-mode.
Date: Fri, 12 Apr 2024 22:23:05 -0700

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

Tags: patch


Adds font-locking for Go operators in `go-ts-mode'. The operator feature
is already added to `treesit-font-lock-feature-list' in `go-ts-mode` - this
just adds the missing rule to `go-ts-mode--font-lock-settings'.


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.33, cairo version 1.16.0) of 2024-04-07 built on noah-X580VD
Repository revision: ee377aaddf9d73116b93c3d54b1ffef9a3a8b925
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Ubuntu 22.04.4 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)]

[0001-Add-font-locking-for-operators-in-go-ts-mode.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noah Peart <noah.v.peart <at> gmail.com>
Cc: 70361 <at> debbugs.gnu.org
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Sat, 13 Apr 2024 10:20:40 +0300

> From: Noah Peart <noah.v.peart <at> gmail.com>
> Date: Fri, 12 Apr 2024 22:23:05 -0700
> 
> Adds font-locking for Go operators in `go-ts-mode'. The operator feature
> is already added to `treesit-font-lock-feature-list' in `go-ts-mode` - this
> just adds the missing rule to `go-ts-mode--font-lock-settings'.

Thanks.

The operators will be fontified only if treesit-font-lock-level is 4,
right?  That's the level at which we decided to fontify operators.

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

From: Noah Peart <noah.v.peart <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70361 <at> debbugs.gnu.org
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Sat, 13 Apr 2024 00:27:26 -0700

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

> The operators will be fontified only if treesit-font-lock-level is 4,
right?

Yea, operator is at level 4 - I didn't change the
treesit-font-lock-feature-list.



On Sat, Apr 13, 2024 at 12:20 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Noah Peart <noah.v.peart <at> gmail.com>
> > Date: Fri, 12 Apr 2024 22:23:05 -0700
> >
> > Adds font-locking for Go operators in `go-ts-mode'. The operator feature
> > is already added to `treesit-font-lock-feature-list' in `go-ts-mode` -
> this
> > just adds the missing rule to `go-ts-mode--font-lock-settings'.
>
> Thanks.
>
> The operators will be fontified only if treesit-font-lock-level is 4,
> right?  That's the level at which we decided to fontify operators.
>

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

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

From: Noah Peart <noah.v.peart <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70361 <at> debbugs.gnu.org
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Sat, 13 Apr 2024 00:32:59 -0700

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

On a somewhat related note, I was wondering why the treesit modes in emacs
define their `treesit-font-lock-feature-list`s in the mode definitions.

Wouldn't it be more user-friendly to `defvar` the feature list?

On Sat, Apr 13, 2024 at 12:27 AM Noah Peart <noah.v.peart <at> gmail.com> wrote:

> > The operators will be fontified only if treesit-font-lock-level is 4,
> right?
>
> Yea, operator is at level 4 - I didn't change the
> treesit-font-lock-feature-list.
>
>
>
> On Sat, Apr 13, 2024 at 12:20 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
>> > From: Noah Peart <noah.v.peart <at> gmail.com>
>> > Date: Fri, 12 Apr 2024 22:23:05 -0700
>> >
>> > Adds font-locking for Go operators in `go-ts-mode'. The operator feature
>> > is already added to `treesit-font-lock-feature-list' in `go-ts-mode` -
>> this
>> > just adds the missing rule to `go-ts-mode--font-lock-settings'.
>>
>> Thanks.
>>
>> The operators will be fontified only if treesit-font-lock-level is 4,
>> right?  That's the level at which we decided to fontify operators.
>>
>

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noah Peart <noah.v.peart <at> gmail.com>, Yuan Fu <casouri <at> gmail.com>
Cc: 70361 <at> debbugs.gnu.org
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Sat, 13 Apr 2024 10:40:09 +0300

> From: Noah Peart <noah.v.peart <at> gmail.com>
> Date: Sat, 13 Apr 2024 00:32:59 -0700
> Cc: 70361 <at> debbugs.gnu.org
> 
> On a somewhat related note, I was wondering why the treesit modes in emacs
> define their `treesit-font-lock-feature-list`s in the mode definitions.
> 
> Wouldn't it be more user-friendly to `defvar` the feature list?

AFAIR, we do that in the mode's settings because the translation of
general categories into mode-specific settings is not easy, and
because we want users to control that via the fontification level, not
below that.

What problems do you see with the current approach that would require
more fine-grained user control?  And why do you think a defvar is the
proper way of providing such control?

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

From: Noah Peart <noah.v.peart <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70361 <at> debbugs.gnu.org, Yuan Fu <casouri <at> gmail.com>
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Sat, 13 Apr 2024 00:53:45 -0700

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

> What problems do you see with the current approach that would require
more fine-grained user control?

The main issue for me is removing the `error` feature from any mode
that adds it with `:override t`. I find the override font-locking is
jarring - in some
Languages half the buffer can switch in and out of parse errors when you
do something as simple as removing a closing paren.

Also, allowing users to rearrange features at different levels seems like a
bonus.

> And why do you think a defvar is the proper way of providing such control?

You're right, I don't think it's the best way - it's just what I've been
doing in
treesit modes I've written till now.

On Sat, Apr 13, 2024 at 12:40 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Noah Peart <noah.v.peart <at> gmail.com>
> > Date: Sat, 13 Apr 2024 00:32:59 -0700
> > Cc: 70361 <at> debbugs.gnu.org
> >
> > On a somewhat related note, I was wondering why the treesit modes in
> emacs
> > define their `treesit-font-lock-feature-list`s in the mode definitions.
> >
> > Wouldn't it be more user-friendly to `defvar` the feature list?
>
> AFAIR, we do that in the mode's settings because the translation of
> general categories into mode-specific settings is not easy, and
> because we want users to control that via the fontification level, not
> below that.
>
> What problems do you see with the current approach that would require
> more fine-grained user control?  And why do you think a defvar is the
> proper way of providing such control?
>

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noah Peart <noah.v.peart <at> gmail.com>
Cc: 70361 <at> debbugs.gnu.org, casouri <at> gmail.com
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Sat, 13 Apr 2024 12:33:15 +0300

> From: Noah Peart <noah.v.peart <at> gmail.com>
> Date: Sat, 13 Apr 2024 00:53:45 -0700
> Cc: Yuan Fu <casouri <at> gmail.com>, 70361 <at> debbugs.gnu.org
> 
> > What problems do you see with the current approach that would require
> more fine-grained user control?
> 
> The main issue for me is removing the `error` feature from any mode
> that adds it with `:override t`. I find the override font-locking is jarring - in some
> Languages half the buffer can switch in and out of parse errors when you
> do something as simple as removing a closing paren.

Maybe we should have a user option to turn error feature on and off.
Yuan, WDYT?

> Also, allowing users to rearrange features at different levels seems like a bonus.

Not sure about this one.  We didn't have that in the traditional
font-lock, did we?

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

From: Yuan Fu <casouri <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70361 <at> debbugs.gnu.org, Noah Peart <noah.v.peart <at> gmail.com>, dev <at> rjt.dev
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Sun, 14 Apr 2024 16:25:54 -0700

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Noah Peart <noah.v.peart <at> gmail.com>
>> Date: Sat, 13 Apr 2024 00:53:45 -0700
>> Cc: Yuan Fu <casouri <at> gmail.com>, 70361 <at> debbugs.gnu.org
>> 
>> > What problems do you see with the current approach that would require
>> more fine-grained user control?
>> 
>> The main issue for me is removing the `error` feature from any mode
>> that adds it with `:override t`. I find the override font-locking is jarring - in some
>> Languages half the buffer can switch in and out of parse errors when you
>> do something as simple as removing a closing paren.
>
> Maybe we should have a user option to turn error feature on and off.
> Yuan, WDYT?

Sure, I’m not a big advocate for highlighting the errors to begin with.
The tree-sitter parse errors aren’t meant to be used for highlighting
anyway. This is just something some major mode provides because it’s a
nice bonus, and it’s effectiveness probably varies by language grammar.

The user can turn off error feature with
treesit-font-lock-recompute-features, like they do for any other
features. Do we want to add a user option in addition? In any case, even
if we add such user option, it should be added at the major mode level,
since the error feature is at major mode level, it isn’t something
provided uniformly across all tree-sitter modes.

>> Also, allowing users to rearrange features at different levels seems like a bonus.
>
> Not sure about this one.  We didn't have that in the traditional
> font-lock, did we?

If major modes define a xxx-mode-feature-list, sure, the user can
redefine that list to their liking. Again that’s a major mode level
decision.

I CC’ed Randy since he’s the maintainer of go-ts-mode.

Yuan

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

From: Yuan Fu <casouri <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70361 <at> debbugs.gnu.org, noah.v.peart <at> gmail.com
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Sun, 14 Apr 2024 16:34:16 -0700

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Noah Peart <noah.v.peart <at> gmail.com>
>> Date: Sat, 13 Apr 2024 00:32:59 -0700
>> Cc: 70361 <at> debbugs.gnu.org
>> 
>> On a somewhat related note, I was wondering why the treesit modes in emacs
>> define their `treesit-font-lock-feature-list`s in the mode definitions.
>> 
>> Wouldn't it be more user-friendly to `defvar` the feature list?
>
> AFAIR, we do that in the mode's settings because the translation of
> general categories into mode-specific settings is not easy, and
> because we want users to control that via the fontification level, not
> below that.
>
> What problems do you see with the current approach that would require
> more fine-grained user control?  And why do you think a defvar is the
> proper way of providing such control?

Replying to Noah, I don’t think there’s anything wrong for a major mode
to define a xxx-mode-feature-list and assign it to
treesit-font-lock-feature-list when setting up the major mode. We just
don’t want a central tree-sitter-font-lock-feature-list that controls
the feature list of every major mode in a single place.

But again, even if a xxx-mode defines xxx-mode-feature-list, it would be
a bit of a hassle to take out or remove a single feature from the list,
since it’s a list of list. (You can just setq the whole list, but then
you need to keep it in sync with upstream.) It’s probably easier to call
treesit-font-lock-recompute-features in the major mode hook anyway.

Yuan

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

From: Noah Peart <noah.v.peart <at> gmail.com>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 70361 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Mon, 15 Apr 2024 05:05:17 -0700

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

Thankyou, this all makes sense.  I do think it would be nice if it was
easier to somehow
add user customizations to the font-locking, especially since it is so easy
to write tree-sitter
font-locking rules.

> We just don’t want a central tree-sitter-font-lock-feature-list that
controls
the feature list of every major mode in a single place.

What would you think of something like a `user` feature that could be
enabled
globally and users could add their mode-specific rules there? Eg.

    (defvar my-treesit-font-lock-rules
      (treesit-font-lock-rules
       :language 'go
       :feature 'user
       '((identifier))

       :language 'typescript
       :feature 'user
       '((identifier))
       ;; ...
       ))

This wouldn't work very well in certain cases where ordering of the rules
was important, however.

On Sun, Apr 14, 2024 at 4:34 PM Yuan Fu <casouri <at> gmail.com> wrote:

>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> From: Noah Peart <noah.v.peart <at> gmail.com>
> >> Date: Sat, 13 Apr 2024 00:32:59 -0700
> >> Cc: 70361 <at> debbugs.gnu.org
> >>
> >> On a somewhat related note, I was wondering why the treesit modes in
> emacs
> >> define their `treesit-font-lock-feature-list`s in the mode definitions.
> >>
> >> Wouldn't it be more user-friendly to `defvar` the feature list?
> >
> > AFAIR, we do that in the mode's settings because the translation of
> > general categories into mode-specific settings is not easy, and
> > because we want users to control that via the fontification level, not
> > below that.
> >
> > What problems do you see with the current approach that would require
> > more fine-grained user control?  And why do you think a defvar is the
> > proper way of providing such control?
>
> Replying to Noah, I don’t think there’s anything wrong for a major mode
> to define a xxx-mode-feature-list and assign it to
> treesit-font-lock-feature-list when setting up the major mode. We just
> don’t want a central tree-sitter-font-lock-feature-list that controls
> the feature list of every major mode in a single place.
>
> But again, even if a xxx-mode defines xxx-mode-feature-list, it would be
> a bit of a hassle to take out or remove a single feature from the list,
> since it’s a list of list. (You can just setq the whole list, but then
> you need to keep it in sync with upstream.) It’s probably easier to call
> treesit-font-lock-recompute-features in the major mode hook anyway.
>
> Yuan
>

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

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

From: Randy Taylor <dev <at> rjt.dev>
To: Noah Peart <noah.v.peart <at> gmail.com>
Cc: 70361 <at> debbugs.gnu.org
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Tue, 16 Apr 2024 01:49:53 +0000

On Saturday, April 13th, 2024 at 01:23, Noah Peart <noah.v.peart <at> gmail.com> wrote:
> Tags: patch
> 
> 
> Adds font-locking for Go operators in `go-ts-mode'. The operator feature
> is already added to `treesit-font-lock-feature-list' in `go-ts-mode` - this
> just adds the missing rule to `go-ts-mode--font-lock-settings'.

Thanks for working on this.

It looks good to me, would someone be able to install it? Thanks in advance.

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

From: Yuan Fu <casouri <at> gmail.com>
To: Noah Peart <noah.v.peart <at> gmail.com>
Cc: 70361 <at> debbugs.gnu.org, eliz <at> gnu.org
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Mon, 15 Apr 2024 22:57:43 -0700

Noah Peart <noah.v.peart <at> gmail.com> writes:

> Thankyou, this all makes sense.  I do think it would be nice if it was easier to somehow 
> add user customizations to the font-locking, especially since it is so easy to write tree-sitter
> font-locking rules.

Right. We (well, I) were focusing on support for writing major modes and
didn’t give enough attention to the customization aspect, especially for
beginners.

>> We just don’t want a central tree-sitter-font-lock-feature-list that controls
> the feature list of every major mode in a single place.
>
> What would you think of something like a `user` feature that could be enabled
> globally and users could add their mode-specific rules there? Eg.
>
>     (defvar my-treesit-font-lock-rules
>       (treesit-font-lock-rules
>        :language 'go
>        :feature 'user
>        '((identifier))
>
>        :language 'typescript
>        :feature 'user
>        '((identifier))
>        ;; ...
>        ))
>
> This wouldn't work very well in certain cases where ordering of the rules
> was important, however.

I think that’s a good start. I like that it’s tied to each language
rather than a major mode, so it avoids problems with derived major
modes.

Indentation can also use some easy customization options.

On a separate note, something like this might be better off in a ELPA
package, such that it has more freedom to evolve and break things.
Things that are added to Emacs core tend to fossilize, or at least
become very hard to change. (To be precise, I mean the interface tend to
be hard to change, implementation details can and do change all the
time.)

I’d say that order wouldn’t be a problem since I imagine most of
tht time, the user wants their fontification override all else.

Yuan

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

From: Yuan Fu <casouri <at> gmail.com>
To: Randy Taylor <dev <at> rjt.dev>
Cc: 70361 <at> debbugs.gnu.org, noah.v.peart <at> gmail.com
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Mon, 15 Apr 2024 22:58:39 -0700

Randy Taylor <dev <at> rjt.dev> writes:

> On Saturday, April 13th, 2024 at 01:23, Noah Peart <noah.v.peart <at> gmail.com> wrote:
>> Tags: patch
>> 
>> 
>> Adds font-locking for Go operators in `go-ts-mode'. The operator feature
>> is already added to `treesit-font-lock-feature-list' in `go-ts-mode` - this
>> just adds the missing rule to `go-ts-mode--font-lock-settings'.
>
> Thanks for working on this.
>
> It looks good to me, would someone be able to install it? Thanks in advance.

Done.

Yuan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 70361 <at> debbugs.gnu.org, dev <at> rjt.dev, noah.v.peart <at> gmail.com
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Tue, 16 Apr 2024 15:34:46 +0300

> Cc: 70361 <at> debbugs.gnu.org, noah.v.peart <at> gmail.com
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Mon, 15 Apr 2024 22:58:39 -0700
> 
> 
> Randy Taylor <dev <at> rjt.dev> writes:
> 
> > Thanks for working on this.
> >
> > It looks good to me, would someone be able to install it? Thanks in advance.
> 
> Done.

Thanks.  Should this bug be closed now?

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

From: Yuan Fu <casouri <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70361-done <at> debbugs.gnu.org, dev <at> rjt.dev, noah.v.peart <at> gmail.com
Subject: Re: bug#70361: [PATCH] Add font-locking for operators in go-ts-mode.
Date: Tue, 16 Apr 2024 19:47:37 -0700

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: 70361 <at> debbugs.gnu.org, noah.v.peart <at> gmail.com
>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Mon, 15 Apr 2024 22:58:39 -0700
>> 
>> 
>> Randy Taylor <dev <at> rjt.dev> writes:
>> 
>> > Thanks for working on this.
>> >
>> > It looks good to me, would someone be able to install it? Thanks in advance.
>> 
>> Done.
>
> Thanks.  Should this bug be closed now?

Yeah we can continue the discussion but close the bug.

Yuan

Previous Next

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