GNU bug report logs - #75794
[PATCH] feat(icomplete): markers and vertical alignment

Previous Next

Full log

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Rahul Martim Juliato <rahuljuliato <at> gmail.com>
Cc: jixiuf <at> qq.com, 75794 <at> debbugs.gnu.org
Subject: Re: bug#75794: [PATCH] feat(icomplete): markers and vertical alignment
Date: Sat, 15 Mar 2025 14:10:45 +0200

> From: Rahul Martim Juliato <rahuljuliato <at> gmail.com>
> Cc: eliz <at> gnu.org, 75794 <at> debbugs.gnu.org
> Date: Wed, 12 Mar 2025 21:09:18 -0300
> 
> > I could reproduce with:
> >
> > (setq show-paren-when-point-inside-paren t)
> >
> 
> Thanks for your feedback!
> 
> Please find attached a new version of this patch.
> 
> Could you try it and tell if this one fixes this bug? For me, it did
> fixed it! :)

Thanks, please see a few minor comments below.

> >From e6f4d614b79949ecf1799e4cc4dc312fd33a5bbd Mon Sep 17 00:00:00 2001
> From: Rahul Martim Juliato <rahul.juliato <at> gmail.com>
> Date: Wed, 12 Mar 2025 21:00:27 -0300
> Subject: [PATCH] Enhance 'icomplete-vertical-mode'
> 
> New user options have been added to enhance 'icomplete-vertical-mode':
> 
>  + 'icomplete-vertical-in-buffer-adjust-list': Aligns in-buffer
> completion to the original cursor column.
>  + 'icomplete-vertical-render-prefix-indicator': When enabled, adds a
> prefix indicator to completion candidates.
>  + 'icomplete-vertical-selected-prefix-indicator': Specifies the prefix
> string for the selected candidate.
>  + 'icomplete-vertical-unselected-prefix-indicator': Specifies the prefix
> string for unselected candidates.
> 
> New faces introduced:
> 
>  + 'icomplete-vertical-selected-prefix-indicator-face': Controls the
> appearance of the selected candidate prefix.
>  + 'icomplete-vertical-unselected-prefix-indicator-face': Controls the
> appearance of unselected candidate prefixes.
> 
> * etc/NEWS: Document the new user options and faces.
> * lisp/icomplete.el(icomplete-vertical-mode): Implement the new options

Please format the commit log message according to our conventions.
Specifically:

  . lines not longer than 63 characters
  . changes described with their file names, not separately
  . Each change description a complete sentence ending in a period

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -269,6 +269,25 @@ return value windows whose buffers share their text with BUFFER-OR-NAME.
>  With such an entry, 'display-buffer-reuse-window' may also choose a
>  window whose buffer shares text with the buffer to display.
>  
> ++++
> +*** New user options for 'icomplete-vertical-mode'.
> +New user options have been added to enhance 'icomplete-vertical-mode':
> +
> + - 'icomplete-vertical-in-buffer-adjust-list': Aligns in-buffer
> +    completion to the original cursor column.
> + - 'icomplete-vertical-render-prefix-indicator': When enabled, adds a
> +    prefix indicator to completion candidates.
> + - 'icomplete-vertical-selected-prefix-indicator': Specifies the prefix
> +    string for the selected candidate.
> + - 'icomplete-vertical-unselected-prefix-indicator': Specifies the prefix
> +    string for unselected candidates.
> +
> +New faces:
> +
> + - 'icomplete-vertical-selected-prefix-indicator-face': Controls the
> +    appearance of the selected candidate prefix.
> + - 'icomplete-vertical-unselected-prefix-indicator-face': Controls the
> +    appearance of unselected candidate prefixes.

Please format NEWS entries according to our conventions (it's a
derivative of Outline mode, using "*" for heading line indentations).

Also, since you haven't updated any manuals, the NEWS entries should
not be marked with "+++".

> +(defface icomplete-vertical-unselected-prefix-indicator-face
> +  '((t :inherit font-lock-keyword-face :weight normal :foreground "gray"))
> +  "Face used for the prefix set by `icomplete-vertical-unselected-prefix-indicator'."

This doc string is too long.  Please keep the lines in doc strings
below 80 characters.

> +(defcustom icomplete-vertical-render-prefix-indicator nil
> +  "Control whether a indicator is added as a prefix to each candidate.
                      ^^^^^^^^^^^
"an indicator"

> +If this is t and `icomplete-vertical-mode' is activated, a indicator,
                                                            ^^^^^^^^^^^
Likewise.

> +controlled by `icomplete-vertical-selected-prefix-indicator' is shown
> +as a prefix to the current under selection candidate, while the
> +remaining of the candidates will receive the indicator controlled
> +by `icomplete-vertical-unselected-prefix-indicator'."
> +  :type 'boolean
> +  :group 'icomplete
> +  :version "31.1")
> +
> +(defcustom icomplete-vertical-selected-prefix-indicator "» "

Is it wise to default to a non_ASCII character without checking first
that it's supported by the display?

> +  "Prefix string used to mark the selected completion candidate.
> +If `icomplete-vertical-render-prefix-indicator' is t, the string
> +defined here is used as a prefix of the currently selected entry in the
> +list.  It can be further customized by the face
> +`icomplete-vertical-selected-prefix-indicator-face'."
> +  :type 'string
> +  :group 'icomplete
> +  :version "31.1")

Regardless of the default value, the options should include an ASCII
and a non-ASCII strings.

Previous Next

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