From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Brad Knotwell <bknotwell@yahoo.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: M4 support take#2
Date: Fri, 13 Apr 2018 22:31:51 +0200 [thread overview]
Message-ID: <87h8oeq24o.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <2126634397.65862.1523072013134@mail.yahoo.com> (Brad Knotwell's message of "Sat, 7 Apr 2018 03:33:33 +0000 (UTC)")
Hello,
Brad Knotwell <bknotwell@yahoo.com> 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
prev parent reply other threads:[~2018-04-13 20:31 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <2126634397.65862.1523072013134.ref@mail.yahoo.com>
2018-04-07 3:33 ` M4 support take#2 Brad Knotwell
2018-04-13 20:31 ` Nicolas Goaziou [this message]
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
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87h8oeq24o.fsf@nicolasgoaziou.fr \
--to=mail@nicolasgoaziou.fr \
--cc=bknotwell@yahoo.com \
--cc=emacs-orgmode@gnu.org \
/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 public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).