cider icon indicating copy to clipboard operation
cider copied to clipboard

Sesman linking not respected

Open lgrapenthin opened this issue 3 years ago • 13 comments

Expected behavior

There was a time when I could have both a CLJ & CLJS REPL of one project at the same time, and when I'd eval something, Cider would know which REPL to use. These times are long over and forgotten. In recent versions, I was able to M-x sesman-browse and via l d associate project subdirs with the desired REPL's. This doesn't work since "Plovdiv". I am aware of https://github.com/clojure-emacs/cider/issues/2946, but since that is a messy ticket discussing multiple solutions, I decided to open a new one specifically about the sesman linking.

Actual behavior

It doesn't work. Every second time I want to go to definition or something it will just not do it. I have to manually switch to the correct REPL and then switch back to the file buffer, and then it somehow will use this REPL.

Suggested behavior

Please just bring back the very simple system that would just associate all .clj files with the next CLJ repl it finds and all .cljs files with the next CLJS repl it finds. It would be more than good enough, especially compared to recent iterations on this.

Steps to reproduce the problem

Update Cider :(

Environment & Version information

CIDER version information

CIDER 1.1.1 (Plovdiv) [2 times]

Include here the version string displayed when CIDER's REPL is launched. Here's an example:

;; CIDER 0.12.0snapshot (package: 20160331.421), nREPL 0.2.12
;; Clojure 1.8.0, Java 1.8.0_31

Lein/Boot version

E.g. Lein 2.6.1 (skip this when you didn't start the nREPL server using Lein or Boot)

Emacs version

26.3

E.g. 24.5 (use M-x emacs-version to check it if unsure)

Operating system

E.g. Fedora 23, OS X 10.11 "El Capitan", Windows 10, etc

lgrapenthin avatar Dec 07 '21 10:12 lgrapenthin

I hope that @vspinu will have some time to look into this. Generally once we're done with CIDER 1.2 I plan to dedicate more time to figuring out what to do with Sesman (fix it or remove it).

bbatsov avatar Dec 07 '21 10:12 bbatsov

@bbatsov I tracked down the issue: cider-repls in cider-connection.el calls sesman-current-session, but should call sesman-current-sessions.

lgrapenthin avatar Dec 07 '21 14:12 lgrapenthin

One session is a wrapper around multiple related REPLs, so it seems reasonable to be using sesman-current-session. I might be missing something, though. PR welcome!

bbatsov avatar Dec 07 '21 15:12 bbatsov

Sesman hasn't been touched since the completion. It was working all as expected when I integrated it in CIDER and probably for much longer after I stopped developing in Clojure/CIDER a few year ago. Cljs/CLJS selection is rather basic, so if that does no longer work, then it probably means something is badly broken in CIDER proper.

I am afraid I probably won't be able to allocate time to look into CIDER/Sessman integration again, so that's unfortunately is entirely on @bbatsov but I can look into improving Sesman if that's needed. Though I am quite skeptical regarding of the possible improvements to the core logic there. The level of abstraction and generality of Sesman is very hard to beat.

vspinu avatar Dec 08 '21 11:12 vspinu

@vspinu The one thing about which most people are complaining about is that you can't just select one session to be used for all evaluation regardless of the context. That's particularly important if you have just a single session and you don't really care about the project context - you just want every eval to get routed to a connection in this session. If we solve this problem I think the majority of people will be reasonably happy with the feature set.

bbatsov avatar Dec 08 '21 11:12 bbatsov

Do you mean that regardless of the project, or directory people want to send everything into one default session? If there is a session for one project and the user wants to evaluate code from another unrelated project it's most likely a mistake. No? And in any case it should be one short key away C-c C-s l.

There is a notion of friendly session which for CIDER means that current directory is in a classpath if I recall correctly. Without knowing the details my hunch is that the issue might be the too simple logic in sesman-friendly-session-p. From what I see it's geared towards java classpaths. I bet it does not detect cljs sessions correctly in all situations.

In an ideal world when that function does that it is supposed to do - to test for relevance of a session to the current buffer - do you still think people would have issues with it? In those hopefully rare cases when the "friendleness" cannot be established or does not make sense in general, one can always do C-c C-s l once per REPL session. Not a big deal right? To me it seems a good tradeoff.

sesman-friendly-session-p method can be overwritten by the user to always return t. Then every session is friendly in all buffers. This would defeat the purpose of session management - everything would be in the same bubble. So I would not like people starting doing that as a workaround. Let's fix the root issue and improve the method. It cannot be done on Sesman side because Sesman does not know anything about the semantic relationship between CIDER session and the current buffer.

That being said, I would be more than happy to fix all session management issues on CIDER side. The emacs/CIDER hacking is not an issue. For me the problem is the startup cost. I am so out of clojure(script) hacking that figuring the setup with all that new stuff that came along or changed in the meanwhile (shadow cljs, boot, nrepl etc) would require a non-trivial time. If someone could provide a step-by-step tutorial of how to setup a simple but a complete and realistic multi-project-multi-session-muti-backend environment, I could take care of all the session management issues that arise.

vspinu avatar Dec 08 '21 18:12 vspinu

There was an older version that had a cider-rotate-connection. It would just switch through the REPLs. That would be good enough.

Manually linking all the dirs with sesman everytime I open a two-REPL project is also no fun, but tolerable.

Regarding my earlier analysis it appears that you are correct in that the right function is called. However, for some reason, sesman doesn't return the correct session, even though all dirs were linked. I debugged it and it seems to obtain paths of buffers correctly, but somehow in the end spits out the wrong result.

Surprisingly though there is code in Cider that filters for the correct REPL (CLJ/CLJS/CLJC) and if I replace the aforementioned call to sesman so that all sesman sessions are returned, it works, thanks to Ciders code. So this works even without sesmans dir linking. So thats what I'm currently doing.

A

lgrapenthin avatar Dec 08 '21 18:12 lgrapenthin

all the dirs with sesman everytime I open a two-REPL project is also no fun, but tolerable.

This step should be rarely necessary if two REPLs are discinct (cljs & clj). All buffers within a project should be automatically linked unless the directory is outside of the project. And even then it should be working automatically if the directory is in the classpath (sesman-friendly-session-p point above).

With two same-type REPLS (each in its own session; e.g. clj + clj) in the same project one would need to explicitly link with directories (obviously cannot be disambiguated from the context alone). In the absence of en explicit link, the most recent REPL is used if I recall correctly.

vspinu avatar Dec 08 '21 19:12 vspinu

all the dirs with sesman everytime I open a two-REPL project is also no fun, but tolerable.

This step should be rarely necessary if two REPLs are discinct (cljs & clj). All buffers within a project should be automatically linked unless the directory is outside of the project. And even then it should be working automatically if the directory is in the classpath (sesman-friendly-session-p point above).

With two same-type REPLS (each in its own session; e.g. clj + clj) in the same project one would need to explicitly link with directories (obviously cannot be disambiguated from the context alone). In the absence of en explicit link, the most recent REPL is used if I recall correctly.

I happen to have two distinct REPLs in the same project CLJ/CLJS. I don't start them with Cider though, just cider-connect. Cider recognizes them as CLJ or CLJS respectively. However even before the current cider version I had to do the sesman dir linking manually, so the automation you are describing isn't working. I'm using pretty bare Emacs, so I'd be surprised to be the only one with this issue...

lgrapenthin avatar Dec 08 '21 21:12 lgrapenthin

@lgrapenthin

Are the connection remote? If so there might be a problem there regarding linking. In general it's not possible to figure out from context what local context to link. Though some heuristics might be useful like auto-linking with the current directory or something. In any case confusion between CLJS and CLJ that you describe is unrelated to sesman project/dir linking. It's a lower level concept internal to CIDER.

Can you describe in more detail your minimal setup and what you do step by step? I will try to reproduce.

vspinu avatar Dec 09 '21 16:12 vspinu

@vspinu This is my setup: https://github.com/leon-computer/basis There are some instructions in the readme. You can try cloning it, starting it, and connecting nrepl into both Clj & Cljs and try if Emacs will find the right REPL depending on which file you are in.

lgrapenthin avatar Dec 22 '21 04:12 lgrapenthin

I can confirm this behavior and I too have the experience of this previously working. Not respecting a manual interaction of sesman-link-with-buffer to fix the automagic is quite annoying.

bennyandresen avatar Apr 18 '22 15:04 bennyandresen

I have just tried on version 1.4.0. Unfortunately, the problem did not go away.

lgrapenthin avatar May 17 '22 09:05 lgrapenthin

We have delivered a substantial number of Sesman- and cljs-specific fixes to CIDER over the last few months.

These are available on CIDER master (not on a stable release yet - it is around the corner).

I've used cljs with these fixes very intensively. I can happily say it all works as intended.

Please just bring back the very simple system that would just associate all .clj files with the next CLJ repl it finds and all .cljs files with the next CLJS repl it finds.

We were also very tempted to do this, however when Sesman<->CIDER integration works as intended, with multiple bugs fixed, then it's no longer necessary to think of workarounds.

In particular, those workarounds could easily bring new issues, that could be harder for users to understand and for us to debug.

The final state we want is simple: spawn one or N repls per project, and only allow evaluation on projects with a repl.

That's much more structured and deterministic than simply looking up a recent or random repl.

Custom workflows can be supported with manual linking.

Cheers - V

vemv avatar Sep 11 '23 13:09 vemv

@vemv I have just upgraded to Cider 1.8.3 and could not notice an improvement. When I switch back from a CLJS file buffer to a CLJ file buffer or vice versa, the file buffer is not connected to a REPL anymore.

Is there at least a binding or command with which I can switch through REPL connections for a file buffer as long as this feature is still being worked on? Since it seems very hard to get it right in the way you envision, it would be nice to have some "quick and dirty" solution at least in the meantime.

The only workaround I could find is to manually switch to the REPL buffer and than back to the file.

When I open sesman-browser, it looks like this

  1: projects/myproj:localhost:43263
      linked-to: proj(~/Documents/projects/myproj/)  
        objects: *cider-repl %s(clj)*  

  2: projects/myproj:localhost:7888
      linked-to: proj(~/Documents/projects/myproj/)  
        objects: *cider-repl %s(cljs)*  

lgrapenthin avatar Oct 20 '23 20:10 lgrapenthin

I'll take a look tomorrow although I've worked with clj/s projects all summer... I also have been working closely with a variety of users, no complaints anymore in this area.

vemv avatar Oct 20 '23 20:10 vemv

Actually, please create a new one - old threads are excessively noisy/taxing.

Also, please only share your problem (accurately, reproducibly, from scratch) - remarks like Since it seems very hard to get it right in the way you envision do not help much.

vemv avatar Oct 20 '23 20:10 vemv