bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

[Wilhelm Kirschbaum]

[-- Attachment #1.1: Type: text/plain, Size: 134 bytes --]

This updates heex-ts-mode to use font-lock-string-face to be consistent
with html-ts-mode on attribute values.

Kind regards,
Wilhelm

[-- Attachment #1.2: Type: text/html, Size: 217 bytes --]

[-- Attachment #2: 0001-Update-fontification-for-attribute-values-in-heex-ts.patch --]
[-- Type: text/x-patch, Size: 1070 bytes --]

From 51dcd9cea634233439db59c977928f8d7a36edff Mon Sep 17 00:00:00 2001
From: Wilhelm Kirschbaum <wkirschbaum@gmail.com>
Date: Mon, 30 Dec 2024 12:56:40 +0200
Subject: [PATCH] Update fontification for attribute values in heex-ts-mode

Attribute values are strings as with html-ts-mode.

* lisp/progmodes/heex-ts-mode.el (heex-ts--font-lock-settings):
Change to attribute value to @font-lock-string-face.
---
 lisp/progmodes/heex-ts-mode.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/progmodes/heex-ts-mode.el b/lisp/progmodes/heex-ts-mode.el
index 797bfd6c8ff..4be987f9d81 100644
--- a/lisp/progmodes/heex-ts-mode.el
+++ b/lisp/progmodes/heex-ts-mode.el
@@ -113,7 +113,7 @@ heex-ts--font-lock-settings
      `((special_attribute_name) @font-lock-keyword-face)
      :language 'heex
      :feature 'heex-string
-     `([(attribute_value) (quoted_attribute_value)] @font-lock-constant-face)
+     `([(attribute_value) (quoted_attribute_value)] @font-lock-string-face)
      :language 'heex
      :feature 'heex-component
      `([
-- 
2.45.2

bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

[Stefan Kangas]

Wilhelm Kirschbaum <wkirschbaum@gmail.com> writes:

> This updates heex-ts-mode to use font-lock-string-face to be consistent
> with html-ts-mode on attribute values.

Should we install this change on master or emacs-30?  Eli, Yuan, WDYT?

> From 51dcd9cea634233439db59c977928f8d7a36edff Mon Sep 17 00:00:00 2001
> From: Wilhelm Kirschbaum <wkirschbaum@gmail.com>
> Date: Mon, 30 Dec 2024 12:56:40 +0200
> Subject: [PATCH] Update fontification for attribute values in heex-ts-mode
>
> Attribute values are strings as with html-ts-mode.
>
> * lisp/progmodes/heex-ts-mode.el (heex-ts--font-lock-settings):
> Change to attribute value to @font-lock-string-face.
> ---
>  lisp/progmodes/heex-ts-mode.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lisp/progmodes/heex-ts-mode.el b/lisp/progmodes/heex-ts-mode.el
> index 797bfd6c8ff..4be987f9d81 100644
> --- a/lisp/progmodes/heex-ts-mode.el
> +++ b/lisp/progmodes/heex-ts-mode.el
> @@ -113,7 +113,7 @@ heex-ts--font-lock-settings
>       `((special_attribute_name) @font-lock-keyword-face)
>       :language 'heex
>       :feature 'heex-string
> -     `([(attribute_value) (quoted_attribute_value)] @font-lock-constant-face)
> +     `([(attribute_value) (quoted_attribute_value)] @font-lock-string-face)
>       :language 'heex
>       :feature 'heex-component
>       `([
> --
> 2.45.2

bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Thu, 2 Jan 2025 01:45:22 -0600
> Cc: Eli Zaretskii <eliz@gnu.org>, Yuan Fu <casouri@gmail.com>
> 
> Wilhelm Kirschbaum <wkirschbaum@gmail.com> writes:
> 
> > This updates heex-ts-mode to use font-lock-string-face to be consistent
> > with html-ts-mode on attribute values.
> 
> Should we install this change on master or emacs-30?  Eli, Yuan, WDYT?

Are we all in agreement with the change itself?  Why should an
attribute be highlighted as a string?

bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Kangas <stefankangas@gmail.com>
>> Date: Thu, 2 Jan 2025 01:45:22 -0600
>> Cc: Eli Zaretskii <eliz@gnu.org>, Yuan Fu <casouri@gmail.com>
>>
>> Wilhelm Kirschbaum <wkirschbaum@gmail.com> writes:
>>
>> > This updates heex-ts-mode to use font-lock-string-face to be consistent
>> > with html-ts-mode on attribute values.
>>
>> Should we install this change on master or emacs-30?  Eli, Yuan, WDYT?
>
> Are we all in agreement with the change itself?  Why should an
> attribute be highlighted as a string?

I'm not familiar with HEEx and can't share a useful opinion, but FWIW, I
simply noted that the code directly above the change said

    :feature 'heex-string

which is why I assumed that using a string face should make sense.

I'll let others chime in here.

bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

[Wilhelm Kirschbaum]

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

>
>
> >> > This updates heex-ts-mode to use font-lock-string-face to be
> consistent
> >> > with html-ts-mode on attribute values.
> >>
> >> Should we install this change on master or emacs-30?  Eli, Yuan, WDYT?
> >
> > Are we all in agreement with the change itself?  Why should an
> > attribute be highlighted as a string?
>

It's the value of an attribute as in <div attribute="attribute_value">.  It
matches html-ts-mode which I believe is more correct.

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

bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Thu, 2 Jan 2025 02:41:12 -0600
> Cc: wkirschbaum@gmail.com, 75192@debbugs.gnu.org, casouri@gmail.com
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Stefan Kangas <stefankangas@gmail.com>
> >> Date: Thu, 2 Jan 2025 01:45:22 -0600
> >> Cc: Eli Zaretskii <eliz@gnu.org>, Yuan Fu <casouri@gmail.com>
> >>
> >> Wilhelm Kirschbaum <wkirschbaum@gmail.com> writes:
> >>
> >> > This updates heex-ts-mode to use font-lock-string-face to be consistent
> >> > with html-ts-mode on attribute values.
> >>
> >> Should we install this change on master or emacs-30?  Eli, Yuan, WDYT?
> >
> > Are we all in agreement with the change itself?  Why should an
> > attribute be highlighted as a string?
> 
> I'm not familiar with HEEx and can't share a useful opinion, but FWIW, I
> simply noted that the code directly above the change said
> 
>     :feature 'heex-string
> 
> which is why I assumed that using a string face should make sense.

So we think that the current face is just a typo?

bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

[Wilhelm Kirschbaum]

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

> > > attribute be highlighted as a string?
> >
> > I'm not familiar with HEEx and can't share a useful opinion, but FWIW, I
> > simply noted that the code directly above the change said
> >
> >     :feature 'heex-string
> >
> > which is why I assumed that using a string face should make sense.
>
> So we think that the current face is just a typo?
>

Originally I was not sure what would be more appropriate, but got a comment
from a user and saw html-ts-mode uses string so thought it would be good to
change before it gets released.

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

bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

[Eli Zaretskii]

> From: Wilhelm Kirschbaum <wkirschbaum@gmail.com>
> Date: Thu, 2 Jan 2025 12:13:27 +0200
> Cc: Stefan Kangas <stefankangas@gmail.com>, 75192@debbugs.gnu.org, casouri@gmail.com
> 
>  > > attribute be highlighted as a string?
>  > 
>  > I'm not familiar with HEEx and can't share a useful opinion, but FWIW, I
>  > simply noted that the code directly above the change said
>  > 
>  >     :feature 'heex-string
>  > 
>  > which is why I assumed that using a string face should make sense.
> 
>  So we think that the current face is just a typo?
> 
> Originally I was not sure what would be more appropriate, but got a comment from a user and saw
> html-ts-mode uses string so thought it would be good to change before it gets released.

OK, then I think installing this on emacs-30 is TRT.

bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

[Yuan Fu]

> On Jan 2, 2025, at 7:24 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Wilhelm Kirschbaum <wkirschbaum@gmail.com>
>> Date: Thu, 2 Jan 2025 12:13:27 +0200
>> Cc: Stefan Kangas <stefankangas@gmail.com>, 75192@debbugs.gnu.org, casouri@gmail.com
>> 
>>>> attribute be highlighted as a string?
>>> 
>>> I'm not familiar with HEEx and can't share a useful opinion, but FWIW, I
>>> simply noted that the code directly above the change said
>>> 
>>>    :feature 'heex-string
>>> 
>>> which is why I assumed that using a string face should make sense.
>> 
>> So we think that the current face is just a typo?
>> 
>> Originally I was not sure what would be more appropriate, but got a comment from a user and saw
>> html-ts-mode uses string so thought it would be good to change before it gets released.
> 
> OK, then I think installing this on emacs-30 is TRT.

Agree.

bug#75192: [PATCH] Update fontification for attribute values in heex-ts-mode

[Stefan Kangas]

Version: 30.1

Yuan Fu <casouri@gmail.com> writes:

>> On Jan 2, 2025, at 7:24 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>>> From: Wilhelm Kirschbaum <wkirschbaum@gmail.com>
>>> Date: Thu, 2 Jan 2025 12:13:27 +0200
>>> Cc: Stefan Kangas <stefankangas@gmail.com>, 75192@debbugs.gnu.org, casouri@gmail.com
>>>
>>>>> attribute be highlighted as a string?
>>>>
>>>> I'm not familiar with HEEx and can't share a useful opinion, but FWIW, I
>>>> simply noted that the code directly above the change said
>>>>
>>>>    :feature 'heex-string
>>>>
>>>> which is why I assumed that using a string face should make sense.
>>>
>>> So we think that the current face is just a typo?
>>>
>>> Originally I was not sure what would be more appropriate, but got a comment from a user and saw
>>> html-ts-mode uses string so thought it would be good to change before it gets released.
>>
>> OK, then I think installing this on emacs-30 is TRT.
>
> Agree.

Thanks all, installed on emacs-30 as commit 921f454f508.  Closing.