From: "numbchild@gmail.com" <numbchild@gmail.com>
To: Rasmus <rasmus@gmx.us>
Cc: Org-mode <emacs-orgmode@gnu.org>
Subject: Re: add some babel supports (PHP, Lua, Redis)
Date: Mon, 23 May 2016 11:09:29 +0800 [thread overview]
Message-ID: <CAL1eYuJ784+Z=e=Su_0Lja7snJ89hg6hiPVjK1kiiFj9yknrqg@mail.gmail.com> (raw)
In-Reply-To: <87ziri9fhu.fsf@gmx.us>
[-- Attachment #1: Type: text/plain, Size: 9274 bytes --]
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] <Hack this world!> GPG key ID: 47C32433
IRC(freeenode): stardiviner Twitter: @numbchild
Key fingerprint = 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 <rasmus@gmx.us> 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" <numbchild@gmail.com> writes:
>
> > 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 Lua
> > src block.
>
> Please capitalize after the colon.
>
> > ---
> > +;;; ob-lua.el --- Execute Lua code within org-mode blocks.
> > +;; Copyright 2016 stardiviner
>
> > +;; Author: stardiviner <numbchild@gmail.com>
> > +;; Maintainer: stardiviner <numbchild@gmail.com>
>
>
>
> > +;; 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’s 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’t think this is necessary to have as a defcustom. There’s 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 → "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’s 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. 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 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 <numbchild@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
> > + "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’t know php, but
> I assume it’s mainly "log messages".
>
> > +;;;###autoload
> > +(eval-after-load "org"
> > + '(add-to-list 'org-src-lang-modes '("php" . php)))
>
> Unnecessary. Also, it seems there’s 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
> > 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 <numbchild@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
> > + "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’t know
> anything about redis).
>
> Or are these "NoSQL" databases similar enough that it would be possible to
> have "unite" them in a single library. Does ob have support for any
> "NoSQL" databases ATM?
>
> I’m not implying that this is necessarily the right way to go. I’m 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!
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 13214 bytes --]
next prev parent reply other threads:[~2016-05-23 3:10 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-10 8:07 add some babel supports (PHP, Lua, Redis) numbchild
2016-05-10 9:42 ` Rasmus
2016-05-10 15:23 ` numbchild
2016-05-10 15:44 ` Rasmus
2016-05-10 15:47 ` Ken Mankoff
2016-05-10 15:58 ` Rasmus
2016-05-10 16:12 ` Ken Mankoff
2016-05-11 1:42 ` numbchild
2016-05-22 17:20 ` Rasmus
2016-05-23 3:09 ` numbchild [this message]
2016-05-27 16:52 ` Rasmus
2016-05-28 0:50 ` numbchild
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAL1eYuJ784+Z=e=Su_0Lja7snJ89hg6hiPVjK1kiiFj9yknrqg@mail.gmail.com' \
--to=numbchild@gmail.com \
--cc=emacs-orgmode@gnu.org \
--cc=rasmus@gmx.us \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.