From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Subject: Re: ob-sed
Date: Wed, 27 May 2015 23:31:41 +0200
Message-ID: <877frtd7wi.fsf@nicolasgoaziou.fr>
References: <3C35DA19-1BA3-4249-9128-99DDA0F4752D@infomedia.uib.no>
	<D5085DF7-B664-4969-8122-2D5093E55E84@infomedia.uib.no>
Mime-Version: 1.0
Content-Type: text/plain
Return-path: <emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:46917)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mail@nicolasgoaziou.fr>) id 1Yxiu3-0003Hz-93
	for emacs-orgmode@gnu.org; Wed, 27 May 2015 17:30:20 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mail@nicolasgoaziou.fr>) id 1Yxity-0001P3-Ip
	for emacs-orgmode@gnu.org; Wed, 27 May 2015 17:30:19 -0400
Received: from relay3-d.mail.gandi.net ([2001:4b98:c:538::195]:41803)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mail@nicolasgoaziou.fr>) id 1Yxity-0001OL-DU
	for emacs-orgmode@gnu.org; Wed, 27 May 2015 17:30:14 -0400
In-Reply-To: <D5085DF7-B664-4969-8122-2D5093E55E84@infomedia.uib.no> (Bjarte
	Johansen's message of "Wed, 27 May 2015 10:56:17 +0200")
List-Id: "General discussions about Org-mode." <emacs-orgmode.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/emacs-orgmode>,
	<mailto:emacs-orgmode-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/emacs-orgmode>
List-Post: <mailto:emacs-orgmode@gnu.org>
List-Help: <mailto:emacs-orgmode-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/emacs-orgmode>,
	<mailto:emacs-orgmode-request@gnu.org?subject=subscribe>
Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org
Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org
To: Bjarte Johansen <bjarte.johansen@infomedia.uib.no>
Cc: emacs-orgmode <emacs-orgmode@gnu.org>

Hello,

Bjarte Johansen <bjarte.johansen@infomedia.uib.no> writes:

> I had loosely based it on my own ob-sparql and ob-awk, I saw that
> there was a remnant of ob-sparql left in there. Here is an updated
> version.

Thank you. Some comments follow.

> ;;; ob-sed.el --- org-babel functions for sed scripts
>
> ;; Copyright (C) 2015 Bjarte Johansen

You need to change the copyright to Free Software Foundation, Inc.
>
> ;; Author: Bjarte Johansen
> ;; Keywords: literate programming, reproducible research
> ;; Version: 0.1.0

You will need to add "This file is part of GNU Emacs."

> ;; Provides a way to evaluate sed scripts in org-mode.

org-mode -> Org mode

> (defvar org-babel-sed-command "sed")

Missing docstring.

> (defun org-babel-execute:sed (body params)
>   "Execute a block of sed code with org-babel.  This function is
> called by `org-babel-execute-src-block'"

org-babel -> Org Babel

"This function is" should be moved to a new line, not on the summary
line.  BODY and PARAMS ought to be explained.

>   (message "executing sed source code block")
>   (let* ((result-params (cdr (assoc :result-params params)))

`assoc' -> `assq' (same goes for other occurrences)

>          (cmd-line (cdr (assoc :cmd-line params)))
>          (in-file (cdr (assoc :in-file params)))

:cmd-line and :in-file look like sed-specific header arguments. If
that's correct, you should create a defconst,
`org-babel-header-args:sed' and list them here, probably with :any
value.

>          (cmd (mapconcat #'identity (remove nil (list org-babel-sed-command
> 						      "-f" code-file
> 						      cmd-line
> 						      in-file))
> 			 " ")))

`remove' -> `remq'

Bonus points for tests, too. Also, "org.texi" needs to be updated.


Regards,

-- 
Nicolas Goaziou