From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: M4 support take#2 Date: Fri, 13 Apr 2018 22:31:51 +0200 Message-ID: <87h8oeq24o.fsf@nicolasgoaziou.fr> References: <2126634397.65862.1523072013134.ref@mail.yahoo.com> <2126634397.65862.1523072013134@mail.yahoo.com> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:53026) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f75MT-0007QX-F0 for emacs-orgmode@gnu.org; Fri, 13 Apr 2018 16:31:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f75MQ-000495-8d for emacs-orgmode@gnu.org; Fri, 13 Apr 2018 16:31:57 -0400 Received: from relay5-d.mail.gandi.net ([217.70.183.197]:41585) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1f75MQ-00047E-1I for emacs-orgmode@gnu.org; Fri, 13 Apr 2018 16:31:54 -0400 In-Reply-To: <2126634397.65862.1523072013134@mail.yahoo.com> (Brad Knotwell's message of "Sat, 7 Apr 2018 03:33:33 +0000 (UTC)") 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: Brad Knotwell Cc: emacs-orgmode@gnu.org Hello, Brad Knotwell writes: > Given the code review from earlier, I've added a second file with the > requested changes. Thank you. Some minor comments follow. > (defconst org-babel-header-args:m4 > '((:cmd-line . :any) > (:quote . :any) > (:unquote . :any) > (:list-start . :any) > (:list-end . :any) > (:prefix-builtins)) Missing allowed type for last header. Maybe :any ? > (defun org-babel--m4-prefix (params) > "Prefix m4_ if :prefix-builtins is set" > (if (assq :prefix-builtins params) "m4_" "")) > > (defun org-babel--m4-changequote (params) > "Declare quoting behavior if start-quote and end-quote are set. Otherwise, return an empty string." The line is too long. The second sentence should go onto another line. > (let ((prefix (org-babel--m4-prefix params)) > (start-quote (cdr (assq :quote params))) > (end-quote (cdr (assq :unquote params)))) > (if (and start-quote end-quote) (format "%schangequote(%s,%s)%sdnl\n" prefix start-quote end-quote prefix) ""))) See above. > (defun org-babel--variable-assignment:m4_generic (params varname values) > "Build the simple macro definitions prepended to the script body." > (let ((prefix (org-babel--m4-prefix params))) > (format "%sdefine(%s,%s)%sdnl\n" prefix varname values prefix))) The (format ...) is not correctly indented. > (defun org-babel--variable-assignment:m4_list (params varname values) > "Build the complex macro definitions prepended to the script body." > (let ((prefix (org-babel--m4-prefix params)) > (list-start (or (cdr (assq :list-start params)) "[")) > (list-end (or (cdr (assq :list-end params)) "]"))) > (format "%sdefine(%s,%s%s%s)%sdnl\n" prefix varname list-start > (mapconcat > (lambda (value) > ;; value could be a numeric table entry as well as a string > (if (= (length value) 1) (format "%s" (car value)) > (concat list-start (mapconcat (lambda (x) (format "%s" x)) value ",") > list-end))) values ",") list-end prefix))) The line is too long. `values' should be below (lambda ...), so does ",". `list-end' and `prefix' should be below "%sdefine..." > (defun org-babel--variable-assignments:m4 (params varnames values) > "Internal helper that converts parameters to m4 definitions." > (pcase values > (`(,_ . ,_) (org-babel--variable-assignment:m4_list params varnames values)) > (_ (org-babel--variable-assignment:m4_generic params varnames values)))) > > (defun org-babel-variable-assignments:m4 (params) > "Interface function that converts parameters to m4 definitions." > (concat (org-babel--m4-changequote params) > (apply #'concat (mapcar (lambda (pair) (org-babel--variable-assignments:m4 > params (car pair) (cdr pair))) > (org-babel--get-vars params))))) (mapcar ...) should be below #'concat. > ;; Required to make tangling work > ;; The final "\n" is needed as GNU m4 errors out if a file doesn't end in a newline. > (defun org-babel-expand-body:m4 (body params) > "Expand BODY according to PARAMS, return the expanded body." > (concat (org-babel-variable-assignments:m4 params) body "\n")) > > (defun org-babel-execute:m4 (body params) > "Execute a block of m4 code with Org Babel. > BODY is the source inside a m4 source block and PARAMS is an > association list over the source block configurations. This > function is called by `org-babel-execute-src-block'." > (message "executing m4 source code block") > (let* ((result-params (cdr (assq :result-params params))) > (cmd-line (cdr (assq :cmd-line params))) > (prefix-builtins (assq :prefix-builtins params)) > (code-file (let ((file (org-babel-temp-file "m4-"))) > (with-temp-file file > (insert (org-babel-expand-body:m4 body params) file)) file)) Last `file' should be below (let ((file ...))). > (stdin (let ((stdin (cdr (assq :stdin params)))) > (when stdin > (let ((tmp (org-babel-temp-file "m4-stdin-")) > (res (org-babel-ref-resolve stdin))) > (with-temp-file tmp > (insert res)) > tmp)))) > (cmd (mapconcat #'identity > (remq nil > (list org-babel-m4-command > cmd-line (if prefix-builtins "-P") "<" code-file)) (and prefix-builtins "-P") "<" and `code-file' should go below `#'identity'. I cannot remember: do you plan to have it integrated into Org proper? If so, have you started the process of signing FSF papers? Regards, -- Nicolas Goaziou