[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#37410: [PATCH] Several doc fixes in package.el
From: |
Eli Zaretskii |
Subject: |
bug#37410: [PATCH] Several doc fixes in package.el |
Date: |
Sun, 15 Sep 2019 19:37:14 +0300 |
> From: Stefan Kangas <stefan@marxist.se>
> 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.
Thanks for working on this.
> (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
"True" is inappropriate here. I'd use "If so, and..." instead.
> (defun package--from-builtin (bi-desc)
> + "Create a `package-desc' object from BI-DESC.
> +BI-DESC should be an `package--bi-desc' object."
^^
"a"
> (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\"."
Instead of "for example", it is better to tell explicitly whet this
does. E.g.,:
"Return full name of package-desc object PKG-DESC.
This is the name of the package with its version appended."
> (defun package-desc-suffix (pkg-desc)
> + "Return suffix of package-desc object PKG-DESC.
I'd say "file-name extension" instead of "suffix".
> (defun package-desc--keywords (pkg-desc)
> + "Return keywords of package-desc object PKG-DESC."
Suggest to say something about what these keywords are and where they
come from. Otherwise, "keywords" is such a vague term that it's
impossible to understand that without reading the code.
> +Convert EXP into a `package-desc' object using the
> +`package-desc-from-define' constructor before pushing it to
> +`package-alist.
^
A closing quote missing there.
> (defun package-archive-base (desc)
> - "Return the archive containing the package NAME."
> + "Return the archive containing the package DESC."
I'd say "the package described by DESC".
> (defun package-install-from-buffer ()
> - "Install a package from the current buffer.
> + "Install package from current buffer.
Why this change?
> ;;;###autoload
> (defun package-install-file (file)
> - "Install a package from a file.
> + "Install package from FILE.
And this?
> (defun describe-package-1 (pkg)
> + "Insert package description of PKG at point.
> +Helper function for `describe-package'."
The "at point" here is ambiguous: does it mean "insert at point" or
"PKG at point"?
> (defun package-install-button-action (button)
> + "Run `package-install' on package defined by BUTTON.
Can a package really be defined by a button?
> (defun package-keyword-button-action (button)
> + "Show *Packages* buffer filtered by keyword from BUTTON label.
*Packages* should be in double quotes.
I generally find this sentence confusing: what do you mean by "keyword
from BUTTON label"?
> +(defun package-make-button (text &rest properties)
> + "Insert button labelled TEXT with button PROPERTIES at point.
^^^^^^^^
"labeled"
> (defun package--print-email-button (name)
> + "Insert a button to email NAME at point.
"To email NAME" is confusing. I'd suggest to rename it ADDRESSEE.
"Insert a button to email" is also confusing. Is this alternative
correct?
Insert a button whose action will send email to ADDRESSEE.
> +NAME should have the form (FULLNAME . EMAIL) where NAME is either
^^^^
FULLNAME
> (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'.")
^^^^^^^^^^^^
"list-packages", I presume?
> (defvar package--emacs-version-list (version-to-list emacs-version)
> - "`emacs-version', as a list.")
> + "Variable `emacs-version' as a list.")
"The value of `emacs-version', as a list."