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

Always match friendly sessions for special buffers #3432

Merged
merged 3 commits into from
Aug 28, 2023

Conversation

vemv
Copy link
Member

@vemv vemv commented Aug 25, 2023

*cider-error*, *cider-result*, and possibly others, always deserve to get an associated repl, if possible.

This normally already is the case, because their default-directory matches that of the current repl.

However, for some reason, occasionally their default-directory will be something like ~/.emacs.d which matches no repl's dir.

So this PR fixes that heisenbug.

But it also is an overall improvement, because conceptually, *cider-error* / *cider-result* belong to no project in particular, so if a repl is available, it should be considered 'friendly' to it.

Cheers - V

@vemv vemv requested a review from bbatsov August 25, 2023 04:21
cider-repl.el Outdated
@@ -1746,72 +1746,76 @@ constructs."
(mapconcat #'identity (cider-repl--available-shortcuts) ", "))))
(error "No command selected")))))

(defvar cider--project-agnostic-buffer-names
Copy link
Member

Choose a reason for hiding this comment

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

Just check that some of those names are not configurable.

Copy link
Member

Choose a reason for hiding this comment

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

Btw, it seems to me this is more or less cider-ancillary-buffers, so I guess you should use it instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great catch!

Pushed

@vemv
Copy link
Member Author

vemv commented Aug 25, 2023

I pushed some unrelated changes, please ignore for now

@vemv
Copy link
Member Author

vemv commented Aug 25, 2023

Reviewable again!

@bbatsov
Copy link
Member

bbatsov commented Aug 25, 2023

Some tests are failing (other than the Windows tests).

@vemv
Copy link
Member Author

vemv commented Aug 25, 2023

The tests pass again if I remove the second commit that observed cider-ancillary-buffers. We observe a hardcoded list again.

Perhaps, it had some nuance?

...For completeness, here's the removed commit: 7d52b87

@bbatsov
Copy link
Member

bbatsov commented Aug 25, 2023

Perhaps the list is not full at the time the test is being run? (you'll see that buffers add themselves there after their definitions) Maybe you can just set it to some explicit value for the purpose of the test?

Copy link
Member Author

@vemv vemv left a comment

Choose a reason for hiding this comment

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

Ready again!

@@ -60,6 +60,12 @@
(before-each
(setq sesman-sessions-hashmap (make-hash-table :test #'equal)
sesman-links-alist nil
cider-ancillary-buffers (seq-filter (lambda (s)
Copy link
Member Author

Choose a reason for hiding this comment

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

Adding this fixed the tests.

@bbatsov
Copy link
Member

bbatsov commented Aug 28, 2023

Looks good to me now.

@vemv vemv merged commit 05c8203 into master Aug 28, 2023
26 checks passed
@vemv vemv deleted the friendly-special-buffers branch August 28, 2023 18:13
@manuel-uberti
Copy link
Contributor

Just passing by to say "thank you!" for this. It happened only sporadically to me, and I've always thought it was a problem with my configuration, but I've never really sit down to debug it better

@vemv
Copy link
Member Author

vemv commented Aug 29, 2023

❤️🍻!

We now also have this troubleshooting page for session linking: https://docs.cider.mx/cider/troubleshooting.html#interactions-fail-with-no-linked-cider-sessions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants