bug#34083: 27.0.50; Default completion, if it exists, should always sort to top

bug#34083: 27.0.50; Default completion, if it exists, should always sort to top

[João Távora]

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

Hi maintainers,

In completion-all-sorted-completions, completions are sorted according
to recency of usage, which is a good idea.

However, for calls to completing-read that are given a DEFAULT (actually
DEF) argument, the sort order isn't very useful.  It means that, when
using icomplete for i.e. M-x describe-symbol on top of the symbol sort

   Describe symbol (default sort): { some-other-symbol | yet-another | sort }
                                     ^^^^^^^^^^^^^^^^^
                                        boldface
                                     
This is very confusing in icomplete as "some-other-symbol" in the
previous example is boldface and gives the idea of a default.  It is the
thing that minibuffer-force-complete-and-exit, bound to C-j, will
complete to immediately.  But since no text has been entered, C-m will
make completion consider "sort" instead.

Here's a recipe:

    Emacs -Q
    M-x icomplete-mode
    M-: (setq icomplete-show-matches-on-no-input t) RET
    type "sort"
    C-h o

Verify that "sort" is the default but it doesn't visually in the
"propects list".  Instead "%" is made boldface and C-j and C-M-i will
immediately complete to it.

This inconsistency is easy to fix in minibuffer.el, as attached in a
patch.  After the patch, "sort" is sorted to the top.

I couldn't figure exactly if there is an impact on non-icomplete.el
usage of completion-all-sorted-completions, because I'm not familiar
with that code.  But since it was already using
minibuffer-history-variable, I don't think this disturbs it much more
than that.

Naturally, this could be coded to work in icomplete-mode only, but doing
that patch cleanly is much harder.

João


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Consider-default-completion-in-completion-all-sorted.patch --]
[-- Type: text/x-patch, Size: 2874 bytes --]

From 26acc0bf504a5c3c29abcbdec7af0544cf2aec02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
Date: Tue, 15 Jan 2019 13:27:01 +0000
Subject: [PATCH] Consider default completion in
 completion-all-sorted-completions

* lisp/minibuffer.el (minibuffer-completion-default): New variable.
(completion-all-sorted-completions): Sort with the default on top.
(completing-read-default): Set minibuffer-completion-default
---
 lisp/minibuffer.el | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 5760a2e49d..578fd9ab63 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1210,6 +1210,9 @@ completion--metadata
     (if (eq (car bounds) base) md-at-point
       (completion-metadata (substring string 0 base) table pred))))
 
+(defvar minibuffer-completion-default nil
+  "Within call to `completing-read', this holds the DEF argument.")
+
 (defun completion-all-sorted-completions (&optional start end)
   (or completion-all-sorted-completions
       (let* ((start (or start (minibuffer-prompt-end)))
@@ -1242,12 +1245,17 @@ completion-all-sorted-completions
           (setq all (if sort-fun (funcall sort-fun all)
                       ;; Prefer shorter completions, by default.
                       (sort all (lambda (c1 c2) (< (length c1) (length c2))))))
-          ;; Prefer recently used completions.
+          ;; Prefer recently used completions and put the default, if
+          ;; it exists, on top.
           (when (minibufferp)
-            (let ((hist (symbol-value minibuffer-history-variable)))
-              (setq all (sort all (lambda (c1 c2)
-                                    (> (length (member c1 hist))
-                                       (length (member c2 hist))))))))
+            (let ((hist (symbol-value minibuffer-history-variable))
+                  (def minibuffer-completion-default))
+              (setq all (sort all
+                              (lambda (c1 c2)
+                                (cond ((equal c1 def) t)
+                                      ((equal c2 def) nil)
+                                      (t (> (length (member c1 hist))
+                                            (length (member c2 hist))))))))))
           ;; Cache the result.  This is not just for speed, but also so that
           ;; repeated calls to minibuffer-force-complete can cycle through
           ;; all possibilities.
@@ -3422,6 +3430,7 @@ completing-read-default
 
   (let* ((minibuffer-completion-table collection)
          (minibuffer-completion-predicate predicate)
+         (minibuffer-completion-default def)
          (minibuffer-completion-confirm (unless (eq require-match t)
                                           require-match))
          (base-keymap (if require-match
-- 
2.19.2

bug#34083: 27.0.50; Default completion, if it exists, should always sort to top

[Drew Adams]

> In completion-all-sorted-completions, completions are sorted according
> to recency of usage, which is a good idea.
> 
> However, for calls to completing-read that are given a DEFAULT
> (actually
> DEF) argument, the sort order isn't very useful.  It means that, when
> using icomplete for i.e. M-x describe-symbol on top of the symbol sort
> 
>    Describe symbol (default sort): { some-other-symbol | yet-another |
> sort }
>                                      ^^^^^^^^^^^^^^^^^
>                                         boldface
> 
> This is very confusing in icomplete as "some-other-symbol" in the
> previous example is boldface and gives the idea of a default.  It is
> the
> thing that minibuffer-force-complete-and-exit, bound to C-j, will
> complete to immediately.  But since no text has been entered, C-m will
> make completion consider "sort" instead.
> 
> Here's a recipe:
> 
>     Emacs -Q
>     M-x icomplete-mode
>     M-: (setq icomplete-show-matches-on-no-input t) RET
>     type "sort"
>     C-h o
> 
> Verify that "sort" is the default but it doesn't visually in the
> "propects list".  Instead "%" is made boldface and C-j and C-M-i will
> immediately complete to it.
> 
> This inconsistency is easy to fix in minibuffer.el, as attached in a
> patch.  After the patch, "sort" is sorted to the top.
> 
> I couldn't figure exactly if there is an impact on non-icomplete.el
> usage of completion-all-sorted-completions, because I'm not familiar
> with that code.  But since it was already using
> minibuffer-history-variable, I don't think this disturbs it much more
> than that.
> 
> Naturally, this could be coded to work in icomplete-mode only, but
> doing that patch cleanly is much harder.

Not a good idea, IMHO.  Do I really care?  My own code
sorts completions differently, so I don't care with
that in mind.  But for vanilla Emacs I do care.

The default value of `completing-read' has nothing
to do with the current sort order or the first
completion of that order.  If someone thinks that
Ido or Icomplete needs massaging in this way then
that's where it should be done.  Certainly not at
the level of `completing-read' or (even worse)
`minibuffer.el' (`completion-all-sorted-completions').

"boldface...gives the idea of a default" is an
Ido/Icomplete thing.  If their UIs need clarifying
in this case then that's where to concentrate efforts.
For example, perhaps a different face should be used
when the default is not the first candidate.  (No, I'm
not proposing that or any other change for these UIs.)

(I don't want to belabor this or put forth arguments
for why this is a bad idea.  If the why is not clear
to others who read this bug thread, too bad.)

bug#34083: 27.0.50; Default completion, if it exists, should always sort to top

[João Távora]

On Tue, Jan 15, 2019 at 5:56 PM Drew Adams <drew.adams@oracle.com> wrote:

> (I don't want to belabor this or put forth arguments
> for why this is a bad idea.  If the why is not clear
> to others who read this bug thread, too bad.)

I think it's very sweet that you don't want to put
forth arguments, there's nothing to refute!

Now really: can you give an example where the
proposed change affects the UI, for vanilla Emacs?

João








-- 
João Távora

bug#34083: 27.0.50; Default completion, if it exists, should always sort to top

[Drew Adams]

> > (I don't want to belabor this or put forth arguments
> > for why this is a bad idea.  If the why is not clear
> > to others who read this bug thread, too bad.)
> 
> I think it's very sweet that you don't want to put
> forth arguments, there's nothing to refute!

Yeah, sorry about that.  I just don't care enough.
Feel free to ignore this one user opinion.

bug#34083: 27.0.50; Default completion, if it exists, should always sort to top

[João Távora]

Fixed in f845f8a279cfc2acd1051b4cd4924e2aede54017.

On Tue, Jan 15, 2019 at 6:46 PM Drew Adams <drew.adams@oracle.com> wrote:
>
> > > (I don't want to belabor this or put forth arguments
> > > for why this is a bad idea.  If the why is not clear
> > > to others who read this bug thread, too bad.)
> >
> > I think it's very sweet that you don't want to put
> > forth arguments, there's nothing to refute!
>
> Yeah, sorry about that.  I just don't care enough.
> Feel free to ignore this one user opinion.
>
>
>


-- 
João Távora