bug#37544: [PATCH] More doc fixes in package.el

bug#37544: [PATCH] More doc fixes in package.el

[Stefan Kangas]

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

Here are some more doc fixes for package.el.  Any comments?

Best regards,
Stefan Kangas

[-- Attachment #2: 0001-More-doc-fixes-in-package.el.patch --]
[-- Type: text/x-patch, Size: 10065 bytes --]

From 4dbcc65645710c17c567cdb5d6f3b7822387c17e Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sun, 22 Sep 2019 02:21:54 +0200
Subject: [PATCH] More doc fixes in package.el

* lisp/emacs-lisp/package.el (package-menu-hide-low-priority)
(package-pinned-packages)
(package-load-descriptor, package-archive-version)
(package-archive-contents, package--read-archive-file)
(package-read-archive-contents, package-unsigned-archives)
(package-read-all-archive-contents)
(package--download-and-read-archives, package-install)
* lisp/subr.el (package--description-file)
* test/lisp/emacs-lisp/package-tests.el: Doc fixes.
---
 lisp/emacs-lisp/package.el            | 63 +++++++++++++++++----------
 lisp/subr.el                          |  1 +
 test/lisp/emacs-lisp/package-tests.el |  7 ++-
 3 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ab1fb8b90f..178ea23b2f 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -36,7 +36,7 @@
 ;; work.)
 
 ;; A package is described by its name and version.  The distribution
-;; format is either  a tar file or a single .el file.
+;; format is either a tar file or a single .el file.
 
 ;; A tar file should be named "NAME-VERSION.tar".  The tar file must
 ;; unpack into a directory named after the package and version:
@@ -239,7 +239,7 @@ package-menu-hide-low-priority
     t: both criteria are used.
 
 This variable has no effect if `package-menu--hide-packages' is
-nil, so it can be toggled with \\<package-menu-mode-map> \\[package-menu-toggle-hiding]."
+nil, so it can be toggled with \\<package-menu-mode-map>\\[package-menu-toggle-hiding]."
   :type '(choice (const :tag "Don't hide anything" nil)
                  (const :tag "Hide per package-archive-priorities"
                         archive)
@@ -282,8 +282,7 @@ package-pinned-packages
 the package will be unavailable."
   :type '(alist :key-type (symbol :tag "Package")
                 :value-type (string :tag "Archive name"))
-  ;; I don't really see why this is risky...
-  ;; I suppose it could prevent you receiving updates for a package,
+  ;; This could prevent you from receiving updates for a package,
   ;; via an entry (PACKAGE . NON-EXISTING).  Which could be an issue
   ;; if PACKAGE has a known vulnerability that is fixed in newer versions.
   :risky t
@@ -368,7 +367,9 @@ package-check-signature
     package-check-signature))
 
 (defcustom package-unsigned-archives nil
-  "List of archives where we do not check for package signatures."
+  "List of archives where we do not check for package signatures.
+This should be a list of strings matching the names of package
+archives in the variable `package-archives'."
   :type '(repeat (string :tag "Archive name"))
   :risky t
   :version "24.4")
@@ -545,9 +546,9 @@ package-desc--keywords
         (nth 1 keywords)
       keywords)))
 
-(defun package-desc-priority (p)
-  "Return the priority of the archive of package-desc object P."
-  (package-archive-priority (package-desc-archive p)))
+(defun package-desc-priority (pkg-desc)
+  "Return the priority of the archive of package-desc object PKG-DESC."
+  (package-archive-priority (package-desc-archive pkg-desc)))
 
 (cl-defstruct (package--bi-desc
                (:constructor package-make-builtin (version summary))
@@ -559,11 +560,13 @@ package-desc-priority
 
 \f
 ;;; Installed packages
+
 ;; The following variables store information about packages present in
 ;; the system.  The most important of these is `package-alist'.  The
 ;; command `package-initialize' is also closely related to this
 ;; section, but it is left for a later section because it also affects
 ;; other stuff.
+
 (defvar package--builtins nil
   "Alist of built-in packages.
 The actual value is initialized by loading the library
@@ -591,6 +594,7 @@ package-activated-list
 (put 'package-activated-list 'risky-local-variable t)
 
 ;;;; Populating `package-alist'.
+
 ;; The following functions are called on each installed package by
 ;; `package-load-all-descriptors', which ultimately populates the
 ;; `package-alist' variable.
@@ -622,7 +626,9 @@ package-process-define-package
       new-pkg-desc)))
 
 (defun package-load-descriptor (pkg-dir)
-  "Load the description file in directory PKG-DIR."
+  "Load the package description file in directory PKG-DIR.
+Create a new `package-desc' object, add it to `package-alist' and
+return it."
   (let ((pkg-file (expand-file-name (package--description-file pkg-dir)
                                     pkg-dir))
         (signed-file (concat pkg-dir ".signed")))
@@ -678,7 +684,9 @@ define-package
 
 \f
 ;;; Package activation
+
 ;; Section for functions used by `package-activate', which see.
+
 (defun package-disabled-p (pkg-name version)
   "Return whether PKG-NAME at VERSION can be activated.
 The decision is made according to `package-load-list'.
@@ -1381,12 +1389,12 @@ package--check-signature
 ;; The `package-initialize' command is also closely related to this
 ;; section, but it has its own section.
 (defconst package-archive-version 1
-  "Version number of the package archive understood by this file.
+  "Version number of the package archive understood by package.el.
 Lower version numbers than this will probably be understood as well.")
 
 ;; We don't prime the cache since it tends to get out of date.
 (defvar package-archive-contents nil
-  "Cache of the contents of the Emacs Lisp Package Archive.
+  "Cache of the contents of all archives in `package-archives'.
 This is an alist mapping package names (symbols) to
 non-empty lists of `package-desc' structures.")
 (put 'package-archive-contents 'risky-local-variable t)
@@ -1482,9 +1490,9 @@ package--add-to-archive-contents
             (package--append-to-alist pkg-desc package-archive-contents)))))
 
 (defun package--read-archive-file (file)
-  "Re-read archive file FILE, if it exists.
-Will return the data from the file, or nil if the file does not exist.
-Will throw an error if the archive version is too new."
+  "Read cached archive FILE data, if it exists.
+Return the data from the file, or nil if the file does not exist.
+If the archive version is too new, signal an error."
   (let ((filename (expand-file-name file package-user-dir)))
     (when (file-exists-p filename)
       (with-temp-buffer
@@ -1497,8 +1505,10 @@ package--read-archive-file
           (cdr contents))))))
 
 (defun package-read-archive-contents (archive)
-  "Re-read archive contents for ARCHIVE.
-If successful, set the variable `package-archive-contents'.
+  "Read cached archive file for ARCHIVE.
+If successful, set or update the variable `package-archive-contents'.
+ARCHIVE should be a string matching the name of a package archive
+in the variable `package-archives'.
 If the archive version is too new, signal an error."
   ;; Version 1 of 'archive-contents' is identical to our internal
   ;; representation.
@@ -1516,8 +1526,8 @@ package--old-archive-priorities
 it again.")
 
 (defun package-read-all-archive-contents ()
-  "Re-read `archive-contents', if it exists.
-If successful, set `package-archive-contents'."
+  "Read cached archive file for all archives in `package-archives'.
+If successful, set or update `package-archive-contents'."
   (setq package-archive-contents nil)
   (setq package--old-archive-priorities package-archive-priorities)
   (dolist (archive package-archives)
@@ -1666,8 +1676,10 @@ package--download-one-archive
 
 (defun package--download-and-read-archives (&optional async)
   "Download descriptions of all `package-archives' and read them.
-This populates `package-archive-contents'.  If ASYNC is non-nil,
-perform the downloads asynchronously."
+Populate `package-archive-contents' with the result.
+
+If optional argument ASYNC is non-nil, perform the downloads
+asynchronously."
   ;; The downloaded archive contents will be read as part of
   ;; `package--update-downloads-in-progress'.
   (dolist (archive package-archives)
@@ -2015,12 +2027,17 @@ package-download-transaction
 ;;;###autoload
 (defun package-install (pkg &optional dont-select)
   "Install the package PKG.
-PKG can be a `package-desc' or a symbol naming one of the available packages
-in an archive in `package-archives'.  Interactively, prompt for its name.
+PKG can be a `package-desc' or a symbol naming one of the
+available packages in an archive in `package-archives'.  When
+called interactively, prompt for the package name.
 
-If called interactively or if DONT-SELECT nil, add PKG to
+Mark the installed package as selected by adding it to
 `package-selected-packages'.
 
+When called from Lisp and optional argument DONT-SELECT is
+non-nil, install the package but do not add it to
+`package-select-packages'.
+
 If PKG is a `package-desc' and it is already installed, don't try
 to install it but still mark it as selected."
   (interactive
diff --git a/lisp/subr.el b/lisp/subr.el
index 45b99a82d2..2dc4e789cf 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -5516,6 +5516,7 @@ package--builtin-versions
 as a list.")
 
 (defun package--description-file (dir)
+  "Return package description file name for package DIR."
   (concat (let ((subdir (file-name-nondirectory
                          (directory-file-name dir))))
             (if (string-match "\\([^.].*?\\)-\\([0-9]+\\(?:[.][0-9]+\\|\\(?:pre\\|beta\\|alpha\\)[0-9]+\\)*\\)" subdir)
diff --git a/test/lisp/emacs-lisp/package-tests.el b/test/lisp/emacs-lisp/package-tests.el
index c757bccf67..f450fd27c2 100644
--- a/test/lisp/emacs-lisp/package-tests.el
+++ b/test/lisp/emacs-lisp/package-tests.el
@@ -28,7 +28,12 @@
 
 ;; Run this in a clean Emacs session using:
 ;;
-;;     $ emacs -Q --batch -L . -l package-test.el -l ert -f ert-run-tests-batch-and-exit
+;;     $ emacs -Q --batch -L . -l package-tests.el -l ert -f ert-run-tests-batch-and-exit
+;;
+;; From the top level directory of the Emacs development repository,
+;; you can use this instead:
+;;
+;;     $ make -C test package-tests
 
 ;;; Code:
 
-- 
2.20.1

bug#37544: [PATCH] More doc fixes in package.el

[Basil L. Contovounesios]

Stefan Kangas <stefan@marxist.se> writes:

> Here are some more doc fixes for package.el.  Any comments?

Just a minor nit in the log entry:

> From 4dbcc65645710c17c567cdb5d6f3b7822387c17e Mon Sep 17 00:00:00 2001
> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 22 Sep 2019 02:21:54 +0200
> Subject: [PATCH] More doc fixes in package.el
>
> * lisp/emacs-lisp/package.el (package-menu-hide-low-priority)
> (package-pinned-packages)
> (package-load-descriptor, package-archive-version)
> (package-archive-contents, package--read-archive-file)
> (package-read-archive-contents, package-unsigned-archives)
> (package-read-all-archive-contents)
> (package--download-and-read-archives, package-install)
                                                       ^^
I think the changelog for each file is expected to end in a colon.

> * lisp/subr.el (package--description-file)
                                           ^^
Ditto.

> * test/lisp/emacs-lisp/package-tests.el: Doc fixes.

Otherwise looks fine to me, but I'm not familiar with the relevant code.

Thanks,

-- 
Basil

bug#37544: [PATCH] More doc fixes in package.el

[Stefan Kangas]

Basil L. Contovounesios <contovob@tcd.ie> writes:

> I think the changelog for each file is expected to end in a colon.

Thanks, I had missed that.

> Otherwise looks fine to me, but I'm not familiar with the relevant code.

Great, thanks for reviewing.

Best regards,
Stefan Kangas

bug#37544: [PATCH] More doc fixes in package.el

[Stefan Kangas]

Basil L. Contovounesios <contovob@tcd.ie> writes:

> Otherwise looks fine to me, but I'm not familiar with the relevant code.

No further comments in 6 days.  Pushed as commit 93dd959711.

Best regards,
Stefan Kangas