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.

Sign up for GitHub

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

Jump to bottom

Bugfix/fix cider selector problem #2711

Merged
merged 5 commits into from
Sep 21, 2019
Merged

Bugfix/fix cider selector problem #2711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd6ef9d
Add note about the problem
ahungry Sep 13, 2019
e8ab9e7
Adjusts cider-selector handling with the following benefits:
ahungry Sep 14, 2019
7b2966e
Further tuning of the selector handling
ahungry Sep 14, 2019
7e4de5c
Update docstring
ahungry Sep 15, 2019
9bc6c89
Merge branch 'master' into bugfix/Fix-cider-selector-problem
bbatsov Sep 21, 2019
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

### Changes

* `cider-selector` has more robust handling for edge cases.

### Bugs fixed

* [#2715](https://github.com/clojure-emacs/cider/issues/2715): Fix the `shadow-cljs` presence check.
Expand Down
22 changes: 15 additions & 7 deletions cider-selector.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,23 @@ DESCRIPTION is a one-line description of what the key selects.")
Not meant to be set by users. It's used internally
by `cider-selector'.")

(defun cider-selector--recently-visited-buffer (mode)
(defun cider-selector--recently-visited-buffer (mode &optional consider-visible-p)
"Return the most recently visited buffer, deriving its `major-mode' from MODE.
Only considers buffers that are not already visible."
CONSIDER-VISIBLE-P will allow handling of visible windows as well.
First pass only considers buffers that are not already visible.
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 we can probably simplify the logic and consider all buffers right away. I've mostly copied this code from SLIME back in the day and I didn't think much about it. Treating differently hidden/visible buffers seems weird to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had thought about that, and on one of the commits actually implemented as such, but that had a side-effect of removing the ability to use cider-selector to flip-flop between the latest 2 buffers of the same type (to change from a clojure-mode buffer to the last visited but not shown clojure-mode buffer - not using the second pass would change this operation into a noop essentially).

Copy link
Member

Choose a reason for hiding this comment

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

Got it. Well, I guess it really depends on what do we consider the purpose of this command to be - for me it was always to jump to a source buffer from some special buffer and vice versa. Of course, there's also the question of how people are actually using something and what do they consider useful. :D

That's why I was thinking we should extend the documentation of cider-selector in general so people would know how it's supposed to be used.

Second pass will attempt one of visible ones for scenarios where the window
is visible, but not focused."
(cl-loop for buffer in (buffer-list)
when (and (with-current-buffer buffer
(derived-mode-p mode))
;; names starting with space are considered hidden by Emacs
(not (string-match-p "^ " (buffer-name buffer)))
(null (get-buffer-window buffer 'visible)))
(or consider-visible-p
(null (get-buffer-window buffer 'visible))))
return buffer
finally (error "Can't find unshown buffer in %S" mode)))
finally (if consider-visible-p
(error "Can't find unshown buffer in %S" mode)
(cider-selector--recently-visited-buffer mode t))))

;;;###autoload
(defun cider-selector (&optional other-window)
Expand Down Expand Up @@ -97,7 +103,7 @@ is chosen. The returned buffer is selected with
`switch-to-buffer'."
(let ((method `(lambda ()
(let ((buffer (progn ,@body)))
(cond ((not (get-buffer buffer))
(cond ((not (and buffer (get-buffer buffer)))
(message "No such buffer: %S" buffer)
(ding))
((get-buffer-window buffer)
Expand Down Expand Up @@ -138,8 +144,10 @@ is chosen. The returned buffer is selected with
(top-level))

(def-cider-selector-method ?r
"Current REPL buffer."
(cider-current-repl))
"Current REPL buffer or as a fallback, the most recently
visited cider-repl-mode buffer."
(or (cider-current-repl)
Copy link
Member

Choose a reason for hiding this comment

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

The docstring probably needs to be adjusted, as now this does something different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

(cider-selector--recently-visited-buffer 'cider-repl-mode)))

(def-cider-selector-method ?m
"Current connection's *nrepl-messages* buffer."
Expand Down