clojure-emacs/clojure-mode

clojure-thread-last breaks with line comments

yuhan0 opened this issue · 2 comments

yuhan0 commented

Expected behavior

Running clojure-thread-last-all on the following

(foo x ;; grobble
  (bar y))

should preserve the comments and not unbalance parens.
eg.

(->> y
  (bar)
  (foo x ;; grobble
    ))

Actual behavior

(->> (bar y)
(foo x ;; grobble))

The closing parens are merged into the comment, breaking the structure.

Steps to reproduce the problem

Run C-c C-r l on an expression with a line comment.
Breaks even when comment is on its own line

(drop 2
  ;; TODO use transducers
  (map inc
    (filter even?
      (range 100))))

This could probably be fixed by adding a (clojure--in-comment-p) check in the right place. But I wonder if some of these refactorings could be written with parseclj to be more robust.. Eg. most don't deal well with metadata or #_ ignored forms. The threading commands happen to be the most complex from the looks of the code.

Environment & Version information

clojure-mode version

clojure-mode (version 5.13.0)

Emacs version

28.0.91

Operating system

macOS 12

vemv commented

This could be as well be done with a tool of the rewrite-clj family, which while would need a CIDER connection, it would seem work that is more likely to happen (and more likely to be reused; CIDER isn't limited in scope to emacs).

It seems already implemented in clojure-lsp, see https://clojure-lsp.io/features/. So whatever its implementation is, either it could be directly used (by using clojure-lsp as-is) or borrowed (via CIDER).

All in all, the broken behavior is trivially fixable by just hitting RET in the right place, so I'd wager that the cost/benefit ratio of a reimplementation would be far from attractive.

PRs welcome in any direction of course.

yuhan0 commented

That's true, also I realised shortly after writing that parseclj is still in alpha and its API isn't the friendliest.

the broken behavior is trivially fixable by just hitting RET in the right place,

Not always, the threading logic also stops after parens have been broken

(a ;; comment
  (b (c (d (e)))))

becomes

(->> (b (c (d (e))))
(a ;; comment))

So it's more like

  1. Notice that something is broken
  2. Recognise the issue (rather than instinctively hitting undo and refactoring by hand)
  3. Navigate to the right spot (with structural navigation broken) and hit RET
  4. (then DEL DEL DEL if Doom Emacs helpfully inserts a ;; to continue the comment)
  5. Navigate to the ->> and C-c C-r t repeatedly until the thread is fully wound, if any other comments are encountered go back to step 3
  • 👍1