From mboxrd@z Thu Jan 1 00:00:00 1970 From: "numbchild@gmail.com" Subject: Re: add some babel supports (PHP, Lua, Redis) Date: Mon, 23 May 2016 11:09:29 +0800 Message-ID: References: <87mvnydzbl.fsf@gmx.us> <87ziri9fhu.fsf@gmx.us> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=94eb2c06c10089eb96053379c6ae Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:57177) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4gFn-00046I-16 for emacs-orgmode@gnu.org; Sun, 22 May 2016 23:10:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b4gFj-00052y-BQ for emacs-orgmode@gnu.org; Sun, 22 May 2016 23:10:01 -0400 Received: from mail-yw0-x231.google.com ([2607:f8b0:4002:c05::231]:34611) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4gFj-00052Y-35 for emacs-orgmode@gnu.org; Sun, 22 May 2016 23:09:59 -0400 Received: by mail-yw0-x231.google.com with SMTP id c127so42269608ywb.1 for ; Sun, 22 May 2016 20:09:58 -0700 (PDT) In-Reply-To: <87ziri9fhu.fsf@gmx.us> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Rasmus Cc: Org-mode --94eb2c06c10089eb96053379c6ae Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable I modified most part of my files. - There are some places I can't improve because I'm not good at elisp. (maybe other people can improve it later) Like ob-redis.el implement ob-sql style configuration. and ob-lua.el make use of lua-mode's running process. - I guess ob-php.el is the only one branch can be merged. - Anyway, check out my updates, if no one can be merged, I will still keep my github repositories. Thanks for your detail reviews. [stardiviner] GPG key ID: 47C32433 IRC(freeenode): stardiviner Twitter: @numbchild Key fingerprint =3D 9BAA 92BC CDDD B9EF 3B36 CB99 B8C4 B8E5 47C3 2433 Blog: http://stardiviner.github.io/ On Mon, May 23, 2016 at 1:20 AM, Rasmus wrote: > Hi, > > Thanks for your patches. Sorry about the delay. I was hoping that > someone with more ob knowledge would step in. > > Please find some comments below. I think some more work is needed before > your libraries are > > "numbchild@gmail.com" writes: > > > From 2589d4e7d28016fb515d2131cbd9ff52797e50eb Mon Sep 17 00:00:00 2001 > > From: stardiviner > > Date: Tue, 10 May 2016 16:03:32 +0800 > > Subject: [PATCH] ob-lua.el: add Lua src block executing support > > > > * contrib/lisp/ob-lua.el (org-babel-execute:lua): support executing Lua > > src block. > > Please capitalize after the colon. > > > --- > > +;;; ob-lua.el --- Execute Lua code within org-mode blocks. > > +;; Copyright 2016 stardiviner > > > +;; Author: stardiviner > > +;; Maintainer: stardiviner > > > > > +;; Keywords: org babel lua > > For whatever reason this seems to be > > ;; Keywords: literate programming, reproducible research > > In most ob files. > > > +;; URL: https://github.com/stardiviner/ob-lua > > > > +;; Created: 12th April 2016 > > This header is unnecessary, but if you like it, it=E2=80=99s fine. > > > +;; Version: 0.0.1 > > +;; Package-Requires: ((org "8")) > > > +;;; Commentary: > > +;; > > +;; Execute Lua code within org-mode blocks. > > > Maybe, > > The file provides Org-Babel support for evaluating Lua code. > > > > > +;;; Code: > > +(require 'org) > > This is unnecessary. Ob implies Org. > > > +(require 'ob) > > + > > +(defgroup ob-lua nil > > + "org-mode blocks for Lua." > > + :group 'org) > > It seems that ob languages do not typically define new groups. > > Also, ob is the filename. Variables are org-babel. > > > +(defcustom ob-lua:default-session "*lua*" > > + "Default Lua session. > > + > > +It is lua inferior process from `run-lua'." > > + :group 'ob-lua > > + :type 'string) > > I don=E2=80=99t think this is necessary to have as a defcustom. There=E2= =80=99s already > :session. Also, you are missing :type. Per above, group is org-babel. > > > +;;;###autoload > This is normally not autoloaded. Babel languages are loaded via > org-babel-do-load-languages. > > > +(defun org-babel-execute:lua (body params) > > + "org-babel lua hook." > > Please capitalize sentences. > > > + (let* ((session (or (cdr (assoc :session params)) > > + ob-lua:default-session)) > > + (cmd (mapconcat 'identity (list "lua -") " "))) > > Is something missing here? AFAIK cmd =E2=86=92 "lua -" always. > > Also, what if my lua is not in my PATH? I got a felling that you might > make a more robust mode by hooking into lua-mode > > https://immerrr.github.io/lua-mode/ > > > + (org-babel-eval cmd body))) > > How are various return values handled? E.g. will a table be correctly > interpreted as such? (It=E2=80=99s a while since I coded in lua). > > > +;;;###Autoload > > +(eval-after-load "org" > > + '(add-to-list 'org-src-lang-modes '("lua" . lua))) > > > This should be unnecessary as the lua-mode is presumably lua-mode. Also, > I think your code depends on lua-mode in order to be able to edit it. Yo= u > need to declare that as a dependency. > > > +(provide 'ob-lua) > > + > > +;;; ob-lua.el ends here > > -- > > 2.8.2 > > > > > > From d2e7202930fcf24e7c90826e69bb768094463a0c Mon Sep 17 00:00:00 2001 > > From: stardiviner > > Date: Tue, 10 May 2016 16:05:38 +0800 > > Subject: [PATCH] ob-php.el: Add PHP src block executing support > > > > * contrib/lisp/ob-php.el (org-babel-execute:php): support executing PHP > > src block. > > Capitalize. > > > --- > > contrib/lisp/ob-php.el | 44 ++++++++++++++++++++++++++++++++++++++++++= ++ > > 1 file changed, 44 insertions(+) > > create mode 100644 contrib/lisp/ob-php.el > > > > diff --git a/contrib/lisp/ob-php.el b/contrib/lisp/ob-php.el > > new file mode 100644 > > index 0000000..31960a5 > > --- /dev/null > > +++ b/contrib/lisp/ob-php.el > > @@ -0,0 +1,44 @@ > > +;;; ob-php.el --- Execute PHP within org-mode blocks. > > +;; Copyright 2016 stardiviner > > + > > +;; Author: stardiviner > > +;; Maintainer: stardiviner > > +;; Keywords: org babel php > > +;; URL: https://github.com/stardiviner/ob-php > > +;; Created: 04th May 2016 > > +;; Version: 0.0.1 > > +;; Package-Requires: ((org "8")) > > + > > +;;; Commentary: > > +;; > > +;; Execute PHP within org-mode blocks. > > + > > +;;; Code: > > +(require 'org) > > +(require 'ob) > > + > > +(defgroup ob-php nil > > + "org-mode blocks for PHP." > > + :group 'org) > > > See comments above. > > > +;; Todo > > Remove. > > > +(defcustom ob-php:inf-php-buffer "*php*" > > + "Default PHP inferior buffer." > > + :group 'ob-php > > + :type 'string) > > :Type is missing and this buffer should not be a defcustom. > > > +;;;###Autoload > > Remove. > > > +(defun org-babel-execute:php (body params) > > + "org-babel PHP hook." > > + ;; Todo > > Remove. > > > + (let* ((cmd (mapconcat 'identity (list "php") " -r "))) > > + (org-babel-eval cmd body) > > + )) > > See comments above re docstring, and robustness. > > What sort of return values can I expect from this? I don=E2=80=99t know = php, but > I assume it=E2=80=99s mainly "log messages". > > > +;;;###autoload > > +(eval-after-load "org" > > + '(add-to-list 'org-src-lang-modes '("php" . php))) > > Unnecessary. Also, it seems there=E2=80=99s an undeclared decency on som= e sort of > php mode. > > > + > > +(provide 'ob-php) > > > From eb3c0cb1467416d141a633c49e1c9050311d92ab Mon Sep 17 00:00:00 2001 > > From: stardiviner > > Date: Tue, 10 May 2016 16:06:19 +0800 > > Subject: [PATCH] ob-redis.el: Add Redis src block executing support > > > > * contrib/lisp/ob-redis.el (org-babel-execute:redis): support for > > executing redis src block. > > Capitalize > > > > --- > > contrib/lisp/ob-redis.el | 44 > ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 44 insertions(+) > > create mode 100644 contrib/lisp/ob-redis.el > > > > diff --git a/contrib/lisp/ob-redis.el b/contrib/lisp/ob-redis.el > > new file mode 100644 > > index 0000000..340b050 > > --- /dev/null > > +++ b/contrib/lisp/ob-redis.el > > @@ -0,0 +1,44 @@ > > +;;; ob-redis.el --- Execute Redis queries within org-mode blocks. > > +;; Copyright 2016 stardiviner > > + > > +;; Author: stardiviner > > +;; Maintainer: stardiviner > > +;; Keywords: org babel redis > > +;; URL: https://github.com/stardiviner/ob-redis > > +;; Created: 28th Feb 2016 > > +;; Version: 0.0.1 > > +;; Package-Requires: ((org "8")) > > + > > +;;; Commentary: > > +;; > > +;; Execute Redis queries within org-mode blocks. > > + > > +;;; Code: > > +(require 'org) > > +(require 'ob) > > + > > +(defgroup ob-redis nil > > + "org-mode blocks for Redis." > > + :group 'org) > > As above. > > > +(defcustom ob-redis:default-db "127.0.0.1:6379" > > + "Default Redis database." > > + :group 'ob-redis > > + :type 'string) > > Does redis support login and different ports and all that jazz? > If so, please compare the configurability to the needs of ob-sql. > > Is it possible to extend ob-sql with another engine? (I don=E2=80=99t kn= ow > anything about redis). > > Or are these "NoSQL" databases similar enough that it would be possible t= o > have "unite" them in a single library. Does ob have support for any > "NoSQL" databases ATM? > > I=E2=80=99m not implying that this is necessarily the right way to go. I= =E2=80=99m just > trying to understand the nature of this. > > > +;;;###Autoload > > Remove. > > > +(defun org-babel-execute:redis (body params) > > + "org-babel redis hook." > > + (let* ((db (or (cdr (assoc :db params)) > > + ob-redis:default-db)) > > + (cmd (mapconcat 'identity (list "redis-cli") " "))) > > + (org-babel-eval cmd body) > > + )) > > > My guess is that connectivity support is not comprehensive enough. > > > +;;;###autoload > > +(eval-after-load "org" > > + '(add-to-list 'org-src-lang-modes '("redis" . redis))) > > Remove. > > > +(provide 'ob-redis) > > + > > +;;; ob-redis.el ends here > > Thanks, > Rasmus > > -- > Vote for proprietary math! > > > > --94eb2c06c10089eb96053379c6ae Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
I modified most part of my files.

- There a= re some places I can't improve because I'm not good at elisp. (mayb= e other =C2=A0 =C2=A0 =C2=A0people can improve it later) Like ob-redis.el i= mplement ob-sql style configuration. and ob-lua.el make use of lua-mode'= ;s running process.

- I guess ob-php.el is the only one bran= ch can be merged.
- Anyway, check out my updates, if no one can be mer= ged, I will still keep my github repositories.

Thanks for yo= ur detail reviews.


[stardiviner]=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 <Hack this world!>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 GPG key ID= : 47C32433
IRC(freeenode): stardiviner =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 Twitter:=C2=A0 @numbchild
Key fingerprint =3D 9BAA 92BC CDDD B9EF= 3B36=C2=A0 CB99 B8C4 B8E5 47C3 2433
Blog: http://stardiviner.github.io/
<= /div>

On Mon, May 23, 2016 at 1:20 AM, Rasmus <rasmus= @gmx.us> wrote:
Hi,

Thanks for your patches.=C2=A0 Sorry about the delay.=C2=A0 I was hoping th= at
someone with more ob knowledge would step in.

Please find some comments below.=C2=A0 I think some more work is needed bef= ore
your libraries are

"numbchild@gmail.com" = <numbchild@gmail.com> writ= es:

> From 2589d4e7d28016fb515d2131cbd9ff52797e50eb Mon Sep 17 00:00:00 2001=
> From: stardiviner <numbchild= @gmail.com>
> Date: Tue, 10 May 2016 16:03:32 +0800
> Subject: [PATCH] ob-lua.el: add Lua src block executing support
>
> * contrib/lisp/ob-lua.el (org-babel-execute:lua): support executing Lu= a
> src block.

Please capitalize after the colon.

> ---
> +;;; ob-lua.el --- Execute Lua code within org-mode blocks.
> +;; Copyright 2016 stardiviner

> +;; Author: stardiviner <num= bchild@gmail.com>
> +;; Maintainer: stardiviner <numbchild@gmail.com>



> +;; Keywords: org babel lua

For whatever reason this seems to be

=C2=A0 =C2=A0 ;; Keywords: literate programming, reproducible research

In most ob files.

> +;; URL: https://github.com/stardiviner/ob-lua


> +;; Created: 12th April 2016

This header is unnecessary, but if you like it, it=E2=80=99s fine.

> +;; Version: 0.0.1
> +;; Package-Requires: ((org "8"))

> +;;; Commentary:
> +;;
> +;; Execute Lua code within org-mode blocks.


Maybe,

=C2=A0 =C2=A0 =C2=A0 =C2=A0 The file provides Org-Babel support for evaluat= ing Lua code.



> +;;; Code:
> +(require 'org)

This is unnecessary.=C2=A0 Ob implies Org.

> +(require 'ob)
> +
> +(defgroup ob-lua nil
> +=C2=A0 "org-mode blocks for Lua."
> +=C2=A0 :group 'org)

It seems that ob languages do not typically define new groups.

Also, ob is the filename.=C2=A0 Variables are org-babel.

> +(defcustom ob-lua:default-session "*lua*"
> +=C2=A0 "Default Lua session.
> +
> +It is lua inferior process from `run-lua'."
> +=C2=A0 :group 'ob-lua
> +=C2=A0 :type 'string)

I don=E2=80=99t think this is necessary to have as a defcustom.=C2=A0 There= =E2=80=99s already
:session.=C2=A0 Also, you are missing :type.=C2=A0 Per above, group is org-= babel.

> +;;;###autoload
This is normally not autoloaded.=C2=A0 Babel languages are loaded via
org-babel-do-load-languages.

> +(defun org-babel-execute:lua (body params)
> +=C2=A0 "org-babel lua hook."

Please capitalize sentences.

> +=C2=A0 (let* ((session (or (cdr (assoc :session params))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 ob-lua:default-session))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(cmd (mapconcat 'identity (list= "lua -") " ")))

Is something missing here?=C2=A0 AFAIK cmd =E2=86=92 "lua -" alwa= ys.

Also, what if my lua is not in my PATH?=C2=A0 I got a felling that you migh= t
make a more robust mode by hooking into lua-mode

=C2=A0 =C2=A0 =C2=A0https://immerrr.github.io/lua-mode/

> +=C2=A0 =C2=A0 (org-babel-eval cmd body)))

How are various return values handled?=C2=A0 E.g. will a table be correctly=
interpreted as such?=C2=A0 (It=E2=80=99s a while since I coded in lua).

> +;;;###Autoload
> +(eval-after-load "org"
> +=C2=A0 '(add-to-list 'org-src-lang-modes '("lua"= ; . lua)))


This should be unnecessary as the lua-mode is presumably lua-mode.=C2=A0 Al= so,
I think your code depends on lua-mode in order to be able to edit it.=C2=A0= You
need to declare that as a dependency.

> +(provide 'ob-lua)
> +
> +;;; ob-lua.el ends here
> --
> 2.8.2
>
>
> From d2e7202930fcf24e7c90826e69bb768094463a0c Mon Sep 17 00:00:00 2001=
> From: stardiviner <numbchild= @gmail.com>
> Date: Tue, 10 May 2016 16:05:38 +0800
> Subject: [PATCH] ob-php.el: Add PHP src block executing support
>
> * contrib/lisp/ob-php.el (org-babel-execute:php): support executing PH= P
>=C2=A0 =C2=A0src block.

Capitalize.

> ---
>=C2=A0 contrib/lisp/ob-php.el | 44 ++++++++++++++++++++++++++++++++++++= ++++++++
>=C2=A0 1 file changed, 44 insertions(+)
>=C2=A0 create mode 100644 contrib/lisp/ob-php.el
>
> diff --git a/contrib/lisp/ob-php.el b/contrib/lisp/ob-php.el
> new file mode 100644
> index 0000000..31960a5
> --- /dev/null
> +++ b/contrib/lisp/ob-php.el
> @@ -0,0 +1,44 @@
> +;;; ob-php.el --- Execute PHP within org-mode blocks.
> +;; Copyright 2016 stardiviner
> +
> +;; Author: stardiviner <num= bchild@gmail.com>
> +;; Maintainer: stardiviner <numbchild@gmail.com>
> +;; Keywords: org babel php
> +;; URL: https://github.com/stardiviner/ob-php
> +;; Created: 04th May 2016
> +;; Version: 0.0.1
> +;; Package-Requires: ((org "8"))
> +
> +;;; Commentary:
> +;;
> +;; Execute PHP within org-mode blocks.
> +
> +;;; Code:
> +(require 'org)
> +(require 'ob)
> +
> +(defgroup ob-php nil
> +=C2=A0 "org-mode blocks for PHP."
> +=C2=A0 :group 'org)


See comments above.

> +;; Todo

Remove.

> +(defcustom ob-php:inf-php-buffer "*php*"
> +=C2=A0 "Default PHP inferior buffer."
> +=C2=A0 :group 'ob-php
> +=C2=A0 :type 'string)

:Type is missing and this buffer should not be a defcustom.

> +;;;###Autoload

Remove.

> +(defun org-babel-execute:php (body params)
> +=C2=A0 "org-babel PHP hook."
> +=C2=A0 ;; Todo

Remove.

> +=C2=A0 (let* ((cmd (mapconcat 'identity (list "php") &q= uot; -r ")))
> +=C2=A0 =C2=A0 (org-babel-eval cmd body)
> +=C2=A0 =C2=A0 ))

See comments above re docstring, and robustness.

What sort of return values can I expect from this?=C2=A0 I don=E2=80=99t kn= ow php, but
I assume it=E2=80=99s mainly "log messages".

> +;;;###autoload
> +(eval-after-load "org"
> +=C2=A0 '(add-to-list 'org-src-lang-modes '("php"= ; . php)))

Unnecessary.=C2=A0 Also, it seems there=E2=80=99s an undeclared decency on = some sort of
php mode.

> +
> +(provide 'ob-php)

> From eb3c0cb1467416d141a633c49e1c9050311d92ab Mon Sep 17 00:00:00 2001=
> From: stardiviner <numbchild= @gmail.com>
> Date: Tue, 10 May 2016 16:06:19 +0800
> Subject: [PATCH] ob-redis.el: Add Redis src block executing support >
> * contrib/lisp/ob-redis.el (org-babel-execute:redis): support for
>=C2=A0 =C2=A0executing redis src block.

Capitalize


> ---
>=C2=A0 contrib/lisp/ob-redis.el | 44 ++++++++++++++++++++++++++++++++++= ++++++++++
>=C2=A0 1 file changed, 44 insertions(+)
>=C2=A0 create mode 100644 contrib/lisp/ob-redis.el
>
> diff --git a/contrib/lisp/ob-redis.el b/contrib/lisp/ob-redis.el
> new file mode 100644
> index 0000000..340b050
> --- /dev/null
> +++ b/contrib/lisp/ob-redis.el
> @@ -0,0 +1,44 @@
> +;;; ob-redis.el --- Execute Redis queries within org-mode blocks.
> +;; Copyright 2016 stardiviner
> +
> +;; Author: stardiviner <num= bchild@gmail.com>
> +;; Maintainer: stardiviner <numbchild@gmail.com>
> +;; Keywords: org babel redis
> +;; URL: https://github.com/stardiviner/ob-redis
> +;; Created: 28th Feb 2016
> +;; Version: 0.0.1
> +;; Package-Requires: ((org "8"))
> +
> +;;; Commentary:
> +;;
> +;; Execute Redis queries within org-mode blocks.
> +
> +;;; Code:
> +(require 'org)
> +(require 'ob)
> +
> +(defgroup ob-redis nil
> +=C2=A0 "org-mode blocks for Redis."
> +=C2=A0 :group 'org)

As above.

> +(defcustom ob-redis:default-db "127.0.0.1:6379"
> +=C2=A0 "Default Redis database."
> +=C2=A0 :group 'ob-redis
> +=C2=A0 :type 'string)

Does redis support login and different ports and all that jazz?
If so, please compare the configurability to the needs of ob-sql.

Is it possible to extend ob-sql with another engine?=C2=A0 (I don=E2=80=99t= know
anything about redis).

Or are these "NoSQL" databases similar enough that it would be po= ssible to
have "unite" them in a single library.=C2=A0 Does ob have support= for any
"NoSQL" databases ATM?

I=E2=80=99m not implying that this is necessarily the right way to go.=C2= =A0 I=E2=80=99m just
trying to understand the nature of this.

> +;;;###Autoload

Remove.

> +(defun org-babel-execute:redis (body params)
> +=C2=A0 "org-babel redis hook."
> +=C2=A0 (let* ((db (or (cdr (assoc :db params))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ob-redi= s:default-db))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(cmd (mapconcat 'identity (list= "redis-cli") " ")))
> +=C2=A0 =C2=A0 (org-babel-eval cmd body)
> +=C2=A0 =C2=A0 ))


My guess is that connectivity support is not comprehensive enough.

> +;;;###autoload
> +(eval-after-load "org"
> +=C2=A0 '(add-to-list 'org-src-lang-modes '("redis&qu= ot; . redis)))

Remove.

> +(provide 'ob-redis)
> +
> +;;; ob-redis.el ends here

Thanks,
Rasmus

--
Vote for proprietary math!




--94eb2c06c10089eb96053379c6ae--