[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: feature/icomplete-vertical

From: Gregory Heytings
Subject: Re: feature/icomplete-vertical
Date: Sat, 19 Sep 2020 04:03:36 +0000
User-agent: Alpine 2.22 (NEB 394 2020-01-19)

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.


Attachment: icomplete.patch
Description: Text Data

Attachment: icomplete-two-ifs.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]