bug#34116: 27.0.50; minibuffer-force-complete-and-exit mostly broken

[João Távora]

On Fri, Jan 18, 2019 at 1:09 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> > It seems to work most of the times, but I could swear it is
> > is doing mischief in some situations (can't tell which tho).
>
> I can believe it.

I'll try to pin it down later.

> test-completion should be pretty fast (and it's called by the subsequent
> completion--complete-and-exit so no matter how slow, adding a call
> cannot slowdown the command by more than a factor 2x)

OK, though not particularly reassuring :-)

> > In other words, there is some arguably poorly chosen cons
> > juggling going on in minibuffer-force-complete.  My fix adds
> > to that, but at least it stays within the same idiom.
>
> I must admit that I find your fix a bit ugly (adding an ad-hoc extra
> arg, then undoing cycling depending on (and dont-cycle
> completion-cycling), ...).

Oh yes, no problem admitting that.  It's hideous!  But small
enough to yield to local analysis and be easy to analyse
to be safe.

> Taking another look at the problem, I believe the change below is the
> better option.  Any objection?

Yes, it elegantly fixes this bug, but complicates #34077.
OTOH #34077 could be solved by your try-completion
thing.  But as I said I think that is unstable (or at least has
much more potential to be unstablel).

commit 74ff04e5cd271a238cb9bd966cdbcf9804d49386
Author: João Távora <joaotavora@gmail.com>
Date:   Tue Jan 15 16:50:40 2019 +0000

  Avoid broken C-M-i cycling in icomplete-mode (bug#34077)

  If there is only one propective candidate and it happens to be a
  directory then (1) C-M-i causes the prospects to be updated to the subfiles of
  the completed directory, otherwise (2) the prospects are merely rotated.

  It is very hard to tell if (1) or (2) happened because the rotated
  prospects may look identical to the updated prospects.  Therefore, in
  icomplete-mode, it is preferable to do (1) always, to avoid
  iconsistencies with the presentation of prospects in this mode.  There
  are already facilities in place to rotate the prospects list.

  * lisp/icomplete.el (icomplete-minibuffer-map): Bind C-M-i to
  icomplete-force-complete.
  (icomplete-force-complete): New command.

The current fix I have for it is

(defvar icomplete-minibuffer-map
   (let ((map (make-sparse-keymap)))
-    (define-key map [?\M-\t] 'minibuffer-force-complete)
+    (define-key map [?\M-\t] 'icomplete-force-complete)
     (define-key map [?\C-j]  'icomplete-force-complete-and-exit)
     (define-key map [?\C-.]  'icomplete-forward-completions)
     (define-key map [?\C-,]  'icomplete-backward-completions)
     map)
   "Keymap used by `icomplete-mode' in the minibuffer.")

+(defun icomplete-force-complete ()
+  "Complete the minibuffer.
+Like `minibuffer-force-complete', but don't cycle."
+  (interactive)
+  ;; FIXME: it _could_ make sense to cycle in certain situations, by
+  ;; analyzing the current thing and the thing to cycle to for
+  ;; instance.  Unfortunately that can't be done until a _very nasty
+  ;; hack_ in `minibuffer-force-complete' is removed.  That hack uses
+  ;; transient maps and prevents two consecutive calls to
+  ;; `icomplete-force-complete'.
+  (minibuffer-force-complete nil nil t))

João Távora