Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cider-ns - refinements #3632

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

- [#3626](https://github.com/clojure-emacs/cider/issues/3626): `cider-ns-refresh`: jump to the relevant file/line on errors.
- [#3628](https://github.com/clojure-emacs/cider/issues/3628): `cider-ns-refresh`: summarize errors as an overlay.
- [#3628](https://github.com/clojure-emacs/cider/issues/3628): `cider-ns-refresh`: accept `clear-and-inhibit` mode.
- It often makes sense not to run the before/after functions around the `clear`ing.
- Bump the injected nREPL to [1.1.1](https://github.com/nrepl/nrepl/blob/v1.1.1/CHANGELOG.md#111-2024-02-20).
- Bump the injected `cider-nrepl` to [0.47.0](https://github.com/clojure-emacs/cider-nrepl/blob/v0.47.0/CHANGELOG.md#0470-2024-03-10).
- Updates [Orchard](https://github.com/clojure-emacs/orchard/blob/v0.23.2/CHANGELOG.md#0232-2024-03-10).
Expand Down
24 changes: 13 additions & 11 deletions cider-ns.el
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,9 @@ presenting the error message as an overlay."
;; jars are unlikely sources of user errors, so we favor the next `cause-dict':
(not (string-prefix-p "jar:" file-url))
line)
(setq buf (cider--find-buffer-for-file file-url))
(list buf (cons line column)))))
(when-let ((found (cider--find-buffer-for-file file-url)))
(setq buf found)
(list buf (cons line column))))))
error)))
(when jump-args
(apply #'cider-jump-to jump-args)
Expand All @@ -152,9 +153,7 @@ presenting the error message as an overlay."
(trimmed-err (funcall cider-inline-error-message-function message)))
(cider--display-interactive-eval-result trimmed-err
'error
(save-excursion
(end-of-defun)
(point))
(cider--semantic-end-of-line)
'cider-error-overlay-face)))))
(cider--render-stacktrace-causes error)
;; Select the window displaying the 'culprit' buffer so that the user can immediately fix it,
Expand Down Expand Up @@ -286,24 +285,27 @@ Uses the configured 'refresh dirs' \(defaults to the classpath dirs).
With a single prefix argument, or if MODE is `refresh-all', reload all
namespaces on the classpath dirs unconditionally.

With a double prefix argument, or if MODE is `clear', clear the state of
the namespace tracker before reloading. This is useful for recovering from
With a double prefix argument, or if MODE is `clear' (or `clear-and-inhibit'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we'll need to document this somewhere in the manual as well, otherwise few people are going to find it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. While we're here, should the new mode be invokable through some prefix incantation?

tbh I don't use prefixes at all so I wouldn't know how to add a new variation here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be nice indeed. I think multiple prefixes had to be checked as some number value in the code to differentiate them. There must be some examples in the code. I do think, however, we should start moving towards transient for prefix arguments as it's UI is so much better, and it's bundled with Emacs 28+.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expanded the user manual

I do think, however, we should start moving towards transient for prefix arguments as it's UI is so much better

In face of this maybe we can refrain from doubling down on prefixes (at least here).

Transient rocks and would love to see a few features / examples of how it would look for CIDER

clear the state of the namespace tracker before reloading.

This is useful for recovering from
some classes of error (for example, those caused by circular dependencies)
that a normal reload would not otherwise recover from. The trade-off of
clearing is that stale code from any deleted files may not be completely
unloaded.

With a negative prefix argument, or if MODE is `inhibit-fns', prevent any
refresh functions (defined in `cider-ns-refresh-before-fn' and
With a negative prefix argument,
or if MODE is `inhibit-fns' (or `clear-and-inhibit'),
prevent any refresh functions (defined in `cider-ns-refresh-before-fn' and
`cider-ns-refresh-after-fn') from being invoked."
(interactive "p")
(cider-ensure-connected)
(cider-ensure-op-supported "refresh")
(cider-ensure-op-supported "cider.clj-reload/reload")
(cider-ns-refresh--save-modified-buffers)
(let ((clear? (member mode '(clear 16)))
(let ((clear? (member mode '(clear clear-and-inhibit 16)))
(all? (member mode '(refresh-all 4)))
(inhibit-refresh-fns (member mode '(inhibit-fns -1))))
(inhibit-refresh-fns (member mode '(inhibit-fns clear-and-inhibit -1))))
(cider-map-repls :clj
(lambda (conn)
;; Inside the lambda, so the buffer is not created if we error out.
Expand Down
22 changes: 22 additions & 0 deletions cider-util.el
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,28 @@ KIND can be the symbols `ns', `var', `emph', `fn', or a face name."
(t x)))
menu-list))

(defun cider--semantic-end-of-line ()
"Returns POINT at the closest EOL that we can move to semantically,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this. What's an EOL to which we can move semantically? I checked the tests as well, but all assertions are in a single tests, so it's pretty hard to figure out what you're testing for exactly.

i.e. using sexp-aware navigation."
(let ((continue t)
(p (point)))
(save-excursion
(while continue
(end-of-line)
(condition-case nil
(save-excursion
;; If we can't `clojure-backward-logical-sexp',
;; it means that we aren't in a 'semantic' position,
;; so functions like `cider--make-result-overlay' (which use `clojure-backward-logical-sexp' internally) would fail
(clojure-backward-logical-sexp))
(scan-error (forward-line)))
(setq p (point))
(setq continue (and (not (eolp)) ;; we're trying to move to the end of the line
(not (eobp)) ;; abort if we reached EOF (infinite loop avoidance)
(not (equal p (point))) ;; abort if we aren't moving (infinite loop avoidance)
)))
(point))))

(provide 'cider-util)

;;; cider-util.el ends here
1 change: 1 addition & 0 deletions codespell.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
edn
fo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why so? That's often a misspelled of.

hist
juxt
nd
Expand Down
13 changes: 10 additions & 3 deletions doc/modules/ROOT/pages/usage/misc_features.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,24 @@ Typing kbd:[C-c M-n r] or kbd:[C-c M-n M-r] will invoke
`cider-ns-refresh` and reload all modified Clojure files on the
classpath.

Adding a prefix argument, kbd:[C-u C-c M-n r], will reload all
Adding a prefix argument, kbd:[C-u C-c M-n r], it will reload all
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "Adding" is not the right terminology here, which affects the rest of the sentence (which reads a bit awkwardly to me). I'd write something like "If you invoke cider-ns-refresh with a prefix argument it will" or something along those lines.

the namespaces on the classpath unconditionally, regardless of their
modification status.

Adding a double prefix argument, kbd:[C-u C-u M-n r], will first
Adding a double prefix argument, kbd:[C-u C-u M-n r], it will first
clear the state of the namespace tracker before reloading. This is
useful for recovering from some classes of error that normal reloads
useful for recovering from some classes of errors that normal reloads
would otherwise not recover from. A good example is circular
dependencies. The trade-off is that stale code from any deleted files
may not be completely unloaded.

Adding a negative prefix argument will inhibit
the `cider-ns-refresh-before-fn` and `cider-ns-refresh-after-fn` functions from being run.

Invoked with a programmatic `clear-and-inhibit` argument,
it will both clear the namespace tracker before reloading (per the previous paragraph),
and inhibit the `cider-ns-refresh-before-fn` and `cider-ns-refresh-after-fn` functions from being run.

`cider-ns-refresh` wraps
https://github.com/clojure/tools.namespace[clojure.tools.namespace], and as
such the same
Expand Down
65 changes: 65 additions & 0 deletions test/cider-util-tests.el
Original file line number Diff line number Diff line change
Expand Up @@ -392,3 +392,68 @@ and some other vars (like clojure.core/filter).
(expect (cider-clojure-major-mode-p) :to-be-truthy)
(expect (cider-clojurescript-major-mode-p) :not :to-be-truthy)
(expect (cider-clojurec-major-mode-p) :to-be-truthy))))

(describe "cider--semantic-end-of-line"
(with-clojure-buffer "|(def foo)
bar
baz"
(expect (cider--semantic-end-of-line) :to-equal 10))

(with-clojure-buffer "(|def foo)
bar
baz"
(expect (cider--semantic-end-of-line) :to-equal 10))

(with-clojure-buffer "(de|f foo)
bar
baz"
(expect (cider--semantic-end-of-line) :to-equal 10))

(with-clojure-buffer "(def| foo)
bar
baz"
(expect (cider--semantic-end-of-line) :to-equal 10))

(with-clojure-buffer "(def |foo)
bar
baz"
(expect (cider--semantic-end-of-line) :to-equal 10))

(with-clojure-buffer "(def fo|o)
bar
baz"
(expect (cider--semantic-end-of-line) :to-equal 10))

(with-clojure-buffer "(def foo|)
bar
baz"
(expect (cider--semantic-end-of-line) :to-equal 10))

(with-clojure-buffer "(def foo)|
bar
baz"
(expect (cider--semantic-end-of-line) :to-equal 10))

(with-clojure-buffer "|(def

foo)
bar
baz"
(expect (cider--semantic-end-of-line) :to-equal 5))

(with-clojure-buffer "(def
|
foo)
bar
baz"
(expect (cider--semantic-end-of-line) :to-equal 6))

(with-clojure-buffer "(def foo)
bar|
baz"
(expect (cider--semantic-end-of-line) :to-equal 14))

(with-clojure-buffer "(def foo)
bar
baz|"
(expect (cider--semantic-end-of-line) :to-equal 18)))
Loading