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

Previous Next

Package: emacs;

Reported by: Rahul Martim Juliato <rahuljuliato <at> gmail.com>

Date: Fri, 24 Jan 2025 02:52:02 UTC

Severity: wishlist

Tags: patch

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


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

From: Rahul Martim Juliato <rahuljuliato <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 75794 <at> debbugs.gnu.org
Subject: Re: bug#75794: [PATCH] feat(icomplete): markers and vertical alignment
Date: Sun, 09 Feb 2025 20:08:56 -0300
[Message part 1 (text/plain, inline)]
Pong! Sorry for the delayed follow up.

Please find attached a new version of the patch, Below my
considerations: 

> 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.
>> 

Done, please let me know if this OK or if I need to change something.

>> > +(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.
>> 

Done.

>> > +(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?
>> 

I agree with you, it was ON for testing purposes only. Changed the
default to nil.

>> > +(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.
>> 

Nice, in all functions and doc-strings this is now an 'indicator'.

>> > +(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".
>> 

Done.

>> > +(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.
>> 

Done.

>> > +(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.
>> 

I think you meant when `truncate-lines' is `nil'. When it is 't, no
problems. When it is nil and we're dealing with wrapped lines, this
"works" with the annoying addition of a 'blank wrapped line' between
candidates.

I could not figure out how to fix it properly. Any tips?

For now, I annotated the doc-string informing this is a feature
limitation.

>> 
>> 
>> 
>> And one other comment: these are user-visible changes, so I think they
>> warrant a NEWS entry.

Done. Please tell me if we need to add/remove something to it.

-- 

Rahul Martim Juliato

[0001-Enhance-icomplete-vertical-mode-with-new-user-option.patch (text/x-diff, attachment)]

This bug report was last modified 110 days ago.

Previous Next


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