Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

org-babel-load-session:prolog has unnecessary parameters #15

Closed
lafrenierejm opened this issue Mar 27, 2018 · 7 comments
Closed

org-babel-load-session:prolog has unnecessary parameters #15

lafrenierejm opened this issue Mar 27, 2018 · 7 comments

Comments

@lafrenierejm
Copy link
Contributor

lafrenierejm commented Mar 27, 2018

The function definition of org-babel-load-session:prolog has unnecessary BODY and PARAMS parameters.

body is never used in the function's body. The only thing params is used for in the function is a let* binding of params; that new, lexically-scoped params is never used.

@lafrenierejm lafrenierejm changed the title org-babel-load-session:prolog has unnecessary PARAMS parameter org-babel-load-session:prolog has unnecessary parameters Mar 28, 2018
@ljos
Copy link
Owner

ljos commented Mar 28, 2018

This is because org-babel-load-in-session uses it like that. I think I just copied this from somewhere when I was learning how ob-babel worked. I can see now that org-babel-load-session:prolog is not doing everything it is supposed to. It doesn't load body into the session as the documentation for load-in-session says. We should fix this.

@lafrenierejm
Copy link
Contributor Author

Can you elaborate on your understanding of what org-babel-load-in-session is supposed to do?

Neither micanzhang/ob-rust nor krisajenkins/ob-translate (commits referenced are those used by MELPA as of this posting) implement org-babel-load-in-session. The built-in emacs-lisp babel also appears to not implement the function.

@ljos
Copy link
Owner

ljos commented Mar 28, 2018

I have to investigate when it is used. It looks to me that org-babel-load-in-session uses it to load the source block with the variables into the current session and then switches to a window with that buffer in it.

@ljos
Copy link
Owner

ljos commented Mar 28, 2018

org-babel-load-in-session doesn't seem to be implemented by a lot of modes, but it seems org-babel-load-in-session is bound to C-l.

@ljos
Copy link
Owner

ljos commented Mar 28, 2018

load-in-session is a built in org-babel command.

@lafrenierejm
Copy link
Contributor Author

If "session" means what I think it does, then a working implementation of org-babel-load-in-session seems that it could be very useful for Prolog. In the meantime, do you think it would be prudent to remove the current, nonfunctional definition of org-babel-load-in-session?

Do you have any guarantees that the other code dealing with sessions is behaving correctly?

@ljos
Copy link
Owner

ljos commented Mar 28, 2018

The org-babel-prolog-evaluate-session code should work. It did last time I worked with it. The org-babel-load-session:prolog code doesn't do what it is supposed to as it only opens the session. It doesn't actually evaluate the current block.

I see that other modes that implement session handling, like ob-R, they also have a org-babel-prep-session:MODE function. That converts the stuff like variables etc.

It should be fairly simple to refactor out the clause loading from org-babel-prolog-evaluate-session and create a new function that can be used in prep-session. Then load-session can use prep-session. The clause handeling is a little strange because of how the prolog repl behaves, but it shouldn't be too difficult.

I am also wondering if there are other commands that org-babel expects that it would be good for prolog to implement.

@ljos ljos closed this as completed in c54bfdd Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants