Kill server with client

[Malabarba]

Whenever the nrepl hangs for one reason or another, you have to kill two different buffers each with its own process. This hook makes it easier to kill the server after killing the client.

[expez]

use => user

[Malabarba]

Done. Also moved the function above that block of comments, since the block is about the following function.

[expez]

This seems like it's duplicating the functionality of cider-quit? Are we doing a too poor job of promoting that function?

[Malabarba]

@expez This is for when the connection hangs. In those cases cider-quit will hang as well trying to close the connection.

[bbatsov]

Btw, reworking cider-quit is high on the todo list. I'm pretty sure most people are surprised that it kills all the connections, instead of only the current one. This makes cider-restart pretty confusing as well.

[bbatsov]

This seems like it's duplicating the functionality of cider-quit? Are we doing a too poor job of promoting that function?

@expez Some people will always try to kill just the current REPL, so this functionality makes sense for me. And as I said cider-quit's current behaviour is definitely not idea. We should definitely be able to kill individual connections and we either need to rename it cider-quit-all and add a different quit command named cider-quit or keep it as is and add a command cider-quit-connection (or something like this).

I still ask myself why this ended up in its current state - most likely because I generally work with only connection and this hasn't been much of an issue for me.

[Malabarba]

Some people will always try to kill just the current REPL, so this functionality makes sense for me.

Not to mention, killing the buffer is a traditionally a viable way of cancelling/stopping things in Emacs, even when a subprocess is involved. So it would be nice for Cider to try to handle that somewhat gracefully too.

We should definitely be able to kill individual connections and we either need to rename it cider-quit-all and add a different quit command named cider-quit or keep it as is and add a command cider-quit-connection (or something like this).

Or change the default behaviour to “quit current” and make the “quit all” behaviour available with a prefix key.

[Malabarba]

But anyway, killing the repl is something I do when it hangs (I suppose it happens more often than it should with me because of all the hacking around the debug middleware :), usually I just cider-quit.

[bbatsov]

Or change the default behaviour to “quit current” and make the “quit all” behaviour available with a prefix key.

Yep, that's a good suggest. Btw, something came to mind - we're killing the connection using a sentinel if the process that created the nrepl server died. Maybe we can use a sentinel for the REPL process as well for consistency. Not sure if we gain anything by this.

[Malabarba]

@bbatsov I had a look the code in nrepl-server-sentinel but I don't quite get something. In the following snippet, why does it close the connection on "hangup" but not on "killed"?

((string-match-p "^killed" event)
  nil)
((string-match-p "^hangup" event)
  (when connection-buffer
    (nrepl-close connection-buffer)))

[bbatsov]

Beats me. I see this was labeled a bug fix in the past https://github.com/clojure-emacs/cider/blob/master/CHANGELOG.md#bugs-fixed-14

Maybe the idea was to be able to kill the server buffer and recycle the REPL buffer or something like this. Guess we can find the PR related to this.

[bbatsov]

Seems to me this was a mistake - #160

[bbatsov]

Btw, in the mean time I've changed the behaviour of cider-quit and cider-restart.

[Malabarba]

I've added something to the client sentinel so that it kills the server process too when the client dies. The server buffer is only offered to be killed if the client buffer is killed. Does it look reasonable?

[bbatsov]

Looks legit. Just add a changelog entry for this.

[Malabarba]

Logged.

[bbatsov]

👍