installed packages long description.

installed packages long description.

[Stephen Leake]

When browsing the *Packages* buffer produced by 'list-packages', the ?
key gives a package description. That includes a "long description",
which for multi-file packages is taken from a README file. For
single-file packages it is taken from a Commentary comment header.

However, if that description was not viewed before a package was
installed, it is not displayed in the description.

The problem is the way describe-package retrieves the long description.
If the package is not installed, the package-desc struct archive field
has a value, and describe-package requests (format "%s-readme.txt" name)
from the archive server. Apparently the webserver on the archive
responds with the contents of <pkg>/README or the Commentary comment
header. describe-package then saves that in
~/.emacs.d/elpa/<pkg>-readme.txt.

However, if the package is installed, the package-desc struct
archive field does not have a value. Then describe-package looks for
~/.emacs.d/elpa/<pkg>-readme.txt, which does not exist if the
description was never viewed before installation.

I see two solutions; describe-package can also look for README or the
Commentary comment header, or package-install can always populate
~/.emacs.d/elpa/<pkg>-readme.txt

preferences?

-- 
-- Stephe

Re: installed packages long description.

[Stefan Monnier]

> I see two solutions; describe-package can also look for README or the
> Commentary comment header, or package-install can always populate
> ~/.emacs.d/elpa/<pkg>-readme.txt

Another is to look for a corresponding package in the archives and
download its readme file.

> preferences?

The ELPA standard doesn't say how/where to find the description within
the package: the <pkg>-readme.txt served via HTTP is the only
documented/official way to get the description (the README file and the
Commentary sections are just what existing archives tend to use in
their own package-building scripts).

While we can come up with a short-term solution, I think in the long run
we should try and improve the ELPA standard in this respect, e.g. by
specifying where the description can be found within the package (and
also by specifying some kind of markup for this description rather than
being limited to plain text).


        Stefan

Re: installed packages long description.

[Stephen Leake]

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

>> I see two solutions; describe-package can also look for README or the
>> Commentary comment header, or package-install can always populate
>> ~/.emacs.d/elpa/<pkg>-readme.txt
>
> Another is to look for a corresponding package in the archives and
> download its readme file.

One problem with this is different versions might be in different
archives, and the package description does not include which archive the
installed package is from.

Perhaps that could be added; it would require a
meta-info file, either in ~/.emacs.d/elpa/<pkg>/, or in ~/.emacs.d/elpa/

Another problem is the installed package could be older than the
current version in the archive; apparently there is no way to retrieve
the readme for an old version.

There's also no way to check that an existing <pkg>-readme.txt
corresponds to the installed package version.

> The ELPA standard

Where is this published/documented? I don't see a reference on
https://elpa.gnu.org/, and a web search for "ELPA standard" returned no
useful hits.

And where are changes to it discussed?

> doesn't say how/where to find the description within the package: the
> <pkg>-readme.txt served via HTTP is the only documented/official way
> to get the description (the README file and the Commentary sections
> are just what existing archives tend to use in their own
> package-building scripts).
>
> While we can come up with a short-term solution,

from elpa/admin/archive-contents.el, that appears to be:

(archive--get-section
  "Commentary"
  '("README" "README.rst" "README.org")
  srcdir mainsrcfile)

That code could be moved to package.el

That would also handle old versions nicely.

> I think in the long run we should try and improve the ELPA standard in
> this respect, e.g. by specifying where the description can be found
> within the package (and also by specifying some kind of markup for
> this description rather than being limited to plain text).

Just document the code above. Markup could be handled by specifying
"markdown", and looking for README.md. Or allow any markup for which
there is an Emacs mode.

--
-- Stephe

Re: installed packages long description.

[Stefan Monnier]

>> The ELPA standard
>
> Where is this published/documented?

I think it's in "Preparing Lisp code for distribution" in the
Elisp manual.  And apparently it does document that it should be either
in the Commentary or in the README.

> from elpa/admin/archive-contents.el, that appears to be:
>
> (archive--get-section
>   "Commentary"
>   '("README" "README.rst" "README.org")
>   srcdir mainsrcfile)
>
> That code could be moved to package.el

Sounds good.

> Just document the code above. Markup could be handled by specifying
> "markdown", and looking for README.md. Or allow any markup for which
> there is an Emacs mode.

IIRC README.md doesn't work so well, because many package use it as
a kind of "homepage" on github, so its content doesn't work very well
in the context of package.el.

As for saying the README and/or Commentary: from now on is assumed to
use Markdown, that will result in ugly text with current/previous
packages which are not written under this assumption.

Also, there's the old discussion of which markup to use (mostly Org or
markdown).


        Stefan

Re: installed packages long description.

[Stephen Leake]

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

>>> The ELPA standard
>>
>> Where is this published/documented?
>
> I think it's in "Preparing Lisp code for distribution" in the
> Elisp manual.

Hmm. "Creating and Maintaining Package Archives" says:

   If you want the archive to be reachable via HTTP, this directory must
   be accessible to a web server. How to accomplish this is beyond the
   scope of this manual.

And it does not describe "the <pkg>-readme.txt served via HTTP"

So we need to add that at least. We could also document the other uses
of 'package--with-response-buffer':

package--check-signature
package--download-one-archive
package-install-from-archive

Then package.el can reference the elisp manual.

> And apparently it does document that it should be either in the
> Commentary or in the README.

Right. We need to add the other allowed file names to that.

>
>> from elpa/admin/archive-contents.el, that appears to be:
>>
>> (archive--get-section
>>   "Commentary"
>>   '("README" "README.rst" "README.org")
>>   srcdir mainsrcfile)
>>
>> That code could be moved to package.el
>
> Sounds good.
>
>> Just document the code above. Markup could be handled by specifying
>> "markdown", and looking for README.md. Or allow any markup for which
>> there is an Emacs mode.
>
> IIRC README.md doesn't work so well, because many package use it as
> a kind of "homepage" on github, so its content doesn't work very well
> in the context of package.el.

We could add README_elpa.* to handle that problem.

> As for saying the README and/or Commentary: from now on is assumed to
> use Markdown, that will result in ugly text with current/previous
> packages which are not written under this assumption.

Right, I would say no markup in Commentary: or README, and rely on the
file extension for a README*.* .

> Also, there's the old discussion of which markup to use (mostly Org or
> markdown).

Easiest to allow any that Emacs can display.

The downside to that is if others try to maintain the file, they may
have to learn a new markup; that's not a big deal for a README.

--
-- Stephe

Re: installed packages long description.

[Stefan Monnier]

> And it does not describe "the <pkg>-readme.txt served via HTTP"
>
> So we need to add that at least. We could also document the other uses
> of 'package--with-response-buffer':
>
> package--check-signature
> package--download-one-archive
> package-install-from-archive
>
> Then package.el can reference the elisp manual.

Sounds good.  Can you take care of it?

>>> from elpa/admin/archive-contents.el, that appears to be:
>>>
>>> (archive--get-section
>>>   "Commentary"
>>>   '("README" "README.rst" "README.org")
>>>   srcdir mainsrcfile)
>>>
>>> That code could be moved to package.el
>>
>> Sounds good.

Can you take care of this as well?

>> As for saying the README and/or Commentary: from now on is assumed to
>> use Markdown, that will result in ugly text with current/previous
>> packages which are not written under this assumption.
> Right, I would say no markup in Commentary: or README, and rely on the
> file extension for a README*.* .

We could support a new section name ("Description:" or
"Commentary.org:" or ...) for the "Commentary: with markup".

This said, there's also the possibility to just use Commentary: along
with a heuristic which would give "mostly correct" results on
existing Commentaries.

Bonus points if we can change elisp-mode's font-lock so the markup is
correctly prettified.

>> Also, there's the old discussion of which markup to use (mostly Org or
>> markdown).
> Easiest to allow any that Emacs can display.

I think currently Org is the only markup format that vanilla Emacs can
render (tho arguably Texinfo is there as well, via makeinfo.el).

Allowing more formats requires more code to handle the various formats,
so I think we'd be better off only supporting one format.


        Stefan

Re: installed packages long description.

[Stephen Leake]

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

>> And it does not describe "the <pkg>-readme.txt served via HTTP"
>>
>> So we need to add that at least. We could also document the other uses
>> of 'package--with-response-buffer':
>>
>> package--check-signature
>> package--download-one-archive
>> package-install-from-archive
>>
>> Then package.el can reference the elisp manual.
>
> Sounds good.  Can you take care of it?

Yes. 

>>>> from elpa/admin/archive-contents.el, that appears to be:
>>>>
>>>> (archive--get-section
>>>>   "Commentary"
>>>>   '("README" "README.rst" "README.org")
>>>>   srcdir mainsrcfile)
>>>>
>>>> That code could be moved to package.el
>>>
>>> Sounds good.
>
> Can you take care of this as well?

Yes. I won't change the elpa code.

I'll submit a patch for review before committing anything.

>>> As for saying the README and/or Commentary: from now on is assumed to
>>> use Markdown, that will result in ugly text with current/previous
>>> packages which are not written under this assumption.
>> Right, I would say no markup in Commentary: or README, and rely on the
>> file extension for a README*.* .
>
> We could support a new section name ("Description:" or
> "Commentary.org:" or ...) for the "Commentary: with markup".
>
> This said, there's also the possibility to just use Commentary: along
> with a heuristic which would give "mostly correct" results on
> existing Commentaries.
>
> Bonus points if we can change elisp-mode's font-lock so the markup is
> correctly prettified.

I'll leave this for later. Simple package authors can switch to a
multi-file package with a README.* file if they want markup.

>>> Also, there's the old discussion of which markup to use (mostly Org or
>>> markdown).
>> Easiest to allow any that Emacs can display.
>
> I think currently Org is the only markup format that vanilla Emacs can
> render (tho arguably Texinfo is there as well, via makeinfo.el).
>
> Allowing more formats requires more code to handle the various formats,
> so I think we'd be better off only supporting one format.

We have markdown-mode in ELPA, which does a reasonable job. There's also
markdown-preview-eww - I didn't try it.

That's what I meant by "any that Emacs can display"; if there is a mode
(built-in or in ELPA) for the format, that does a reasonable job of
display, then the package author can decide to use that format.

Hmm. It also has to work well on the ELPA web page; that's a
complication.

We might have to use multi-major-mode in the display buffer, for the
headers provided by 'describe-package'.

If the mode is in ELPA and not installed, there needs to be some header
text saying "please install foo-mode to display this better". Or
something. That would be a reason to use only modes bundled with the
standard distribution.

Or the author could provide a plain text README as a fall-back. I
suspect most markdown formatters have an option to output plain text, so
that could be automated.

I don't see an "rst-mode", but I guess that format is meant to be
readable as plain text.

This is less important than getting the correct long description for
installed packages.

-- 
-- Stephe

Re: installed packages long description.

[Stefan Monnier]

> I'll submit a patch for review before committing anything.

Thanks.

> We have markdown-mode in ELPA, which does a reasonable job.

Do we?

> We might have to use multi-major-mode in the display buffer, for the
> headers provided by 'describe-package'.

That should not be a big deal: the buffer is read-only, so it's an easy
case of multi-major-mode.  Basically, we only need to handle the
highlighting and the hyperlinks.

> I don't see an "rst-mode", but I guess that format is meant to be
> readable as plain text.

It's built-in:

    rst-mode is an interactive autoloaded compiled Lisp function in ‘rst.el’.

but I don't think it comes with a renderer.

> This is less important than getting the correct long description for
> installed packages.

Agreed.  One step at a time.


        Stefan

Re: installed packages long description.

[Stephen Leake]

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

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

>> I'll submit a patch for review before committing anything.
>
> Thanks.

Attached. I'm not clear if this rates a NEWS entry?


-- 
-- Stephe

[-- Attachment #2: pkg-long-desc.diff --]
[-- Type: text/x-patch, Size: 9023 bytes --]

diff --git a/doc/lispref/package.texi b/doc/lispref/package.texi
index 37c1ee6697..730decc378 100644
--- a/doc/lispref/package.texi
+++ b/doc/lispref/package.texi
@@ -22,6 +22,7 @@ Packaging
 * Simple Packages::         How to package a single .el file.
 * Multi-file Packages::     How to package multiple files.
 * Package Archives::        Maintaining package archives.
+* Archive Web Server::      Interfacing to an archive web server.
 @end menu
 
 @node Packaging Basics
@@ -249,7 +250,8 @@ Multi-file Packages
 @end defun
 
   If the content directory contains a file named @file{README}, this
-file is used as the long description.
+file is used as the long description (overriding any @samp{;;;
+Commentary:} section).
 
   If the content directory contains a file named @file{dir}, this is
 assumed to be an Info directory file made with @command{install-info}.
@@ -311,8 +313,8 @@ Package Archives
 
   A package archive is simply a directory in which the package files,
 and associated files, are stored.  If you want the archive to be
-reachable via HTTP, this directory must be accessible to a web server.
-How to accomplish this is beyond the scope of this manual.
+reachable via HTTP, this directory must be accessible to a web server;
+@xref{Archive Web Server}.
 
   A convenient way to set up and update a package archive is via the
 @code{package-x} library.  This is included with Emacs, but not loaded
@@ -393,3 +395,28 @@ Package Archives
 @pxref{Top,, GnuPG, gnupg, The GNU Privacy Guard Manual}.  Emacs comes
 with an interface to GNU Privacy Guard, @pxref{Top,, EasyPG, epa,
 Emacs EasyPG Assistant Manual}.
+
+@node Archive Web Server
+@section Interfacing to an archive web server
+@cindex archive web server
+
+A web server providing access to a package archive must support the
+following queries:
+
+@table @asis
+@item archive-contents
+Return a lisp form describing the archive contents. The form is a list
+of 'package-desc' structures (see @file{package.el}), except the first
+element of the list is the archive version.
+
+@item <package name>-readme.txt
+Return the long description of the package.
+
+@item <file name>.sig
+Return the signature for the file.
+
+@item <file name>
+Return the file. This will be the tarball for a multi-file
+package, or the single file for a simple package.
+
+@end table
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index dcede1a5b2..1752c7e9fe 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2123,6 +2123,9 @@ package-delete
            (add-hook 'post-command-hook #'package-menu--post-refresh)
            (delete-directory dir t)
            ;; Remove NAME-VERSION.signed and NAME-readme.txt files.
+           ;;
+           ;; NAME-readme.txt files are no longer created, but they
+           ;; may be left around from an earlier install.
            (dolist (suffix '(".signed" "readme.txt"))
              (let* ((version (package-version-join (package-desc-version pkg-desc)))
                     (file (concat (if (string= suffix ".signed")
@@ -2233,6 +2236,45 @@ package--print-help-section
 
 (declare-function lm-commentary "lisp-mnt" (&optional file))
 
+(defun package--get-description (desc)
+  "Return a string containing the long description of the package DESC.
+The description is read from the installed package files."
+  ;; Installed packages have nil for kind, so we look for README
+  ;; first, then fall back to the Commentary header.
+
+  ;; We don’t include README.md here, because that is often the home
+  ;; page on a site like github, and not suitable as the package long
+  ;; description.
+  (let ((files '("README-elpa" "README-elpa.md" "README" "README.rst" "README.org"))
+        file
+        (srcdir (package-desc-dir desc))
+        result)
+    (while (and files
+                (not result))
+      (setq file (pop files))
+      (when (file-readable-p (expand-file-name file srcdir))
+        ;; Found a README.
+        (with-temp-buffer
+          (insert-file-contents (expand-file-name file srcdir))
+          (setq result (buffer-string)))))
+
+    (or
+     result
+
+     ;; Look for Commentary header.
+     (let ((mainsrcfile (expand-file-name (format "%s.el" (package-desc-name desc))
+                                          srcdir)))
+       (when (file-readable-p mainsrcfile)
+         (with-temp-buffer
+           (insert (or (lm-commentary mainsrcfile) ""))
+           (goto-char (point-min))
+           (when (re-search-forward "^;;; Commentary:\n" nil t)
+             (replace-match ""))
+           (while (re-search-forward "^\\(;+ ?\\)" nil t)
+             (replace-match ""))
+           (buffer-string))))
+     )))
+
 (defun describe-package-1 (pkg)
   (require 'lisp-mnt)
   (let* ((desc (or
@@ -2406,7 +2448,8 @@ describe-package-1
     (insert "\n")
 
     (if built-in
-        ;; For built-in packages, insert the commentary.
+        ;; For built-in packages, get the description from the
+        ;; Commentary header.
         (let ((fn (locate-file (format "%s.el" name) load-path
                                load-file-rep-suffixes))
               (opoint (point)))
@@ -2417,27 +2460,25 @@ describe-package-1
               (replace-match ""))
             (while (re-search-forward "^\\(;+ ?\\)" nil t)
               (replace-match ""))))
-      (let* ((basename (format "%s-readme.txt" name))
-             (readme (expand-file-name basename package-user-dir))
-             readme-string)
-        ;; For elpa packages, try downloading the commentary.  If that
-        ;; fails, try an existing readme file in `package-user-dir'.
-        (cond ((and (package-desc-archive desc)
-                    (package--with-response-buffer (package-archive-base desc)
-                      :file basename :noerror t
-                      (save-excursion
-                        (goto-char (point-max))
-                        (unless (bolp)
-                          (insert ?\n)))
-                      (write-region nil nil
-                                    (expand-file-name readme package-user-dir)
-                                    nil 'silent)
-                      (setq readme-string (buffer-string))
-                      t))
-               (insert readme-string))
-              ((file-readable-p readme)
-               (insert-file-contents readme)
-               (goto-char (point-max))))))))
+
+      (if (package-installed-p desc)
+          ;; For installed packages, get the description from the installed files.
+          (insert (package--get-description desc))
+
+        ;; For non-built-in, non-installed packages, get description from the archive.
+        (let* ((basename (format "%s-readme.txt" name))
+               readme-string)
+
+          (package--with-response-buffer (package-archive-base desc)
+            :file basename :noerror t
+            (save-excursion
+              (goto-char (point-max))
+              (unless (bolp)
+                (insert ?\n)))
+            (setq readme-string (buffer-string))
+            t)
+          (insert readme-string))
+        ))))
 
 (defun package-install-button-action (button)
   (let ((pkg-desc (button-get button 'package-desc)))
diff --git a/test/lisp/emacs-lisp/package-tests.el b/test/lisp/emacs-lisp/package-tests.el
index f08bc92ff2..17431f31f8 100644
--- a/test/lisp/emacs-lisp/package-tests.el
+++ b/test/lisp/emacs-lisp/package-tests.el
@@ -435,11 +435,24 @@ package-test-desc-version-string
      (save-excursion (should (search-forward "Summary: A single-file package with no dependencies" nil t)))
      (save-excursion (should (search-forward "Homepage: http://doodles.au" nil t)))
      (save-excursion (should (re-search-forward "Keywords: \\[?frobnicate\\]?" nil t)))
-     ;; No description, though. Because at this point we don't know
-     ;; what archive the package originated from, and we don't have
-     ;; its readme file saved.
+     (save-excursion (should (search-forward "This package provides a minor mode to frobnicate"
+                                             nil t)))
      )))
 
+(ert-deftest package-test-describe-installed-multi-file-package ()
+  "Test displaying of the readme for installed multi-file package."
+
+  (with-package-test ()
+    (package-initialize)
+    (package-refresh-contents)
+    (package-install 'multi-file)
+    (with-fake-help-buffer
+     (describe-package 'multi-file)
+     (goto-char (point-min))
+     (should (search-forward "Homepage: http://puddles.li" nil t))
+     (should (search-forward "This is a bare-bones readme file for the multi-file"
+                             nil t)))))
+
 (ert-deftest package-test-describe-non-installed-package ()
   "Test displaying of the readme for non-installed package."
 

Re: installed packages long description.

[Stefan Monnier]

>>> I'll submit a patch for review before committing anything.
>> Thanks.
> Attached.

LGTM

> I'm not clear if this rates a NEWS entry?

I think it's just a bug fix.


        Stefan

Re: installed packages long description.

[Stephen Leake]

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

>>>> I'll submit a patch for review before committing anything.
>>> Thanks.
>> Attached.
>
> LGTM

Thanks, committed.

-- 
-- Stephe