[Top][All Lists]

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

Re: [PATCH] `completing-read`: Add `group-function` support to completio

From: Daniel Mendler
Subject: Re: [PATCH] `completing-read`: Add `group-function` support to completion metadata (REVISED PATCH VERSION 2)
Date: Sun, 2 May 2021 02:32:41 +0200

On 5/1/21 9:54 PM, Juri Linkov wrote:
> I think both completions-detailed and completions-group should be
> checked only by the API user like in help-fns.el.  Otherwise,
> there is duplication that you can see in my previous patch
> for read-char-by-name:
>   (let* ((enable-recursive-minibuffers t)
>        (completion-ignore-case t)
>        (completion-tab-width 4)
>        (completions-group read-char-by-name-group)
>                             =======================
>        (input
>         (completing-read
>          prompt
>          (lambda (string pred action)
>            (if (eq action 'metadata)
>                `(metadata
>                  (group-function
>                   . ,(when read-char-by-name-group
>                              =======================
>                        #'mule--ucs-names-group))
> The same user option read-char-by-name-group is checked twice.
> It should suffice to leave only the latter.

This is a matter of preference. In this case I think I would prefer to
have the settings checked only once centrally in order to avoid the code
duplicatication in every completion table. Furthermore it seems that the
style to check the setting locally in every completion table will lead
to an unnecessary proliferation of configuration variables, since you
introduced the variable `read-char-by-name-group` here. I don't think we
should introduce an extra configuration variable per completion table.


reply via email to

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