Implement friendly sessions #2483
Conversation
|
Nice! Note that there may be some unfortunate intricacies with (non)absolute paths in the classpath here, and buffers whose file names aren't absolute. Spoken as someone who uses a lot of symlinks ;) |
Is this the case? I don't see simlinks in my classpaths. I am afraid running
This is taken care of. |
| (or (null host) | ||
| (equal host (plist-get ses-params :host)))))) | ||
| (sesman-linked-sessions 'CIDER '(project))))) | ||
| (sesman-current-sessions 'CIDER '(project))))) |
There was a problem hiding this comment.
Btw, shouldn't this be just sesman-sessions or something? Both linked and current seem misleading to me.
There was a problem hiding this comment.
There is already sesman-sessions which returns ALL sessions for a system. linked and current have slightly different semantics. Linked should be very clear given that linking is such a central concept in sesman; "current" conveys a general idea of "active" sessions in the current context. It basically means linked + friendly sessions.
There was a problem hiding this comment.
Yeah, I get this, but I feel that sesman-sessions should just return all sessions by default and narrow this down depending on optional params passed to it. Seems to me we've used relevant instead of current in CIDER, but I generally feel that in the present of the params it's pretty clear what sessions we're talking about without adding more functions to the API.
There was a problem hiding this comment.
I agree in general and in case of sesman-sessions this might actually work better. There is also sesman-links and sesman-current-links. Plus sesman-current-session which is the (car (sesman-current-sessions)). Let me see if I could simplify the api without making too many tradeoffs.
There was a problem hiding this comment.
I gave it a bit of thought and I am afraid the sesman-current-sessions and sesman-current-links will have to stay. The reason for the "current" instead of "relevant" is because those are sessions active in the current context and the sesman-current-session is the most relevant session for the current context. The latter is also how CIDER calls its most relevant repl. The reason for not subsuming sesman-current-session into sesman-sessions is because I would need to subsume sesman-current-links (which is a bit trickier) and the CIDER code would have to look like (sesman-sessions 'CIDER '(linked friendly) 'sort) which is rather annoying as all readers will have to learn about all those internals, while sesman-current-sessions is kinda self-explanatory.
It makes sense to subsume sesman-friendly-sessions and sesman-linked-sessions though. Those are somewhat internal. Looking into that right now.
cider-connection.el
Outdated
| (file-len (length file))) | ||
| (seq-find (lambda (path) | ||
| (when (>= file-len (length path)) | ||
| (string= path (substring file 0 (length path))))) |
There was a problem hiding this comment.
Can't you just simplify all of this using string-prefix-p or something along those lines?
There was a problem hiding this comment.
Ha. I didn't know about string-prefix-p. Will do.
| - <kbd>C-c C-s p</kbd> `sesman-link-with-project` | ||
| - <kbd>C-c C-s u</kbd> `sesman-unlink` | ||
|
|
||
| ## Friendly Sessions |
There was a problem hiding this comment.
I don't like much the friendly term, but I'm not sure what'd be better here. Did you entertain any other names for this?
There was a problem hiding this comment.
I thought of "external" after similar concept in project.el but I don't like it much. I think "friendly" is pretty suggestive and generic. There might be other notions of "friendliness" than dependency. If you have other ideas I am all ears.
And jars I guess. |
Yes. |
vspinu
force-pushed
the
friendly-sessions
branch
from
f082a17 to
783943d
Compare
|
Done! |
|
Thanks! |
Implement "friendly" sessions - sessions linked to dependent projects. Fix #2446, close #2464 and re-fix #2408 in a more elegant way.
The best way I could find to implement this is by searching through the classpath for the buffer file path. I have no estimation of how fast this is and whether it could cause performance issues. Remains to be seen. I am caching the classpath on the first use and haven't seen any issues in my experiments.
Regarding #2464, I will add at some point an option to sesman for picking first available session in non-linked contexts, but let's first see if this is really necessary for the CIDER's use case.
Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
make test)make lint) which is based on [elisp-lint]