emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: stardiviner <numbchild@gmail.com>
To: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Cc: org-mode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] Add support for Babel with Eshell, (updated PATCH)
Date: Mon, 23 Apr 2018 21:22:40 +0800	[thread overview]
Message-ID: <87h8o26orj.fsf@gmail.com> (raw)
In-Reply-To: <87po2q5kqk.fsf@nicolasgoaziou.fr>

-----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-----

  reply	other threads:[~2018-04-23 13:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-22  2:00 [PATCH] Add support for Babel with Eshell, and some questions stardiviner
2018-04-22  2:01 ` stardiviner
2018-04-22  2:31   ` [PATCH] Add support for Babel with Eshell, (updated PATCH) stardiviner
2018-04-23  9:34     ` Nicolas Goaziou
2018-04-23 13:22       ` stardiviner [this message]
2018-04-23 21:35         ` Nicolas Goaziou
2018-04-24  2:39           ` stardiviner
2018-04-25  8:31             ` Nicolas Goaziou
2018-04-26 10:53               ` stardiviner
2018-05-02 14:53                 ` stardiviner
2018-05-31 21:13                 ` Nicolas Goaziou
2018-06-01  6:26                   ` stardiviner
2018-06-02 10:16                     ` Nicolas Goaziou
2018-06-04  2:11                       ` stardiviner
2018-06-14 19:12                         ` Nicolas Goaziou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h8o26orj.fsf@gmail.com \
    --to=numbchild@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=mail@nicolasgoaziou.fr \
    --subject='Re: [PATCH] Add support for Babel with Eshell, (updated PATCH)' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).