GNU bug report logs -
#73877
30; rust-ts-mode: highlight the possible type suffix of number literals
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 73877 in the body.
You can then email your comments to 73877 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Sat, 19 Oct 2024 11:02:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Christophe TROESTLER <Christophe.TROESTLER <at> umons.ac.be>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Sat, 19 Oct 2024 11:02:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi,
In order to make Rust number literals such as 1usize, 1.0_f64,... more legible, the type suffix should be highlighted. The attached patch to rust-ts-mode does that.
Best,
Christophe
[0001-rust-ts-mode-highlight-the-possible-type-suffix-of-n.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Sun, 20 Oct 2024 12:02:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 73877 <at> debbugs.gnu.org (full text, mbox):
Christophe TROESTLER <Christophe.TROESTLER <at> umons.ac.be> writes:
> Hi,
>
> In order to make Rust number literals such as 1usize, 1.0_f64,... more legible, the type suffix should be highlighted. The attached patch to rust-ts-mode does that.
>
> Best,
> Christophe
Thanks.
Randy, do you have any comments on the below patch?
> From e1cf2f8c59f2abd75688721d08653327c5427da9 Mon Sep 17 00:00:00 2001
> From: Christophe Troestler <Christophe.Troestler <at> umons.ac.be>
> Date: Fri, 18 Oct 2024 23:50:06 +0200
> Subject: [PATCH] rust-ts-mode: highlight the possible type suffix of number
> literals
> Content-Type: text/plain; charset="utf-8"
>
> ---
> lisp/progmodes/rust-ts-mode.el | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
> index 571ffa9b220..0454c46261f 100644
> --- a/lisp/progmodes/rust-ts-mode.el
> +++ b/lisp/progmodes/rust-ts-mode.el
> @@ -116,6 +116,12 @@ rust-ts-mode--indent-rules
> ((parent-is "use_list") parent-bol rust-ts-mode-indent-offset)))
> "Tree-sitter indent rules for `rust-ts-mode'.")
>
> +(defconst rust-ts-mode--number-types
> + (regexp-opt '("u8" "i8" "u16" "i16" "u32" "i32" "u64"
> + "i64" "u128" "i128" "usize" "isize" "f32" "f64"))
> + "Regexp that matches any suffix on number litterals as documented
> +at https://doc.rust-lang.org/reference/tokens.html#suffixes")
> +
> (defvar rust-ts-mode--builtin-macros
> '("concat_bytes" "concat_idents" "const_format_args"
> "format_args_nl" "log_syntax" "trace_macros" "assert" "assert_eq"
> @@ -221,7 +227,8 @@ rust-ts-mode--font-lock-settings
>
> :language 'rust
> :feature 'number
> - '([(float_literal) (integer_literal)] @font-lock-number-face)
> + '([(float_literal) (integer_literal)]
> + @rust-ts-mode--fontify-number-literal)
>
> :language 'rust
> :feature 'operator
> @@ -365,6 +372,22 @@ 'rust-ts-mode--fontify-pattern
> (treesit-node-start id) (treesit-node-end id)
> 'font-lock-variable-name-face override start end)))))))
>
> +(defun rust-ts-mode--fontify-number-literal (node override start stop &rest _)
> + "Fontify number literals, highlighting the optional type if present"
> + (let* ((beg (treesit-node-start node))
> + (end (treesit-node-end node)))
> + (save-excursion
> + (goto-char end)
> + (if (looking-back rust-ts-mode--number-types beg)
> + (let* ((ty (match-beginning 0))
> + (nb (if (eq (char-before ty) ?_) (1- ty) ty)))
> + (treesit-fontify-with-override
> + ty end 'font-lock-type-face override start stop)
> + (treesit-fontify-with-override
> + beg nb 'font-lock-number-face override start stop))
> + (treesit-fontify-with-override
> + beg end 'font-lock-number-face override start stop)))))
> +
> (defun rust-ts-mode--defun-name (node)
> "Return the defun name of NODE.
> Return nil if there is no name or if NODE is not a defun node."
> --
> 2.45.2
Added tag(s) patch.
Request was from
Stefan Kangas <stefankangas <at> gmail.com>
to
control <at> debbugs.gnu.org
.
(Sun, 20 Oct 2024 12:02:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Tue, 22 Oct 2024 23:07:03 GMT)
Full text and
rfc822 format available.
Message #13 received at 73877 <at> debbugs.gnu.org (full text, mbox):
> Christophe TROESTLER Christophe.TROESTLER <at> umons.ac.be writes:
>
> > Hi,
> >
> > In order to make Rust number literals such as 1usize, 1.0_f64,... more legible, the type suffix should be highlighted. The attached patch to rust-ts-mode does that.
> >
> > Best,
> > Christophe
Thanks for working on this Christophe.
I think this will need to be gated behind a variable which should retain the existing behaviour by default.
Would you also be able to add some tests for this?
You can see how the font-lock tests are done in the test/lisp/progmodes/rust-ts-mode-tests.el file,
and add some to the rust-ts-mode-resources/font-lock.rs file.
We might need a new file for this one, actually, since we'd want to test both cases.
I have some comments below:
+ "Regexp that matches any suffix on number litterals as documented
^ literals
+at https://doc.rust-lang.org/reference/tokens.html#suffixes")
Add a period at the end.
+ "Fontify number literals, highlighting the optional type if present"
Add a period at the end.
We may also want to say something like: "highlighting the optional type based on the value of `name-of-the-variable'."
once the variable to control it is added.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Thu, 24 Oct 2024 15:24:02 GMT)
Full text and
rfc822 format available.
Message #16 received at 73877 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi,
Thanks for your comments. Here is an updated patch (against emacs-30, let me know if you would prefer the emacs-29 branch).
Best,
C.
On 23 October 2024 at 01:05 +02, Randy Taylor <dev <at> rjt.dev> wrote:
> […] I think this will need to be gated behind a variable which should
> retain the existing behaviour by default.
>
> Would you also be able to add some tests for this? […]
[0001-rust-ts-mode-highlight-the-possible-type-suffix-of-n.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Sun, 27 Oct 2024 19:09:02 GMT)
Full text and
rfc822 format available.
Message #19 received at 73877 <at> debbugs.gnu.org (full text, mbox):
On Thursday, October 24th, 2024 at 11:22, Christophe TROESTLER <Christophe.TROESTLER <at> umons.ac.be> wrote:
>
>
> Hi,
>
> Thanks for your comments. Here is an updated patch (against emacs-30, let me know if you would prefer the emacs-29 branch).
>
> Best,
> C.
Thanks Christophe.
This should be against the master branch.
It will need a commit message (see the CONTRIBUTE file) and a NEWS entry.
+(defcustom rust-ts-mode-highlight-number-literal-type nil
I'm unsure about this name since it's a little misleading. The type
is still highlighted if this is nil, but the same as the number.
I don't really have a better suggestion though, and anything else
would be longer and probably a little unruly. I was thinking about
rust-ts-mode-highlight-number-literal-type-separately but...
Maybe we drop the literal part?
Does anyone else have any thoughts/suggestions?
+ :version "30.1"
Should be "31.1".
For rust-ts-test-font-lock, I'm wondering if we should split that up to
a new ert-deftest. If we have more customizations in the future we'd
probably end up doing that anyway.
Also, I'm not sure we need to bother resetting the rust-ts-mode-highlight-number-literal-type variable.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Mon, 28 Oct 2024 21:58:01 GMT)
Full text and
rfc822 format available.
Message #22 received at 73877 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi,
Here is an updated patch that (hopefully) addresses all of your remarks except the last one: when executing the tests interactively in the instance of Emacs one works with, it is good IMHO that global variables are not changed under our feet. That being said, it is not a strong opinion and I can remove the reset if you wish.
Best,
C.
On 27 October 2024 at 20:07 +01, Randy Taylor <dev <at> rjt.dev> wrote:
> […] This should be against the master branch.
>
> It will need a commit message (see the CONTRIBUTE file) and a NEWS entry.
>
> +(defcustom rust-ts-mode-highlight-number-literal-type nil
> I'm unsure about this name since it's a little misleading. The type
> is still highlighted if this is nil, but the same as the number.
> I don't really have a better suggestion though, and anything else
> would be longer and probably a little unruly. I was thinking about
> rust-ts-mode-highlight-number-literal-type-separately but...
>
> Maybe we drop the literal part?
>
> Does anyone else have any thoughts/suggestions?
>
> + :version "30.1"
> Should be "31.1".
>
> For rust-ts-test-font-lock, I'm wondering if we should split that up to
> a new ert-deftest. If we have more customizations in the future we'd
> probably end up doing that anyway.
> Also, I'm not sure we need to bother resetting the
> rust-ts-mode-highlight-number-literal-type variable.
[0001-Rust-ts-fontify-as-type-the-possible-suffix-of-numbe.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Sat, 02 Nov 2024 11:02:01 GMT)
Full text and
rfc822 format available.
Message #25 received at 73877 <at> debbugs.gnu.org (full text, mbox):
> Cc: "73877 <at> debbugs.gnu.org" <73877 <at> debbugs.gnu.org>,
> Stefan Kangas <stefankangas <at> gmail.com>
> From: Christophe TROESTLER <Christophe.TROESTLER <at> umons.ac.be>
> Date: Mon, 28 Oct 2024 21:56:52 +0000
>
> Here is an updated patch that (hopefully) addresses all of your remarks except the last one: when executing the tests interactively in the instance of Emacs one works with, it is good IMHO that global variables are not changed under our feet. That being said, it is not a strong opinion and I can remove the reset if you wish.
Thanks, a few more nits to be fixed:
> * lisp/progmodes/rust-ts-mode.el (rust-ts-mode--fontify-number-literal):
> perform the improved fontification of numbers.
The description should start with a capital letter.
Also, please mention the bug number somewhere in the log message.
> +---
> +*** New user option 'rust-ts-mode-fontify-number-suffix-as-type'.
> +Rust number literals may have an optional type suffix. When this option
> +is non-nil, this suffix is fontified as a type.
^^^^^^^^^
I think "using the 'font-lock-type-face'" is a more clear description
than "as a type".
> +(defcustom rust-ts-mode-fontify-number-suffix-as-type nil
> + "If non-nil, fontify the optional type suffix of number literals with
> +`font-lock-type-face' instead of `font-lock-number-face'."
The first line of a doc string should be a single complete sentence.
So please make it a summary of what this option does, and describe the
details in the subsequent sentences.
> +(defconst rust-ts-mode--number-types
> + (regexp-opt '("u8" "i8" "u16" "i16" "u32" "i32" "u64"
> + "i64" "u128" "i128" "usize" "isize" "f32" "f64"))
> + "Regexp that matches any suffix on number literals as documented
> +at https://doc.rust-lang.org/reference/tokens.html#suffixes.")
Same here.
> +(defun rust-ts-mode--fontify-number-literal (node override start stop &rest _)
> + "Fontify number literals, highlighting the optional suffix as a type
> +based on the value of `rust-ts-mode-highlight-number-literal-type'."
And here.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Sat, 02 Nov 2024 19:49:02 GMT)
Full text and
rfc822 format available.
Message #28 received at 73877 <at> debbugs.gnu.org (full text, mbox):
On Monday, October 28th, 2024 at 17:56, Christophe TROESTLER <Christophe.TROESTLER <at> umons.ac.be> wrote:
>
>
> Hi,
>
> Here is an updated patch that (hopefully) addresses all of your remarks except the last one
Sorry for the delay.
The patch looks good to me with Eli's points addressed (thanks Eli for
taking a look) and the below addressed.
>...: when executing the tests interactively in the instance of Emacs one works with, it is good IMHO that global variables are not changed under our feet. That being said, it is not a strong opinion and I can remove the reset if you wish.
Unless I am missing something, the variables should reset automatically.
They are only changed in the context of the test.
How are you running them that you are seeing otherwise?
>
> Best,
> C.
>
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Mon, 11 Nov 2024 12:33:02 GMT)
Full text and
rfc822 format available.
Message #31 received at 73877 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Thanks Eli Zaretskii and Randy Taylor for your remarks. An undated patch is attached.
On 2 November 2024 at 20:48 +01, Randy Taylor <dev <at> rjt.dev> wrote:
> […] Unless I am missing something, the variables should reset
> automatically. They are only changed in the context of the test.
>
> How are you running them that you are seeing otherwise?
I now am using the command line but, if I evaluate the tests (C-xC-e) and then run them with M-x ert, the variable 'rust-ts-mode-fontify-number-suffix-as-type' is not reset to its original value.
Best,
C.
[0001-Rust-ts-fontify-as-type-the-possible-suffix-of-numbe.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Fri, 15 Nov 2024 00:13:01 GMT)
Full text and
rfc822 format available.
Message #34 received at 73877 <at> debbugs.gnu.org (full text, mbox):
On Monday, November 11th, 2024 at 07:32, Christophe TROESTLER <Christophe.TROESTLER <at> umons.ac.be> wrote:
>
>
>
> Thanks Eli Zaretskii and Randy Taylor for your remarks. An undated patch is attached.
>
> On 2 November 2024 at 20:48 +01, Randy Taylor dev <at> rjt.dev wrote:
>
> > […] Unless I am missing something, the variables should reset
> > automatically. They are only changed in the context of the test.
> >
> > How are you running them that you are seeing otherwise?
>
>
> I now am using the command line but, if I evaluate the tests (C-xC-e) and then run them with M-x ert, the variable 'rust-ts-mode-fontify-number-suffix-as-type' is not reset to its original value.
>
> Best,
> C.
Ah, sorry. I had only tried with treesit-font-lock-level myself since
the patch didn't apply cleanly for me and I didn't have a chance to
manually apply it, and I glossed over how the variables are set
differently. If you do the same as treesit-font-lock-level, i.e.,
place it in the let instead of setq, then it will reset properly.
You will also need to: (require 'rust-ts-mode)
After you make that change, I think you should be able to get rid of
this:
+(put 'rust-ts-mode-fontify-number-suffix-as-type 'safe-local-variable
+ 'booleanp)
In the font-lock-number.rs file, it looks like there is an extraneous
newline at the top of the file.
BTW the current patch still doesn't apply against the latest master
for me.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Mon, 18 Nov 2024 15:16:02 GMT)
Full text and
rfc822 format available.
Message #37 received at 73877 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 15 November 2024 at 01:11 +01, Randy Taylor <dev <at> rjt.dev> wrote:
> […] the patch didn't apply cleanly for me […]
Here is an updated version of the patch against today's master branch.
> […] If you do the same as treesit-font-lock-level, […]
For some reason it did not work in the past but it does now, so that's how I proceeded.
> You will also need to: (require 'rust-ts-mode)
Ok — some tests do that, others don't, so I did not originally change that. I've now added the “require”.
> After you make that change, I think you should be able to get rid of
> this:
> +(put 'rust-ts-mode-fontify-number-suffix-as-type 'safe-local-variable
> + 'booleanp)
I first thought that one may want to set that change for specific file but, you are right, it is more of a user choice that should be set in your own config. files.
> In the font-lock-number.rs file, it looks like there is an extraneous
> newline at the top of the file.
Fixed.
> BTW the current patch still doesn't apply against the latest master
> for me.
I have rebased it on the current origin/master.
Best,
C.
[0001-Rust-ts-fontify-as-type-the-possible-suffix-of-numbe.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Fri, 22 Nov 2024 00:58:02 GMT)
Full text and
rfc822 format available.
Message #40 received at 73877 <at> debbugs.gnu.org (full text, mbox):
On Monday, November 18th, 2024 at 10:14, Christophe TROESTLER <Christophe.TROESTLER <at> umons.ac.be> wrote:
>
> On 15 November 2024 at 01:11 +01, Randy Taylor dev <at> rjt.dev wrote:
>
> > […] the patch didn't apply cleanly for me […]
>
>
> Here is an updated version of the patch against today's master branch.
For some reason it still doesn't apply for me - not sure why. Maybe it's just me.
I always do "git format-patch -1" to generate my patches.
>
> > […] If you do the same as treesit-font-lock-level, […]
>
>
> For some reason it did not work in the past but it does now, so that's how I proceeded.
>
> > You will also need to: (require 'rust-ts-mode)
>
>
> Ok — some tests do that, others don't, so I did not originally change that. I've now added the “require”.
>
> > After you make that change, I think you should be able to get rid of
> > this:
> > +(put 'rust-ts-mode-fontify-number-suffix-as-type 'safe-local-variable
> > + 'booleanp)
>
>
> I first thought that one may want to set that change for specific file but, you are right, it is more of a user choice that should be set in your own config. files.
>
> > In the font-lock-number.rs file, it looks like there is an extraneous
> > newline at the top of the file.
>
>
> Fixed.
>
> > BTW the current patch still doesn't apply against the latest master
> > for me.
>
>
> I have rebased it on the current origin/master.
>
> Best,
> C.
Thanks for working on this and putting up with me - the patch looks
good to me. Eli, please install if you have no further comments.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#73877
; Package
emacs
.
(Fri, 22 Nov 2024 07:24:02 GMT)
Full text and
rfc822 format available.
Message #43 received at 73877 <at> debbugs.gnu.org (full text, mbox):
On 22 November 2024 at 01:57 +01, Randy Taylor <dev <at> rjt.dev> wrote:
> On Monday, November 18th, 2024 at 10:14, Christophe TROESTLER
> <Christophe.TROESTLER <at> umons.ac.be> wrote:
>>
>> On 15 November 2024 at 01:11 +01, Randy Taylor dev <at> rjt.dev wrote:
>>
>> > […] the patch didn't apply cleanly for me […]
>>
>>
>> Here is an updated version of the patch against today's master branch.
>
> For some reason it still doesn't apply for me - not sure why. Maybe
> it's just me.
> I always do "git format-patch -1" to generate my patches.
I used magit to do it. “git format-patch -1” generated an identical one.
Best,
C.
Reply sent
to
Eli Zaretskii <eliz <at> gnu.org>
:
You have taken responsibility.
(Sat, 23 Nov 2024 12:59:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Christophe TROESTLER <Christophe.TROESTLER <at> umons.ac.be>
:
bug acknowledged by developer.
(Sat, 23 Nov 2024 12:59:03 GMT)
Full text and
rfc822 format available.
Message #48 received at 73877-done <at> debbugs.gnu.org (full text, mbox):
> Date: Fri, 22 Nov 2024 00:57:32 +0000
> From: Randy Taylor <dev <at> rjt.dev>
> Cc: "73877 <at> debbugs.gnu.org" <73877 <at> debbugs.gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
>
> On Monday, November 18th, 2024 at 10:14, Christophe TROESTLER <Christophe.TROESTLER <at> umons.ac.be> wrote:
> >
> > On 15 November 2024 at 01:11 +01, Randy Taylor dev <at> rjt.dev wrote:
> >
> > > […] the patch didn't apply cleanly for me […]
> >
> >
> > Here is an updated version of the patch against today's master branch.
>
> For some reason it still doesn't apply for me - not sure why. Maybe it's just me.
> I always do "git format-patch -1" to generate my patches.
>
> >
> > > […] If you do the same as treesit-font-lock-level, […]
> >
> >
> > For some reason it did not work in the past but it does now, so that's how I proceeded.
> >
> > > You will also need to: (require 'rust-ts-mode)
> >
> >
> > Ok — some tests do that, others don't, so I did not originally change that. I've now added the “require”.
> >
> > > After you make that change, I think you should be able to get rid of
> > > this:
> > > +(put 'rust-ts-mode-fontify-number-suffix-as-type 'safe-local-variable
> > > + 'booleanp)
> >
> >
> > I first thought that one may want to set that change for specific file but, you are right, it is more of a user choice that should be set in your own config. files.
> >
> > > In the font-lock-number.rs file, it looks like there is an extraneous
> > > newline at the top of the file.
> >
> >
> > Fixed.
> >
> > > BTW the current patch still doesn't apply against the latest master
> > > for me.
> >
> >
> > I have rebased it on the current origin/master.
> >
> > Best,
> > C.
>
> Thanks for working on this and putting up with me - the patch looks
> good to me. Eli, please install if you have no further comments.
Thanks, installed on master, and closing the bug.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Sun, 22 Dec 2024 12:24:15 GMT)
Full text and
rfc822 format available.
This bug report was last modified 175 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.