> On 29 May 2015, at 11:00, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
> 
> Bjarte Johansen <bjarte.johansen@infomedia.uib.no> 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.
> 
>> -@item GNU Screen @tab screen @tab shell @tab sh 
>> -@item SQL @tab sql @tab SQLite @tab sqlite
>> +@item GNU Screen @tab screen Sed @tab sed
>> +@item @tab shell @tab sh @item SQL @tab sql
>> +@item @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,