Subject: Re: bug#63708: 29.0.60; rust-ts-mode not properly handling apostrophe

From: Eli Zaretskii <[email-masked]>
Date: Mon, 29 May 2023 14:25:50 +0300

> Date: Sun, 28 May 2023 19:52:38 +0000
> From: Йордан Миладинов <[email-masked]>
> Cc: Eli Zaretskii <[email-masked]>, [email-masked], [email-masked]
> 
> And here's the patch...

Thanks, a few comments below:

> * lisp/progmodes/rust-ts-mode.el (rust-ts-mode--syntax-propertize)
> 
>   Treat apostrophes as strings if used to define character literals.
> 
>   Treat LT and GT as pairs if used to define type parameters (formerly
>   they were treated as pairs only for type arguments).
> 
> * lisp/progmodes/go-ts-mode.el (go-ts-mode)
> 
>   Treat apostrophes as strings if used to define rune literals.

We don't leave empty lines between the name of the function and the
description of the change.  Like this:

* lisp/progmodes/rust-ts-mode.el (rust-ts-mode--syntax-propertize):
Treat apostrophes as strings if used to define character literals.
Treat LT and GT as pairs if used to define type parameters
(formerly they were treated as pairs only for type arguments).

* lisp/progmodes/go-ts-mode.el (go-ts-mode): Treat apostrophes as
strings if used to define rune literals.

Also note: no indentation, and a colon before the description.

> +(defun go-ts-mode--syntax-propertize (beg end)
> +    "Apply syntax properties to various special characters with
> +contextual meaning between BEG and END.

The first line of a doc string should be a single full sentence, and
should not exceed 78 characters.

>  (defun rust-ts-mode--syntax-propertize (beg end)
> -  "Apply syntax text property to template delimiters between BEG and END.
> +  "Apply syntax properties to various special characters with
> +contextual meaning between BEG and END.

Same here.

> +This function scans the changed RANGES for these special
> +characters and applies the appropriate text properties to reflect
> +their syntactical meaning."

This uses "RANGES", upper-case, which is our convention for arguments,
but there's no argument RANGES in this function.  (Yes, I know that
the original doc string also had this problem.)

>    (while (re-search-forward (rx (or "<" ">")) end t)
> -    (pcase (treesit-node-type
> -            (treesit-node-parent
> -             (treesit-node-at (match-beginning 0))))
> -      ("type_arguments"
> -       (put-text-property (match-beginning 0)
> -                          (match-end 0)
> -                          'syntax-table
> -                          (pcase (char-before)
> -                            (?< '(4 . ?>))
> -                            (?> '(5 . ?<))))))))
> +    (let ((parent-type (treesit-node-type
> +                        (treesit-node-parent
> +                         (treesit-node-at (match-beginning 0))))))
> +      (when (or (string-equal parent-type "type_arguments")
> +                (string-equal parent-type "type_parameters"))
> +        (put-text-property (match-beginning 0)
> +                           (match-end 0)
> +                           'syntax-table
> +                           (pcase (char-before)
> +                             (?< '(4 . ?>))
> +                             (?> '(5 . ?<))))))))

Here, AFAIU you needed to add a single line, but instead you rewrote a
'pcase' as a 'when', and got a much larger change.  Was that really
needed?

I also don't understand how a bug for go-ts-mode led to a change that
also affects rust-ts-mode.  Can you explain?

Finally, and most importantly: I don't see a copyright assignment from
you on file, which means we can only accept a small contribution from
you, less than 15 non-trivial lines of changes.  This patch is
significantly larger.  So we can only accept it if you start your
legal paperwork now, and only after the paperwork is finished.  Would
you like to start the papers rolling at this time?  If yes, I will
send you the form to fill.

Thanks again for working on this.