unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Adam Porter <adam@alphapapa.net>
To: emacs-devel@gnu.org
Subject: Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
Date: Tue, 07 Sep 2021 05:26:31 -0500	[thread overview]
Message-ID: <87ilzc65rc.fsf@alphapapa.net> (raw)
In-Reply-To: jwv5yvd3xof.fsf-monnier+emacs@gnu.org

[-- Attachment #1: Type: text/plain, Size: 6937 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> 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:

No problem.  Thanks.

>> +(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.

Oops, I wrongly assumed that it was just like `elpaa--call', but
sandboxed.  :)  I removed the check for the exit code, so
`elpaa--call-sandboxed' can signal an error when it checks the code
itself.

> `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.

Apologies for not noticing that.  I've changed it to write to and read
from a temp file.

> 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).

Done.

> [ 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.  ]

Sometimes Org changes can be annoying to compensate for.  Hopefully the
org-export infrastructure won't change too much.  I tested this on Emacs
26.3 and it seems to work.

>> @@ -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.

Oops, fixed.

>> +                 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.

I don't disagree, but I'm not sure what you have in mind.  Since this
code also writes to the temp buffer made by the containing function,
`elpaa--html-make-pkg', it would seem awkward to me to move that code
as-is to another function that would both write to a file and insert
into the current buffer, but maybe that's not what you meant, or maybe I
just haven't learned your preferred style.  :)

> 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).

I guess you forgot to attach the patch, but maybe the one I'm attaching
fixes that problem, anyway.  :)

> 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?

I'm not sure what you have in mind.  I felt confused by that function's
signature and it's "overloading", shall we say, in that it may return a
file's contents or a section's contents.  In the patched version of
`elpaa--html-make-pkg', I tried to label the logic to make it easier for
me to follow.

Anyway, here's an updated patch that I hope will basically work
correctly on your end.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-elpa-admin.el-Export-Org-readmes-to-ASCII-and-HTML.patch --]
[-- Type: text/x-diff, Size: 5175 bytes --]

From 05ccce75900f42d28aaf8dff241e22d146918bbf Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Sun, 29 Aug 2021 17:45:22 -0500
Subject: [PATCH] * elpa-admin.el: Export Org readmes to ASCII and HTML

(elpaa--export-org): New function.
(elpaa--org-export-options): New variable.

(elpaa--html-make-pkg): Export Org readmes to HTML and plain-text, and
other readme formats to plain-text.

(elpaa--get-README): Remove function.
---
 elpa-admin.el | 70 +++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 56 insertions(+), 14 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index ac72f2f..c38d69a 100644
--- a/elpa-admin.el
+++ b/elpa-admin.el
@@ -69,6 +69,11 @@ to be installed and has only been tested on some Debian systems.")
 
 (defvar elpaa--debug nil)
 
+(defvar elpaa--org-export-options
+  '(:with-author nil :with-creator nil :with-broken-links t)
+  "Options used common to all Org export backends.
+See variable `org-export-options-alist'.")
+
 (unless (fboundp 'ignore-error)
   (defmacro ignore-error (condition &rest body)
     `(condition-case nil (progn ,@body) (,condition nil))))
@@ -1209,15 +1214,30 @@ return section under HEADER in package's main file."
           (delete-region (point) (point-max))
           (buffer-string)))))))
 
-(defun elpaa--get-README (pkg-spec dir)
-  (elpaa--get-section
-   "Commentary" (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"))
-   dir pkg-spec))
+(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")
+  ;; "emacs --batch" loads site-init files, which may pollute output,
+  ;; so we write it to a temp file.
+  (let ((output-filename (make-temp-file "elpaa--export-org-")))
+    (unwind-protect
+        (progn
+          (with-temp-buffer
+            (elpaa--call-sandboxed
+             t "emacs" "--batch" "-l" (format "ox-%S" backend)
+             file
+             "--eval" (format "(write-region (org-export-as '%s nil nil %S '%S) nil %S)"
+                              backend body-only ext-plist output-filename)))
+          (with-temp-buffer
+            (insert-file-contents output-filename)
+            (buffer-string)))
+      (delete-file output-filename))))
 
 (defun elpaa--get-NEWS (pkg-spec dir)
   (let ((text
@@ -1313,11 +1333,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
+                                 srcdir pkg-spec)
+                 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"))
-- 
2.7.4


  parent reply	other threads:[~2021-09-07 10:26 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-29 22:52 [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii Adam Porter
2021-08-29 23:28 ` Adam Porter
2021-08-29 23:38 ` Clément Pit-Claudel
2021-08-30  0:01   ` Adam Porter
2021-08-30  1:49     ` Clément Pit-Claudel
2021-08-30  2:15       ` Adam Porter
2021-08-30  0:48 ` Stefan Monnier
2021-08-30  1:29   ` Adam Porter
2021-08-30  2:13   ` [ELPA/elpa-admin] Render README.org as HTML with ox-html Adam Porter
2021-09-03  2:01     ` Adam Porter
2021-09-07  3:31       ` Stefan Monnier
2021-09-07  8:12         ` Philip Kaludercic
2021-09-07 10:26         ` Adam Porter [this message]
2021-09-10 20:58           ` Stefan Monnier
2021-09-12 13:03             ` Adam Porter
2021-09-20  4:29               ` Stefan Monnier
2021-09-20  6:41                 ` Stefan Kangas
2021-09-20 13:40                   ` Basil L. Contovounesios
2021-09-20 19:57                   ` Adam Porter
2021-09-20 23:26                 ` Adam Porter
2021-10-09 15:08                   ` Stefan Monnier
2021-10-09 16:39                     ` Eric Abrahamsen
2021-10-10  3:37                       ` Stefan Monnier
2021-10-10  3:54                         ` Corwin Brust
2021-10-10 13:27                           ` Stefan Monnier
2021-10-10  4:32                         ` Eric Abrahamsen
2021-10-10 14:50                     ` Adam Porter
2021-10-10 15:30                       ` Stefan Monnier
2021-08-30 17:49   ` [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii Philip Kaludercic

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.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ilzc65rc.fsf@alphapapa.net \
    --to=adam@alphapapa.net \
    --cc=emacs-devel@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.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).