Package: emacs;
Reported by: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Date: Sat, 15 Jul 2023 12:35:02 UTC
Severity: normal
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
Message #53 received at 64647 <at> debbugs.gnu.org (full text, mbox):
From: Theodor Thornhill <theo <at> thornhill.no> To: Vincenzo Pupillo <v.pupillo <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org> Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net Subject: Re: bug#64647: treesit-query-error due to a recent change to tree-sitter-javascript grammar definition Date: Wed, 19 Jul 2023 07:11:05 +0200
Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:
> Hi,
>
> In data domenica 16 luglio 2023 20:19:38 CEST, Eli Zaretskii ha scritto:
>> > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
>> > Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
>> > Date: Sun, 16 Jul 2023 20:00:43 +0200
>> >
>> > In my patch for java-ts-mode I used treesit-query-capture to figure out
>> > whether a symbol was defined or not. Check out the
>> > java-ts-mode--string-highlight- helper function.
>>
>> Can you do something similar in this case? That would be good enough
>> for Emacs 29.1.
>
> In attachment you can find the new version of the patches (similar to the patch
> that i made for java-ts-mode).
> The patches were made on the branch emacs-29.
>
> Both work with the latest grammar. The javascript version is reliable, the
> typescrypt version seems reliable. In fact, drum roll, with typescrypt both
> tests:
> 1. (treesit-query-capture 'typescript '((member_expression) @capture)) ;; the
> new node type
> 2. (treesit-query-capture 'typescript '((nested_identifier) @capture)) ;; the
> old node type
> both return nil !!!
> If you use #2, then treesitter-ts-mode font lock gives an error!
> This happens only for font-lock, while for indentation everything works
> correctly.
For Typescript these changes should go into 'tsx-ts-mode, not
'typescript-ts-mode. That may be why you are seeing some strange results?
>
> No problem for javascript, it works as expected for font-lock. The old node
> type returns an error with treesit-query-capture.
>
> @Theo: I am not sure if the indentation works if I add the new rules to
> treesit-simple-indent-rules in the Init function. I am writing a major-mode
> for php and, unless I am mistaken or due to problems with the various
> treesitter parsers, the order seems to be important.
Thanks! Some minor comments (all of which apply to both patches, even
though the comment is only for one of them):
>
> From c6a93b510378756f2eff01a11ef4f9127a5e5d17 Mon Sep 17 00:00:00 2001
> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Date: Mon, 17 Jul 2023 22:20:44 +0200
> Subject: [PATCH] Updated JSX support due to changes in tree-sitter-javascript
>
> A recent change in tree-sitter-javascript grammar support for
> JSX (commit bb1f97b), changed two things:
> 1. renamed nested_identifier to member_expression
> 2. removed jsx_fragment, jsx_text is used instead
>
> * lisp/progmodes/js.el: (js--treesit-indent-helper): indent helper
> function for handle different tree-sitter-javascript version
> * lisp/progmodes/js.el: (js--treesit-indent-rules): use the new
> function
> * lisp/progmodes/js.el: (js--treesit-font-lock-helper): font lock
> helper function for handle different tree-sitter-javascript version
> * lisp/progmodes/js.el: (js--treesit-font-lock-settings): use the new
> function
> ---
"... function to handle ..."
> lisp/progmodes/js.el | 65 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
> index a05bd758dbc..f5158195500 100644
> --- a/lisp/progmodes/js.el
> +++ b/lisp/progmodes/js.el
> @@ -3427,6 +3427,18 @@ This function is intended for use in `after-change-functions'."
>
> ;;; Tree sitter integration
>
> +(defun js--treesit-indent-helper ()
> + "Indent rules helper, for handle different release of tree-sitter-javascript.
> +Check if a node type is available, then return the right indent rules."
"Indent rules helper, to handle different releases of tree-sitter-javascript."
> + ;; handle commit bb1f97b
> + (condition-case nil
> + (progn (treesit-query-capture 'javascript '((jsx_fragment) @capture))
> + `(((match "<" "jsx_fragment") parent 0)
> + ((parent-is "jsx_fragment") parent js-indent-level)))
> + (error
> + `(((match "<" "jsx_text") parent 0)
> + ((parent-is "jsx_text") parent js-indent-level)))))
> +
> (defvar js--treesit-indent-rules
> (let ((switch-case (rx "switch_" (or "case" "default"))))
> `((javascript
> @@ -3462,8 +3474,9 @@ This function is intended for use in `after-change-functions'."
> ((parent-is "statement_block") parent-bol js-indent-level)
>
> ;; JSX
> - ((match "<" "jsx_fragment") parent 0)
> - ((parent-is "jsx_fragment") parent js-indent-level)
> + ;; ((match "<" "jsx_fragment") parent 0)
> + ;; ((parent-is "jsx_fragment") parent js-indent-level)
> + (js--treesit-indent-helper)
> ((node-is "jsx_closing_element") parent 0)
> ((match "jsx_element" "statement") parent js-indent-level)
> ((parent-is "jsx_element") parent js-indent-level)
> @@ -3490,6 +3503,36 @@ This function is intended for use in `after-change-functions'."
> "&&" "||" "!")
> "JavaScript operators for tree-sitter font-locking.")
>
> +(defun js--treesit-font-lock-helper ()
> + "Font lock rules helper, for handle different release of tree-sitter-javascript.
> +Check if a node type is available, then return the right font lock rules."
Same little comment here.
> + ;; handle commit bb1f97b
> + (condition-case nil
> + (progn (treesit-query-capture 'javascript '((member_expression) @capture))
> + '((jsx_opening_element
> + [(member_expression (identifier)) (identifier)]
> + @font-lock-function-call-face)
> +
> + (jsx_closing_element
> + [(member_expression (identifier)) (identifier)]
> + @font-lock-function-call-face)
> +
> + (jsx_self_closing_element
> + [(member_expression (identifier)) (identifier)]
> + @font-lock-function-call-face)))
> + (error
> + '((jsx_opening_element
> + [(nested_identifier (identifier)) (identifier)]
> + @font-lock-function-call-face)
> +
> + (jsx_closing_element
> + [(nested_identifier (identifier)) (identifier)]
> + @font-lock-function-call-face)
> +
> + (jsx_self_closing_element
> + [(nested_identifier (identifier)) (identifier)]
> + @font-lock-function-call-face)))))
> +
The indentation here looks off. Can you format this?
> (defvar js--treesit-font-lock-settings
> (treesit-font-lock-rules
>
> @@ -3599,21 +3642,9 @@ This function is intended for use in `after-change-functions'."
>
> :language 'javascript
> :feature 'jsx
> - '((jsx_opening_element
> - [(nested_identifier (identifier)) (identifier)]
> - @font-lock-function-call-face)
> -
> - (jsx_closing_element
> - [(nested_identifier (identifier)) (identifier)]
> - @font-lock-function-call-face)
> -
> - (jsx_self_closing_element
> - [(nested_identifier (identifier)) (identifier)]
> - @font-lock-function-call-face)
> -
> - (jsx_attribute
> - (property_identifier)
> - @font-lock-constant-face))
> + (append
> + (js--treesit-font-lock-helper)
> + '((jsx_attribute (property_identifier) @font-lock-constant-face)))
>
> :language 'javascript
> :feature 'number
> --
> 2.41.0
>
> From 263c9f0eca3a7df7cb29306297d32358f0e6537c Mon Sep 17 00:00:00 2001
> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Date: Mon, 17 Jul 2023 22:32:13 +0200
> Subject: [PATCH] Updated JSX support due to changes in tree-sitter-typescript
>
> A recent change in tree-sitter-typescript grammar support for
> JSX (commit b893426), changed two things:
> 1. renamed nested_identifier to member_expression
> 2. removed jsx_fragment, jsx_text is used instead
>
> * lisp/progmodes/typescript-ts-mode.el: (typescript-ts-mode--indent-helper): indent helper
> function for handle different tree-sitter-javascript version
> * lisp/progmodes/typescript-ts-mode.el: (typescript-ts-mode--indent-rules): use the new
> function
> * lisp/progmodes/typescript-ts-mode.el: (typescript-ts-mode--font-lock-helper): font lock
> helper function for handle different tree-sitter-javascript version
> * lisp/progmodes/typescript-ts-mode.el: (typescript-ts-mode--font-lock-settings): use the new
> function
> ---
> lisp/progmodes/typescript-ts-mode.el | 64 +++++++++++++++++++++-------
> 1 file changed, 49 insertions(+), 15 deletions(-)
>
> diff --git a/lisp/progmodes/typescript-ts-mode.el b/lisp/progmodes/typescript-ts-mode.el
> index 5df34de0472..2de7587e43a 100644
> --- a/lisp/progmodes/typescript-ts-mode.el
> +++ b/lisp/progmodes/typescript-ts-mode.el
> @@ -75,6 +75,18 @@
> table)
> "Syntax table for `typescript-ts-mode'.")
>
This seems to not be properly converted to tsx from javascript, both in
the docstring and code. Also, I think the name is wrong. Maybe it
should describe its intent a little more closely, something like
"tsx-ts-mode--indent-compatibility-b893426"?
> +(defun typescript-ts-mode--indent-helper ()
^^^^^^^tsx-ts-mode
> + "Indent rules helper, for handle different release of tree-sitter-typescript.
> +Check if a node type is available, then return the right indent rules."
> + ;; handle commit b893426
> + (condition-case nil
> + (progn (treesit-query-capture 'javascript '((jsx_fragment) @capture))
'tsx^^^^^^^, right?
> + `(((match "<" "jsx_fragment") parent 0)
> + ((parent-is "jsx_fragment") parent typescript-ts-mode-indent-offset)))
> + (error
> + `(((match "<" "jsx_text") parent 0)
> + ((parent-is "jsx_text") parent typescript-ts-mode-indent-offset)))))
> +
> (defun typescript-ts-mode--indent-rules (language)
> "Rules used for indentation.
> Argument LANGUAGE is either `typescript' or `tsx'."
> @@ -110,8 +122,7 @@ Argument LANGUAGE is either `typescript' or `tsx'."
> ((parent-is "binary_expression") parent-bol typescript-ts-mode-indent-offset)
>
> ,@(when (eq language 'tsx)
> - `(((match "<" "jsx_fragment") parent 0)
> - ((parent-is "jsx_fragment") parent typescript-ts-mode-indent-offset)
> + `((typescript-ts-mode--indent-helper)
> ((node-is "jsx_closing_element") parent 0)
> ((match "jsx_element" "statement") parent typescript-ts-mode-indent-offset)
> ((parent-is "jsx_element") parent typescript-ts-mode-indent-offset)
> @@ -142,6 +153,39 @@ Argument LANGUAGE is either `typescript' or `tsx'."
> "&&" "||" "!" "?.")
> "TypeScript operators for tree-sitter font-locking.")
>
Same naming comment here, and the language is wrong. It should be 'tsx,
not 'typescript.
> +(defun typescript-ts-mode--font-lock-helper ()
> + "Font lock rules helper, for handle different release of tree-sitter-typescript.
> +Check if a node type is available, then return the right font lock rules."
> + ;; handle commit bb1f97b
> + ;; Warning: treesitter-query-capture says both node types are valid,
> + ;; but then raises an error if the wrong node type is used. So it is
> + ;; important to check with the new node type (member_expression)
> + (condition-case nil
> + (progn (treesit-query-capture 'typescript '((member_expression) @capture))
> + '((jsx_opening_element
> + [(member_expression (identifier)) (identifier)]
> + @typescript-ts-jsx-tag-face)
> +
> + (jsx_closing_element
> + [(member_expression (identifier)) (identifier)]
> + @typescript-ts-jsx-tag-face)
> +
> + (jsx_self_closing_element
> + [(member_expression (identifier)) (identifier)]
> + @typescript-ts-jsx-tag-face)))
> + (error
> + '((jsx_opening_element
> + [(nested_identifier (identifier)) (identifier)]
> + @typescript-ts-jsx-tag-face)
> +
> + (jsx_closing_element
> + [(nested_identifier (identifier)) (identifier)]
> + @typescript-ts-jsx-tag-face)
> +
> + (jsx_self_closing_element
> + [(nested_identifier (identifier)) (identifier)]
> + @typescript-ts-jsx-tag-face)))))
> +
> (defun typescript-ts-mode--font-lock-settings (language)
> "Tree-sitter font-lock settings.
> Argument LANGUAGE is either `typescript' or `tsx'."
> @@ -293,19 +337,9 @@ Argument LANGUAGE is either `typescript' or `tsx'."
>
> :language language
> :feature 'jsx
> - `((jsx_opening_element
> - [(nested_identifier (identifier)) (identifier)]
> - @typescript-ts-jsx-tag-face)
> -
> - (jsx_closing_element
> - [(nested_identifier (identifier)) (identifier)]
> - @typescript-ts-jsx-tag-face)
> -
> - (jsx_self_closing_element
> - [(nested_identifier (identifier)) (identifier)]
> - @typescript-ts-jsx-tag-face)
> -
> - (jsx_attribute (property_identifier) @typescript-ts-jsx-attribute-face))
> + (append
> + (typescript-ts-mode--font-lock-helper)
> + `((jsx_attribute (property_identifier) @typescript-ts-jsx-attribute-face)))
>
> :language language
> :feature 'number
> --
> 2.41.0
Thanks for the patch :)
Theo
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.