Re: [O] ob-sed

[Nicolas Goaziou]

Bjarte Johansen <address@hidden> writes:

> I think I have addressed all your comments in the attached patch.

Thank you. Some more comments follow.

> Subject: [PATCH] Org Babel now supports sed scripts

You should add something like the following to your commit message:

  * doc/org.texi: Signal new Babel language

  * lisp/ob-sed.el:
  * testing/examples/ob-sed-test.org:
  * testing/lisp/test-ob-sed.el: New files.

> address@hidden GNU Screen @tab screen @tab shell @tab sh 
> address@hidden SQL @tab sql @tab SQLite @tab sqlite
> address@hidden GNU Screen @tab screen Sed @tab sed
> address@hidden @tab shell @tab sh @item SQL @tab sql
> address@hidden @tab SQLite @tab sqlite @tab @tab

This looks wrong. I think it should be:

  @item GNU Screen @tab screen @tab Sed @tab sed
  @item shell @tab sh @tab SQL @tab sql
  @item SQLite @tab sqlite @tab @tab

> +;;; Usage:
> +
> +;; Add to your Emacs config:
> +
> +;; (org-babel-do-load-languages
> +;;  'org-babel-load-languages
> +;;  '((sed . t)))

You may want to introduce usage for :cmd-line and :in-file arguments in
"Usage" section.

> +(defconst org-babel-header-args:sed
> +  '((:cmd-line :any
> +     :in-file  :any))
> +  "Sed specific header arguments.")

It should be

  '((:cmd-line . :any)
    (:in-file  . :any))

See, for example `org-babel-header-args:R'

> +(defun org-babel-execute:sed (body params)
> +  "Execute a block of sed code with Org Babel.
> +BODY is the source inside a sed source block and PARAMS is an
> +association list over the source block configurations. This
                                                        ^^^
                                                     two spaces


Regards,