unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#68214: Completion sorting customization by category
@ 2024-01-02 17:07 Juri Linkov
  2024-01-03  7:20 ` Daniel Mendler
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2024-01-02 17:07 UTC (permalink / raw)
  To: 68214

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

0. emacs -Q
1. M-x calendar RET
2. 'g d' (calendar-goto-date)
3. type any year and RET
4. on the prompt "Month name: " type TAB

Month names are sorted alphabetically that makes no sense:

  April August December February January July
  June  March  May      November October September

What is worse is that currently it's impossible to customize
this sorting order.

This was discussed recently in
https://lists.gnu.org/archive/html/emacs-devel/2023-11/msg01233.html

Here is a patch that allows such customization

 (setopt completion-category-overrides
         '((calendar-month
            (display-sort-function . identity))))

that will sort month names chronologically:

  January February March     April   May      June
  July    August   September October November December


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: completion-sort.patch --]
[-- Type: text/x-diff, Size: 3907 bytes --]

diff --git a/lisp/calendar/calendar.el b/lisp/calendar/calendar.el
index a25684f7b5d..e01d5d792a6 100644
--- a/lisp/calendar/calendar.el
+++ b/lisp/calendar/calendar.el
@@ -2339,7 +2339,11 @@ calendar-read-date
          (month (cdr (assoc-string
                       (completing-read
                        (format-prompt "Month name" defmon)
-                       (append month-array nil)
+                       (lambda (string pred action)
+	                 (if (eq action 'metadata)
+		             '(metadata (category . calendar-month))
+	                   (complete-with-action
+                            action (append month-array nil) string pred)))
                        nil t nil nil defmon)
                       (calendar-make-alist month-array 1) t)))
          (defday (calendar-extract-day default-date))
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index fa2dcb4f698..67870ad8054 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1106,6 +1111,13 @@ completion--cycling-threshold-type
            (const :tag "Always cycle" t)
            (integer :tag "Threshold")))
 
+(defconst completion--sorting-type
+  '(choice (const :tag "Undefined" nil)
+           (const :tag "Keep unsorted" identity)
+           (const :tag "Alphabetical sorting" alphabetical)
+           (const :tag "Historical sorting" historical)
+           (function :tag "Custom function")))
+
 (defcustom completion-styles
   ;; First, use `basic' because prefix completion has been the standard
   ;; for "ever" and works well in most cases, so using it first
@@ -1171,7 +1183,11 @@ completion-category-overrides
 		 ,completion--styles-type)
            (cons :tag "Completion Cycling"
 		 (const :tag "Select one value from the menu." cycle)
-                 ,completion--cycling-threshold-type))))
+                 ,completion--cycling-threshold-type)
+           (cons :tag "Completion Sorting"
+                 (const :tag "Select one value from the menu."
+                        display-sort-function)
+                 ,completion--sorting-type))))
 
 (defun completion--category-override (category tag)
   (or (assq tag (cdr (assq category completion-category-overrides)))
@@ -1334,10 +1350,7 @@ completions-sort
 If the completion-specific metadata provides a
 `display-sort-function', that function overrides the value of
 this variable."
-  :type '(choice (const :tag "No sorting" nil)
-                 (const :tag "Alphabetical sorting" alphabetical)
-                 (const :tag "Historical sorting" historical)
-                 (function :tag "Custom function"))
+  :type completion--sorting-type
   :version "30.1")
 
 (defcustom completions-group nil
@@ -1618,6 +1631,12 @@ completion--metadata
     (if (eq (car bounds) base) md-at-point
       (completion-metadata (substring string 0 base) table pred))))
 
+(defun completion--display-sort-function (metadata)
+  (let* ((cat (completion-metadata-get metadata 'category))
+         (over (completion--category-override cat 'display-sort-function)))
+    (if over (cdr over)
+      (completion-metadata-get metadata 'display-sort-function))))
+
 (defun minibuffer--sort-by-key (elems keyfun)
   "Return ELEMS sorted by increasing value of their KEYFUN.
 KEYFUN takes an element of ELEMS and should return a numerical value."
@@ -2522,7 +2541,7 @@ minibuffer-completion-help
              (aff-fun (or (completion-metadata-get all-md 'affixation-function)
                           (plist-get completion-extra-properties
                                      :affixation-function)))
-             (sort-fun (completion-metadata-get all-md 'display-sort-function))
+             (sort-fun (completion--display-sort-function all-md))
              (group-fun (completion-metadata-get all-md 'group-function))
              (mainbuf (current-buffer))
              ;; If the *Completions* buffer is shown in a new

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-02 17:07 bug#68214: Completion sorting customization by category Juri Linkov
@ 2024-01-03  7:20 ` Daniel Mendler
  2024-01-03 16:07   ` Juri Linkov
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Mendler @ 2024-01-03  7:20 UTC (permalink / raw)
  To: 68214

Hi Juri,

thank you for this patch. Would it make sense to define a general
`completion-metadata-override-get` function instead of
`completion--display-sort-function`? This function could be used to look
up the other meta data functions too, `display-sort-function`,
`annotation-function`, `affixation-function`, `group-function`, etc.

(defun completion-metadata-override-get (metadata prop)
  (if-let ((cat (completion-metadata-get metadata 'category))
           (over (completion--category-override cat prop)))
      (cdr over)
    (completion-metadata-get metadata prop)))

I suggest to use `if-let` instead of `let*`, such that an override is
not retrieved if the category is nil.

Besides that, in the `completion-category-overrides`, you use
`completion--sorting-type` for the `display-sort-function`, while it
should just be `function`.

Daniel





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-03  7:20 ` Daniel Mendler
@ 2024-01-03 16:07   ` Juri Linkov
  2024-01-03 17:12     ` Daniel Mendler
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2024-01-03 16:07 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 68214

[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]

> Would it make sense to define a general
> `completion-metadata-override-get` function instead of
> `completion--display-sort-function`?

Thanks for the suggestion, Daniel.  This is used now in a new patch below.

> This function could be used to look up the other meta data
> functions too, `display-sort-function`, `annotation-function`,
> `affixation-function`, `group-function`, etc.

All these meta data functions could be added later to
completion-category-overrides after pushing the current patch.

> (defun completion-metadata-override-get (metadata prop)
>   (if-let ((cat (completion-metadata-get metadata 'category))
>            (over (completion--category-override cat prop)))
>       (cdr over)
>     (completion-metadata-get metadata prop)))
>
> I suggest to use `if-let` instead of `let*`, such that an override is
> not retrieved if the category is nil.

Maybe customization of completion-category-overrides could support
a catch-all category `nil` for completions without metadata, like
e.g. `nil` can be used in .dir-locals.el as a catch-all for all modes.
But I'm not sure how useful this would be.

> Besides that, in the `completion-category-overrides`, you use
> `completion--sorting-type` for the `display-sort-function`, while it
> should just be `function`.

A new patch doesn't use anymore `completion--sorting-type`
since there is a small difference between customization choices
in `completions-sort` and `completion-category-overrides`:
`completion-category-overrides` needs the value `identity`
that prevents a fallback to `completions-sort` as `nil` does.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: completion-metadata-override-get.patch --]
[-- Type: text/x-diff, Size: 2964 bytes --]

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index fa2dcb4f698..ac07b92acb8 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1148,10 +1153,16 @@ completion-category-defaults
 
 (defcustom completion-category-overrides nil
   "List of category-specific user overrides for completion styles.
+
 Each override has the shape (CATEGORY . ALIST) where ALIST is
 an association list that can specify properties such as:
 - `styles': the list of `completion-styles' to use for that category.
 - `cycle': the `completion-cycle-threshold' to use for that category.
+- `display-sort-function': where `nil' means to use either the sorting
+function from metadata or if it's nil then fall back to `completions-sort';
+`identity' means to not use any sorting to keep the original order;
+and other values are the same as in `completions-sort'.
+
 Categories are symbols such as `buffer' and `file', used when
 completing buffer and file names, respectively.
 
@@ -1171,12 +1182,26 @@ completion-category-overrides
 		 ,completion--styles-type)
            (cons :tag "Completion Cycling"
 		 (const :tag "Select one value from the menu." cycle)
-                 ,completion--cycling-threshold-type))))
+                 ,completion--cycling-threshold-type)
+           (cons :tag "Completion Sorting"
+                 (const :tag "Select one value from the menu."
+                        display-sort-function)
+                 (choice (const :tag "Use default" nil)
+                         (const :tag "No sorting" identity)
+                         (const :tag "Alphabetical sorting" alphabetical)
+                         (const :tag "Historical sorting" historical)
+                         (function :tag "Custom function"))))))
 
 (defun completion--category-override (category tag)
   (or (assq tag (cdr (assq category completion-category-overrides)))
       (assq tag (cdr (assq category completion-category-defaults)))))
 
+(defun completion-metadata-override-get (metadata prop)
+  (if-let ((cat (completion-metadata-get metadata 'category))
+           (over (completion--category-override cat prop)))
+      (cdr over)
+    (completion-metadata-get metadata prop)))
+
 (defun completion--styles (metadata)
   (let* ((cat (completion-metadata-get metadata 'category))
          (over (completion--category-override cat 'styles)))
@@ -2522,7 +2547,7 @@ minibuffer-completion-help
              (aff-fun (or (completion-metadata-get all-md 'affixation-function)
                           (plist-get completion-extra-properties
                                      :affixation-function)))
-             (sort-fun (completion-metadata-get all-md 'display-sort-function))
+             (sort-fun (completion-metadata-override-get all-md 'display-sort-function))
              (group-fun (completion-metadata-get all-md 'group-function))
              (mainbuf (current-buffer))
              ;; If the *Completions* buffer is shown in a new

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-03 16:07   ` Juri Linkov
@ 2024-01-03 17:12     ` Daniel Mendler
  2024-01-04 17:21       ` Juri Linkov
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Mendler @ 2024-01-03 17:12 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 68214

On 1/3/24 17:07, Juri Linkov wrote:
>> This function could be used to look up the other meta data
>> functions too, `display-sort-function`, `annotation-function`,
>> `affixation-function`, `group-function`, etc.
> 
> All these meta data functions could be added later to
> completion-category-overrides after pushing the current patch.

Makes sense, the `display-sort-function' is a good start. I suggest to
at least add the `cycle-sort-function' too. The cycle threshold can be
customized too.

For later - in some cases one may also want to customize the
`annotation-' or `affixation-function'. We have a whole package which
does only that - see Marginalia on GNU ELPA. In contrast to sorting, for
annotations one has to work a bit harder, since one may not have access
to locally let-bound candidate-related data, which is only accessible by
the completion table closure and not globally.

> Maybe customization of completion-category-overrides could support
> a catch-all category `nil` for completions without metadata, like
> e.g. `nil` can be used in .dir-locals.el as a catch-all for all modes.
> But I'm not sure how useful this would be.

Okay. I suggest t instead of nil for the catch-all category. This seems
more aligned with other customization options or Lisp language
constructs like `cond'.

Daniel





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-03 17:12     ` Daniel Mendler
@ 2024-01-04 17:21       ` Juri Linkov
  2024-01-05  7:59         ` Juri Linkov
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2024-01-04 17:21 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 68214

>>> This function could be used to look up the other meta data
>>> functions too, `display-sort-function`, `annotation-function`,
>>> `affixation-function`, `group-function`, etc.
>> 
>> All these meta data functions could be added later to
>> completion-category-overrides after pushing the current patch.

Now the patch is pushed to master.

> Makes sense, the `display-sort-function' is a good start. I suggest to
> at least add the `cycle-sort-function' too. The cycle threshold can be
> customized too.

Looks like the cycle threshold is already supported,
so I'll add the other meta data.

> For later - in some cases one may also want to customize the
> `annotation-' or `affixation-function'. We have a whole package which
> does only that - see Marginalia on GNU ELPA. In contrast to sorting, for
> annotations one has to work a bit harder, since one may not have access
> to locally let-bound candidate-related data, which is only accessible by
> the completion table closure and not globally.

I guess users should be able to customize completion-category-overrides
in such a way that categories could be associated with Marginalia functions.
So users could enable Marginalia for one category, and disable for another.

>> Maybe customization of completion-category-overrides could support
>> a catch-all category `nil` for completions without metadata, like
>> e.g. `nil` can be used in .dir-locals.el as a catch-all for all modes.
>> But I'm not sure how useful this would be.
>
> Okay. I suggest t instead of nil for the catch-all category. This seems
> more aligned with other customization options or Lisp language
> constructs like `cond'.

This could be done as well.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-04 17:21       ` Juri Linkov
@ 2024-01-05  7:59         ` Juri Linkov
  2024-01-05  8:33           ` Daniel Mendler
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2024-01-05  7:59 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 68214

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

>>>> This function could be used to look up the other meta data
>>>> functions too, `display-sort-function`, `annotation-function`,
>>>> `affixation-function`, `group-function`, etc.
>>>
>>> All these meta data functions could be added later to
>>> completion-category-overrides after pushing the current patch.
>>
>> Makes sense, the `display-sort-function' is a good start. I suggest to
>> at least add the `cycle-sort-function' too. The cycle threshold can be
>> customized too.
>
> Looks like the cycle threshold is already supported,
> so I'll add the other meta data.

This looks like a complete patch, but it's not completely tested yet:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: completion-metadata-override-get.patch --]
[-- Type: text/x-diff, Size: 6375 bytes --]

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index b7aebae63a8..fbd9a03d921 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1180,6 +1185,10 @@ completion-category-overrides
            (cons :tag "Completion Cycling"
 		 (const :tag "Select one value from the menu." cycle)
                  ,completion--cycling-threshold-type)
+           (cons :tag "Cycle Sorting"
+                 (const :tag "Select one value from the menu."
+                        cycle-sort-function)
+                 (choice (function :tag "Custom function")))
            (cons :tag "Completion Sorting"
                  (const :tag "Select one value from the menu."
                         display-sort-function)
@@ -1189,7 +1198,19 @@ completion-category-overrides
                                 minibuffer-sort-alphabetically)
                          (const :tag "Historical sorting"
                                 minibuffer-sort-by-history)
-                         (function :tag "Custom function"))))))
+                         (function :tag "Custom function")))
+           (cons :tag "Completion Annotation"
+                 (const :tag "Select one value from the menu."
+                        annotation-function)
+                 (choice (function :tag "Custom function")))
+           (cons :tag "Completion Affixation"
+                 (const :tag "Select one value from the menu."
+                        affixation-function)
+                 (choice (function :tag "Custom function")))
+           (cons :tag "Completion Groups"
+                 (const :tag "Select one value from the menu."
+                        group-function)
+                 (choice (function :tag "Custom function"))))))
 
 (defun completion--category-override (category tag)
   (or (assq tag (cdr (assq category completion-category-overrides)))
@@ -1761,8 +1782,8 @@ completion-all-sorted-completions
                                            base-size md
                                            minibuffer-completion-table
                                            minibuffer-completion-predicate))
-             (sort-fun (completion-metadata-get all-md 'cycle-sort-function))
-             (group-fun (completion-metadata-get all-md 'group-function)))
+             (sort-fun (completion-metadata-override-get all-md 'cycle-sort-function))
+             (group-fun (completion-metadata-override-get all-md 'group-function)))
         (when last
           (setcdr last nil)
 
@@ -2540,14 +2561,14 @@ minibuffer-completion-help
                                            base-size md
                                            minibuffer-completion-table
                                            minibuffer-completion-predicate))
-             (ann-fun (or (completion-metadata-get all-md 'annotation-function)
+             (ann-fun (or (completion-metadata-override-get all-md 'annotation-function)
                           (plist-get completion-extra-properties
                                      :annotation-function)))
-             (aff-fun (or (completion-metadata-get all-md 'affixation-function)
+             (aff-fun (or (completion-metadata-override-get all-md 'affixation-function)
                           (plist-get completion-extra-properties
                                      :affixation-function)))
              (sort-fun (completion-metadata-override-get all-md 'display-sort-function))
-             (group-fun (completion-metadata-get all-md 'group-function))
+             (group-fun (completion-metadata-override-get all-md 'group-function))
              (mainbuf (current-buffer))
              ;; If the *Completions* buffer is shown in a new
              ;; window, mark it as softly-dedicated, so bury-buffer in
@@ -4466,9 +4487,9 @@ completion--flex-adjust-metadata
   "If `flex' is actually doing filtering, adjust sorting."
   (let ((flex-is-filtering-p completion-pcm--regexp)
         (existing-dsf
-         (completion-metadata-get metadata 'display-sort-function))
+         (completion-metadata-override-get metadata 'display-sort-function))
         (existing-csf
-         (completion-metadata-get metadata 'cycle-sort-function)))
+         (completion-metadata-override-get metadata 'cycle-sort-function)))
     (cl-flet
         ((compose-flex-sort-fn (existing-sort-fn)
            (lambda (completions)
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index d49714f3204..00f69e4ad72 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -789,12 +789,12 @@ icomplete--augment
 `group-function'.  Consecutive `equal' sections are avoided.
 COMP is the element in PROSPECTS or a transformation also given
 by `group-function''s second \"transformation\" protocol."
-  (let* ((aff-fun (or (completion-metadata-get md 'affixation-function)
+  (let* ((aff-fun (or (completion-metadata-override-get md 'affixation-function)
                       (plist-get completion-extra-properties :affixation-function)))
-         (ann-fun (or (completion-metadata-get md 'annotation-function)
+         (ann-fun (or (completion-metadata-override-get md 'annotation-function)
                       (plist-get completion-extra-properties :annotation-function)))
          (grp-fun (and completions-group
-                       (completion-metadata-get md 'group-function)))
+                       (completion-metadata-override-get md 'group-function)))
          (annotated
           (cond (aff-fun
            (funcall aff-fun prospects))
diff --git a/lisp/completion-preview.el b/lisp/completion-preview.el
index baadb4714b1..d1d6c5cac50 100644
--- a/lisp/completion-preview.el
+++ b/lisp/completion-preview.el
@@ -231,8 +231,8 @@ completion-preview--try-table
          (exit-fn (plist-get props :exit-function))
          (string (buffer-substring beg end))
          (md (completion-metadata string table pred))
-         (sort-fn (or (completion-metadata-get md 'cycle-sort-function)
-                      (completion-metadata-get md 'display-sort-function)
+         (sort-fn (or (completion-metadata-override-get md 'cycle-sort-function)
+                      (completion-metadata-override-get md 'display-sort-function)
                       completion-preview-sort-function))
          (all (let ((completion-lazy-hilit t))
                 (completion-all-completions string table pred

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-05  7:59         ` Juri Linkov
@ 2024-01-05  8:33           ` Daniel Mendler
  2024-01-06 17:33             ` Juri Linkov
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Mendler @ 2024-01-05  8:33 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 68214

Juri Linkov <juri@linkov.net> writes:

>>>>> This function could be used to look up the other meta data
>>>>> functions too, `display-sort-function`, `annotation-function`,
>>>>> `affixation-function`, `group-function`, etc.
>>>>
>>>> All these meta data functions could be added later to
>>>> completion-category-overrides after pushing the current patch.
>>>
>>> Makes sense, the `display-sort-function' is a good start. I suggest to
>>> at least add the `cycle-sort-function' too. The cycle threshold can be
>>> customized too.
>>
>> Looks like the cycle threshold is already supported,
>> so I'll add the other meta data.
>
> This looks like a complete patch, but it's not completely tested yet:

If we are going with a solution which supports customizing arbitrary
metadata based on category, we could also adjust
`completion-metadata-get' and avoid the introduction of a separate
function `completion-metadata-override-get'. This will result in a
smaller and simpler code change overall.

(defun completion-metadata-get (metadata prop)
  "Get PROP from completion METADATA.
If the metadata specifies a completion category, the variables
`completion-category-overrides' and
`completion-category-defaults' take precedence."
  (if-let (((not (eq prop 'category)))
           (cat (alist-get 'category metadata))
           (over (completion--category-override cat prop)))
      (cdr over)
    (alist-get prop metadata)))





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-05  8:33           ` Daniel Mendler
@ 2024-01-06 17:33             ` Juri Linkov
  2024-01-06 17:59               ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2024-01-06 17:33 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 68214

> If we are going with a solution which supports customizing arbitrary
> metadata based on category, we could also adjust
> `completion-metadata-get' and avoid the introduction of a separate
> function `completion-metadata-override-get'. This will result in a
> smaller and simpler code change overall.
>
> (defun completion-metadata-get (metadata prop)
>   "Get PROP from completion METADATA.
> If the metadata specifies a completion category, the variables
> `completion-category-overrides' and
> `completion-category-defaults' take precedence."
>   (if-let (((not (eq prop 'category)))
>            (cat (alist-get 'category metadata))
>            (over (completion--category-override cat prop)))
>       (cdr over)
>     (alist-get prop metadata)))

This is what I already considered but hesitated to make this change
since it modifies the default behavior.  OTOH, probably there is
no possible harm from this change.  So I'll give this a try.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-06 17:33             ` Juri Linkov
@ 2024-01-06 17:59               ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-01-07 18:05                 ` Juri Linkov
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-06 17:59 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 68214

Juri Linkov <juri@linkov.net> writes:

>> If we are going with a solution which supports customizing arbitrary
>> metadata based on category, we could also adjust
>> `completion-metadata-get' and avoid the introduction of a separate
>> function `completion-metadata-override-get'. This will result in a
>> smaller and simpler code change overall.
>>
>> (defun completion-metadata-get (metadata prop)
>>   "Get PROP from completion METADATA.
>> If the metadata specifies a completion category, the variables
>> `completion-category-overrides' and
>> `completion-category-defaults' take precedence."
>>   (if-let (((not (eq prop 'category)))
>>            (cat (alist-get 'category metadata))
>>            (over (completion--category-override cat prop)))
>>       (cdr over)
>>     (alist-get prop metadata)))
>
> This is what I already considered but hesitated to make this change
> since it modifies the default behavior.  OTOH, probably there is
> no possible harm from this change.  So I'll give this a try.

Thanks. Seems better to take that route, instead of changing all
`completion-metadata-get' calls to `completion-metadata-override-get'
and leaving `completion-metadata-get' mostly unused. As long as we don't
modify `completion-category-defaults' or `completion-category-overrides'
there won't be a change in behavior.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-06 17:59               ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-01-07 18:05                 ` Juri Linkov
  2024-01-07 18:37                   ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2024-01-07 18:05 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 68214

>> This is what I already considered but hesitated to make this change
>> since it modifies the default behavior.  OTOH, probably there is
>> no possible harm from this change.  So I'll give this a try.
>
> Thanks. Seems better to take that route, instead of changing all
> `completion-metadata-get' calls to `completion-metadata-override-get'
> and leaving `completion-metadata-get' mostly unused. As long as we don't
> modify `completion-category-defaults' or `completion-category-overrides'
> there won't be a change in behavior.

Thanks for the suggestion.  Now pushed in commit 18de131222e,
so all metadata properties are supported now in
`completion-category-overrides'.

What do you think about doing the same for
`completion-extra-properties'?

This basically means moving `plist-get completion-extra-properties'
to `completion-metadata-get' that will replace this:

  (ann-fun (or (completion-metadata-get all-md 'annotation-function)
               (plist-get completion-extra-properties
                          :annotation-function)))
  (aff-fun (or (completion-metadata-get all-md 'affixation-function)
               (plist-get completion-extra-properties
                          :affixation-function)))

with just:

  (ann-fun (completion-metadata-get all-md 'annotation-function))
  (aff-fun (completion-metadata-get all-md 'affixation-function))





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-07 18:05                 ` Juri Linkov
@ 2024-01-07 18:37                   ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-01-09 17:59                     ` Juri Linkov
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-07 18:37 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 68214

Juri Linkov <juri@linkov.net> writes:

> Thanks for the suggestion.  Now pushed in commit 18de131222e,
> so all metadata properties are supported now in
> `completion-category-overrides'.

Thanks.

> What do you think about doing the same for
> `completion-extra-properties'?

Yes, makes sense. The documentation of `completion-extra-properties'
would have to be extended accordingly, mentioning all the newly
supported properties. It bothers me a little bit that the
`completion-metadata-get' lookup wouldn't be allocation-free anymore
(because of keywords versus plain symbols), but one could cache the
keyword symbol.

(defun completion-metadata-get (metadata prop)
  "Get property PROP from completion METADATA.
If the metadata specifies a completion category, the variables
`completion-category-overrides' and
`completion-category-defaults' take precedence for
category-specific overrides.  If the completion metadata does not
specify the property, the `completion-extra-properties' plist is
consulted.  Note that the keys of the
`completion-extra-properties' plist are keyword symbols, not
plain symbols."
  (if-let (((not (eq prop 'category)))
           (cat (alist-get 'category metadata))
           (over (completion--category-override cat prop)))
      (cdr over)
    (or (alist-get prop metadata)
        (plist-get completion-extra-properties
                   ;; Cache the keyword
                   (or (get prop 'completion-extra-properties--keyword)
                       (put prop 'completion-extra-properties--keyword
                            (intern (concat ":" (symbol-name prop)))))))))

Daniel





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-07 18:37                   ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-01-09 17:59                     ` Juri Linkov
  2024-01-09 18:14                       ` Juri Linkov
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2024-01-09 17:59 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 68214

>> What do you think about doing the same for
>> `completion-extra-properties'?
>
> Yes, makes sense. The documentation of `completion-extra-properties'
> would have to be extended accordingly, mentioning all the newly
> supported properties. It bothers me a little bit that the
> `completion-metadata-get' lookup wouldn't be allocation-free anymore
> (because of keywords versus plain symbols), but one could cache the
> keyword symbol.
>
> (defun completion-metadata-get (metadata prop)
>   "Get property PROP from completion METADATA.
> If the metadata specifies a completion category, the variables
> `completion-category-overrides' and
> `completion-category-defaults' take precedence for
> category-specific overrides.  If the completion metadata does not
> specify the property, the `completion-extra-properties' plist is
> consulted.  Note that the keys of the
> `completion-extra-properties' plist are keyword symbols, not
> plain symbols."
>   (if-let (((not (eq prop 'category)))
>            (cat (alist-get 'category metadata))
>            (over (completion--category-override cat prop)))
>       (cdr over)
>     (or (alist-get prop metadata)
>         (plist-get completion-extra-properties
>                    ;; Cache the keyword
>                    (or (get prop 'completion-extra-properties--keyword)
>                        (put prop 'completion-extra-properties--keyword
>                             (intern (concat ":" (symbol-name prop)))))))))

Thanks, this is now pushed as well in the commit aff1d53cd46.

Now I tried to use a more user-friendly let-binding of
'completion-extra-properties' like below, but then discovered
that it doesn't work for the category.

Maybe 'completion-metadata-get' could try to get a category
from 'completion-extra-properties' as well?

diff --git a/lisp/calendar/calendar.el b/lisp/calendar/calendar.el
index e01d5d792a6..2c3e7d28301 100644
--- a/lisp/calendar/calendar.el
+++ b/lisp/calendar/calendar.el
@@ -2337,14 +2337,12 @@ calendar-read-date
          (defmon (aref month-array (1- (calendar-extract-month default-date))))
          (completion-ignore-case t)
          (month (cdr (assoc-string
-                      (completing-read
-                       (format-prompt "Month name" defmon)
-                       (lambda (string pred action)
-	                 (if (eq action 'metadata)
-		             '(metadata (category . calendar-month))
-	                   (complete-with-action
-                            action (append month-array nil) string pred)))
-                       nil t nil nil defmon)
+                      (let ((completion-extra-properties
+                             '(:category calendar-month)))
+                        (completing-read
+                         (format-prompt "Month name" defmon)
+                         (append month-array nil)
+                         nil t nil nil defmon))
                       (calendar-make-alist month-array 1) t)))
          (defday (calendar-extract-day default-date))
          (last (calendar-last-day-of-month month year)))





^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-09 17:59                     ` Juri Linkov
@ 2024-01-09 18:14                       ` Juri Linkov
  2024-01-09 18:31                         ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2024-01-09 18:14 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 68214

> Now I tried to use a more user-friendly let-binding of
> 'completion-extra-properties' like below, but then discovered
> that it doesn't work for the category.
>
> Maybe 'completion-metadata-get' could try to get a category
> from 'completion-extra-properties' as well?

Sorry, this was imprecise.  I meant another layer of indirection:
to get a category from 'completion-extra-properties'
to be able to get 'display-sort-function' by category.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-09 18:14                       ` Juri Linkov
@ 2024-01-09 18:31                         ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-01-10  7:35                           ` Juri Linkov
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-09 18:31 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 68214

Juri Linkov <juri@linkov.net> writes:

>> Now I tried to use a more user-friendly let-binding of
>> 'completion-extra-properties' like below, but then discovered
>> that it doesn't work for the category.
>>
>> Maybe 'completion-metadata-get' could try to get a category
>> from 'completion-extra-properties' as well?
>
> Sorry, this was imprecise.  I meant another layer of indirection:
> to get a category from 'completion-extra-properties'
> to be able to get 'display-sort-function' by category.

Indeed. I forgot about this indirection. This should be added as well.
Not sure if there a significantly easier way to write this than as
follows with a separate helper function?

(defun completion--metadata-get-1 (metadata prop)
  (or (alist-get prop metadata)
      (plist-get completion-extra-properties
                 ;; Cache the keyword
                 (or (get prop 'completion-extra-properties--keyword)
                     (put prop 'completion-extra-properties--keyword
                          (intern (concat ":" (symbol-name prop))))))))

(defun completion-metadata-get (metadata prop)
  "Get property PROP from completion METADATA.
If the metadata specifies a completion category, the variables
`completion-category-overrides' and
`completion-category-defaults' take precedence for
category-specific overrides.  If the completion metadata does not
specify the property, the `completion-extra-properties' plist is
consulted.  Note that the keys of the
`completion-extra-properties' plist are keyword symbols, not
plain symbols."
  (if-let (((not (eq prop 'category)))
           (cat (completion--metadata-get-1 'category metadata))
           (over (completion--category-override cat prop)))
      (cdr over)
    (completion--metadata-get-1 prop metadata)))





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-09 18:31                         ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-01-10  7:35                           ` Juri Linkov
  2024-01-10  8:53                             ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2024-01-10  7:35 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: 68214

close 68214 30.0.50
thanks

>>> Now I tried to use a more user-friendly let-binding of
>>> 'completion-extra-properties' like below, but then discovered
>>> that it doesn't work for the category.
>>>
>>> Maybe 'completion-metadata-get' could try to get a category
>>> from 'completion-extra-properties' as well?
>>
>> Sorry, this was imprecise.  I meant another layer of indirection:
>> to get a category from 'completion-extra-properties'
>> to be able to get 'display-sort-function' by category.
>
> Indeed. I forgot about this indirection. This should be added as well.
> Not sure if there a significantly easier way to write this than as
> follows with a separate helper function?

I think other ways such as calling the same function recursively 
would be too ugly.  But a separate function is more clear.

> (defun completion--metadata-get-1 (metadata prop)
>   (or (alist-get prop metadata)
>       (plist-get completion-extra-properties
>                  ;; Cache the keyword
>                  (or (get prop 'completion-extra-properties--keyword)
>                      (put prop 'completion-extra-properties--keyword
>                           (intern (concat ":" (symbol-name prop))))))))
>
> (defun completion-metadata-get (metadata prop)

Now this is pushed as well.

Since all existing metadata functions are completely covered now,
it's time to close this feature request.

Thank you for helping to bring this nice feature to fruition!





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68214: Completion sorting customization by category
  2024-01-10  7:35                           ` Juri Linkov
@ 2024-01-10  8:53                             ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-10  8:53 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 68214

Juri Linkov <juri@linkov.net> writes:

> Now this is pushed as well.
>
> Since all existing metadata functions are completely covered now,
> it's time to close this feature request.
>
> Thank you for helping to bring this nice feature to fruition!

Thank you for finishing this feature! I will add the new
`completion-metadata-get' implementation to the Compat package, such
that ELPA packages can potentially also take advantage of the improved
metadata access.

Daniel





^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-01-10  8:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-02 17:07 bug#68214: Completion sorting customization by category Juri Linkov
2024-01-03  7:20 ` Daniel Mendler
2024-01-03 16:07   ` Juri Linkov
2024-01-03 17:12     ` Daniel Mendler
2024-01-04 17:21       ` Juri Linkov
2024-01-05  7:59         ` Juri Linkov
2024-01-05  8:33           ` Daniel Mendler
2024-01-06 17:33             ` Juri Linkov
2024-01-06 17:59               ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-07 18:05                 ` Juri Linkov
2024-01-07 18:37                   ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-09 17:59                     ` Juri Linkov
2024-01-09 18:14                       ` Juri Linkov
2024-01-09 18:31                         ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-10  7:35                           ` Juri Linkov
2024-01-10  8:53                             ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).