Package: emacs;
Reported by: Juri Linkov <juri <at> linkov.net>
Date: Thu, 8 Feb 2024 17:42:01 UTC
Severity: normal
Fixed in version 30.0.50
Done: Juri Linkov <juri <at> linkov.net>
Bug is archived. No further changes may be made.
Message #14 received at 68993 <at> debbugs.gnu.org (full text, mbox):
From: Yuan Fu <casouri <at> gmail.com> To: Juri Linkov <juri <at> linkov.net> Cc: 68993 <at> debbugs.gnu.org Subject: Re: bug#68993: treesitter support for forward-sexp-default-function Date: Sun, 11 Feb 2024 17:42:34 -0800
> On Feb 8, 2024, at 9:38 AM, Juri Linkov <juri <at> linkov.net> wrote:
>
> 'treesit-forward-sentence' supports the node type 'text',
> and for matching nodes it uses the fallback to
> 'forward-sentence-default-function'.
>
> This patch does exactly the same for 'treesit-forward-sexp':
> for nodes that match a new node type 'comment',
> it uses the fallback to the new function
> 'forward-sexp-default-function'.
>
> diff --git a/lisp/treesit.el b/lisp/treesit.el
> index 82b2f97b4a5..284c4915f3a 100644
> --- a/lisp/treesit.el
> +++ b/lisp/treesit.el
> @@ -2137,7 +2137,10 @@ treesit-forward-sexp
> (interactive "^p")
> (let ((arg (or arg 1))
> (pred (or treesit-sexp-type-regexp 'sexp)))
> - (or (if (> arg 0)
> + (or (when (treesit-node-match-p (treesit-node-at (point)) 'comment t)
> + (funcall #'forward-sexp-default-function arg)
> + t)
> + (if (> arg 0)
> (treesit-end-of-thing pred (abs arg) 'restricted)
> (treesit-beginning-of-thing pred (abs arg) 'restricted))
> ;; If we couldn't move, we should signal an error and report
> diff --git a/lisp/emacs-lisp/lisp.el b/lisp/emacs-lisp/lisp.el
> index 4b722b4e9a7..d3c3bf55de3 100644
> --- a/lisp/emacs-lisp/lisp.el
> +++ b/lisp/emacs-lisp/lisp.el
> @@ -45,7 +45,12 @@ parens-require-spaces
> :type 'boolean
> :group 'lisp)
>
> -(defvar forward-sexp-function nil
> +(defun forward-sexp-default-function (&optional arg)
> + "Default function for `forward-sexp-function'."
> + (goto-char (or (scan-sexps (point) arg) (buffer-end arg)))
> + (if (< arg 0) (backward-prefix-chars)))
> +
> +(defvar forward-sexp-function #'forward-sexp-default-function
> ;; FIXME:
> ;; - for some uses, we may want a "sexp-only" version, which only
> ;; jumps over a well-formed sexp, rather than some dwimish thing
> @@ -74,10 +79,7 @@ forward-sexp
> "No next sexp"
> "No previous sexp"))))
> (or arg (setq arg 1))
> - (if forward-sexp-function
> - (funcall forward-sexp-function arg)
> - (goto-char (or (scan-sexps (point) arg) (buffer-end arg)))
> - (if (< arg 0) (backward-prefix-chars)))))
> + (funcall forward-sexp-function arg)))
>
> (defun backward-sexp (&optional arg interactive)
> "Move backward across one balanced expression (sexp).
>
> Maybe the node type 'comment' is not the best name,
> but it was intended to allow using the default function
> to be able to move with 'M-C-f' in the comments and strings
> there tree-sitter has no information.
>
> It makes sense to support the default movement with 'M-C-f'
> in the comments and strings of all ts modes. The second patch
> shows how this could be achieved by adding the default
> 'comment' match to 'treesit-thing-settings' of all modes.
> Or maybe this should be customizable?
I think treesit-thing-settings is something we want to left for major mode’s to set. They’ll need to define other “things” in treesit-thing-settings anyway. Sure, it’s nice if we can set a few definitions automatically, but I don’t think the gain is worth that much; OTOH, it’s nice to have clear boundaries, and minimizes the possibility of confusion.
>
> diff --git a/lisp/treesit.el b/lisp/treesit.el
> index 82b2f97b4a5..284c4915f3a 100644
> --- a/lisp/treesit.el
> +++ b/lisp/treesit.el
> @@ -3054,6 +3057,18 @@ treesit-major-mode-setup
> (setq-local outline-search-function #'treesit-outline-search
> outline-level #'treesit-outline-level))
>
> + (dolist (parser (treesit-parser-list))
> + (let ((language (treesit-parser-language parser))
> + (comment (regexp-opt '("comment" "string" "string_content"))))
> + (unless (treesit-thing-defined-p 'comment language)
> + (if-let ((l (alist-get language treesit-thing-settings)))
> + (progn
> + (setf (alist-get 'comment l) (list comment))
> + (setf (alist-get language treesit-thing-settings) l))
> + (setq-local treesit-thing-settings
> + (append `((,language (comment ,comment)))
> + treesit-thing-settings))))))
> +
> ;; Remove existing local parsers.
> (dolist (ov (overlays-in (point-min) (point-max)))
> (when-let ((parser (overlay-get ov 'treesit-parser)))
>
> The third patch demonstrates how it's possible to close bug#67036
> that was impossible to fix without more general changes in treesit.el.
>
> The problem is that e.g. Ruby parser to such text:
>
> hash[:key]
>
> produces such syntax tree:
>
> (element_reference object: (identifier) [ (simple_symbol) ])
>
> so when point is on [ then 'M-C-f' can't move to ].
>
> This is fixed now by the third patch:
>
> diff --git a/lisp/progmodes/ruby-ts-mode.el b/lisp/progmodes/ruby-ts-mode.el
> index 598eaa461ff..4d0ae2e9303 100644
> --- a/lisp/progmodes/ruby-ts-mode.el
> +++ b/lisp/progmodes/ruby-ts-mode.el
> @@ -1170,7 +1170,20 @@ ruby-ts-mode
> "global_variable"
> )
> eol)
> - #'ruby-ts--sexp-p)))))
> + #'ruby-ts--sexp-p))
> + (comment ,(lambda (node)
> + (or (member (treesit-node-type node)
> + '("comment" "string_content"))
> + (and (member (treesit-node-text node)
> + '("[" "]"))
> + (equal (treesit-node-type
> + (treesit-node-parent node))
> + "element_reference"))
> + (and (member (treesit-node-text node)
> + '("#{" "}"))
> + (equal (treesit-node-type
> + (treesit-node-parent node))
> + "interpolation"))))))))
>
> ;; AFAIK, Ruby can not nest methods
> (setq-local treesit-defun-prefer-top-level nil)
IIUC, this doesn’t look like a good idea: you don’t want to mark something that’s not comment as comment. In the future, other packages will start using these thing definitions, and I’m sure you don’t want them consider regular code as comments.
For the specific problem you described, maybe the change made in #68899 can help?
Yuan
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.