emacs-devel
[Top][All Lists]
Advanced

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

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


From: Juri Linkov
Subject: Re: [PATCH] `completing-read`: Add `group-function` support to completion metadata (REVISED PATCH VERSION 2)
Date: Fri, 07 May 2021 20:03:48 +0300
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (x86_64-pc-linux-gnu)

>>>   (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.
>
> I agree, `read-char-by-name-group` is obsolete by your new option
> `completions-group`.

I tried to remove `read-char-by-name-group`, but it has a feature
currently not supported by `group-function`:

  (defcustom read-char-by-name-group nil
    "How to group characters for `read-char-by-name' completion.
  When t, split characters to sections of Unicode blocks
  sorted alphabetically."
  =====================

It seems a new function is needed to sort groups, e.g. `group-sort-function`.

Maybe better to push your current patches, so it would be easier
to base the next patches on master?



reply via email to

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