Re: [PATCH] Add support for Babel with Eshell, (updated PATCH)

[stardiviner <numbchild@gmail.com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256


Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> stardiviner <numbchild@gmail.com> writes:
>
>> From 596da7b0384d64f3c1c22a49bc9bced8d0d8abf8 Mon Sep 17 00:00:00 2001
>> From: stardiviner <numbchild@gmail.com>
>> Date: Sun, 22 Apr 2018 09:37:40 +0800
>> Subject: [PATCH] ob-eshell.el: Add Eshell support for Babel.
>
> Thank you. This could go into master branch once Org 9.2 is out.

It's fine to be merged later.

Hi, Nicolas, thank you always review my Org-mode patches. Yesterday, I
ask Bastein about Worg ox-org question. He mentioned I should at least
say "Hi" in message. I'm not good at English culture of something else.
But I just want to say, I'm glad you reviewed my patches. Really. Thanks
and that means a lot to me.

>
>>  | D          | d          | ditaa          | ditaa      |
>>  | Graphviz   | dot        | Emacs Calc     | calc       |
>>  | Emacs Lisp | emacs-lisp | Fortran        | fortran    |
>> +| Shell      | sh         | Eshell         | eshell     |
>>  | Gnuplot    | gnuplot    | Haskell        | haskell    |
>>  | Java       | java       | Javascript     | js         |
>>  | LaTeX      | latex      | Ledger         | ledger     |
>
> Shell sh pair is already defined later in the table (ordered
> alphabetically). Those are spurious.

Aha, I found it, is there a way to auto sort this table automatically?
(Maybe format it into two columns instead of four columns even though it
looks short and better.) Adding one language into this table, then move
following languages in table one by one is awful.

>
>> +;; Org-Babel support for evaluating Eshell source code.
>
> Org Babel support....
>
>> +(defvar org-babel-default-header-args:eshell '())
>> +
>> +(defun org-babel-execute:eshell (body params)
>> +  "Execute a block of Eshell code.
>> +This function is called by `org-babel-execute-src-block'."
>
> Could you expound a bit and explain what are the arguments, i.e., BODY
> and PARAMS?
>
>> +(defun org-babel-prep-session:eshell (session params)
>> +  "Prepare SESSION according to the header arguments specified in PARAMS."
>> +  (let* ((session (org-babel-eshell-initiate-session session))
>> +	 ;; Eshell session buffer is read from variable `eshell-buffer-name'.
>> +	 (eshell-buffer-name session)
>> +	 (var-lines (org-babel-variable-assignments:eshell params)))
>> +    (call-interactively 'eshell)
>
>     #'eshell
>
>> +    (dolist (var-line var-lines)
>> +      (eshell-command var-line))
>
>     (mapc #'eshell-command var-lines)

I previously patches, you mentioned to use `dolist`, so I used it this
time, what's the difference between mapc and dolist? I thought might be
performance or something else. Maybe looks cleaner?

>
>> +    session))
>> +
>> +(defun ob-eshell-session-live-p (session)
>> +  "Detect Eshell SESSION exist."
>
> "Non-nil if Eshell SESSION exists."
>
>> +  (and (get-buffer session) t))
>
> Simply:
>
>     (get-buffer session)
>
>> +;;; test-ob-eshell.el
>> +
>> +;; Copyright (c) 2010-2014 Eric Schulte
>
> I don't think so ;)

I copy form other test-ob-*.el file template. forgot to change copyright
name. should I use my own name "stardiviner"?

>
> Barring the minor comments above, it looks good.
>
> Could you send an updated patch?
>
> Regards,

I will update patch, and resend later after those question answer fixed.

And thanks again. :) :) :)

- --
[ stardiviner ] don't need to convince with trends.
       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEE8J9lDX1nSBmJJZFAG13xyVromsMFAlrd3iAACgkQG13xyVro
msMvOwf8DDLfUSJrdaS7mvBhNm8mYJhlpSbiWMnBTMGcMDr8afmzpT9JsSXEjtW3
RyiMPF9dBjNIoKbqCMkTU18DnFEfT7mNcB5nasI0gxxfPgF/3Ueb/xDcoKm9GLyB
NXmei4WwXqLnyRSmBfXocp/tbuX4GmdzQi5Yv8ITMsceb+/LCoWxvw0gHH1QFjeB
sPA0LsMeSZpEdCNe2L6q2/y13D00yAjIi8ufBSO4KQProLOKVmZhU7i3iz+8acLJ
QxLDsLDpYHCV1RK29WWhHfBcHe2Sx9SXLhOOgUAQLaUuhKSncnLQu5sjVfe8GMBL
5KD+bZxvyvDkKBCfDwCtM6cvlu3lKA==
=F76/
-----END PGP SIGNATURE-----