Subject: bug#37410: [PATCH] Several doc fixes in package.el

From: Stefan Kangas <[email-masked]>
Date: Sun, 15 Sep 2019 18:01:17 +0200

Seeing as the documentation in package.el leaves much to be desired, I
spent some time adding doc strings and fixing checkdoc and stylistic
errors.  I've attached a patch with my results, which should improve
the situation a little bit at least.  Is this okay to install?

Best regards,
Stefan Kangas
From a5c324028dd001a3a759534b99866afa05a2535b Mon Sep 17 00:00:00 2001
From: Stefan Kangas <[email-masked]>
Date: Sun, 15 Sep 2019 17:41:27 +0200
Subject: [PATCH] Several doc fixes in package.el

* lisp/emacs-lisp/package.el (top-level)
(package-check-signature, package--from-builtin)
(package-desc-full-name, package-desc-suffix)
(package-desc--keywords, package--bi-desc)
(package-process-define-package, package-archive-base)
(package-install-from-archive, package-install-from-buffer)
(package-install-file, package-autoremove, describe-package-1)
(package-install-button-action, package-delete-button-action)
(package-keyword-button-action, package-make-button)
(package--print-email-button, package-list-unversioned)
(package--emacs-version-list, package-menu-toggle-hiding)
(package-hidden-regexps, package-menu-hide-package)
(package-menu-get-status, package-menu--find-upgrades)
(package-menu--post-refresh): Doc fixes.
---
 lisp/emacs-lisp/package.el | 85 +++++++++++++++++++++++++++++---------
 1 file changed, 65 insertions(+), 20 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ef0c5171de..edc62c5394 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -120,9 +120,9 @@
 ;; - "installed" instead of a blank in the status column
 ;; - tramp needs its files to be compiled in a certain order.
 ;;   how to handle this?  fix tramp?
-;; - maybe we need separate .elc directories for various emacs versions
-;;   and also emacs-vs-xemacs.  That way conditional compilation can
-;;   work.  But would this break anything?
+;; - maybe we need separate .elc directories for various emacs
+;;   versions.  That way conditional compilation can work.  But would
+;;   this break anything?
 ;; - William Xu suggests being able to open a package file without
 ;;   installing it
 ;; - Interface with desktop.el so that restarting after an install
@@ -354,9 +354,9 @@ package-check-signature
 
 (defun package-check-signature ()
   "Check whether we have a usable OpenPGP configuration.
-If true, and `package-check-signature' is `allow-unsigned',
-return `allow-unsigned', otherwise return the value of
-`package-check-signature'."
+If true, and variable `package-check-signature' is
+`allow-unsigned', return `allow-unsigned', otherwise return the
+value of variable `package-check-signature'."
   (if (eq package-check-signature 'allow-unsigned)
       (progn
         (require 'epg-config)
@@ -472,6 +472,8 @@ package--default-summary
   signed)
 
 (defun package--from-builtin (bi-desc)
+  "Create a `package-desc' object from BI-DESC.
+BI-DESC should be an `package--bi-desc' object."
   (package-desc-create :name (pop bi-desc)
                        :version (package--bi-desc-version bi-desc)
                        :summary (package--bi-desc-summary bi-desc)
@@ -509,11 +511,22 @@ package-version-join
       (apply #'concat (nreverse str-list)))))
 
 (defun package-desc-full-name (pkg-desc)
+  "Return full name of package-desc object PKG-DESC.
+For example, if the package is named \"foo\" and has version
+\"1.2.3\", then return \"foo-1.2.3\"."
   (format "%s-%s"
           (package-desc-name pkg-desc)
           (package-version-join (package-desc-version pkg-desc))))
 
 (defun package-desc-suffix (pkg-desc)
+  "Return suffix of package-desc object PKG-DESC.
+Depending on the `package-desc-kind' of PKG-DESC, this is one of:
+
+   'single - \".el\"
+   'tar    - \".tar\"
+   'dir    - \"\"
+
+If the kind is none of the above, signal an error."
   (pcase (package-desc-kind pkg-desc)
     ('single ".el")
     ('tar ".tar")
@@ -521,6 +534,7 @@ package-desc-suffix
     (kind (error "Unknown package kind: %s" kind))))
 
 (defun package-desc--keywords (pkg-desc)
+  "Return keywords of package-desc object PKG-DESC."
   (let ((keywords (cdr (assoc :keywords (package-desc-extras pkg-desc)))))
     (if (eq (car-safe keywords) 'quote)
         (nth 1 keywords)
@@ -530,10 +544,10 @@ package-desc-priority
   "Return the priority of the archive of package-desc object P."
   (package-archive-priority (package-desc-archive p)))
 
-;; Package descriptor format used in finder-inf.el and package--builtins.
 (cl-defstruct (package--bi-desc
                (:constructor package-make-builtin (version summary))
                (:type vector))
+  "Package descriptor format used in finder-inf.el and package--builtins."
   version
   reqs
   summary)
@@ -575,7 +589,15 @@ package-activated-list
 ;; The following functions are called on each installed package by
 ;; `package-load-all-descriptors', which ultimately populates the
 ;; `package-alist' variable.
+
 (defun package-process-define-package (exp)
+  "Process define-package expression EXP and push it to `package-alist'.
+EXP should be a form read from a foo-pkg.el file.
+Convert EXP into a `package-desc' object using the
+`package-desc-from-define' constructor before pushing it to
+`package-alist.
+If there already exists a package by that name in
+`package-alist', replace that definition with the new one."
   (when (eq (car-safe exp) 'define-package)
     (let* ((new-pkg-desc (apply #'package-desc-from-define (cdr exp)))
            (name (package-desc-name new-pkg-desc))
@@ -866,6 +888,7 @@ package--alist-to-plist-args
   (mapcar #'macroexp-quote
           (apply #'nconc
                  (mapcar (lambda (pair) (list (car pair) (cdr pair))) alist))))
+
 (defun package-unpack (pkg-desc)
   "Install the contents of the current buffer as a package."
   (let* ((name (package-desc-name pkg-desc))
@@ -1900,11 +1923,11 @@ package--sort-by-dependence
 ;; requirements (dependencies) are always satisfied by looking in
 ;; `package-archive-contents'.
 (defun package-archive-base (desc)
-  "Return the archive containing the package NAME."
+  "Return the archive containing the package DESC."
   (cdr (assoc (package-desc-archive desc) package-archives)))
 
 (defun package-install-from-archive (pkg-desc)
-  "Download and install a tar package."
+  "Download and install a tar package defined by PKG-DESC."
   ;; This won't happen, unless the archive is doing something wrong.
   (when (eq (package-desc-kind pkg-desc) 'dir)
     (error "Can't install directory package from archive"))
@@ -2043,7 +2066,7 @@ package-strip-rcs-id
 
 ;;;###autoload
 (defun package-install-from-buffer ()
-  "Install a package from the current buffer.
+  "Install package from current buffer.
 The current buffer is assumed to be a single .el or .tar file or
 a directory.  These must follow the packaging guidelines (see
 info node `(elisp)Packaging').
@@ -2081,7 +2104,7 @@ package-install-from-buffer
 
 ;;;###autoload
 (defun package-install-file (file)
-  "Install a package from a file.
+  "Install package from FILE.
 The file can either be a tar file, an Emacs Lisp file, or a
 directory."
   (interactive "fPackage file name: ")
@@ -2217,7 +2240,7 @@ package-reinstall
 
 ;;;###autoload
 (defun package-autoremove ()
-  "Remove packages that are no more needed.
+  "Remove packages that are no longer needed.
 
 Packages that are no more needed by other packages in
 `package-selected-packages' and their dependencies
@@ -2334,6 +2357,8 @@ package--get-description
      )))
 
 (defun describe-package-1 (pkg)
+  "Insert package description of PKG at point.
+Helper function for `describe-package'."
   (require 'lisp-mnt)
   (let* ((desc (or
                 (if (package-desc-p pkg) pkg)
@@ -2563,6 +2588,9 @@ describe-package-1
       (browse-url-add-buttons))))
 
 (defun package-install-button-action (button)
+  "Run `package-install' on package defined by BUTTON.
+Used for the 'action property of buttons in the buffer created by
+`describe-package'."
   (let ((pkg-desc (button-get button 'package-desc)))
     (when (y-or-n-p (format-message "Install package `%s'? "
                                     (package-desc-full-name pkg-desc)))
@@ -2571,6 +2599,9 @@ package-install-button-action
       (goto-char (point-min)))))
 
 (defun package-delete-button-action (button)
+  "Run `package-delete' on package defined by BUTTON.
+Used for the 'action property of buttons in the buffer created by
+`describe-package'."
   (let ((pkg-desc (button-get button 'package-desc)))
     (when (y-or-n-p (format-message "Delete package `%s'? "
                                     (package-desc-full-name pkg-desc)))
@@ -2579,10 +2610,16 @@ package-delete-button-action
       (goto-char (point-min)))))
 
 (defun package-keyword-button-action (button)
+  "Show *Packages* buffer filtered by keyword from BUTTON label.
+Used for the 'action property of buttons in the buffer created by
+`describe-package'."
   (let ((pkg-keyword (button-get button 'package-keyword)))
     (package-show-package-list t (list pkg-keyword))))
 
-(defun package-make-button (text &rest props)
+(defun package-make-button (text &rest properties)
+  "Insert button labelled TEXT with button PROPERTIES at point.
+PROPERTIES are passed to `insert-text-button', for which this
+function is a convenience wrapper used by `describe-package-1'."
   (let ((button-text (if (display-graphic-p) text (concat "[" text "]")))
         (button-face (if (display-graphic-p)
                          '(:box (:line-width 2 :color "dark grey")
@@ -2590,9 +2627,12 @@ package-make-button
                                 :foreground "black")
                        'link)))
     (apply #'insert-text-button button-text 'face button-face 'follow-link t
-           props)))
+           properties)))
 
 (defun package--print-email-button (name)
+  "Insert a button to email NAME at point.
+NAME should have the form (FULLNAME . EMAIL) where NAME is either
+a full name or nil, and EMAIL is a valid email address."
   (when (car name)
     (insert (car name)))
   (when (and (car name) (cdr name))
@@ -2701,13 +2741,13 @@ package--push
      (push (cons ,pkg-desc ,status) ,listname)))
 
 (defvar package-list-unversioned nil
-  "If non-nil include packages that don't have a version in `list-package'.")
+  "If non-nil, include packages that don't have a version in `list-package'.")
 
 (defvar package-list-unsigned nil
   "If non-nil, mention in the list which packages were installed w/o signature.")
 
 (defvar package--emacs-version-list (version-to-list emacs-version)
-  "`emacs-version', as a list.")
+  "Variable `emacs-version' as a list.")
 
 (defun package--incompatible-p (pkg &optional shallow)
   "Return non-nil if PKG has no chance of being installable.
@@ -2782,7 +2822,7 @@ package-menu--hide-packages
 Installed obsolete packages are always displayed.")
 
 (defun package-menu-toggle-hiding ()
-  "Toggle visibility of obsolete available packages."
+  "In Package Menu, toggle visibility of obsolete available packages."
   (interactive)
   (unless (derived-mode-p 'package-menu-mode)
     (user-error "The current buffer is not a Package Menu"))
@@ -2840,7 +2880,7 @@ package-hidden-regexps
 omitted from the package menu.  To toggle this, type \\[package-menu-toggle-hiding].
 
 Values can be interactively added to this list by typing
-\\[package-menu-hide-package] on a package"
+\\[package-menu-hide-package] on a package."
   :version "25.1"
   :type '(repeat (regexp :tag "Hide packages with name matching")))
 
@@ -3100,7 +3140,7 @@ package-menu-refresh
   (package-refresh-contents package-menu-async))
 
 (defun package-menu-hide-package ()
-  "Hide a package under point.
+  "Hide a package under point in Package Menu.
 If optional arg BUTTON is non-nil, describe its associated package."
   (interactive)
   (declare (interactive-only "change `package-hidden-regexps' instead."))
@@ -3199,6 +3239,7 @@ package-menu-quick-help
   'package-menu-view-commentary 'package-menu-describe-package "24.1")
 
 (defun package-menu-get-status ()
+  "Return status text of package at point in Package Menu."
   (let* ((id (tabulated-list-get-id))
          (entry (and id (assoc id tabulated-list-entries))))
     (if entry
@@ -3224,6 +3265,10 @@ package-desc-priority-version
         (package-desc-version pkg-desc)))
 
 (defun package-menu--find-upgrades ()
+  "In Package Menu, return an alist of packages that can be upgraded.
+The alist has the same form as `package-alist', namely a list
+of (PKG . DESCS), but where DESCS is the `package-desc' object
+corresponding to the newer version."
   (let (installed available upgrades)
     ;; Build list of installed/available packages in this buffer.
     (dolist (entry tabulated-list-entries)
@@ -3487,7 +3532,7 @@ package-menu--find-and-notify-upgrades

 (defun package-menu--post-refresh ()
-  "If there's a *Packages* buffer, revert it and check for new packages and upgrades.
+  "Revert *Packages* buffer and check for new packages and upgrades.
 Do nothing if there's no *Packages* buffer.
 
 This function is called after `package-refresh-contents' and it
-- 
2.20.1