clojure-emacs / cider

The Clojure Interactive Development Environment that Rocks for Emacs
https://cider.mx
GNU General Public License v3.0
3.55k stars 645 forks source link

`cider-browse-ns-all` works incorrectly with single-name namespaces #3221

Open kxygk opened 2 years ago

kxygk commented 2 years ago

Expected behavior

When I go into cider-browse-ns-all and hit ENTER on a namespace, I start browsing that namespace's variables

Actual behavior

When the namespace is a simple one (just one name with no . separators) then it's somehow treated incorrectly and it open a docs window

Steps to reproduce the problem

Here is a minimal example

https://gist.github.com/kxygk/2932bc0fb53f28f8330815fbbf1ecfd5

I create lots of little single-file libraries in flat directories like this. I see that mynamespace.core is more common - but I'd rather not repackage them all with extra useless folders and boiler plate. (unless I'm missing some benefit here..)

If you jack-in and eval the buffer you can then run cider-browse-ns. Then type in ciderbug0. You are greated with the expected window

ciderbug0
  launch-the-missiles Another docstr
  super-secret-str Some docstr

(fancy syntax highlighting missing here..)

However if you instead type in cider-browse-ns-all - navigate to ciderbug0 and hit the ENTER key then it pops up the docs page

ciderbug0/ciderbug0
  Not documented.

ciderbug0/ciderbug0 is defined in ciderbug0.clj.

Other namespaces with . separators don't see to have this behavior

CIDER 1.3.0-snapshot (package: 20220216.732), nREPL 0.9.0

Clojure 1.10.3, Java 11.0.14.1

GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.16.0) of 2022-01-25, modified by Debian

OS: Ubuntu 22.04 jammy

juszczakn commented 2 years ago

Maybe related to #3217 that was just recently merged?

Looks like the problem is that the whitespace before/after each ns in the list isn't propertized correctly, so in cider-browse-ns--thing-at-point, ns-p doesn't return a result.

If you move point to be over the actual ns name, and not the beginning of the line, it seems to work as expected.

I think the correct fix might be to propertize the whitespace correctly, but I hacked this together that works, as well /shrug

modified   cider-browse-ns.el
@@ -455,13 +455,12 @@ var-meta map."
   "Get the thing at point.
 Return a list of the type ('ns or 'var) and the value."
   (let ((ns-p (get-text-property (point) 'ns))
-        (line (car (split-string (string-trim (thing-at-point 'line)) " "))))
-    (if (or ns-p (string-match "\\." line))
+        (line (car (split-string (string-trim (thing-at-point 'line)) " ")))
+        (things-ns (or (get-text-property (point) 'cider-browse-ns-current-ns)
+                       cider-browse-ns-current-ns)))
+    (if (or ns-p (string-match "\\." line) (not things-ns))
         `(ns ,line)
-      `(var ,(format "%s/%s"
-                     (or (get-text-property (point) 'cider-browse-ns-current-ns)
-                         cider-browse-ns-current-ns)
-                     line)))))
+      `(var ,(format "%s/%s" things-ns line)))))

 (defun cider-browse-ns-toggle-all ()
   "Toggle showing all of the items in the browse-ns buffer."
bbatsov commented 2 years ago

@zkry Can you look into this?