Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html

[Stefan Monnier <monnier@iro.umontreal.ca>]

> After some more thinking, the ASCII-only patch seemed redundant, so here
> is a new, simpler patch that also accounts for a few other details I had
> overlooked.  It exports Org readmes to HTML in the ELPA Web page and to
> plain-text for the PACKAGE-readme.txt file.

Sorry for not getting back to you sooner.
I still haven't been able to devote the attention that this deserves,
but here's some comments for now:

> +(cl-defun elpaa--export-org (file backend &key body-only ext-plist)
> +  "Return Org FILE as an exported string.
> +BACKEND and EXT-PLIST are passed to `org-export-as', which see.
> +Uses `elpaa--call-sandboxed', since exporting with Org may run
> +arbitrary code."
> +  (declare (indent defun))
> +  (cl-check-type backend symbol)
> +  (cl-assert (memq body-only '(nil t)) t
> +             "BODY-ONLY may only be nil or t")
> +  (with-temp-buffer
> +    (unless (zerop (elpaa--call-sandboxed
> +                    t "emacs" "--batch" "-l" (format "ox-%s" backend)
> +                    file
> +                    "--eval" (format "(message \"%%s\" (org-export-as '%s nil nil %S '%S))"
> +                                     backend body-only ext-plist)))
> +      (error "Unable to export Org file: %S" file))
> +    (buffer-string)))

`elpaa--call-sandboxed` doesn't return the exit status of the process.

`emacs --batch` will load the site-init files which may emit messages,
and those will "pollute" your temp buffer.

It's not hypothetical, because the Emacs that's in Debian does exactly
that, so the Emacs on elpa.gnu.org (which is arguably the most important
Emacs for this code) emits such warnings (that's how I know ;-).
So you probably want to pass the result of `org-export-as` to
`write-region` to save it explicitly to some file of your choice.

I'd also use `%S` rather than `%s` for `backend` (basically I follow
the rule that we should always use `%S` except for those cases where the
arg is a string and we want to plug its contents).

[ Also, I haven't yet tried the code on elpa.gnu.org but that's still
  running Emacs-26, so there might be compatibility issues for Org.
  It should be upgraded to Emacs-27 "real soon now", tho, so maybe it's
  not worth the trouble.  ]

> @@ -1313,11 +1326,33 @@ return section under HEADER in package's main file."
>        (insert (format "<p>To install this package, run in Emacs:</p>
>                         <pre>M-x <span class=\"kw\">package-install</span> RET <span class=\"kw\">%s</span> RET</pre>"
>                        name))
> -      (let ((rm (elpaa--get-README pkg-spec srcdir)))
> -        (when rm
> -          (write-region rm nil (concat name "-readme.txt"))
> -          (insert "<h2>Full description</h2><pre>\n" (elpaa--html-quote rm)
> -                  "\n</pre>\n")))
> +      (let ((readme-file-name
> +             (elpaa--spec-get pkg-spec :readme
> +                              '("README" "README.rst"
> +                                ;; Most README.md files seem to be currently
> +                                ;; worse than the Commentary: section :-(
> +                                ;; "README.md"
> +                                "README.org")))
> +            (output-filename (concat name "-readme.txt"))
> +            readme-content page-content)
> +        (pcase readme-file-name
> +          ("README.org"
> +           (setf readme-content (elpaa--export-org readme-file-name 'ascii
> +                                  :ext-plist (append '(:ascii-charset utf-8)
> +                                                     elpaa--org-export-options))
> +                 page-content (elpaa--export-org readme-file-name 'html
> +                                :body-only t :ext-plist elpaa--org-export-options)))
> +          (_ ;; Non-Org readme.
> +           (setf readme-content (elpaa--get-section
> +                                 "Commentary" readme-file-name
> +                                 dir pkg-spec)

The byte-compiler tells me `dir` is an unknown variable.

> +                 page-content (when readme-content
> +                                (concat "<pre>\n"
> +                                        (elpaa--html-quote readme-content)
> +                                        "\n</pre>\n")))))
> +        (when readme-content
> +          (write-region readme-content nil output-filename)
> +          (insert "<h2>Full description</h2>\n" page-content)))
>        ;; (message "latest=%S; files=%S" latest files)
>        (unless (< (length files) (if (zerop (length latest)) 1 2))
>          (insert (format "<h2>Old versions</h2><table>\n"))

Hmm... this function is already too long, so please move this to
a separate function.

After tweaking your code a bit to get it to work (see patch below), I'm
still not getting the HTML I expect (the "HTML" doc only contains
Debian Emacs's extraneous init messages but not the actual HTML from
org-export-as, even though running the Emacs command by hand does
output it, I haven't had time to dig deeper into the problem).

One other thing.  I think it would make sense to make
`elpaa--get-section` return a pair of the string content and some "type"
(org, markdown, plaintext, ...) so as to automatically use your HTML
renderer for any .org file we find instead of only when the org file is
specified explicitly via `:readme "README.org"`.  WDYT?


        Stefan