bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#41086: [PATCH] Add user-defined column widths to package-list (packa


From: Stefan Kangas
Subject: bug#41086: [PATCH] Add user-defined column widths to package-list (package.el)
Date: Mon, 04 May 2020 23:02:57 +0200

Chris McMahan <cmcmahan@gmail.com> writes:

> When executing the function `package-list-packages', the column widths are
> too narrow, cutting off a significant
> portion of the package names.
>
> This minor patch adds four variables that can be set by the user to adjust
> the width of each column within the package list.

Thanks for the patch, this makes sense.  I have some comments below.

Is this your first patch to Emacs?  I can't find your name in the git
log, but maybe I missed it.  Since your patch is over 15 lines, we
would need a copyright assignment from you to be able to apply it.
Would you be willing to sign such papers?

Please see here for more: https://www.gnu.org/licenses/why-assign.html

---

Now, here are my technical comments.

Could you send your patch as formatted by 'git format-patch -1'
instead?  It will make it easier for us to apply it.

> Changelog entry
> --------------------------------
> 2020-05-04  Chris McMahan  <cmcmahan@gmail.com>
>
> * package.el ((define-derived-mode package-menu-mode tabulated-list-mode
> "Package Menu"):
> User can now adjust column widths of the package list by setting the values
> of the following:

This is better formatted like:

* package.el (package-menu-mode): User can now adjust column widths of
the package list by setting the values of the below defcustoms.

> package-name-column-width (defaults to 30 columns)
> package-version-column-width (14 column)
> package-status-column-width (12 columns)
> package-archive-column-width (14 columns)

This should be formatted like so:

    (package-archive-column-width): New defcustom.

> +(defcustom package-name-column-width 30
> +  "Column width of the 'Package' name within the package list
> +invoked through `package-list-packages'."
> +  :type 'number
> +  :version "27.1")

Should be :version "28.1", since this will most likely not go to the
emacs-27 branch this close to the release.

Also the first line should be a complete sentence in this defcustom
and the others.  Please see M-x customize-group RET package RET if you
want to see why.

We prefer "package menu" to "package list".  I'm not sure why, but I
think it has historical reasons.  Ideal or not, I think we should try
to stay consistent.

Possibly the docstrings could look something much like:

"Column width for the Package name in the package menu."

Refer to Info node `(elisp) Documentation Tips' for more.

> -Letters do not insert themselves; instead, they are commands.
> -\\<package-menu-mode-map>
> -\\{package-menu-mode-map}"
> +  Letters do not insert themselves; instead, they are commands.
> +  \\<package-menu-mode-map>
> +  \\{package-menu-mode-map}"

Why this reformatting?  This will make the docstring look unusual in
C-h f, I think.

Thanks again for your patch.

Best regards,
Stefan Kangas





reply via email to

[Prev in Thread] Current Thread [Next in Thread]