Re: [PATCH] (icomplete-vertical-mode): Add support for affixations and, annotations

[João Távora]

[Just sent you this reply off-list, here it is on-list]

Gregory Heytings <gregory@heytings.org> writes:

>> Just because it's in master doesn't mean it's written in stone.
> That's most often the case, but let's see whether this one is an
> exception.

What, really?  Lol, then what are all these commits to master doing?
:-)

> 1. Your code is not modular enough, for example, IMO it would be much
> better if the the annotation function and the candidate number/total
> number of candidates were (user-configurable) hooks, instead of
> hard-coding these operations in the code.

Why do you think the code is not modular enough? Previously, it mixed
vertical and non-vertical mode with a bunch of ifs, now vertical mode is
handled in its own function.  Not ideal, but at least more modular to
me.

Maybe you're conflating the ability to configure a system with the
modularity of its implementation in code.

> Not everyone wants to see
> these numbers, not everyone wants to see annotations even if they are
> available, some might want to add annotations to the candidates list
> with their own chosen formatting, and so forth.

Who do you think doesn't want to see those numbers?  You personally?  If
so, add a patch (I think Daniel is working on this).  But keep the
numbers on by default in fido-mode + fido-vertical-mode.  It's a new
feature so we can what we think is the best default.

Overlay, to clarify it seems you regard icomplete-vertical-mode as a
long-supported feature of Emacs, it's not.  It's a new thing for Emacs
28.  Your implementation, which I reworked and merged some time ago, was
very simple (a good thing!) but also primitive in functionality (when
compared to other completers).  I was a good proof of concept but it
makes sense to rework it.

> 2. I don't understand why you used "icomplete-scroll" for the variable
> name, and why it is not a defcustom.  IMO it should have been
> something more explicit like
> "icomplete-vertical-scroll-candidate-list".  With a hook, this
> variable would not be necessary.

A hook is a variable, and considerably more complex than a boolean.  Why
is that better here and how would it solve the problem?

If you want to change the variable name, it's vertainly possible, but
really your proposal is mouthful of words, IMHO.

I don't make stuff customizable unless people request the customization,
i.e. I don't add customization knobs gratutiously.  But if you really
want to make it a defcustom, of course it can be added.

As for fido-mode, which is an opinionated completer, it overrides it (as
it already does a lot of other defcustoms).

> 3. As I feared, the code to scroll the candidates list does not work
> correctly alas.  I've put lots of efforts to make "icomplete-vertical"
> work correctly in virtually every possible case, so I find this very
> regrettable.  For example, with frame-height = 47, 10 candidates are
> displayed, yet the candidates list rotates on the 7th candidate
> (instead of the 11th one).

This is by design.  Works like Vertico and many other completers.  But
are you experimenting with:

  fido-mode + icomplete-vertical-mode

or

  icomplete-mode + icomplete-vertical-mode

?

The former uses scrolling by default, the latter doesn't.  It uses
rotation, and always "rotates" in the first character.  So I am confused
when you speak of rotations.

> With frame-height = 30, 6 candidates are
> displayed, and the candidates list rotates on the 6th candidate
> (instead of the 7th one).

Can't reproduce this, I see it scroll (not rotate) on the third visual
candidate.

> With frame-height = 22, 4 candidates are displayed, and the candidates
> list correctly rotates on the 5th candidate.  With frame-height = 15,
> two candidates are displayed, yet the candidates list rotates on the
> 4th candidate (instead of the 3th one, and in this case you don't even
> see the current candidate anymore).

I also can't reproduce these two cases, not with Emacs -Q.  Maybe you're
using some extra configuration?  What?

> If I start working on this (again), I would likely change most of your
> code into something else, and it would take some time, because it's a
> difficult problem.  Would you agree with this?

First say what you would want to change it to, and why.

João