Re: feature/icomplete-vertical

[Gregory Heytings]

Hi Ergus,

Thanks for your comments.

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

Okay, these lines in my patch can just be dropped indeed.  This makes the 
whole thing even simpler.

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

I see.  I did not think about that potential problem before sending the 
patch.  I attach an updated patch (still very short) in which this problem 
is (I think) fixed.

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

That could be an additional feature indeed.  I do not understand why they 
are annoying, it's common in completion mechanisms to have such 
indications, but why not.  But I think it is better to separate concerns.

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

That could also be an additional feature, again I don't really see why it 
would be useful, but why not.  OTOH, it is not possible to do this with 
the current version of icomplete in horizontal mode, and again I think it 
is better to separate concerns.

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

The patch I propose has only five "if icomplete-vertical", that's not much 
I think.  And in fact it's easy to remove three of them, and to have only 
two "if icomplete-vertical" (see the other attached patch).  If horizontal 
icomplete remains the default behavior (as I think it will), these if's 
make it clear where the code differs between the two cases.

Gregory

icomplete.patch
Description: Text Data

icomplete-two-ifs.patch
Description: Text Data