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

[João Távora]

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

>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