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

Previous Next

Full log

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: rahuljuliato <at> gmail.com
Cc: 75794 <at> debbugs.gnu.org
Subject: Re: bug#75794: [PATCH] feat(icomplete): markers and vertical alignment
Date: Sat, 08 Feb 2025 11:24:11 +0200

Ping!  Rahul, would you please submit an improved version of the patch
with my comments addressed, so that we could proceed to installing
this?

> Cc: 75794 <at> debbugs.gnu.org
> Date: Fri, 24 Jan 2025 10:55:27 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > From: Rahul Martim Juliato <rahuljuliato <at> gmail.com>
> > Date: Thu, 23 Jan 2025 23:51:06 -0300
> > 
> > I am submitting a patch for icomplete-mode that introduces two
> > enhancements aimed at improving the user experience with completion
> > candidates:
> > 
> > Vertical Alignment: When using icomplete-vertical-mode for in-buffer
> > completion, the candidates will now align vertically with the column
> > where the cursor was when completion started. This provides a cleaner,
> > more intuitive visual experience when scrolling through completion
> > suggestions.
> > 
> > Customizable Markers: I’ve introduced two customizable markers for
> > icomplete-vertical-mode. The first marker will be applied to the
> > selected candidate, and the second will apply to the rest of the
> > list. Both markers are customizable via Emacs faces, allowing users to
> > tailor the appearance to their preferences.
> > 
> > These enhancements are intended to improve the usability and flexibility
> > of the icomplete interface. The ability to align the completion
> > candidates and customize the markers will make the completion process
> > smoother, especially in environments where visual clarity is essential.
> > 
> > In case you would like to explore the context further, I’ve written two
> > blog posts discussing in-buffer icomplete and these enhancements:
> 
> Thanks, please see some comments below.
> 
> > Add two enhancements to icomplete-mode:
> > 
> > - Vertical alignment when using icomplete-vertical-mode with
> >   in-buffer completion, so candidates are aligned with
> >   the column where the cursor was when completion started.
> > 
> > - Customizable markers to icomplete-vertical-mode, providing
> >   each candidate a prefix if currently selected and another
> >   prefix for the rest of the list. Faces can also be customized.
> 
> This commit log should include a ChangeLog-style descriptions of
> added/modified variables and functions.  See CONTRIBUTE for the
> details.
> 
> > +(defface icomplete-vertical-selected-prefix-face
> > +  '((t :inherit font-lock-keyword-face :weight bold :foreground "cyan"))
> > +  "Face used for the prefix set by `icomplete-vertical-selected-prefix-marker'."
> > +  :group 'icomplete
> > +  :version "31")
> > +
> > +(defface icomplete-vertical-unselected-prefix-face
> > +  '((t :inherit font-lock-keyword-face :weight normal :foreground "gray"))
> > +  "Face used for the prefix set by `icomplete-vertical-unselected-prefix-marker'."
> > +  :group 'icomplete
> > +  :version "31")
>               ^^^^
> This should be "31.1" (here and elsewhere in the patch.
> 
> > +(defcustom icomplete-vertical-in-buffer-adjust-list t
> > +  "Control whether in-buffer completion should align the cursor position.
> 
> Is it necessary to turn the new behavior ON by default?  Would people
> be surprised or annoyed by this behavior change?
> 
> > +(defcustom icomplete-vertical-render-prefix-marker t
> > +  "Control whether a marker is added as a prefix to each candidate.
> 
> I think you are talking about some indication, not a marker.  "Marker"
> in Emacs has special meaning, not what you mean by that here.  So I
> suggest to reword this (and other) doc string(s) accordingly.
> 
> > +(defcustom icomplete-vertical-selected-prefix-marker "» "
> > +  "Prefix string used to mark the selected completion candidate.
> > +If `icomplete-vertical-render-prefix-marker' is t, the string
> > +setted here is used as a prefix of the currently selected entry in the
>    ^^^^^^
> "defined", not "setted".
> 
> > +(defcustom icomplete-vertical-unselected-prefix-marker "  "
> > +  "Prefix string used on the unselected completion candidates.
> > +If `icomplete-vertical-render-prefix-marker' is t, the string
> > +setted here is used as a prefix for all unselected entries in the list.
>    ^^^^^^
> Same.
> 
> > +(defun icomplete-vertical--adjust-lines-for-column (lines buffer data)
> > +  "Adjust the LINES to align with the column in BUFFER based on DATA."
> > +  (if icomplete-vertical-in-buffer-adjust-list
> > +      (let ((column
> > +             (with-current-buffer buffer
> > +               (save-excursion
> > +                 (goto-char (car data))
> > +                 (current-column)))))
> > +        (dolist (l lines)
> > +          (add-text-properties
> > +           0 1 `(display ,(concat (make-string column ?\s) (substring l 0 1)))
> > +           l))
> > +        lines)
> > +    lines))
> 
> Did you test this with a window horizontally scrolled (when
> truncate-lines is non-nil)?  What about continuation lines?  In both
> these cases, current-column is different from the visual column on the
> screen, counted from the left edge of the window.
> 
> 
> 
> 
> And one other comment: these are user-visible changes, so I think they
> warrant a NEWS entry.

Previous Next

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