GNU bug report logs - #68929
[PATCH] Copy which-key from GNU ELPA into core

Previous Next

Full log

Message #17 received at 68929 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jeremy Bryant <jb <at> jeremybryant.net>
Cc: 68929 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, justin <at> burkett.cc
Subject: Re: bug#68929: [PATCH] Copy which-key from GNU ELPA into core
Date: Mon, 05 Feb 2024 14:32:19 +0200

> Date: Sun, 04 Feb 2024 22:02:50 +0000
> From:  Jeremy Bryant via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> This patch brings the latest version of which-key from GNU ELPA into
> Emacs core
> 
> If suitable I will then work on the updates to user-facing aspects such as
> manual and NEWS.

This "needs work"...  Based on a very superficial reading of this huge
file:

> +;; which-key provides the minor mode which-key-mode for Emacs. The mode displays
                                                                ^^
Two spaces between sentences in comments and doc strings, here and
elsewhere.

> +(defcustom which-key-idle-delay 1.0
> +  "Delay (in seconds) for which-key buffer to popup.
> +This variable should be set before activating `which-key-mode'.
> +
> +A value of zero might lead to issues, so a non-zero value is
> +recommended
> +(see https://github.com/justbur/emacs-which-key/issues/134)."
> +  :group 'which-key
> +  :type 'float)

Please add a :version tag to all defcustom's and defface's.


> +(defcustom which-key-idle-secondary-delay nil
> +  "Once the which-key buffer shows once for a key sequence reduce
> +the idle time to this amount (in seconds). This makes it possible

The first sentence of a doc string should be a single complete
sentence.  In this case I suggest

  Number of seconds to reduce idle-timer delay after showing which-key once.

> +(defcustom which-key-min-column-description-width 0
> +  "Every column should at least have this width."

This needs details, otherwise it's mot very useful.

> +(defcustom which-key-unicode-correction 3
> +  "Correction for wide unicode characters.
> +Since we measure width in terms of the number of characters,
> +Unicode characters that are wider than ASCII characters throw off
> +the calculation for available width in the which-key buffer.  This
> +variable allows you to adjust for the wide unicode characters by
> +artificially reducing the available width in the buffer.
> +
> +The default of 3 means allow for the total extra width
> +contributed by any wide unicode characters to be up to one
> +additional ASCII character in the which-key buffer.  Increase this
> +number if you are seeing characters get cutoff on the right side
> +of the which-key popup."

This seems to be a sub-optimal way of accounting for character widths.
We have better ways, e.g., char-width, string-pixel-width etc.

> +(defcustom which-key-dont-use-unicode nil
> +  "If non-nil, don't use any unicode characters in default setup."

This should mention the settings affected by it.

Please go through all the doc strings and make sure they are
sufficiently self-explanatory.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.