Navigation commands should no-op when called in an invalid position

[yuhan0]

In regular clojure-mode, calling the commands up-list and backward-up-list in a position without an enclosing form results in a no-op, raising a user error

At top level

E.g. with point at :here in the following, press C-M-u (backward-up-list) / M-x up-list repeatedly to move out of successive forms.

(first "expression")

(:you {:are [:here]})

(last "expression")

Upon reaching the top level (beginning/end of line), the expected behavior is for the point to remain stationary (and a user error to be thrown)

In clojure-ts-mode, this results instead in the point moving all the way to the beginning or end of the buffer, which can be quite disconcerting.

Additional note: This affects various navigation commands in other packages like Paredit and Lispy, which ultimately delegate to (backward)-up-list.

[rrudakov]

It looks like an Emacs bug.

In treesit.el in treesit-major-mode-setup it sets custom forward-sexp-function:

(when (treesit-thing-defined-p 'sexp nil)
    (setq-local forward-sexp-function #'treesit-forward-sexp))

which in turn triggers a branch in up-list-default-function https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/emacs-lisp/lisp.el#n292

(if (null forward-sexp-function)
                (goto-char (or (scan-lists (point) inc 1)
                               (buffer-end arg)))
              (condition-case err
                  (while (progn (setq pos (point))
                                (forward-sexp inc)
                                (/= (point) pos)))
                (scan-error (goto-char (nth (if (> arg 0) 3 2) err))))
              (if (= (point) pos)
                  (signal 'scan-error
                          (list "Unbalanced parentheses" (point) (point)))))

Looks like this code just calls this treesit-forward-sexp until it reaches the end/beginning of the buffer.

I think it should be reported to Emacs' bug tracker.

[rrudakov]

I've found a solution. We should set custom up-list-function during clojure-ts-mode setup:

(setq-local up-list-function #'treesit-up-list)

This fixes the problem.

[rrudakov]

We might also consider setting the following to improve user experience:

(setq-local up-list-function #'treesit-up-list
                down-list-function #'treesit-down-list
                forward-list-function #'treesit-forward-list
                transpose-sexps-function #'treesit-transpose-sexps)

[yuhan0]

Hmm, it seems like the functions you've linked to haven't been merged into a release branch of Emacs - I'm on the latest 30.1 release and there's no sign of a up-list-function. Git-blaming points to the following commits and bug report:

https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ec8dd27f008bca810209354a189d241479fe4d32
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=687ff86e802c9883f292f58a890178d08311a821
https://lists.gnu.org/archive/html/bug-gnu-emacs/2024-09/msg01410.html

I wonder if this hasn't already been solved on the latest master branch - @rrudakov are you running Emacs directly off HEAD?

[bbatsov]

Yeah, I was about to say that's the first time I hear of those functions.

[rrudakov]

Ah, sorry, indeed, I'm on master :)

[yuhan0]

In any case it appears that (the master branch version of) treesit.el already performs the above duties of setting up-list-function etc. as long as some list thing is defined
https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/treesit.el#n4232

[rrudakov]

In any case it appears that (the master branch version of) treesit.el already performs the above duties of setting up-list-function etc. as long as some list thing is defined https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/treesit.el#n4232

Yes, but list thing is not defined for clojure-ts-mode, only sexp and text, so it should either be defined explicitly, or we should set up-list-function ourselves.

[yuhan0]

Yeah, I don't know enough about treesitter integration to judge which approach is better, and also if we should be making changes against 'unreleased' APIs in the first place?

I'd vote to close this issue for now and revisit it when up-list-function etc. actually gets introduced.

[bbatsov]

Let's keep the issue open and investigate options. I've been spending more time playing with TreeSitter lately (even wrote a simple TS mode for OCaml https://github.com/bbatsov/neocaml) and I plan to double down on clojure-ts-mode this year.

[rrudakov]

Probably the issue still could be reported to Emacs' bug tracker? There is a chance that it could be fixed in version 30.2.

[rrudakov]

I guess we could try setting forward-sexp-function to nil to fallback to the original behavior of up-list.

[bbatsov]

@rrudakov Yeah, I think we can't go wrong to report this. Would you mind doing this?

[rrudakov]

@rrudakov Yeah, I think we can't go wrong to report this. Would you mind doing this?

sure

[yuhan0]

Thanks, I can confirm that this solves the original issue– will see if it produces any other unforeseen effects:

(add-hook 'clojure-ts-mode-hook
          (defun +clojure-ts-tweaks ()
            (setq-local forward-sexp-function nil)))

[rrudakov]

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=76784

[rrudakov]

So far no one of the maintainers replied, but I received a suggestion how to fix it properly for emacs-31.

@@ -918,10 +918,16 @@
     "unquote_splicing_lit" "unquoting_lit")
   "A regular expression that matches nodes that can be treated as s-expressions.")
 
+(defconst clojure-ts--list-nodes
+  '("list_lit" "anon_fn_lit" "read_cond_lit" "splicing_read_cond_lit"
+    "map_lit" "ns_map_lit" "vec_lit" "set_lit")
+  "A regular expression that matches nodes that can be treated as lists.")
+
 (defconst clojure-ts--thing-settings
   `((clojure
-     (sexp ,(regexp-opt clojure-ts--sexp-nodes)
-           text ,(regexp-opt '("comment"))))))
+     (sexp ,(regexp-opt clojure-ts--sexp-nodes))
+     (list ,(regexp-opt clojure-ts--list-nodes))
+     (text ,(regexp-opt '("comment"))))))
 
 (defvar clojure-ts-mode-map
   (let ((map (make-sparse-keymap)))
@@ -1043,7 +1049,8 @@
       ;; Workaround for treesit-transpose-sexps not correctly working with
       ;; treesit-thing-settings on Emacs 30.
       ;; Once treesit-transpose-sexps it working again this can be removed
-      (when (fboundp 'transpose-sexps-default-function)
+      (when (and (fboundp 'transpose-sexps-default-function)
+                 (< emacs-major-version 31))
         (setq-local transpose-sexps-function #'transpose-sexps-default-function)))))
 
 ;; For Emacs 30+, so that `clojure-ts-mode' is treated as deriving from

it adds a definition for list thing, so treesit.el would set all corresponding functions automatically. I think it would be a good improvement and we can apply it now without breaking anything. @bbatsov what do you think?

[bbatsov]

I agree. Let's do this!