emacs-devel
[Top][All Lists]
Advanced

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

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


From: Daniel Mendler
Subject: Re: [PATCH] (icomplete-vertical-mode): Add support for affixations and, annotations
Date: Mon, 24 May 2021 21:53:24 +0200


On 5/24/21 9:13 PM, João Távora wrote:
> João Távora <joaotavora@gmail.com> writes:
> 
>> Daniel Mendler <mail@daniel-mendler.de> writes:
>>
>>> On 5/23/21 11:54 PM, João Távora wrote:
>>
>>> But regarding merging or not merging the patch, I don't agree with your
>>> argument of taking this as leverage which makes the discussion more or
>>> less difficult.
>>
>> I'm not taking this as "leverage", I just don't think icomplete.el
>> should embark into what I consider (and apparently others) a misdesigned
>> API.  We should strive to come up with maintainable and reliable
>> systems, not just merge something because it happens to work and look
>> nice (which is plain to see that it does).

Okay, I agree with what you say about maintainable and reliable systems.
I am thankful that you are putting work into the
`icomplete-vertical-mode`. And I hope we find a good solution for the
annotations.

My proposal to merge this is not just because it "looks nice". It
replicates what is already present in the default UI which is part of
the core functionality in minibuffer.el. Everything that you consider to
be wrong here should also be fixed there.

> By the way, another reason not to merge your patch as it is originally
> is that it seems to affixate too many prospects, way way more than are
> shown.  My alternative patch should, in principle, only annotate the
> candidates that are to be displayed.  It also does suffix alignment (but
> not prefix yet).  So my view is that there are edges to polish in
> multiple angles and we shouldn't rush this.

If that is indeed the case this is a fair criticism and my patch should
not be merged. However this implies that there was something wrong with
the code all along since the prospects I am annotating are concatenated
right away. Of course the concatenation should only be done for the
candidates which are visible.

Regarding the patches you are working one, I think we should first agree
on a way forward regarding the `affixation-function`. I don't think it
is a good idea to end up with the `affixation-function` in addition the
enhanced `annotation-function` with its prefix/suffix annotations. If we
go with the enhanced `annotation-function` the `affixation-function`
should be removed.

I wonder about the semantics of your 'prefix and 'suffix annotations.
Will the 'suffix take precedence over the string itself then or are they
supposed to be concatenated? Wouldn't it be sufficient to only add a
'prefix annotation? Is there a possibility to add more annotations? In
my other mail I had thought about supporting multiple annotation fields
which are then displayed in columns. Do you consider consider something
like this to be realistic?

Daniel



reply via email to

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