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: João Távora
Subject: Re: [PATCH] (icomplete-vertical-mode): Add support for affixations and, annotations
Date: Wed, 2 Jun 2021 00:22:44 +0100

On Wed, Jun 2, 2021 at 12:04 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 01.06.2021 21:47, João Távora wrote:
> > In what way?  Really, all other things being equal, in what way is
> > threading arguments through lots of functions, bloating the arglist,
> > implementations and the docstrings of said functions, calling group-fun
> > multiple times and in multiple places in minibuffer.el better than_not_
> > doing those things?
>
> Why do you think it forces you to "thread" them?

The current implementation in minibuffer.el threads the function down
to the point where it is called twice again (once with the same
second argument -- a possible inefficiency -- and then with a non-nil
second argument). If the results of a single call were stored in
the candidate string, this wouldn't be necessary.

> Firs of all, you can construct the same composite values you wanted
> using the current calling convention of the group-function. And then use
> them the same way.

Indeed.  Indeed that does show that we simplify the code and can keep
the current calling convention.  But then why should we? Its akwardness
becomes even clearer there: calling the same function twice in quick
succession with different second arg for two different types of return value.
To save a cons?

> Whether this results in better minibuffer.el code is debatable: your
> diff has more additions than deletions, and one rather long line.

Oh sure, that function is slightly more complicated, but if you look at

completion--insert-strings
completion--insert-horizontal
completion--insert-vertical
completion--insert-one-column
display-completion-list
minibuffer-completion-help

You'll find they are all simplified.

> OTOH, someone in a different codebase might appreciate that the
> group-function decouples the mapping to group names from the
> transformations of the items, and thus one can do the latter closer to
> where the items are printed, instead of having to "thread" the
> transformed values to their place of use (from where the grouping logic
> was called).

Exactly, but my proposal doesn't prevent doing this near the
print locus.

> Using a text property is a handy workaround, but ideally one shouldn't
> have to use it.

There's nothing wrong with it? If you see a transformation function as
a property of a candidate (which it is in reality -- it's given by a fucntion of
a candidate), then it's something that fits naturally.  If a candidate was
some non-string object, I doubt that there would be any doubt as to what
to do here.  But just because it's a string doesn't change things that much.

João



reply via email to

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