GNU bug report logs - #60105
[PATCH] Add yaml-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 60105 in the body.
You can then email your comments to 60105 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: Randy Taylor <dev <at> rjt.dev>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: [PATCH] Add yaml-ts-mode
Date: Thu, 15 Dec 2022 22:19:58 +0000

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

X-Debbugs-CC: casouri <at> gmail.com

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

[0001-Add-yaml-ts-mode.patch (text/x-patch, attachment)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Randy Taylor <dev <at> rjt.dev>, 60105 <at> debbugs.gnu.org
Cc: casouri <at> gmail.com
Subject: Re: bug#60105: [PATCH] Add yaml-ts-mode
Date: Thu, 15 Dec 2022 16:20:04 -0800

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

> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index 9c5a361df7..4319623e64 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -224,7 +224,7 @@ eglot-server-programs
>                                  ((tex-mode context-mode texinfo-mode bibtex-mode)
>                                   . ,(eglot-alternatives '("digestif" "texlab")))
>                                  (erlang-mode . ("erlang_ls" "--transport" "stdio"))
> -                                (yaml-mode . ("yaml-language-server" "--stdio"))
> +                                ((yaml-mode yaml-ts-mode) . ("yaml-language-server" "--stdio"))

This is a nit, but I think it might make more sense to users if we put
the built-in mode first in the list.  After all, the one we ship with
Emacs is, or will be, the standard mode.

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

From: Randy Taylor <dev <at> rjt.dev>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: casouri <at> gmail.com, 60105 <at> debbugs.gnu.org
Subject: Re: bug#60105: [PATCH] Add yaml-ts-mode
Date: Fri, 16 Dec 2022 02:01:42 +0000

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

On Thursday, December 15th, 2022 at 19:20, Stefan Kangas <stefankangas <at> gmail.com> wrote:
> 
> Randy Taylor dev <at> rjt.dev writes:
> 
> > diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> > index 9c5a361df7..4319623e64 100644
> > --- a/lisp/progmodes/eglot.el
> > +++ b/lisp/progmodes/eglot.el
> > @@ -224,7 +224,7 @@ eglot-server-programs
> > ((tex-mode context-mode texinfo-mode bibtex-mode)
> > . ,(eglot-alternatives '("digestif" "texlab")))
> > (erlang-mode . ("erlang_ls" "--transport" "stdio"))
> > - (yaml-mode . ("yaml-language-server" "--stdio"))
> > + ((yaml-mode yaml-ts-mode) . ("yaml-language-server" "--stdio"))
> 
> 
> This is a nit, but I think it might make more sense to users if we put
> the built-in mode first in the list. After all, the one we ship with
> Emacs is, or will be, the standard mode.

Sounds good to me. I'll submit another patch tomorrow making that change for the rest of the modes we've added (unless someone beats me to it).

Attached is a new patch that applies cleanly against emacs-29 and has yaml-ts-mode come first for eglot.

[0001-Add-yaml-ts-mode-Bug-60105.patch (text/x-patch, attachment)]

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

From: Yuan Fu <casouri <at> gmail.com>
To: Randy Taylor <dev <at> rjt.dev>
Cc: Stefan Kangas <stefankangas <at> gmail.com>, 60105-done <at> debbugs.gnu.org
Subject: Re: bug#60105: [PATCH] Add yaml-ts-mode
Date: Fri, 16 Dec 2022 14:55:34 -0800

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

> On Thursday, December 15th, 2022 at 19:20, Stefan Kangas <stefankangas <at> gmail.com> wrote:
>> 
>> Randy Taylor dev <at> rjt.dev writes:
>> 
>> > diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
>> > index 9c5a361df7..4319623e64 100644
>> > --- a/lisp/progmodes/eglot.el
>> > +++ b/lisp/progmodes/eglot.el
>> > @@ -224,7 +224,7 @@ eglot-server-programs
>> > ((tex-mode context-mode texinfo-mode bibtex-mode)
>> > . ,(eglot-alternatives '("digestif" "texlab")))
>> > (erlang-mode . ("erlang_ls" "--transport" "stdio"))
>> > - (yaml-mode . ("yaml-language-server" "--stdio"))
>> > + ((yaml-mode yaml-ts-mode) . ("yaml-language-server" "--stdio"))
>> 
>> 
>> This is a nit, but I think it might make more sense to users if we put
>> the built-in mode first in the list. After all, the one we ship with
>> Emacs is, or will be, the standard mode.
>
> Sounds good to me. I'll submit another patch tomorrow making that change for the rest of the modes we've added (unless someone beats me to it).
>
> Attached is a new patch that applies cleanly against emacs-29 and has yaml-ts-mode come first for eglot.
>

LGTM, I applied the patch, thanks!

Yuan

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

From: Juri Linkov <juri <at> linkov.net>
To: Randy Taylor <dev <at> rjt.dev>
Cc: casouri <at> gmail.com, 60105 <at> debbugs.gnu.org
Subject: Re: bug#60105: [PATCH] Add yaml-ts-mode
Date: Mon, 02 Jan 2023 20:52:19 +0200

> +   :language 'yaml
> +   :feature 'string
> +   :override t
> +   '([(block_scalar)
> +      (double_quote_scalar)
> +      (single_quote_scalar)
> +      (string_scalar)] @font-lock-string-face)

Thanks, yaml-ts-mode works great.  One problem is that
with the above setting everything is (over)fontified in the buffer.
This is the only mode I have seen where 100% of text has
non-default colors making it so called "angry fruit salad".
In this regard yaml-mode is not better: it fontifies only text in quotes
that makes an unnecessary distinction between quoted and unquoted text.
I know it's possible to configure this in a hackish way:

  (with-eval-after-load 'yaml-ts-mode
    (setq yaml-ts-mode--font-lock-settings
          (seq-remove (lambda (e) (eq (nth 2 e) 'string))
                      yaml-ts-mode--font-lock-settings)))

But what I propose is to add a customizable option to enable/disable
font-lock-string-face on most text to lessen the color burden on users.

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

From: Randy Taylor <dev <at> rjt.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: casouri <at> gmail.com, 60105 <at> debbugs.gnu.org
Subject: Re: bug#60105: [PATCH] Add yaml-ts-mode
Date: Mon, 02 Jan 2023 21:58:57 +0000

On Monday, January 2nd, 2023 at 13:52, Juri Linkov <juri <at> linkov.net> wrote:
> 
> > + :language 'yaml
> 
> > + :feature 'string
> > + :override t
> > + '([(block_scalar)
> > + (double_quote_scalar)
> > + (single_quote_scalar)
> > + (string_scalar)] @font-lock-string-face)
> 
> 
> Thanks, yaml-ts-mode works great. One problem is that
> with the above setting everything is (over)fontified in the buffer.
> This is the only mode I have seen where 100% of text has
> non-default colors making it so called "angry fruit salad".
> In this regard yaml-mode is not better: it fontifies only text in quotes
> that makes an unnecessary distinction between quoted and unquoted text.
> I know it's possible to configure this in a hackish way:
> 
> (with-eval-after-load 'yaml-ts-mode
> (setq yaml-ts-mode--font-lock-settings
> (seq-remove (lambda (e) (eq (nth 2 e) 'string))
> yaml-ts-mode--font-lock-settings)))
> 
> But what I propose is to add a customizable option to enable/disable
> font-lock-string-face on most text to lessen the color burden on users.

I think using treesit-font-lock-recompute-features is the way to adjust which features you want, and is what is expected for cases like this (but Yuan would know best).

Alternatively, treesit-font-lock-level dictates which level of features should be included for highlighting. The default is level 3, and string is on level 2. We can move string to the 4th level, which may be an OK compromise?

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

From: Juri Linkov <juri <at> linkov.net>
To: Randy Taylor <dev <at> rjt.dev>
Cc: casouri <at> gmail.com, 60105 <at> debbugs.gnu.org
Subject: Re: bug#60105: [PATCH] Add yaml-ts-mode
Date: Tue, 03 Jan 2023 20:21:49 +0200

>> But what I propose is to add a customizable option to enable/disable
>> font-lock-string-face on most text to lessen the color burden on users.
>
> I think using treesit-font-lock-recompute-features is the way to
> adjust which features you want, and is what is expected for cases like
> this (but Yuan would know best).

The reason why I proposed a new customizable option is because
ruby-ts-mode provides an option ruby-ts-highlight-predefined-constants
that enables some rules in ruby-ts--font-lock-settings.  But maybe
there is no way to avoid this fine-grained setting in ruby-ts-mode.

But you are right that (treesit-font-lock-recompute-features '() '(string))
is the right way to customize it.

> Alternatively, treesit-font-lock-level dictates which level of
> features should be included for highlighting. The default is level 3,
> and string is on level 2. We can move string to the 4th level, which
> may be an OK compromise?

It would be nice if you will decide to move 'string' to the 4th level
by default since the 4th level is intended for maximal fontification.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>, Randy Taylor <dev <at> rjt.dev>
Cc: casouri <at> gmail.com, 60105 <at> debbugs.gnu.org
Subject: Re: bug#60105: [PATCH] Add yaml-ts-mode
Date: Thu, 5 Jan 2023 01:30:41 +0200

On 03/01/2023 20:21, Juri Linkov wrote:
> The reason why I proposed a new customizable option is because
> ruby-ts-mode provides an option ruby-ts-highlight-predefined-constants
> that enables some rules in ruby-ts--font-lock-settings.  But maybe
> there is no way to avoid this fine-grained setting in ruby-ts-mode.

But there is. What do you think about this change?

diff --git a/lisp/progmodes/ruby-ts-mode.el b/lisp/progmodes/ruby-ts-mode.el
index 5c173ad24c7..93039c27511 100644
--- a/lisp/progmodes/ruby-ts-mode.el
+++ b/lisp/progmodes/ruby-ts-mode.el
@@ -87,11 +87,6 @@ ruby-ts
   :prefix "ruby-ts-"
   :group 'languages)

-(defcustom ruby-ts-highlight-predefined-constants t
-  "When non-nil, the pre-defined constants are highlighted.
-They will be highlighted the same way as the pre-defined variables."
-  :type 'boolean)
-
 (defvar ruby-ts--operators
   '("+" "-" "*" "/" "%" "**"
     "==" "!=" ">" "<" ">=" "<=" "<=>" "==="
@@ -202,9 +197,11 @@ ruby-ts--font-lock-settings

    :language language
    :feature 'builtin
-   `(((global_variable) @var (:match ,ruby-ts--predefined-variables 
@var)) @font-lock-builtin-face
-     ,@(when ruby-ts-highlight-predefined-constants
-         `(((constant) @var (:match ,ruby-ts--predefined-constants 
@var)) @font-lock-builtin-face)))
+   `(((global_variable) @var (:match ,ruby-ts--predefined-variables 
@var)) @font-lock-builtin-face)
+
+   :language language
+   :feature 'builtin-constant
+   `(((constant) @var (:match ,ruby-ts--predefined-constants @var)) 
@font-lock-builtin-face)

    :language language
    :feature 'keyword
@@ -932,7 +929,7 @@ ruby-ts-mode
   (setq-local treesit-font-lock-feature-list
               '(( comment method-definition )
                 ( keyword regexp string type)
-                ( builtin constant
+                ( builtin builtin-constant constant
                   delimiter escape-sequence global
                   instance
                   interpolation literal symbol variable)

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Randy Taylor <dev <at> rjt.dev>, casouri <at> gmail.com, 60105 <at> debbugs.gnu.org
Subject: Re: bug#60105: [PATCH] Add yaml-ts-mode
Date: Thu, 05 Jan 2023 20:09:22 +0200

>> The reason why I proposed a new customizable option is because
>> ruby-ts-mode provides an option ruby-ts-highlight-predefined-constants
>> that enables some rules in ruby-ts--font-lock-settings.  But maybe
>> there is no way to avoid this fine-grained setting in ruby-ts-mode.
>
> But there is. What do you think about this change?
>
> @@ -202,9 +197,11 @@ ruby-ts--font-lock-settings
>
>     :language language
>     :feature 'builtin
> -   `(((global_variable) @var (:match ,ruby-ts--predefined-variables @var)) @font-lock-builtin-face
> -     ,@(when ruby-ts-highlight-predefined-constants
> -         `(((constant) @var (:match ,ruby-ts--predefined-constants @var))
>          @font-lock-builtin-face)))
> +   `(((global_variable) @var (:match ,ruby-ts--predefined-variables @var)) @font-lock-builtin-face)
> +
> +   :language language
> +   :feature 'builtin-constant
> +   `(((constant) @var (:match ,ruby-ts--predefined-constants @var)) @font-lock-builtin-face)

Look like this is a clear name.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: Randy Taylor <dev <at> rjt.dev>, casouri <at> gmail.com, 60105 <at> debbugs.gnu.org
Subject: Re: bug#60105: [PATCH] Add yaml-ts-mode
Date: Fri, 6 Jan 2023 03:55:11 +0200

On 05/01/2023 20:09, Juri Linkov wrote:
>>> The reason why I proposed a new customizable option is because
>>> ruby-ts-mode provides an option ruby-ts-highlight-predefined-constants
>>> that enables some rules in ruby-ts--font-lock-settings.  But maybe
>>> there is no way to avoid this fine-grained setting in ruby-ts-mode.
>> But there is. What do you think about this change?
>>
>> @@ -202,9 +197,11 @@ ruby-ts--font-lock-settings
>>
>>      :language language
>>      :feature 'builtin
>> -   `(((global_variable) @var (:match ,ruby-ts--predefined-variables @var)) @font-lock-builtin-face
>> -     ,@(when ruby-ts-highlight-predefined-constants
>> -         `(((constant) @var (:match ,ruby-ts--predefined-constants @var))
>>           @font-lock-builtin-face)))
>> +   `(((global_variable) @var (:match ,ruby-ts--predefined-variables @var)) @font-lock-builtin-face)
>> +
>> +   :language language
>> +   :feature 'builtin-constant
>> +   `(((constant) @var (:match ,ruby-ts--predefined-constants @var)) @font-lock-builtin-face)
> Look like this is a clear name.

Very good. Pushed.

Previous Next

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