cider-switch-to-relevant-repl-buffer tramples cider-switch-to-repl-buffer

[jonpither]

Some issues around cider-switch-to-relevant-repl-buffer and cider-switch-to-repl-buffer.

This is around #361, where @vedang split cider-switch-to-relevant-repl-buffer out of cider-switch-to-repl-buffer.

1. In my setup, C-c C-z still calls cider-switch-to-relevant-repl-buffer, even though it's mapped to C-c C-Z. A little research makes me think both C-c C-Z and C-c C-z cannot co-exist mapped to different actions. Users might now be using the "relevance" feature without realising it's the unintended function.

2. There is no mention of C-c C-Z in the README.

3. In the readme I think C-u C-u C-c C-z is wrong. C-u C-c C-Z does what is described, and now there is no way of doing both selecting a project-dir via IDO and switching to the NS based on the current NS. This may not be a big deal but the docstrings are incorrect.

4. The if statement in cider-switch-to-relevant-repl-buffer now has 3 clauses. It seems to work still but I'm not entirely sure why.

[jonpither]

C-c C-z in cider-repl-mode is wrong in the README, it switches user to the file of where the repl was instantiated. Pretty funky, but undocumented.

C-u C-u C-c C-z should not be there at all in the cider-repl-mode part of README.

[jonpither]

Actually, C-c C-z for cider-repl-mode is partially correct. It does "select the last Clojure buffer" which matches the docstring for cider-switch-to-last-clojure-buffer.

However it's wrong in that C-u C-c C-z will "switch the Clojure buffer to the namespace in the current buffer." That doesn't make sense.

I'd say also that "Select the last Clojure buffer. " is just too vague.

[bbatsov]

Yep, the docstrings seem to need some improving.

[jonpither]

Yup, though my main point in this issue is what exactly does C-c C-z do?

[bbatsov]

Well, it seems that we can't use C-c C-Z and C-c C-z together indeed and that's the root of this problem.

[jonpither]

Are you sure about the "relevance" behavour being split out of the standard C-c C-z that used to be there?

In general I'd be interested to know what feedback there is for what features ppl use..

[bbatsov]

AFAIR the relevance behaviour wasn't working properly if someone was connecting to a remote nREPL instance.

[jonpither]

It wasn't that it didn't work properly using a remote nREPL instance, more that the functionality wasn't desirable according to @vedang in #361 (see his initial comment in the issue).

Options as I see them:

1. Rebind "cider-switch-to-relevant-repl-buffer" to a new hotkey and clean up the docs. This is basically continuing the work in Fix `nrepl-switch-to-repl-buffer' to work with all connections #361.

2. Reverse the split and make the "relevant repl buffer" behavour a configurable option than you can toggle.

Just occurred to me that we could update the relevance behavour to output a message making it clear what repl has been switched into.

[vedang]

To add my 2 cents, I would prefer the two functions to remain separate and have separate key bindings. The default behavior, mapped to C-c C-z, should be cider-switch-to-repl-buffer IMHO, but I'm okay with it either way. We can consider using the original key binding I had proposed (C-c C-v along the lines of find alternate file) for the other function.

[bbatsov]

I refreshed my knowledge of the issue and I think a single command might be more sensible provided it features some safeguards when there is ambiguity as to which REPL to select (maybe it could prompt the user to manually select the proper REPL buffer). Having a message displayed when the REPL buffer was changed would be a nice improvement as well. Btw, those changes would be needed even if we decide to keep around the two separate commands.

That said - I'm also OK with having two separate commands. The downside is this approach would be only that fewer people would understand about the presence of this behaviour.

[jonpither]

It's good we're rethinking this.

I'm concerned with having 2 commands as it makes using CIDER more complicated than it need be. We've had a couple of warnings in that the docs couldn't keep up and we didn't notice that one of the commands was inaccessible via a keybinding.

If we decide we can't merge the two commands another option is to drop the 'relevance' behavour or to move it out as an extension.

I'm happy to do some more work on this as I don't want to leave it in a messy state, but it needs a consensus first.

Digging into @bbatsov's comment around providing "safeguards when there is ambiguity as to which REPL to select" - Yes we can do this. To be clear, as soon as you open up a remote nREPL session there will be ambiguity, as you can't tie the remote session to a project in a directory structure.

Having a communicative safeguard that doesn't attempt any cleverness in this situation might be sensible, and might alleviate @vedang's concern?

[bbatsov]

I guess it will. I should mention that people have been using switch-to-relevant-repl for a few months now (because of the keybinding bug) and I got no complaints, so it seems it's not causing any significant problems for anyone. Improving upon it seems like a logical step.

[jonpither]

OK. Unless anyone objects I'll begin a pull request based on this conversation. Once my new year hangover has abated that is.

[vedang]

@bbatsov the code has been working because of a check on L193 in cider-interaction.el, which was introduced in PR #361. No one seems to notice or care for the error message "No relevant repl found" which is printed to the minibuffer in this case, since the code works as expected by the user. I guess I'll call this a happy accident

@jonpither looking at the code for cider-switch-to-relevant-repl-buffer, modifying the error messages / changing the code slightly might be the only change required. We can then do away with the other function (cider-switch-to-repl-buffer). As you've already mentioned, the doc strings might need to be updated.

[vedang]

To clarify further, ^ this is working because people generally don't tend to mix cider-jack-in and cider. For example, I always connect with M-x cider. I just reproduced the bug reported in the #361 (comment) by connecting to a remote repl using M-x cider and a local repl for the same project using M-x cider-jack-in