unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii
@ 2021-08-29 22:52 Adam Porter
  2021-08-29 23:28 ` Adam Porter
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Adam Porter @ 2021-08-29 22:52 UTC (permalink / raw)
  To: emacs-devel

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

Hi Stefan, et al,

Having added taxy.el to ELPA, I noticed that its README.org file isn't
very readable on the ELPA site, because it's rendered as a raw file,
including long lines that extend beyond the edge of the HTML PRE block,
raw Org-syntax, etc.

Thankfully, Org has an ASCII/UTF-8 export backend that cleanly renders
Org to plain text.  It only took a few lines of to make use of it.
Please see the attached patches.  (While I was at it, I took the liberty
of adding a couple of docstrings and renaming a few variables to help me
understand the code.)

Please note, I haven't built the whole ELPA repo to test this change,
but I tested the code on my taxy.el readme, and it returns a string like
the old function did, so it "should work."  :)

In the long run, as the TODO in the commentary says, it would be good to
render as HTML, but this is a big improvement for a very small change in
the code, so a step in the right direction.

Thanks,
Adam

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-elpa-admin.el-elpaa-get-section-Add-docstring-rename.patch --]
[-- Type: text/x-diff, Size: 2331 bytes --]

From 85c9d3070c6d87695f57613ed51a43e7ebd5bafa Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Sun, 29 Aug 2021 17:35:59 -0500
Subject: [PATCH 1/2]  * elpa-admin.el (elpaa--get-section): Add docstring,
 rename vars

---
 elpa-admin.el | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index 4c84360..ac72f2f 100644
--- a/elpa-admin.el
+++ b/elpa-admin.el
@@ -1167,25 +1167,28 @@ Rename DIR/ to PKG-VERS/, and return the descriptor."
          (insert-file-contents mainsrcfile)
          (lm-header prop))))))
 
-(defun elpaa--get-section (hsection fsection srcdir pkg-spec)
-  (when (consp fsection)
-    (while (cdr-safe fsection)
-      (setq fsection
-            (if (file-readable-p (expand-file-name (car fsection) srcdir))
-                (car fsection)
-              (cdr fsection))))
-    (when (consp fsection) (setq fsection (car fsection))))
+(defun elpaa--get-section (header file srcdir pkg-spec)
+  "Return specified section as a string from SRCDIR for PKG-SPEC.
+If FILE is readable in SRCDIR, return its contents.  Otherwise
+return section under HEADER in package's main file."
+  (when (consp file)
+    (while (cdr-safe file)
+      (setq file
+            (if (file-readable-p (expand-file-name (car file) srcdir))
+                (car file)
+              (cdr file))))
+    (when (consp file) (setq file (car file))))
   (cond
-   ((file-readable-p (expand-file-name fsection srcdir))
+   ((file-readable-p (expand-file-name file srcdir))
     (with-temp-buffer
-      (insert-file-contents (expand-file-name fsection srcdir))
+      (insert-file-contents (expand-file-name file srcdir))
       (buffer-string)))
    ((file-readable-p (expand-file-name (elpaa--main-file pkg-spec) srcdir))
     (with-temp-buffer
       (insert-file-contents
        (expand-file-name (elpaa--main-file pkg-spec) srcdir))
-      (emacs-lisp-mode)       ;lm-section-start needs the outline-mode setting.
-      (let ((start (lm-section-start hsection)))
+      (emacs-lisp-mode) ;lm-section-start needs the outline-mode setting.
+      (let ((start (lm-section-start header)))
         (when start
           ;; FIXME: Emacs<28 had a bug in `lm-section-end', so cook up
           ;; our own ad-hoc replacement.
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-elpa-admin.el-elpaa-get-README-Docstring-export-Org-.patch --]
[-- Type: text/x-diff, Size: 2009 bytes --]

From 7e34b7fb6396513560ea7e2994ee73d3e2a9820a Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Sun, 29 Aug 2021 17:45:22 -0500
Subject: [PATCH 2/2] * elpa-admin.el (elpaa--get-README): Docstring, export
 Org readmes

Exports "README.org" files using ox-ascii.el, which is more readable
on the ELPA Web site.
---
 elpa-admin.el | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index ac72f2f..dfe56f0 100644
--- a/elpa-admin.el
+++ b/elpa-admin.el
@@ -1210,14 +1210,27 @@ return section under HEADER in package's main file."
           (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))
+  "Return readme for PKG-SPEC in DIR as a string.
+If readme is an Org file, render it to plain-text using Org
+Export."
+  (let ((readme-file
+         (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"))))
+    (pcase readme-file
+      ("README.org"
+       (require 'org)
+       (require 'ox)
+       (with-temp-buffer
+         (insert-file-contents readme-file)
+         (org-export-as 'ascii nil nil nil (:ascii-charset utf-8 :with-broken-links t)))
+       )
+      (_ (elpaa--get-section
+          "Commentary" readme-file
+          dir pkg-spec)))))
 
 (defun elpaa--get-NEWS (pkg-spec dir)
   (let ((text
-- 
2.7.4


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii
  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:48 ` Stefan Monnier
  2 siblings, 0 replies; 21+ messages in thread
From: Adam Porter @ 2021-08-29 23:28 UTC (permalink / raw)
  To: emacs-devel

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

Someday I will learn to send patches without simple mistakes on the
first try.  Until then, here's a corrected version of the second patch
(and I'll re-attach the first for simplicity's sake).

Thanks,
Adam

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-elpa-admin.el-elpaa-get-section-Add-docstring-rename.patch --]
[-- Type: text/x-diff, Size: 2331 bytes --]

From 85c9d3070c6d87695f57613ed51a43e7ebd5bafa Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Sun, 29 Aug 2021 17:35:59 -0500
Subject: [PATCH 1/2]  * elpa-admin.el (elpaa--get-section): Add docstring,
 rename vars

---
 elpa-admin.el | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index 4c84360..ac72f2f 100644
--- a/elpa-admin.el
+++ b/elpa-admin.el
@@ -1167,25 +1167,28 @@ Rename DIR/ to PKG-VERS/, and return the descriptor."
          (insert-file-contents mainsrcfile)
          (lm-header prop))))))
 
-(defun elpaa--get-section (hsection fsection srcdir pkg-spec)
-  (when (consp fsection)
-    (while (cdr-safe fsection)
-      (setq fsection
-            (if (file-readable-p (expand-file-name (car fsection) srcdir))
-                (car fsection)
-              (cdr fsection))))
-    (when (consp fsection) (setq fsection (car fsection))))
+(defun elpaa--get-section (header file srcdir pkg-spec)
+  "Return specified section as a string from SRCDIR for PKG-SPEC.
+If FILE is readable in SRCDIR, return its contents.  Otherwise
+return section under HEADER in package's main file."
+  (when (consp file)
+    (while (cdr-safe file)
+      (setq file
+            (if (file-readable-p (expand-file-name (car file) srcdir))
+                (car file)
+              (cdr file))))
+    (when (consp file) (setq file (car file))))
   (cond
-   ((file-readable-p (expand-file-name fsection srcdir))
+   ((file-readable-p (expand-file-name file srcdir))
     (with-temp-buffer
-      (insert-file-contents (expand-file-name fsection srcdir))
+      (insert-file-contents (expand-file-name file srcdir))
       (buffer-string)))
    ((file-readable-p (expand-file-name (elpaa--main-file pkg-spec) srcdir))
     (with-temp-buffer
       (insert-file-contents
        (expand-file-name (elpaa--main-file pkg-spec) srcdir))
-      (emacs-lisp-mode)       ;lm-section-start needs the outline-mode setting.
-      (let ((start (lm-section-start hsection)))
+      (emacs-lisp-mode) ;lm-section-start needs the outline-mode setting.
+      (let ((start (lm-section-start header)))
         (when start
           ;; FIXME: Emacs<28 had a bug in `lm-section-end', so cook up
           ;; our own ad-hoc replacement.
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-elpa-admin.el-elpaa-get-README-Docstring-export-Org-.patch --]
[-- Type: text/x-diff, Size: 2010 bytes --]

From d8faeb64a954a238debe2c83f47c6fd7dcca411d Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Sun, 29 Aug 2021 17:45:22 -0500
Subject: [PATCH 2/2] * elpa-admin.el (elpaa--get-README): Docstring, export
 Org readmes

Exports "README.org" files using ox-ascii.el, which is more readable
on the ELPA Web site.
---
 elpa-admin.el | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index ac72f2f..608dea2 100644
--- a/elpa-admin.el
+++ b/elpa-admin.el
@@ -1210,14 +1210,27 @@ return section under HEADER in package's main file."
           (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))
+  "Return readme for PKG-SPEC in DIR as a string.
+If readme is an Org file, render it to plain-text using Org
+Export."
+  (let ((readme-file
+         (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"))))
+    (pcase readme-file
+      ("README.org"
+       (require 'org)
+       (require 'ox)
+       (with-temp-buffer
+         (insert-file-contents readme-file)
+         (org-export-as 'ascii nil nil nil '(:ascii-charset utf-8 :with-broken-links t)))
+       )
+      (_ (elpaa--get-section
+          "Commentary" readme-file
+          dir pkg-spec)))))
 
 (defun elpaa--get-NEWS (pkg-spec dir)
   (let ((text
-- 
2.7.4


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii
  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  0:48 ` Stefan Monnier
  2 siblings, 1 reply; 21+ messages in thread
From: Clément Pit-Claudel @ 2021-08-29 23:38 UTC (permalink / raw)
  To: emacs-devel

On 8/29/21 6:52 PM, Adam Porter wrote:
> Hi Stefan, et al,
> 
> Having added taxy.el to ELPA, I noticed that its README.org file isn't
> very readable on the ELPA site, because it's rendered as a raw file,
> including long lines that extend beyond the edge of the HTML PRE block,
> raw Org-syntax, etc.
> 
> Thankfully, Org has an ASCII/UTF-8 export backend that cleanly renders
> Org to plain text.  It only took a few lines of to make use of it.
> Please see the attached patches.  (While I was at it, I took the liberty
> of adding a couple of docstrings and renaming a few variables to help me
> understand the code.)

How much does security matter in this case?  AFAIR exporting an Org file can run arbitrary code; would this patch allow a package in ELPA to subvert the build process of another package? 

And if so, is that a problem, or is there sufficient scrutiny of the inputs to ELPA?  IIRC any package author can push to ELPA and updates will propagate immediately, so the worry would be that in the time between the introduction of a worm and its detection a large number of end users might install bad code.

Clément.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii
  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
  0 siblings, 1 reply; 21+ messages in thread
From: Adam Porter @ 2021-08-30  0:01 UTC (permalink / raw)
  To: emacs-devel

Clément Pit-Claudel <cpitclaudel@gmail.com> writes:

> On 8/29/21 6:52 PM, Adam Porter wrote:
>> Hi Stefan, et al,
>> 
>> Having added taxy.el to ELPA, I noticed that its README.org file isn't
>> very readable on the ELPA site, because it's rendered as a raw file,
>> including long lines that extend beyond the edge of the HTML PRE block,
>> raw Org-syntax, etc.
>> 
>> Thankfully, Org has an ASCII/UTF-8 export backend that cleanly renders
>> Org to plain text.  It only took a few lines of to make use of it.
>> Please see the attached patches.  (While I was at it, I took the liberty
>> of adding a couple of docstrings and renaming a few variables to help me
>> understand the code.)
>
> How much does security matter in this case?  AFAIR exporting an Org
> file can run arbitrary code; would this patch allow a package in ELPA
> to subvert the build process of another package?
>
> And if so, is that a problem, or is there sufficient scrutiny of the
> inputs to ELPA?  IIRC any package author can push to ELPA and updates
> will propagate immediately, so the worry would be that in the time
> between the introduction of a worm and its detection a large number of
> end users might install bad code.

Hi Clément,

Yes, that's a good question.  I don't know if I can fully answer it,
myself.

I would guess that those who have commit access to ELPA are considered
trusted, and regardless of potentially using Org Export while building
packages, those committers could already add malicious code that could
end up being distributed to users until someone noticed and reverted the
changes.

Also, AFAIU, ELPA already runs Makefiles for packages as part of the
build process, and those can run arbitrary code, which I guess could do
things like modify other packages, modify the build process or scripts,
or anything else that the user account the build process runs as could
do on the server.

So, based on my understanding, this change wouldn't make the build
process less secure, per se.  One might say that it could offer a new
way in which to obfuscate or hide malicious code, but I'd guess that,
since we already seem to trust the people who can commit to ELPA now,
this wouldn't change the status quo.

Please let me know if I'm missing something.  :)  Thanks.




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii
  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:48 ` Stefan Monnier
  2021-08-30  1:29   ` Adam Porter
                     ` (2 more replies)
  2 siblings, 3 replies; 21+ messages in thread
From: Stefan Monnier @ 2021-08-30  0:48 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

> From 85c9d3070c6d87695f57613ed51a43e7ebd5bafa Mon Sep 17 00:00:00 2001
> From: Adam Porter <adam@alphapapa.net>
> Date: Sun, 29 Aug 2021 17:35:59 -0500
> Subject: [PATCH 1/2]  * elpa-admin.el (elpaa--get-section): Add docstring,
>  rename vars

Thanks, looks good, pushed.

> From 7e34b7fb6396513560ea7e2994ee73d3e2a9820a Mon Sep 17 00:00:00 2001
> From: Adam Porter <adam@alphapapa.net>
> Date: Sun, 29 Aug 2021 17:45:22 -0500
> Subject: [PATCH 2/2] * elpa-admin.el (elpaa--get-README): Docstring, export
>  Org readmes

I hope we can generate HTML rather than "clean text", but in the mean
time, clean text is better than raw source, so: thanks.

There's one problem with your patch, tho: `org-export-as` can run
arbitrary code so we want to run that in a sandbox (and hence in
a separate Emacs process).


        Stefan




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii
  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-08-30 17:49   ` [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii Philip Kaludercic
  2 siblings, 0 replies; 21+ messages in thread
From: Adam Porter @ 2021-08-30  1:29 UTC (permalink / raw)
  To: emacs-devel

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

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

> I hope we can generate HTML rather than "clean text", but in the mean
> time, clean text is better than raw source, so: thanks.

Yes, that shouldn't be a big deal, I think, but it might require a
little fiddling to integrate the Org-generated HTML into the
ELPA-generated HTML.

> There's one problem with your patch, tho: `org-export-as` can run
> arbitrary code so we want to run that in a sandbox (and hence in
> a separate Emacs process).

Oops, I didn't realize that ELPA already had sandbox functions.  You're
way ahead of me.  :)  Please see the attached patch, which uses
elpaa--call-sandboxed.


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

From da325b9ef1f9e3ae139256b33a5da7e19eead4b8 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Sun, 29 Aug 2021 17:45:22 -0500
Subject: [PATCH 2/2] * elpa-admin.el: Export Org readmes to readable text

Exports "README.org" files using ox-ascii.el, which is more readable
on the ELPA Web site.

(elpaa--export-org): New function.
(elpaa--get-README): Add docstring, call new function.
---
 elpa-admin.el | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index ac72f2f..3682bec 100644
--- a/elpa-admin.el
+++ b/elpa-admin.el
@@ -1210,14 +1210,36 @@ return section under HEADER in package's main file."
           (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))
+  "Return readme for PKG-SPEC in DIR as a string.
+If readme is an Org file, render it to plain-text using Org
+Export."
+  (let ((readme-file
+         (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"))))
+    (pcase readme-file
+      ("README.org"
+       (elpaa--export-org readme-file 'ascii '(:ascii-charset utf-8 :with-broken-links t)))
+      (_ (elpaa--get-section
+          "Commentary" readme-file
+          dir pkg-spec)))))
+
+(defun elpaa--export-org (file backend &optional 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."
+  (with-temp-buffer
+    (unless (zerop (elpaa--call-sandboxed
+                    t "emacs" "--batch" "-l" "ox-ascii"
+                    file
+                    "--eval" (format "(message \"%%s\" (org-export-as '%s nil nil nil '%S))"
+                                     backend ext-plist)))
+      (error "Unable to export Org file: %S" file))
+    (buffer-string)))
 
 (defun elpaa--get-NEWS (pkg-spec dir)
   (let ((text
-- 
2.7.4


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii
  2021-08-30  0:01   ` Adam Porter
@ 2021-08-30  1:49     ` Clément Pit-Claudel
  2021-08-30  2:15       ` Adam Porter
  0 siblings, 1 reply; 21+ messages in thread
From: Clément Pit-Claudel @ 2021-08-30  1:49 UTC (permalink / raw)
  To: emacs-devel

On 8/29/21 8:01 PM, Adam Porter wrote:
> I would guess that those who have commit access to ELPA are considered
> trusted, and regardless of potentially using Org Export while building
> packages, those committers could already add malicious code that could
> end up being distributed to users until someone noticed and reverted the
> changes.

The scary part is not so much altering a package (or a few packages) with bad code (though that is scary), but having the ability to alter all of them (sure, you could push to all package branches, but that's more easily detected that altering one readme).

> Also, AFAIU, ELPA already runs Makefiles for packages as part of the
> build process, and those can run arbitrary code, which I guess could do
> things like modify other packages, modify the build process or scripts,
> or anything else that the user account the build process runs as could
> do on the server.

Good catch, and indeed given this running org doesn't make things worse.  Thanks.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  2021-08-30  0:48 ` Stefan Monnier
  2021-08-30  1:29   ` Adam Porter
@ 2021-08-30  2:13   ` Adam Porter
  2021-09-03  2:01     ` Adam Porter
  2021-08-30 17:49   ` [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii Philip Kaludercic
  2 siblings, 1 reply; 21+ messages in thread
From: Adam Porter @ 2021-08-30  2:13 UTC (permalink / raw)
  To: emacs-devel

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

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

> I hope we can generate HTML rather than "clean text", but in the mean
> time, clean text is better than raw source, so: thanks.

Building on the previous patches, here's my attempt at exporting Org to
HTML.  I don't know if it accounts for everything that should be
accounted for (there's much I don't understand about the build process),
so it might need a few changes, but I think it should basically work.


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

From d54be5068756e2a4845e866332c82508512e3145 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Sun, 29 Aug 2021 17:45:22 -0500
Subject: [PATCH 1/2] * elpa-admin.el: Export Org readmes to readable text

Exports "README.org" files using ox-ascii.el, which is more readable
on the ELPA Web site.

(elpaa--export-org): New function.
(elpaa--get-README): Add docstring, call new function.
---
 elpa-admin.el | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index ac72f2f..3682bec 100644
--- a/elpa-admin.el
+++ b/elpa-admin.el
@@ -1210,14 +1210,36 @@ return section under HEADER in package's main file."
           (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))
+  "Return readme for PKG-SPEC in DIR as a string.
+If readme is an Org file, render it to plain-text using Org
+Export."
+  (let ((readme-file
+         (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"))))
+    (pcase readme-file
+      ("README.org"
+       (elpaa--export-org readme-file 'ascii '(:ascii-charset utf-8 :with-broken-links t)))
+      (_ (elpaa--get-section
+          "Commentary" readme-file
+          dir pkg-spec)))))
+
+(defun elpaa--export-org (file backend &optional 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."
+  (with-temp-buffer
+    (unless (zerop (elpaa--call-sandboxed
+                    t "emacs" "--batch" "-l" "ox-ascii"
+                    file
+                    "--eval" (format "(message \"%%s\" (org-export-as '%s nil nil nil '%S))"
+                                     backend ext-plist)))
+      (error "Unable to export Org file: %S" file))
+    (buffer-string)))
 
 (defun elpaa--get-NEWS (pkg-spec dir)
   (let ((text
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-elpa-admin.el-Export-Org-to-HTML-content-and-files.patch --]
[-- Type: text/x-diff, Size: 4303 bytes --]

From 4c7d4b5c1de11f80f411f7f2e7e423a0b893f7a5 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Sun, 29 Aug 2021 21:03:37 -0500
Subject: [PATCH 2/2] * elpa-admin.el: Export Org to HTML content and files

(elpaa--export-org): Export to specified backend, optionally with
body-only.

(elpaa--get-README): Use elpaa--export-org to export HTML.

(elpaa--html-make-pkg): Export Org readmes to HTML, and other readme
formats to plain text.
---
 elpa-admin.el | 46 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index 3682bec..791bc67 100644
--- a/elpa-admin.el
+++ b/elpa-admin.el
@@ -1222,22 +1222,27 @@ Export."
                             "README.org"))))
     (pcase readme-file
       ("README.org"
-       (elpaa--export-org readme-file 'ascii '(:ascii-charset utf-8 :with-broken-links t)))
+       (elpaa--export-org
+        readme-file 'html
+        :body-only t :ext-plist '(:ascii-charset utf-8 :with-broken-links t)))
       (_ (elpaa--get-section
           "Commentary" readme-file
           dir pkg-spec)))))
 
-(defun elpaa--export-org (file backend &optional ext-plist)
+(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."
+  (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" "ox-ascii"
+                    t "emacs" "--batch" "-l" (format "ox-%s" backend)
                     file
-                    "--eval" (format "(message \"%%s\" (org-export-as '%s nil nil nil '%S))"
-                                     backend ext-plist)))
+                    "--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)))
 
@@ -1335,10 +1340,33 @@ arbitrary code."
       (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)
+      (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 readme-content page-content)
+        (pcase readme-file-name
+          ("README.org"
+           (setf output-filename (concat name "-readme.html")
+                 readme-content (elpaa--export-org
+                                 readme-file-name 'html
+                                 :ext-plist '(:with-broken-links t))
+                 page-content (elpaa--export-org
+                               readme-file-name 'html
+                               :body-only t :ext-plist '(:with-broken-links t))))
+          (_ ;; Non-Org readme.
+           (setf output-filename (concat name "-readme.txt")
+                 readme-content (elpaa--get-section
+                                 "Commentary" readme-file-name
+                                 dir pkg-spec)
+                 page-content (when readme-content
+                                (elpaa--html-quote readme-content)))))
+        (when readme-content
+          (write-region readme-content nil output-filename)
+          (insert "<h2>Full description</h2><pre>\n" page-content
                   "\n</pre>\n")))
       ;; (message "latest=%S; files=%S" latest files)
       (unless (< (length files) (if (zerop (length latest)) 1 2))
-- 
2.7.4


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii
  2021-08-30  1:49     ` Clément Pit-Claudel
@ 2021-08-30  2:15       ` Adam Porter
  0 siblings, 0 replies; 21+ messages in thread
From: Adam Porter @ 2021-08-30  2:15 UTC (permalink / raw)
  To: emacs-devel

Clément Pit-Claudel <cpitclaudel@gmail.com> writes:

> The scary part is not so much altering a package (or a few packages)
> with bad code (though that is scary), but having the ability to alter
> all of them (sure, you could push to all package branches, but that's
> more easily detected that altering one readme).

Yes, we should be very careful about that, and I'm glad people like you
and Stefan are keeping it in mind.  :)  In fact...

>> Also, AFAIU, ELPA already runs Makefiles for packages as part of the
>> build process, and those can run arbitrary code, which I guess could do
>> things like modify other packages, modify the build process or scripts,
>> or anything else that the user account the build process runs as could
>> do on the server.
>
> Good catch, and indeed given this running org doesn't make things
> worse.  Thanks.

As Stefan mentioned, it appears that he's is way ahead of both of us, as
he's already implemented some sandboxing in the build process.  :)




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii
  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-08-30 17:49   ` Philip Kaludercic
  2 siblings, 0 replies; 21+ messages in thread
From: Philip Kaludercic @ 2021-08-30 17:49 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Adam Porter, emacs-devel

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

>> From 85c9d3070c6d87695f57613ed51a43e7ebd5bafa Mon Sep 17 00:00:00 2001
>> From: Adam Porter <adam@alphapapa.net>
>> Date: Sun, 29 Aug 2021 17:35:59 -0500
>> Subject: [PATCH 1/2]  * elpa-admin.el (elpaa--get-section): Add docstring,
>>  rename vars
>
> Thanks, looks good, pushed.
>
>> From 7e34b7fb6396513560ea7e2994ee73d3e2a9820a Mon Sep 17 00:00:00 2001
>> From: Adam Porter <adam@alphapapa.net>
>> Date: Sun, 29 Aug 2021 17:45:22 -0500
>> Subject: [PATCH 2/2] * elpa-admin.el (elpaa--get-README): Docstring, export
>>  Org readmes
>
> I hope we can generate HTML rather than "clean text", but in the mean
> time, clean text is better than raw source, so: thanks.

I think that raw rendering would also be good for C-h P, unless Emacs
renders the documentation using eww or something.

> There's one problem with your patch, tho: `org-export-as` can run
> arbitrary code so we want to run that in a sandbox (and hence in
> a separate Emacs process).
>
>
>         Stefan

-- 
	Philip Kaludercic



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  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
  0 siblings, 1 reply; 21+ messages in thread
From: Adam Porter @ 2021-09-03  2:01 UTC (permalink / raw)
  To: emacs-devel

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

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.

[-- 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: 4853 bytes --]

From 60c959ddfad184988be74c1ed32f1759d1a24610 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.
---
 elpa-admin.el | 63 ++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 14 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index ac72f2f..40a355f 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,23 @@ 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")
+  (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)))
 
 (defun elpaa--get-NEWS (pkg-spec dir)
   (let ((text
@@ -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)
+                 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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  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
  0 siblings, 2 replies; 21+ messages in thread
From: Stefan Monnier @ 2021-09-07  3:31 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

> 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




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  2021-09-07  3:31       ` Stefan Monnier
@ 2021-09-07  8:12         ` Philip Kaludercic
  2021-09-07 10:26         ` Adam Porter
  1 sibling, 0 replies; 21+ messages in thread
From: Philip Kaludercic @ 2021-09-07  8:12 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Adam Porter, emacs-devel

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

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

There seems to be no patch attached to your message?

-- 
	Philip Kaludercic



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  2021-09-07  3:31       ` Stefan Monnier
  2021-09-07  8:12         ` Philip Kaludercic
@ 2021-09-07 10:26         ` Adam Porter
  2021-09-10 20:58           ` Stefan Monnier
  1 sibling, 1 reply; 21+ messages in thread
From: Adam Porter @ 2021-09-07 10:26 UTC (permalink / raw)
  To: emacs-devel

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  2021-09-07 10:26         ` Adam Porter
@ 2021-09-10 20:58           ` Stefan Monnier
  2021-09-12 13:03             ` Adam Porter
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Monnier @ 2021-09-10 20:58 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

Hi Adam,

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

[ No need for apologies, really.  ]

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

Great, thanks.

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

I didn't have anything specific in mind, sorry.

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

Hmm... so now I really have to read and understand your code, eh?  ;-)

OK, I see what you mean now.  Indeed, we need to get both
a plaintext version (for `<pkg>-readme.txt`) and an HTML version (to
insert into the temp buffer).  See below.

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

Thanks, indeed.

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

We want to support packages where the "readme" is either inside a file
or inside a `Commentary:` section (and same thing for the news which can
be in a file or in the `News:` section), which is why we have this
function with this kind of "overloading".

Currently regardless of its origin (file or section) the content is
taken to be "plain text".  I'd like to extend this to allow not just
`foo.org` files (as your patch does) but also allow the use of Org
format in sections (maybe by calling it `Commentary.org:`).  It's not
necessary to provide this support right now, but we can keep
`elpaa--get-section` more or less as it is now, just extend it to return
(TYPE . CONTENT).  Currently the TYPE would be `text/plain` in all cases
except when the content comes from a file and the file's name ends in
`.org`.

And we'd add 2 new functions: one that converts (TYPE . CONTENT) to
plain text (i.e. trivial when the type is `text/plain`) and
another that converts it to HTML (using `elpaa--html-quote` for the
`text/plain` case).

Does it make more sense?


        Stefan




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  2021-09-10 20:58           ` Stefan Monnier
@ 2021-09-12 13:03             ` Adam Porter
  2021-09-20  4:29               ` Stefan Monnier
  0 siblings, 1 reply; 21+ messages in thread
From: Adam Porter @ 2021-09-12 13:03 UTC (permalink / raw)
  To: emacs-devel

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

Hi Stefan,

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

> We want to support packages where the "readme" is either inside a file
> or inside a `Commentary:` section (and same thing for the news which can
> be in a file or in the `News:` section), which is why we have this
> function with this kind of "overloading".
>
> Currently regardless of its origin (file or section) the content is
> taken to be "plain text".  I'd like to extend this to allow not just
> `foo.org` files (as your patch does) but also allow the use of Org
> format in sections (maybe by calling it `Commentary.org:`).  It's not
> necessary to provide this support right now, but we can keep
> `elpaa--get-section` more or less as it is now, just extend it to return
> (TYPE . CONTENT).  Currently the TYPE would be `text/plain` in all cases
> except when the content comes from a file and the file's name ends in
> `.org`.
>
> And we'd add 2 new functions: one that converts (TYPE . CONTENT) to
> plain text (i.e. trivial when the type is `text/plain`) and
> another that converts it to HTML (using `elpaa--html-quote` for the
> `text/plain` case).
>
> Does it make more sense?

Thanks, now I have a better idea of what you want.  Hopefully this patch
is close to what you have in mind.

[-- 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: 10784 bytes --]

From b09569c310ecb242e918a345fe5ee69b8ac00973 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, elpaa--section-to-plain-text,
elpaa--section-to-html): New functions.

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

(elpaa--get-section): Return (TYPE . CONTENT) cons.

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

(elpaa--get-README): Remove function.

See discussion at
<https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg00108.html>.

Thanks to Stefan Monnier for his guidance.
---
 elpa-admin.el | 180 +++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 135 insertions(+), 45 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index ac72f2f..19a787e 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))))
@@ -1167,10 +1172,57 @@ Rename DIR/ to PKG-VERS/, and return the descriptor."
          (insert-file-contents mainsrcfile)
          (lm-header prop))))))
 
+(defun elpaa--section-to-plain-text (section)
+  "Return SECTION as plain text.
+SECTION should be a cons as returned by `elpaa--get-section',
+which see.  If SECTION's type is \"text/plain\" or
+\"text/markdown\", its contents are returned as-is.  If
+\"application/x-org\", its contents are exported to UTF-8 plain
+text with `elpaa--export-org', which see."
+  (pcase-exhaustive section
+    (`(,(or "text/plain" "text/markdown") . ,content)
+     content)
+    (`("text/x-org" . ,content)
+     (let ((temp-file (make-temp-file "elpaa--section-to-plain-text--")))
+       (unwind-protect
+           (progn
+             (with-temp-file temp-file
+               (insert content))
+             (elpaa--export-org temp-file 'ascii
+               :ext-plist (append '(:ascii-charset utf-8)
+                                  elpaa--org-export-options)))
+         (delete-file temp-file))))))
+
+(defun elpaa--section-to-html (section)
+  "Return SECTION as HTML.
+SECTION should be a cons as returned by `elpaa--get-section',
+which see.  If SECTION's type is \"text/plain\" or
+\"text/markdown\", its contents are escaped with
+`elpaa--html-quote' and wrapped in HTML PRE tags.  If
+\"application/x-org\", its contents are exported to HTML with
+`elpaa--export-org', which see."
+  (pcase-exhaustive section
+    (`(,(or "text/plain" "text/markdown") . ,content)
+     (concat "<pre>\n"
+             (elpaa--html-quote content)
+             "\n</pre>\n"))
+    (`("text/x-org" . ,content)
+     (let ((temp-file (make-temp-file "elpaa--section-to-html--")))
+       (unwind-protect
+           (progn
+             (with-temp-file temp-file
+               (insert content))
+             (elpaa--export-org temp-file 'html
+               :body-only t
+               :ext-plist elpaa--org-export-options))
+         (delete-file temp-file))))))
+
 (defun elpaa--get-section (header file srcdir pkg-spec)
-  "Return specified section as a string from SRCDIR for PKG-SPEC.
-If FILE is readable in SRCDIR, return its contents.  Otherwise
-return section under HEADER in package's main file."
+  "Return specified section for PKG-SPEC.
+Returns (TYPE . CONTENT) cons, where TYPE is a MIME-type string,
+and CONTENT is the content string.  If FILE is readable in
+SRCDIR, return its contents.  Otherwise return section under
+HEADER in package's main file."
   (when (consp file)
     (while (cdr-safe file)
       (setq file
@@ -1180,44 +1232,73 @@ return section under HEADER in package's main file."
     (when (consp file) (setq file (car file))))
   (cond
    ((file-readable-p (expand-file-name file srcdir))
-    (with-temp-buffer
-      (insert-file-contents (expand-file-name file srcdir))
-      (buffer-string)))
+    ;; Return FILE's contents.
+    (let ((type
+           (pcase (mailcap-extension-to-mime (file-name-extension file))
+             ((and `nil
+                   (guard (member-ignore-case
+                           (file-name-extension file) '("md" "markdown"))))
+              ;; `mailcap-extension-to-mime' returns nil for Markdown
+              ;; files, at least on Emacs 26.3.
+              "text/markdown")
+             (else else)))
+          (content (with-temp-buffer
+                     (insert-file-contents (expand-file-name file srcdir))
+                     (buffer-string))))
+      (cons type content)))
    ((file-readable-p (expand-file-name (elpaa--main-file pkg-spec) srcdir))
-    (with-temp-buffer
-      (insert-file-contents
-       (expand-file-name (elpaa--main-file pkg-spec) srcdir))
-      (emacs-lisp-mode) ;lm-section-start needs the outline-mode setting.
-      (let ((start (lm-section-start header)))
-        (when start
-          ;; FIXME: Emacs<28 had a bug in `lm-section-end', so cook up
-          ;; our own ad-hoc replacement.
-          (goto-char start) (forward-line 1)
-          (re-search-forward "^\\(;;;[^;\n]\\|[^; \n]\\)" nil t)
-          (insert
-           (prog1
-               (buffer-substring start (match-beginning 0))
-             (erase-buffer)))
-          (emacs-lisp-mode)
-          (goto-char (point-min))
-          (delete-region (point) (line-beginning-position 2))
-          (uncomment-region (point-min) (point-max))
-          (when (looking-at "^\\([ \t]*\n\\)+")
-            (replace-match ""))
-          (goto-char (point-max))
-          (skip-chars-backward " \t\n")
-          (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))
+    ;; Return specified section from package's main source file.
+    (let ((type "text/plain")
+          (content (with-temp-buffer
+                     (insert-file-contents
+                      (expand-file-name (elpaa--main-file pkg-spec) srcdir))
+                     (emacs-lisp-mode) ;lm-section-start needs the outline-mode setting.
+                     (let ((start (lm-section-start header)))
+                       (when start
+                         ;; FIXME: Emacs<28 had a bug in `lm-section-end', so cook up
+                         ;; our own ad-hoc replacement.
+                         (goto-char start) (forward-line 1)
+                         (re-search-forward "^\\(;;;[^;\n]\\|[^; \n]\\)" nil t)
+                         (insert
+                          (prog1
+                              (buffer-substring start (match-beginning 0))
+                            (erase-buffer)))
+                         (emacs-lisp-mode)
+                         (goto-char (point-min))
+                         (delete-region (point) (line-beginning-position 2))
+                         (uncomment-region (point-min) (point-max))
+                         (when (looking-at "^\\([ \t]*\n\\)+")
+                           (replace-match ""))
+                         (goto-char (point-max))
+                         (skip-chars-backward " \t\n")
+                         (delete-region (point) (point-max))
+                         (buffer-string))))))
+      (cons type content)))))
+
+(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 +1394,20 @@ 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* ((package-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")))
+             (readme-section (elpaa--get-section "Commentary" package-readme-file-name
+                                                 srcdir pkg-spec))
+             (readme-content (elpaa--section-to-plain-text readme-section))
+             (page-content (elpaa--section-to-html readme-section))
+             (readme-output-filename (concat name "-readme.txt")))
+        (write-region readme-content nil readme-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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  2021-09-12 13:03             ` Adam Porter
@ 2021-09-20  4:29               ` Stefan Monnier
  2021-09-20  6:41                 ` Stefan Kangas
  2021-09-20 23:26                 ` Adam Porter
  0 siblings, 2 replies; 21+ messages in thread
From: Stefan Monnier @ 2021-09-20  4:29 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

> Thanks, now I have a better idea of what you want.  Hopefully this patch
> is close to what you have in mind.

Looks good, thanks.

> -    (with-temp-buffer
> -      (insert-file-contents (expand-file-name file srcdir))
> -      (buffer-string)))
> +    ;; Return FILE's contents.
> +    (let ((type
> +           (pcase (mailcap-extension-to-mime (file-name-extension file))
> +             ((and `nil
> +                   (guard (member-ignore-case
> +                           (file-name-extension file) '("md" "markdown"))))
> +              ;; `mailcap-extension-to-mime' returns nil for Markdown
> +              ;; files, at least on Emacs 26.3.
> +              "text/markdown")

Here `mailcap-extension-to-mime` could fail if `mailcap` is not yet
loaded.  Also, the problem you describe seems to depend on
/etc/mime.types rather than the version of Emacs.  Here (a Debian
machine) I got another problem which is that my /etc/mime.types says:

    [...]
    application/vnd.lotus-organizer                 or3 or2 org
    [...]

so I got an error from elpaa--section-to-* complaining that it doesn't
know how to convert `application/vnd.lotus-organizer` :-(

I think we're going to have to use our own mapping from extensions to
types :-(

After fixing this problem, I got another one which was in the part that
generates the "news" because it calls `elpaa--get-section` but was not
updated to adjust to the new return value.

I fixed these and made a few other cosmetic changes.
You can see the result in the `scratch/prettify-readme.org` branch of elpa.git

One thing I noticed in the resulting <pkg>.html file is that the doc is
not clearly delimited any more (e.g. it tends to use <h2> headers for
its own top-level entities whereas the surrounding HTML uses <h2> as
the header that introduces the doc).
:-(


        Stefan




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  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
  1 sibling, 2 replies; 21+ messages in thread
From: Stefan Kangas @ 2021-09-20  6:41 UTC (permalink / raw)
  To: Stefan Monnier, Adam Porter; +Cc: emacs-devel

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

> One thing I noticed in the resulting <pkg>.html file is that the doc is
> not clearly delimited any more (e.g. it tends to use <h2> headers for
> its own top-level entities whereas the surrounding HTML uses <h2> as
> the header that introduces the doc).

This should be rather easy to fix with CSS.  You wrap it in some div
using a particular class (IIRC it already is wrapped) and then you use
CSS selectors based on that to set the font sizes to something more
suitable.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  2021-09-20  6:41                 ` Stefan Kangas
@ 2021-09-20 13:40                   ` Basil L. Contovounesios
  2021-09-20 19:57                   ` Adam Porter
  1 sibling, 0 replies; 21+ messages in thread
From: Basil L. Contovounesios @ 2021-09-20 13:40 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Stefan Monnier, Adam Porter, emacs-devel

Stefan Kangas [2021-09-19 23:41 -0700] wrote:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> One thing I noticed in the resulting <pkg>.html file is that the doc is
>> not clearly delimited any more (e.g. it tends to use <h2> headers for
>> its own top-level entities whereas the surrounding HTML uses <h2> as
>> the header that introduces the doc).
>
> This should be rather easy to fix with CSS.  You wrap it in some div
> using a particular class (IIRC it already is wrapped) and then you use
> CSS selectors based on that to set the font sizes to something more
> suitable.

We'll "fix it in post" ;).

-- 
Basil



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  2021-09-20  6:41                 ` Stefan Kangas
  2021-09-20 13:40                   ` Basil L. Contovounesios
@ 2021-09-20 19:57                   ` Adam Porter
  1 sibling, 0 replies; 21+ messages in thread
From: Adam Porter @ 2021-09-20 19:57 UTC (permalink / raw)
  To: emacs-devel

Stefan Kangas <stefankangas@gmail.com> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> One thing I noticed in the resulting <pkg>.html file is that the doc is
>> not clearly delimited any more (e.g. it tends to use <h2> headers for
>> its own top-level entities whereas the surrounding HTML uses <h2> as
>> the header that introduces the doc).
>
> This should be rather easy to fix with CSS.  You wrap it in some div
> using a particular class (IIRC it already is wrapped) and then you use
> CSS selectors based on that to set the font sizes to something more
> suitable.

This should be fixable in the export settings, i.e. see:

  (defcustom org-html-toplevel-hlevel 2
    "The <H> level for level 1 headings in HTML export.
  This is also important for the classes that will be wrapped around headlines
  and outline structure.  If this variable is 1, the top-level headlines will
  be <h1>, and the corresponding classes will be outline-1, section-number-1,
  and outline-text-1.  If this is 2, all of these will get a 2 instead.
  The default for this variable is 2, because we use <h1> for formatting the
  document title."
    :group 'org-export-html
    :type 'integer)




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [ELPA/elpa-admin] Render README.org as HTML with ox-html
  2021-09-20  4:29               ` Stefan Monnier
  2021-09-20  6:41                 ` Stefan Kangas
@ 2021-09-20 23:26                 ` Adam Porter
  1 sibling, 0 replies; 21+ messages in thread
From: Adam Porter @ 2021-09-20 23:26 UTC (permalink / raw)
  To: emacs-devel

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

> Here `mailcap-extension-to-mime` could fail if `mailcap` is not yet
> loaded.  Also, the problem you describe seems to depend on
> /etc/mime.types rather than the version of Emacs.  Here (a Debian
> machine) I got another problem which is that my /etc/mime.types says:
>
>     [...]
>     application/vnd.lotus-organizer                 or3 or2 org
>     [...]
>
> so I got an error from elpaa--section-to-* complaining that it doesn't
> know how to convert `application/vnd.lotus-organizer` :-(
>
> I think we're going to have to use our own mapping from extensions to
> types :-(

Ah, that makes sense, yes.  (I might have added that Org entry to my
local mime.types and forgotten about it.)

> After fixing this problem, I got another one which was in the part that
> generates the "news" because it calls `elpaa--get-section` but was not
> updated to adjust to the new return value.
>
> I fixed these and made a few other cosmetic changes.
> You can see the result in the `scratch/prettify-readme.org` branch of
> elpa.git

Thanks, that looks good.

> One thing I noticed in the resulting <pkg>.html file is that the doc is
> not clearly delimited any more (e.g. it tends to use <h2> headers for
> its own top-level entities whereas the surrounding HTML uses <h2> as
> the header that introduces the doc).
> :-(

I think that should be fixed by using the appropriate export option,
like this:

  (cl-defmethod elpaa--section-to-html ((section (head text/x-org)))
    (elpaa--export-org (cdr section) 'html
                       :body-only t
                       :ext-plist (append '(:html-toplevel-hlevel 3)
                                          elpaa--org-export-options)))




^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2021-09-20 23:26 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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-08-30 17:49   ` [ELPA/elpa-admin] Render README.org as ASCII with ox-ascii Philip Kaludercic

Code repositories for project(s) associated with this 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 NNTP newsgroup(s).