Support multiple sessions

ciusy commented 7 years ago

It's would be great to support multiple sessions.

Right now, I found org-babel-eval-in-repl always runs in the same session, even if I specify a new session name in header argument, like for shell:

#+BEGIN_SRC sh :session session1
echo "session1"
#+END_SRC

#+BEGIN_SRC sh :session session2
echo "session2"
#+END_SRC

diadochos commented 7 years ago

Thank you for the suggestion. I will look into this, but I am not sure if eval-in-repl supports multiple sessions.

(If it doesn't, then we are at the edge to decide whether to implement an original way to send lines to REPL buffers selectively, or not to implement this. As for now, I am inclined to the latter, because otherwise this package will be by far useless as an extension to org-mode.)

diadochos commented 7 years ago

I've looked into this, but eval-in-repl doesn't seem to support multiple sessions yet.

We need to modify eval-in-repl itself to achieve this, and the changes required are language-specific. (fun-change-to-repl defined for each language need to be modified, I think). Yet, it is probably not difficult to implement. It's just that 10 different changes are needed for 10 different languages.

For now, I don't have the time to do that, but sending a PR to eval-in-repl to support multiple sessions will greatly help. After it's supported, I will immediately add multiple session support to this package as well.

Oh, I totally agree that this feature is needed, by the way.

actondev commented 5 years ago

hey @diadochos thanks for this package! when I stumbled upon this i thought it was everything I ever wanted but the lack of session was a bummer.

Wanted to let you know that I started working on modifying the eval-in-repl and this package itself for supporting sh sessions. I just needed sh sessions to setup my dev workflow :) I have a working thing actually, but since I'm a complete lisp newbie I spend some time trying out things still..

actondev commented 4 years ago

Hi @diadochos sorry it took me so long to get back. I've done the different sessions (for shell scripts) but I'd like your input about my aproach. This is my first real attempt to do anything in emacs-lisp. If you think it's fine I will proceed with the eval-in-repl PR.

Here are the changes https://github.com/diadochos/org-babel-eval-in-repl/compare/develop...actonDev:feature/multiple_sh_sessions?expand=1

And here are the changes for the eval in repl https://github.com/kaz-yos/eval-in-repl/compare/master...actonDev:feature%2Fmultiple_sh_sessions?diff=split&expand=1#diff-77108f8d11c4c47af045eb1e9173fd91L44

So essentially

I added a customizable eir-shell-buffer-name variable (defaulting to *shell*)
In org-mode, if a source block a :session header (eg :session *other-shell*), before executing the code it sets the eir-shell-buffer-name accordingly, runs the code, and reverts it to its previous value

Note: It also work if you set #+PROPERTY: header-args :session *dev-sh* at the top of an org document

To try it in action, you can use straight.el

(use-package eval-in-repl
  :straight
  (:type git :host github :repo "kaz-yos/eval-in-repl"
     :fork (:host github :repo "actondev/eval-in-repl" :branch "feature/multiple_sh_sessions"))
  )

(use-package org-babel-eval-in-repl
  :straight
  (:type git :host github :repo "diadochos/org-babel-eval-in-repl"
     :fork (:host github :repo "actondev/org-babel-eval-in-repl" :branch "feature/multiple_sh_sessions"))
  :bind
  (:map org-mode-map
    ("C-<return>" . ober-eval-in-repl)))

diadochos commented 4 years ago

Hi @actonDev , thank you very much for the code! Sorry for the late reply.

As for the PR to `org-babel-eval-in-repl`

The change looks good, but I have two suggestions:

Could you kindly turn the literals "*shell*" and "none" (inside ober-get-sh-session-name) into customizable variables (like you did in the modification for eval-in-repl pakcage) with these values being the defaults? I think that should help us in the future.
For backward compatibility, could you add an if clause to ober-eval-sh to check if eir-shell-buffer-name exists, and fall-back to just calling (eir-eval-in-shell) if it doesn't? This should help us avoid the error until the PR to eval-in-repl is accepted.

I would appreciate it very much if you could consider adding these two modifications and create a PR.

As for the PR to `eval-in-repl`

I don't have an answer to the question written as the comment (the regexp matching part). Except for that part, the code looks good to me. Maybe you want to consider sending a question issue to the package first, and then create a pull request?

As for testing the code

In fact, I am a little busy and do not have time to thoroughly test the behavior for now. So allow me to just trust you and accept your PR once you file it.

Again, thank you so much for all the effort and suggestions and your participation!

actondev commented 4 years ago

/ping @diadochos Ok, that's going slow but it will be done :)

so, I'm gonna open a PR to stay sane.
a pushed a fix. the new feature can work with the current version of eval-in-repl. But i'd say wait to merge it after eval-in-repl gets the fix. Cause they might want changes

Also, a Q: What do you mean with the turning the literals to variables? Some clarification:

the *shell* literal is used by the emacs whenever you open a shell.
the "none" is what org-mode returns when you don't have a :session configuration in the src block.

Having these in mind do you still consider setting them in custom variables?

Edit: see the other PR here

diadochos commented 4 years ago

Thanks a lot for your time!

wait to merge it after eval-in-repl gets the fix. Cause they might want changes

I see and agreed. Thank you so much for the fix.

What do you mean with the turning the literals to variables?

Thanks for the clarification. I meant setting those strings ("shell" and "none") in custom variables (like you correctly interpreted it).

Here are my answers to the question:

the "shell" literal

Given your new PR kaz-yos/eval-in-repl#37, I think the default shell name for kaz-yos/eval-in-repl is now configurable.
- (and there are reasons why one may want to change the default, e.g., if one uses shell-pop or some other libraries which use \*shell\*.)
I think it makes sense to prioritize a user-configured default over the Emacs default.
However, directly referencing eir-shell-buffer-name would make the packages too tightly connected, so making it a configurable variable is probably a better way.

What do you think?

the "none" literal

For this one, I think you're right. My first thought was that the behavior of org-mode may change and it may be easier to adapt to it when we have a configurable variable, but on a second thought with your clarification, I think it's better to not set it to a variable (since the value is unlikely to vary among different users).

Again, thanks for your work and let me hear what you think!

actondev commented 4 years ago

About the shell literal

In my view the usecase in this extension is

user running something for a src (sh) block without a custom :session
.. with a custom :session

Now, it indeed would make sense to have a variable like ober-default-sh-buffer-name to handle the 1st case.
Would something be needed to override the buffer name in 2nd case? (not to use the one of :session). IMO : I don't think so

So, if we are on the same page (and I indeed understood the usecase you have in mind) - and agree - I will go on and implement what I described here.

Waiting for your ok, or any other input.

PS: a last idea The ober-default-sh-buffer-name could as well be a.. function? In a use case that you run a block and you'd want a dialog to popup and ask you in which shell to run the code. But I think that goes a little far from my current knowledge, and wouldn't be up for implementing this - not right now at least.

diadochos commented 4 years ago

Thanks a lot! I think we are on the same page and I agree with you.

last idea

I think your implementation already serves the switching purpose unless one wants a frequent switch among different sessions (which, in my opinion, is unlikely).

actondev commented 4 years ago

Aloha. I committed yesterday that last thing and now.. we just wait for the https://github.com/kaz-yos/eval-in-repl/pull/37 :)

kaz-yos commented 4 years ago

Thanks for your cool work, @diadochos and @actonDev! I'm leaving this comment to get notifications. We are working out the details of https://github.com/kaz-yos/eval-in-repl/pull/37 (@actonDev) along with an older PR https://github.com/kaz-yos/eval-in-repl/pull/34 (@terlar), which I completely missed.

actondev commented 4 years ago

@diadochos we're set! eval-in-repl PR is merged :) Thank you @kaz-yos

diadochos commented 4 years ago

Cool! I'll merge your PR (#25) now. Thank you for your brilliant work :D

actondev commented 4 years ago

Well thank YOU for the extension! I think it will be really useful for stuff live devops etc (was my main motivation/idea to add this) A future thing would be to add some gifs in this repo's readme (which are always nice to have - in any extension's readme) or even some video presentation for how it could be used for devops.

Check that gif https://user-images.githubusercontent.com/1299928/68528644-d6ab1200-02f5-11ea-8bf9-9997b3755429.gif if you think it could serve for the readme (I think a smaller gif, and with less clutter should be better though)

Anyways, enjoy the weekend

diadochos commented 4 years ago

@actonDev Thanks! The GIF is super cool! (I was going to ask about it when I first saw this cool demo in the issue for kaz-yos/eval-in-repl) As long as you are happy, how about we first add your gif to the readme (in the example section) and then see if we come up with a better demonstration later? (Personally, I think your gif is already straight to the point and excellent)

diadochos / org-babel-eval-in-repl