Improvements for REPL output pre-processing and locref navigation #2083

vspinu · 2017-08-28T14:23:48Z

Improving on #2043 a bit for navigation. Pass through file metatdata when available from location references and no info from var names could be retrieved. cider-find-file does a good job in those cases.

bbatsov · 2017-08-29T16:37:43Z

cider-repl.el

-                       (or (cider-sync-request:ns-path var)
-                           (nrepl-dict-get (cider-sync-request:info var) "file"))
-                     (plist-get loc :file))))
+             (file (or (when var


Who does this improve the code? Seems exactly the same to me.

The difference is that now :file metadata is picked up when first two resolutions fail (those within or). This happens when a var is a protocol method. This was also the original intention btw.

Ah, I see your point now - var might be present, but its branch might return nil. Add some comments as it's somewhat hard to follow the what the assumptions here otherwise.

Yes, I used "var" loosely to refer to the variable at point, which need not correspond to clojure's var. Will add a couple of notes.

bbatsov · 2017-08-29T16:39:00Z

cider-repl.el

@@ -561,7 +561,7 @@ When there is a possible unfinished ansi control sequence,
                   (buffer-local-value 'cider-repl--ns-forms-plist connection)
                   ns)))))

-(defvar cider-repl--root-ns-highlight-template "\\<\\(%s\\)[^$/: \t\n]+"
+(defvar cider-repl--root-ns-highlight-template "\\<\\(%s\\)[^$/: \t\n()]+"


What's the purpose of this change?

Improve original regexp. Currently things like ns1.foo.bar(file.clj:123) are highlighted all the way to 123. This change stops them at (.

Originally I thought all those reference have $ in their name, but that's not always the case.

bbatsov · 2017-08-29T16:40:04Z

cider-repl.el

@@ -1051,7 +1051,7 @@ regexes from `cider-locref-regexp-alist' to infer locations at point."
                             (nrepl-dict-get (cider-sync-request:info var) "file")))
                       (plist-get loc :file))))
        (if file
-            (cider--jump-to-loc-from-info (nrepl-dict "file" file "line" line))
+            (cider--jump-to-loc-from-info (nrepl-dict "file" file "line" line) t)


I think it's subjective whether this is better or worse - in such cases I always add a defcustom.

P.S. Mind the commit message style. 😃

It's not subjective. These pseudo links now behave like links in cider stacktrace buffer. Also like links in every other buffer which has a bunch of links in it (occur, grep, compilation etc) behave like that. It's common to click on multiple error references till you get what you want.

Mind the commit message style. 😃

What do you mean more concretely?

What do you mean more concretely?

Follow this guide.

When clicked on locref in REPL, jump to other window.

doesn't adhere to #4 or #5.

Ohh, dot at the end of the message. Fixing that :) The subject line is already in imperative mood.

The subject line is already in imperative mood.

I'm not a stickler for this stuff, but no, it's not.

Jump to other window when clicking on locref in REPL

would be.

Indeed. The commit messages should ideally always start with a verb.

bbatsov · 2017-08-30T12:43:12Z

CHANGELOG.md

@@ -8,6 +8,7 @@

 ### Bugs Fixed

+* Improve file retrieval when jumping to REPLs location references and Clojure vars cannot be resolved.


How come you can just to something that's not resolved? I guess you made some mistake in the description here. Or this just for the scope of the REPL defs. Frankly I don't remember much this code and I don't have time to dive into it, but I still have to understand what exactly is this supposed to fix.

That code is recent. I added it in #2043 and now I am just improving on it.

I just said what I did there - when locrefs cannot be referenced from variables at point, use file metadata from the locrefs themselves. I can state it more generically "Improve locations lookup by using file metadata when available". Not accurate but conveys the improvement.

AFAIC, this need not be documented at all as being a small and technical fix on a recent change, but knowing how zealous you are on change-logs I added it.

…ware

vspinu · 2017-08-30T20:30:58Z

I am piling a bunch of related changes and improvements here. With a bit more doc this time, so should be self explanatory.

Also fixing failing tests to account for recent buttercup changes.

Two notable new features:

cider-preoutput-hook which implements a customizable pipeline of output pre-processing and includes most of our output pre-processing as "middleware".
cider-repl-highlight-spec-keywords pre-output processor which highlights clojure.spec "explain" keywords. Clojure spec explain output is extremely difficult to read.. Make it a bit easy by showing this:

instead of

xiongtx · 2017-08-30T20:48:00Z

I am piling a bunch of related changes and improvements here.

It'd be better to open more PRs. A bunch of unrelated changes in a single PR is hard to review.

- Disallow highlight spilling over () as in ns.foo.bar(file.clj:123)). - Start matching at the beginning of the symbol, not word. Particularly inhibit matching of namespaced keywords (which could be many).

This allows for customization of the modification of the REPL output before it is inserted into the buffer. New functions to be used as part of this hook: - cider-repl-add-locref-help-echo - cider-repl-highlight-current-project

vspinu force-pushed the locref-file-fix branch from daa6f65 to 498e90c Compare August 28, 2017 19:08

bbatsov reviewed Aug 29, 2017

View reviewed changes

vspinu force-pushed the locref-file-fix branch from 498e90c to b5fb91d Compare August 29, 2017 18:49

bbatsov reviewed Aug 30, 2017

View reviewed changes

vspinu force-pushed the locref-file-fix branch 2 times, most recently from d7ab529 to 052d90b Compare August 30, 2017 19:44

Rely on cider-find-file when locref cannot be resolved by info middle…

d76a25a

…ware

vspinu force-pushed the locref-file-fix branch 2 times, most recently from fc22b8d to 74fee61 Compare August 30, 2017 20:16

vspinu changed the title ~~Rely on cider-find-file when locref of vars cannot be resolved~~ Improvements for REPL output pre-processing and locref navigation Aug 30, 2017

vspinu added 4 commits August 31, 2017 00:05

Improve on cider-repl--root-ns-highlight-template regexp

f876643

- Disallow highlight spilling over () as in ns.foo.bar(file.clj:123)). - Start matching at the beginning of the symbol, not word. Particularly inhibit matching of namespaced keywords (which could be many).

Jump to other window when clicking on a locref in REPL

1104d83

New utility function cider-run-chained-hook

10afc9c

New utility function cider-add-face

40c89c4

vspinu force-pushed the locref-file-fix branch 2 times, most recently from 6437a59 to 8d59067 Compare August 30, 2017 22:15

vspinu added 4 commits August 31, 2017 00:16

New non-standard hook cider-repl-preoutput-hook

5f67a50

This allows for customization of the modification of the REPL output before it is inserted into the buffer. New functions to be used as part of this hook: - cider-repl-add-locref-help-echo - cider-repl-highlight-current-project

Add cider-repl-highlight-spec-keywords pre-output processor

d847640

Document recent changes

1a4f382

Fix tests

f93a5fe

vspinu force-pushed the locref-file-fix branch from 8d59067 to f93a5fe Compare August 30, 2017 22:16

bbatsov merged commit 7a72259 into clojure-emacs:master Sep 5, 2017

vspinu deleted the locref-file-fix branch June 20, 2018 10:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements for REPL output pre-processing and locref navigation #2083

Improvements for REPL output pre-processing and locref navigation #2083

vspinu commented Aug 28, 2017

bbatsov Aug 29, 2017

vspinu Aug 29, 2017

bbatsov Aug 30, 2017

vspinu Aug 30, 2017

bbatsov Aug 29, 2017

vspinu Aug 29, 2017

bbatsov Aug 29, 2017

vspinu Aug 29, 2017

xiongtx Aug 29, 2017 •

edited

vspinu Aug 29, 2017 •

edited

xiongtx Aug 29, 2017

bbatsov Aug 31, 2017

bbatsov Aug 30, 2017 •

edited

vspinu Aug 30, 2017

vspinu commented Aug 30, 2017

xiongtx commented Aug 30, 2017 •

edited

		@@ -8,6 +8,7 @@

		### Bugs Fixed

		* Improve file retrieval when jumping to REPLs location references and Clojure vars cannot be resolved.

Improvements for REPL output pre-processing and locref navigation #2083

Improvements for REPL output pre-processing and locref navigation #2083

Conversation

vspinu commented Aug 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiongtx Aug 29, 2017 • edited

Choose a reason for hiding this comment

vspinu Aug 29, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbatsov Aug 30, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vspinu commented Aug 30, 2017

xiongtx commented Aug 30, 2017 • edited

xiongtx Aug 29, 2017 •

edited

vspinu Aug 29, 2017 •

edited

bbatsov Aug 30, 2017 •

edited

xiongtx commented Aug 30, 2017 •

edited