Skip to content

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

Open
@yuhan0

Description

@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.

Activity

rrudakov

rrudakov commented on Mar 6, 2025

@rrudakov
Contributor

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

rrudakov commented on Mar 6, 2025

@rrudakov
Contributor

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

rrudakov commented on Mar 6, 2025

@rrudakov
Contributor

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

yuhan0 commented on Mar 6, 2025

@yuhan0
ContributorAuthor

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

bbatsov commented on Mar 6, 2025

@bbatsov
Member

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

rrudakov

rrudakov commented on Mar 6, 2025

@rrudakov
Contributor

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

yuhan0

yuhan0 commented on Mar 6, 2025

@yuhan0
ContributorAuthor

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

rrudakov commented on Mar 6, 2025

@rrudakov
Contributor

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

yuhan0 commented on Mar 6, 2025

@yuhan0
ContributorAuthor

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

bbatsov commented on Mar 6, 2025

@bbatsov
Member

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

rrudakov commented on Mar 6, 2025

@rrudakov
Contributor

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

rrudakov commented on Mar 6, 2025

@rrudakov
Contributor

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

bbatsov

bbatsov commented on Mar 6, 2025

@bbatsov
Member

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

10 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Navigation commands should no-op when called in an invalid position · Issue #62 · clojure-emacs/clojure-ts-mode