bug#70383: Conf-mode[toml] badly set face property

bug#70383: Conf-mode[toml] badly set face property

[Alastair Irving]

When editting TOML files with conf-mode[toml] the face property is being 
set badly on certain characters.  This occurs in Emacs 29.3 (running 
with -q) but has been present in older versions. Steps to reproduce:

1. Create a file test.toml containing the following:


[package]
name = "test"
# See more keys and their definitions at 
https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]

2. Navigate to the d in dependencies and evaluate

(text-properties-at (point))

The output for me looks like

(face (font-lock-type-face font-lock-type-face font-lock-type-face 
font-lock-type-face font-lock-type-face font-lock-ty\
pe-face font-lock-type-face font-lock-type-face font-lock-type-face 
font-lock-type-face


It looks like something is wrong in the font-lock setup and the same 
face is being applied very many times.

bug#70383: Conf-mode[toml] badly set face property

[Eli Zaretskii]

> Date: Sun, 14 Apr 2024 11:25:08 +0100
> From: Alastair Irving <alastair.j.irving@gmail.com>
> 
> When editting TOML files with conf-mode[toml] the face property is being 
> set badly on certain characters.  This occurs in Emacs 29.3 (running 
> with -q) but has been present in older versions.  Steps to reproduce:
> 
> 1. Create a file test.toml containing the following:
> 
> 
> [package]
> name = "test"
> # See more keys and their definitions at 
> https://doc.rust-lang.org/cargo/reference/manifest.html
> [dependencies]
> 
> 2. Navigate to the d in dependencies and evaluate
> 
> (text-properties-at (point))
> 
> The output for me looks like
> 
> (face (font-lock-type-face font-lock-type-face font-lock-type-face 
> font-lock-type-face font-lock-type-face font-lock-ty\
> pe-face font-lock-type-face font-lock-type-face font-lock-type-face 
> font-lock-type-face
> 
> 
> It looks like something is wrong in the font-lock setup and the same 
> face is being applied very many times.

I cannot reproduce this here, neither in Emacs 29 nor in Emacs 30.
What version of the TOML grammar library are you using?

Jostein and Yuan, can you please look into this?

bug#70383: Conf-mode[toml] badly set face property

[Alastair Irving]

[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]


On 14/04/2024 17:46, Eli Zaretskii wrote:
>> Date: Sun, 14 Apr 2024 11:25:08 +0100
>> From: Alastair Irving <alastair.j.irving@gmail.com>
>>
>> When editting TOML files with conf-mode[toml] the face property is being
>> set badly on certain characters.  This occurs in Emacs 29.3 (running
>> with -q) but has been present in older versions.  Steps to reproduce:
>>
>> 1. Create a file test.toml containing the following:
>>
>>
>> [package]
>> name = "test"
>> # See more keys and their definitions at
>> https://doc.rust-lang.org/cargo/reference/manifest.html
>> [dependencies]
>>
>> 2. Navigate to the d in dependencies and evaluate
>>
>> (text-properties-at (point))
>>
>> The output for me looks like
>>
>> (face (font-lock-type-face font-lock-type-face font-lock-type-face
>> font-lock-type-face font-lock-type-face font-lock-ty\
>> pe-face font-lock-type-face font-lock-type-face font-lock-type-face
>> font-lock-type-face
>>
>>
>> It looks like something is wrong in the font-lock setup and the same
>> face is being applied very many times.
> I cannot reproduce this here, neither in Emacs 29 nor in Emacs 30.
> What version of the TOML grammar library are you using?
I don't think it's using a TOML grammar library, just conf-mode bundled 
with EMACS "The major mode is Conf[TOML] mode defined in conf-mode.el".  
I think an extra newline was inserted in the TOML I pasted (in the 
comment before the URL), it should be 4 lines total. I've attached the 
exact file.
>
> Jostein and Yuan, can you please look into this?

[-- Attachment #2: test.toml --]
[-- Type: text/plain, Size: 136 bytes --]

[package]
name = "test"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]

bug#70383: Conf-mode[toml] badly set face property

[Jostein Kjønigsen]

[-- Attachment #1: Type: text/plain, Size: 2311 bytes --]

From what I can tell Alastair is using conf-mode, not toml-ts-mode.

Alastait: toml-ts-mode is a new builtin tree-sitter based major-mode for working with TOML files.

If you have issues with conf-mode and certain TOML files, you could try that instead. I just tried it with your test-files and I don't see any incorrect fontification, so it seems to work fairly well.

Eli: If there's a bug to fix in conf-mode, I'm going to assume it would be better to CC the conf-mode maintainer than the toml-ts-mode maintainer. It might gain more traction.

—
Kind Regards
Jostein Kjønigsen

> On 14 Apr 2024, at 21:59, Alastair Irving <alastair.j.irving@gmail.com> wrote:
> 
> 
> On 14/04/2024 17:46, Eli Zaretskii wrote:
>>> Date: Sun, 14 Apr 2024 11:25:08 +0100
>>> From: Alastair Irving <alastair.j.irving@gmail.com>
>>> 
>>> When editting TOML files with conf-mode[toml] the face property is being
>>> set badly on certain characters.  This occurs in Emacs 29.3 (running
>>> with -q) but has been present in older versions.  Steps to reproduce:
>>> 
>>> 1. Create a file test.toml containing the following:
>>> 
>>> 
>>> [package]
>>> name = "test"
>>> # See more keys and their definitions at
>>> https://doc.rust-lang.org/cargo/reference/manifest.html
>>> [dependencies]
>>> 
>>> 2. Navigate to the d in dependencies and evaluate
>>> 
>>> (text-properties-at (point))
>>> 
>>> The output for me looks like
>>> 
>>> (face (font-lock-type-face font-lock-type-face font-lock-type-face
>>> font-lock-type-face font-lock-type-face font-lock-ty\
>>> pe-face font-lock-type-face font-lock-type-face font-lock-type-face
>>> font-lock-type-face
>>> 
>>> 
>>> It looks like something is wrong in the font-lock setup and the same
>>> face is being applied very many times.
>> I cannot reproduce this here, neither in Emacs 29 nor in Emacs 30.
>> What version of the TOML grammar library are you using?
> I don't think it's using a TOML grammar library, just conf-mode bundled with EMACS "The major mode is Conf[TOML] mode defined in conf-mode.el".  I think an extra newline was inserted in the TOML I pasted (in the comment before the URL), it should be 4 lines total. I've attached the exact file.
>> 
>> Jostein and Yuan, can you please look into this?
> <test.toml>


[-- Attachment #2: Type: text/html, Size: 5275 bytes --]

bug#70383: Conf-mode[toml] badly set face property

[Eli Zaretskii]

> From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
> Date: Mon, 15 Apr 2024 08:43:59 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  "Mr. Jostein Kjønigsen" <jostein@kjonigsen.net>,
>  Yuan Fu <casouri@gmail.com>,
>  70383@debbugs.gnu.org
> 
> From what I can tell Alastair is using conf-mode, not toml-ts-mode.

Right, sorry.

bug#70383: Conf-mode[toml] badly set face property

[Eli Zaretskii]

> Date: Sun, 14 Apr 2024 20:59:02 +0100
> Cc: 70383@debbugs.gnu.org
> From: Alastair Irving <alastair.j.irving@gmail.com>
> 
> > I cannot reproduce this here, neither in Emacs 29 nor in Emacs 30.
> > What version of the TOML grammar library are you using?
> I don't think it's using a TOML grammar library, just conf-mode bundled 
> with EMACS "The major mode is Conf[TOML] mode defined in conf-mode.el".  
> I think an extra newline was inserted in the TOML I pasted (in the 
> comment before the URL), it should be 4 lines total. I've attached the 
> exact file.

Thanks.  Does the patch below give good results (I don't have too many
conf files around to test it).

Stefan, does the fix look good to you?

diff --git a/lisp/textmodes/conf-mode.el b/lisp/textmodes/conf-mode.el
index 5e16360..8a201bd 100644
--- a/lisp/textmodes/conf-mode.el
+++ b/lisp/textmodes/conf-mode.el
@@ -180,7 +180,7 @@ conf-font-lock-keywords
      (1 'font-lock-variable-name-face)
      (2 'font-lock-constant-face nil t))
     ;; section { ... } (do this last because some assign ...{...)
-    ("^[ \t]*\\([^#=:\n]+?\\)[ \t\n]*{[^{}]*?$" 1 'font-lock-type-face prepend))
+    ("^[ \t]*\\([^[#=:\n]+?\\)[ \t\n]*{[^{}]*?$" 1 'font-lock-type-face prepend))
   "Keywords to highlight in Conf mode.")
 
 (defvar conf-javaprop-font-lock-keywords

bug#70383: Conf-mode[toml] badly set face property

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> diff --git a/lisp/textmodes/conf-mode.el b/lisp/textmodes/conf-mode.el
> index 5e16360..8a201bd 100644
> --- a/lisp/textmodes/conf-mode.el
> +++ b/lisp/textmodes/conf-mode.el
> @@ -180,7 +180,7 @@ conf-font-lock-keywords
>       (1 'font-lock-variable-name-face)
>       (2 'font-lock-constant-face nil t))
>      ;; section { ... } (do this last because some assign ...{...)
> -    ("^[ \t]*\\([^#=:\n]+?\\)[ \t\n]*{[^{}]*?$" 1 'font-lock-type-face prepend))
> +    ("^[ \t]*\\([^[#=:\n]+?\\)[ \t\n]*{[^{}]*?$" 1 'font-lock-type-face prepend))
>    "Keywords to highlight in Conf mode.")

AFAICT this rule is not even activated in for TOML files.
The patch below seems to DTRT for me.
[ I'd even argue that it should (goto-char end).  ]

[ I found the culprit after noticing that the number of
  `font-lock-type-face` elements in the list in the `face` property was
  the same as the number of characters between the two "sections".  ]


        Stefan


diff --git a/lisp/textmodes/conf-mode.el b/lisp/textmodes/conf-mode.el
index 5e1636033f6..e74409128df 100644
--- a/lisp/textmodes/conf-mode.el
+++ b/lisp/textmodes/conf-mode.el
@@ -613,7 +613,6 @@
   "Font-lock helper function for `conf-toml-mode'.
 Handles recognizing TOML section names, like [section],
 \[[section]], or [something.\"else\".section]."
-  (save-excursion
     ;; Skip any number of "[" to handle things like [[section]].
     (when (re-search-forward "^\\s-*\\[+" limit t)
       (let ((start (point)))
@@ -633,7 +632,7 @@
             (goto-char start)
             (unless (search-forward "," end t)
               (set-match-data (list start end))
-              t)))))))
+            t))))))
 
 ;;;###autoload
 (define-derived-mode conf-toml-mode conf-mode "Conf[TOML]"

bug#70383: Conf-mode[toml] badly set face property

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> The patch below seems to DTRT for me.
> [ I'd even argue that it should (goto-char end).  ]

Oh, it already does, sorry.  OK I pushed it to `master`.
I think this is it.
Can the OP confirm it fixes the problem for them?


        Stefan

bug#70383: Conf-mode[toml] badly set face property

[Alastair Irving]

On 17/04/2024 19:31, Stefan Monnier wrote:
>> The patch below seems to DTRT for me.
>> [ I'd even argue that it should (goto-char end).  ]
> Oh, it already does, sorry.  OK I pushed it to `master`.
> I think this is it.
> Can the OP confirm it fixes the problem for them?
Thank you Stefan, that's fixed it.

bug#70383: Conf-mode[toml] badly set face property

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Thank you Stefan, that's fixed it.

Thanks for confirming, closing,


        Stefan