emacs-devel
[Top][All Lists]
Advanced

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

Re: feature/icomplete-vertical


From: Ergus
Subject: Re: feature/icomplete-vertical
Date: Sat, 19 Sep 2020 08:15:31 +0200

On Sat, Sep 19, 2020 at 04:03:36AM +0000, Gregory Heytings wrote:

Hi Ergus,

Thanks for your comments.


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

I thought the same on the beginning, but then the sizes issues came here
and there.


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.

There is another corner case that is when

(frame-parameter nil 'minibuffer) is 'only. This was also a problem
reported some days ago needed when using frames and not the minibuffer.


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.

The point is that they process the output and substitute the [] () {}
and | with an advise.

Not only in the external package but also some custom configurations
they sent by mail made many magics with this. And it was explicitly
requested by a user some days ago.

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.

With the advise it is possible and more or less simple.

That's why I have been playing with the formats to see if I can provide
that with a simple code. But ofcourse, it is expected to be an
additional feature.


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.

The other problem with the patch is that due to rounding and floor when
using different fonts there is too much extra space missing and
sometimes missed a candidate at the end. This was also reported some
days ago.


Gregory

Look at the attached patch as it is partially simplified in my local
branch.

It can be still very improved (the separator part, for example needs
some extra checks to assert it contains at least a '\n', I am actually
thinking in removing the icomplete-format variable and just look for a
\n in the separator in the setup; it will be simpler indeed) but also
some actions are repeated with no reason.

But at least it includes the minimal changes, the corner cases with the
windows/frames/fonts size, fixes the prompt hidden issue, respects the
user separator variable and reduces the conditions we check to only 1 in
the setup.


Best,
Ergus.

Attachment: icomplete-vertical.patch
Description: Text document


reply via email to

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