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

[Daniel Mendler <mail@daniel-mendler.de>]

On 6/1/21 4:47 PM, Gregory Heytings wrote:
> 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" indication were (user-configurable) hooks, instead 
> of hard-coding these operations in the 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.

Regarding the annotations I disagree. The default *Completions* UI also
annotates by default with not possibility for the user to turn this off,
except by overwriting the `annotation-function` itself for example by
advising `completion-metadata-get`.

> 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".  And with a 
> hook, this variable would not be necessary.

Agree, a `defcustom` is better.

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

Okay, I didn't see something out of the ordinary, but I've only given it
a bit of light testing earlier. Hopefully these issues can be fixed.

Daniel