Re: [O] language support for m4

[Nicolas Goaziou]

Hello,

Brad Knotwell <address@hidden> writes:

> The attached file adds m4 support.  It was tested against org-9.1.7
> and used ob-sed.el and ob-shell.el for inspiration.  Both code
> execution and tangling have been tested with simple inputs as well as
> tables (easiest way to verify correctness is to inspect the tangled
> output).  It was good fun for an evening project (converting the
> tables to m4 list syntax was the trickiest part and required a foray
> into ob-shell.el; pcase is a nifty function).
> When putting it together, I debated between putting the definitions
> representing variables in as -DNAME=value pairs in the command-line or
> putting them in the body of the generate file.  Even though it's a bit
> uglier, I decided to put them in the body of the file as it makes the
> generated file capable of being run by hand.

Thank you. Some comments follow.

First, what is your status wrt FSF papers. If you haven't signed them,
this can go in contrib/ until it is sorted out.

> ;; Copyright (C) 2015-2018 Free Software Foundation, Inc.

The copyright years are probably not accurate.

> ;; build the passed-in macro definitions as a single string to prepend to the 
> body
> ;; the hocus-pocus with :prefix-builtins is necessary for the -P option to 
> work

Could you make two separate sentences? The above is a bit annoying to
grok.

> (defun __org-babel-m4-prefix (params) (if (assq :prefix-builtins params) 
> "m4_"))

The name of the function is not right. It should probably be
`org-babel--m4-prefix'.  Also, it needs a docstring.

Nitpick:

  (and (assq :prefix-builtins params) "m4_")

> (defun org-babel--variable-assignment:m4_generic (params varname values)

Ditto about the missing docstring.

>   (concat (__org-babel-m4-prefix params)
>         "define(" (format "%s" varname) "," (format "%s" values) ")"
>         (__org-babel-m4-prefix params)
>         "dnl\n"))

  (format "define(%s,%s)" varname values)

>
> (defun org-babel--variable-assignment:m4_list (params varname values)

Ditto.

>   (concat (__org-babel-m4-prefix params)
>         "define(" (format "%s,[" varname)

Per above:

  (format "define(%s,[%s])"
          varname
          (mapconcat ...))

>         (mapconcat (lambda (value)
>                      (if (= (length value) 1) (format "%s" (car value))

  (and (= (length value) 1) (format "%s" (car values)))

Aren't `values' strings already?

>                      (concat "["
>                              (mapconcat (lambda (x) (format "%s" x)) value 
> ",")
>                              "]"))) values ",")
>         "])" (__org-babel-m4-prefix params) "dnl\n"))
>
> (defun org-babel--variable-assignments:m4 (params varnames values)
>   "Represent the parameters as 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)
>   (apply 'concat (mapcar (lambda (pair) (org-babel--variable-assignments:m4
>                                        params (car pair) (cdr pair)))
>                        (org-babel--get-vars params))))

  (apply #'concat ...)
  
>
> ;; required to make tangling work
> ;; the final "\n" is required to make m4 work as the body doesn't end in a 
> newline

Missing capitals and full stops.

> ;; Test examples below:

These could go into a "test-ob-m4.el" file.

Regards,

-- 
Nicolas Goaziou