Re: feature/icomplete-vertical

[Ergus]

On Fri, Sep 18, 2020 at 09:39:58PM +0000, Gregory Heytings via Emacs 
development discussions. wrote:
> Hi Ergus,
>
> I don't understand why you change so many things in icomplete.el for 
> icomplete-vertical.  I attach a patch which implements 
> icomplete-vertical by adding only 20 lines to that file.  It seems to 
> work correctly.  Use it with:
>
> (icomplete-mode 1)
> (setq icomplete-vertical t)
>
> and set `icomplete-prospects-height' to the maximal number of 
> completion candidates you want to display, for example:
>
> (setq icomplete-prospects-height 10)
>
> Gregory


Hi Gregory, thanks for commenting.

I know the branch has too many changes. Usually I do a full refactor and
a squash before merging to master to simplify all the experimental
things I try during development. My error was probably to call it
feature and not scratch branch.

The changes I introduced are actually not too many, the others are just
details and customs to avoid to interfere or changing the defaults of
icomplete while developing. Most of them will disappear in the final
patch or come latter as different features. (I have the bad habit to
overwrite too much my histories ;p)

OTOH: The problem with the approach in your patch (that you can see is
essentially similar to the first commit in the branch) are basically:

1) Icomplete shouldn't call shrink-window because the minibuffer resize
must respect the max-mini-window-height and resize-mini-windows policy.

2) When using different fonts (as a user jixiuf did) the prompt
disappears because there is a mismatch between the visible lines and the
ones the minibuffer shows. So we need to fix that in a general way and
calculate sizes in pixels to avoid this problem.

3) In vertical the [] and () are annoying; more than in horizontal so we
should give some control for them. Now there is a mechanism a bit
complex, but just because it is under development. I am actually trying
to simplify that.

4) Even in vertical the icomplete-separator must be respected somehow
because some users reported to use it (with the external package) to
separate the candidates from the left of the window, add prompts and so
on. (for example adding a > before the first candidate (changing the {)
and a space before the others (changing the separator)) So our changes
need to support all that in a simpler way. Also some users requested to
have two \n as a separator (don't ask me why).

5) I opted for a more modular approach because filling the code with
several 'if' it harder to read and very error prone to maintain
latter. So I just separated the setups when vertical and horizontal so
if we want to add more policies and features we have to deal with less
race conditions.

For now I am only worried about functionality; when that works as
expected and cover all the popular use cases I will cleanup I promise :)

Best,
Ergus