imenu is broken after commit `54a62cc02a0901400a1a3660182a4c898626116d`

[rrudakov]

Expected behavior

When I execute M-x imenu or any enhanced version of it (like helm-imenu) I expect to see list of function names:

Actual behavior

If definition has some metadata it's rendered in the imenu instead of definition name:

Steps to reproduce the problem

1. Create a new buffer with clojure-mode
2. Type the following:

(def ^:private first-var "Hello")

(def ^{:private true} second-var "World")

(defn some-func
  []
  (print (str first-var second-var)))

3. Execute M-x imenu

Note: Reverting 54a62cc fixes the issue.

Environment & Version information

clojure-mode version

M-x clojure-mode-display-version returns the following:

clojure-mode (version nil)

Emacs version

GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.34, cairo version 1.17.6) of 2022-07-14

Operating system

Linux 5.18.14-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 23 Jul 2022 11:46:17 +0000 x86_64 GNU/Linux

[vemv]

Thanks for the report!

Perhaps 'broken' is bit of an overstatement because metadata isn't present all the time, but yeah this should be fixed.

@OknoLombarda, would you like to check out this issue?

[OknoLombarda]

I'll take a look

[OknoLombarda]

Well, first things first, that's completely my fault.

The reason this happens is because before my fix, after skipping metadata, function still went through another iteration to find var name. Again, for example, if we have this definition

(def ^:dynamic a 1)

then before checking if current sexp is var metadata, function calls bounds-of-thing-at-point to determine bounds of potential var name.

clojure-mode/clojure-mode.el

Line 756 in bb327cb

(cl-destructuring-bind (def-beg . def-end) (bounds-of-thing-at-point 'sexp)

If var has metadata, this call will return bounds of metadata sexp (^:dynamic). Previously, after that function would go through another iteration and find correct var name. Now, because of this condition

clojure-mode/clojure-mode.el

Line 760 in bb327cb

(when (or (not (char-equal ?^ (char-after (point))))

after metadata sexp is successfully skipped, the function exits the loop with incorrect bounds for var name. I managed to fix this by changing this condition

-            (when (or (not (char-equal ?^ (char-after (point))))
+            (when (or (not (char-equal ?^ (char-after def-beg)))

Initially I added it so that function completes for vars without metadata: if char after point is not ^, then function exits the loop, but the problem is that after successfully skipping metadata, the point is no longer at ^, so condition evaluates to true. If, instead of checking the char after point, we check beginning of previously captured sexp bounds (def-beg), then the condition will evaluate to false and function will continue to the next iteration. (I tried my best, but this is really hard to explain)

It seems to me, this should work, but I'll check it more thoroughly tomorrow and if everything is okay, send a PR

[rrudakov]

Thanks everyone!

[vemv]

clojure-mode 5.15.1 will be available in MELPA within a couple hours

[bbatsov]

And on NonGNU ELPA as well.