sanel / monroe

Clojure nREPL client for Emacs
162 stars 21 forks source link

Support for multiple connections #16

Open technomancy opened 7 years ago

technomancy commented 7 years ago

It seems like right now monroe only supports connecting to a single nrepl instance at a time.

Are you interested in adding support for more than one active *monroe* buffer, or is it better to use separate Emacs instances for that?

sanel commented 7 years ago

I was planning to add it (made a few attempts), just didn't have enough time to complete it. However, if you are able to do it, keeping the code simple, I'll be happy to accept the patch :)

technomancy commented 7 years ago

Good to know; thanks. No promises, but I might take a shot at this at some point.

bbuccianti commented 5 years ago

I'm thinking of making an attempt too. Maybe we can discuss a little about how to do it?

sanel commented 5 years ago

Feel free @bbuccianti :) Maybe check inferior-lisp mode (inf-lisp.el in Emacs) for multiple processes support.

sogaiu commented 5 years ago

@bbuccianti

For reference, if you want to see an existing attempt for a socket repl-based thing with multiple connections, I've had some luck w/:

It's been working pretty well for me so far.

I started with sesman, but decided it was much more than I needed.

bbuccianti commented 5 years ago

@sanel @technomancy @sogaiu

Can you help me test this?

https://github.com/bbuccianti/monroe/

I'm sure there is a lot of ways to improve it, please help me!

sogaiu commented 5 years ago

Do you mean the multiple-connections branch?

https://github.com/bbuccianti/monroe/tree/multiple-connections

bbuccianti commented 5 years ago

This is the correct commit:

https://github.com/bbuccianti/monroe/commit/38a1488f54596ccdb35769c83ce0def2380b233c

sogaiu commented 5 years ago

Had success interacting with 2 separate processes from within a single emacs instance.

Opened two files -- each one from a different project -- ran M-x monroe in each and tried M-x monroe-eval-region. Seems to work :+1:

IIUC, there is at most one connection per project and at most one process per project. Is that correct?

bbuccianti commented 5 years ago

Yes, it's using the project folder as a pointer concatenating it with monroe and monroe-conn buffers.

I've created a pull request #33

sanel commented 5 years ago

Thanks @bbuccianti for your work and @sogaiu for fast response!

Is #33 will add support for multiple connections? Title of PR confuses me :)

Also, does it handle reading .nrepl-port from project tree?

bbuccianti commented 5 years ago

It still uses monroe-locate-port-file.

The only change is that instead of naming the buffers *monroe* and *monroe-conn* I conj the project folder to it. Then, when you try to send code to the nrepl it searches for the correct buffers using the project folder.

bbuccianti commented 5 years ago

@sanel I just improved the title of the PR

sanel commented 5 years ago

Thanks!

Here are few issues I found (my Emacs is 26.3).

After first expression is entered (in single or multiple REPL connections), prompt is not shown any more.

Next issue is connecting to REPLs outside of working directories. For example, you run REPL at localhost:7888 in project foo, then in external terminal run REPL in project baz at localhost:7889, but keep Emacs in project foo, it will connect to foo for both REPL and inputs and outputs will be shared.

The same applies if you connect Emacs to any multiple projects without entering to project folders.

Since you are calculating process name from working directory, I'd suggest to use combination of host+port instead, because they have to be unique on given system.

Also, I'd suggest to pop out exising monroe connection REPL if user attempt to connect to that session again. This will prevent multiple REPLs that will echo inputs and outputs between each other.

bbuccianti commented 5 years ago

I can't reproduce the issue with prompt. Maybe I'm evaluating trivial things, but seems to work fine for me.

https://github.com/bbuccianti/monroe/commit/05b37d975da409210762a343b131e9ccb41b9ab6

Now it's using the host:port combination to name buffers and also call (monroe-disconnect) before trying to attempt another connection.

Let me know!

sanel commented 4 years ago

Thanks @bbuccianti for your work so far! However, I still have issue with prompt. What Emacs are you using?

Here is how to reproduce it:

  1. Run lein repl somewhere.
  2. In new Emacs session, run monroe, it will ask for connection.
  3. After you connect, you will see prompt. Enter something like (+ 1 2 3) in REPL. It will print result, but prompt is no longer visible.

Here is how it looks for me:

user=> (+ 1 2 3)                                                                                                                                                                                                    
6
;; no prompt here                                                                                                                                                                                                                   
(println "abc")                                                                                                                                                                                                     
abc
;; no prompt here
bbuccianti commented 4 years ago

Are you using this? https://github.com/sanel/monroe/pull/33

I'm currently using GNU Emacs 26.3 (build 1)

sanel commented 4 years ago

Yes, using PR #33 and same Emacs version. Tried even with emacs -Q.

Can you try loading PR with emacs -Q please?

bbuccianti commented 4 years ago

Well, that works for reproducing the issue. Any suggestion on how to fix it?

sanel commented 4 years ago

I'm curious how it worked for you - maybe some package "fixed" that :) This happened before for me and I believe it is up to comint-mode now as well (comint can break in weird ways if you messed up options or processes). I'll check this one.

bbuccianti commented 4 years ago

I've tested with commit https://github.com/sanel/monroe/commit/2f472fdc09c1b36c291ddb5ed9aecc331fd7e082 and it's not working in emacs -q !

sanel commented 4 years ago

Hm... does it work for you if you revert to this: https://github.com/sanel/monroe/commit/a7cf37e6e29a1e31d9a6e9ce19d5324eca09465a ?

bbuccianti commented 4 years ago

Yeah! Can confirm that commit a7cf37e it's working as expected.

Also I want to tell that seems like a callback done with monroe-make-response-handler isn't running with the "done" status. Then monroe-requests isn't cleaned so the prompt is never added again.

But is really strange, because status is clearly truthy but can't go inside the (when status ;cleaning) code. I'm really new to those kind of callbacks and don't know really how monroe-dbind-response is doing is magic, but I'm learning a lot. Would be great if you @sanel can give some advice in how to debug that part.

Thanks!

bbuccianti commented 4 years ago

@sanel no advances on this?

sanel commented 4 years ago

Hey Benjamín,

sadly I'm a bit busy currently :S If you have time to take a look at it, I'll be happy to review it.

bbuccianti commented 4 years ago

I force-pushed to #33

Thanks @sanel

sanel commented 4 years ago

Thanks @bbuccianti ! Will take a look :)

sanel commented 4 years ago

Implemented with https://github.com/sanel/monroe/pull/33

sanel commented 4 years ago

Sorry @bbuccianti but this still doesn't work. To demonstrate:

Worse, with multiple connections, it mess up where evaluation should go. To reproduce:

How you manage to get this working?

sanel commented 4 years ago

@technomancy after https://github.com/sanel/monroe/commit/1c2559b6862e1d4f8780d370973ee63b383b6122 prompt is no longer shown when you enter something like + in fresh monroe session. What is reasoning behind this change?

Does it work for you?

bbuccianti commented 4 years ago

I think it's my error. I assumed that you would open a monroe session from a project folder. So it's searching for the .nrepl-port file in order to get the correct buffer to send eval string to the correct process.

That's why connecting from another places it's screwing everything. I don't know how to solve that problem (yet), I need to think more.

Maybe we can add a prompt in order to ask for the project folder when it doesn't find that .nrepl-port file?

bbuccianti commented 4 years ago

I'm testing b540e13 in Emacs 28.0.5 and seems to resolve ok different connections, without the need of .nrepl.-port. I read now that it tries to get port from the .nrepl-port file, if it doesn't find one it is extracted from the buffer. That's way the buffer now it's called "monroe: %s" being %s the host:port.

Maybe it's an issue with Emacs 26.3 version? But I think @technomancy was using that version of 25, I don't remember exactly.

sanel commented 4 years ago

I had the same issue with 26.1 I think... we must support older versions as well, including currently stable one :)

Now, I haven't dig deeper into the code, but the strategy of reading ports should be different IMO. When new connection is requested, it should only read .nrepl-port file to suggest host and port (as was doing before). After that, it should encode those inside buffer name.

I'm curious, how clojure-mode buffer (buffer where clojure code is) knows to which monroe repl to send code?

technomancy commented 4 years ago

@sanel re: 1c2559b the old logic was only to print the prompt when a message was received that did not contain certain keys in it; however, IMO this was taking advantage of an implementation detail in the reference implementation of the Clojure nrepl server.

There is nothing inherent in "a message with no output" that guarantees it is equal to "the input you sent is done being evaluated"; it's just an accidental property that the one nrepl server happens to have, and other servers do not have. In that commit I made it based on whether there were any outstanding requests in the monroe-requests table, but that's no longer a good criteria now that multiple connections are possible.

Probably what it should have been based on from the start is when a message is received where the status field contains "done".

bbuccianti commented 4 years ago

It's still the same approach. It reads from .nrepl-port or you can provide host:port or :port using localhost as default. Nothing changes there.

What changes is the name of the monroe buffer. Before it was monroe. Now with this change we create a buffer as monroe: host:port. And for the connection it's similar. It creates a monroe-connection: host:port.

In this way then, to return the correct buffer we use monroe-repl-buffer function. If you found .nrepl-host, it returns the monroe: host:port buffer, otherwise it's assuming you are in the monroe buffer and it extracts host:port from the buffer name.

bbuccianti commented 4 years ago

I see an error that I can't fix. I can show you the output of both versions of Emacs 26 and 27, reactioning to the same input.

decoded: ((dict (id . 1) (ns . user) (session . a54cd27f-152b-4b2e-aa87-384597d4486b) (value . 5)) (dict (id . 1) (session . a54cd27f-152b-4b2e-aa87-384597d4486b) (status done)))
net-filter response: (dict (id . 1) (ns . user) (session . a54cd27f-152b-4b2e-aa87-384597d4486b) (value . 5))
response: (dict (id . 1) (ns . user) (session . a54cd27f-152b-4b2e-aa87-384597d4486b) (value . 5))
net-filter response: (dict (id . 1) (session . a54cd27f-152b-4b2e-aa87-384597d4486b) (status done))
response: (dict (id . 1) (session . a54cd27f-152b-4b2e-aa87-384597d4486b) (status done))
GNU Emacs 27.0.91 (build 1, x86_64-pc-linux-musl, X toolkit, cairo version 1.16.0) of 2020-07-03
Monroe connected.
decoded: ((dict (id . 1) (ns . user) (session . fec70c4b-59a9-4258-a884-c6e270720ae3) (value . 5)) (dict (id . 1) (session . fec70c4b-59a9-4258-a884-c6e270720ae3) (status done)))
net-filter response: (dict (id . 1) (ns . user) (session . fec70c4b-59a9-4258-a884-c6e270720ae3) (value . 5))
response: (dict (id . 1) (ns . user) (session . fec70c4b-59a9-4258-a884-c6e270720ae3) (value . 5))
GNU Emacs 26.3 (build 1, x86_64-unknown-linux-musl) of 2019-08-29

I'm trying to investigate why net-filter isn't called again.

bbuccianti commented 4 years ago

I think I have a fix in #34