Stefan Monnier 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 "

To install this package, run in Emacs:

>>
M-x package-install RET %s RET
" >> name)) >> - (let ((rm (elpaa--get-README pkg-spec srcdir))) >> - (when rm >> - (write-region rm nil (concat name "-readme.txt")) >> - (insert "

Full description

\n" (elpaa--html-quote rm)
>> -                  "\n
\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 "
\n"
>> +                                        (elpaa--html-quote readme-content)
>> +                                        "\n
\n"))))) >> + (when readme-content >> + (write-region readme-content nil output-filename) >> + (insert "

Full description

\n" page-content))) >> ;; (message "latest=%S; files=%S" latest files) >> (unless (< (length files) (if (zerop (length latest)) 1 2)) >> (insert (format "

Old versions

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