bug#17545: 24.4.50; icomplete conflicts with minibuffer default

bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Dan McKinley]

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

When completing in the minibuffer using icomplete-mode, an invisible
completion can circumvent the minibuffer's default selection.

For example, say you have icomplete enabled, and you try to kill the
current buffer. The minibuffer will show a confirmation message like,
"Kill buffer (default: foo)". If you type C-j to trigger icomplete
without having actually typed any characters, icomplete will terminate
an unrelated buffer and won't even tell you which it was. It will
terminate whatever buffer it decides is the completion of nothing at
that time.

This is confusing because the minibuffer prompt suggests a completely
different default. Here's a function that fixes the behavior:

(defun icomplete-complete-or-default ()
  (interactive)
  (let* ((start (minibuffer-prompt-end))
         (end (point-max))
         (phrase (buffer-substring start end)))

    (if (zerop (length phrase))
        ; Select the minibuffer's default if there's no text after
        ; the prompt.
        (minibuffer-complete-and-exit)

      ; Select icomplete's default completion if the user has typed
      ; something.
      (minibuffer-force-complete-and-exit))
  ))

Then remapping C-j to call that in the icomplete keymap fixes the bug:

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

I came across this trying to remap icomplete to <return> instead of C-j
so that the keys are more like the (apparently dead) iswitchb. That
doesn't seem like an unreasonable thing to do, although I don't know how
many people used iswitchb.

[-- Attachment #2: Type: text/html, Size: 2635 bytes --]

bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Stefan Monnier]

> "Kill buffer (default: foo)". If you type C-j to trigger icomplete
> without having actually typed any characters, icomplete will terminate
> an unrelated buffer and won't even tell you which it was.

Good point.

> This is confusing because the minibuffer prompt suggests a completely
> different default. Here's a function that fixes the behavior:

I think a better fix is to change icomplete such that if the default is
among the possible completions, then it appears first.


        Stefan

bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Stefan Monnier]

>> This is confusing because the minibuffer prompt suggests a completely
>> different default. Here's a function that fixes the behavior:
> I think a better fix is to change icomplete such that if the default is
> among the possible completions, then it appears first.

In the end I installed patch following the same idea as yours (but
folded into minibuffer-force-complete-and-exit).  Thank you.


        Stefan

Re: bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Dmitry Gutov]

On 05/16/2015 05:23 PM, Stefan Monnier wrote:
>> It seems the easiest way to resolve this is to always make the default
>> the first item, even when it's not in the collection passed to
>> `completing-read'.
>
> But if the user hit C-., that should change the default.

Why? Suppose I've pressed C-. a few times, didn't find what I wanted, 
typed a few chars (didn't find what I wanted there either), erased them, 
then resigned to my fate and pressed RET. Why would the end selection be 
the result of those random C-. presses, rather than the default 
specified by the caller?

On the other hand, we could even call the complaint that started this 
bug "working as intended". Because C-j is bound to 
minibuffer-force-complete-and-exit, and its docstring promises it will 
use the first of the matches. The default isn't even mentioned (the 
current implementation is against the spec in this regard).

But if we wanted to keep that behavior, we could devise a more direct 
way for a completing-read-function to tell minibuffer that some input 
has been performed. (eq (minibuffer-prompt-end) (point-max)) apparently 
doesn't cut it.

Or rather whether any completions are visible. Because if 
icomplete-show-matches-on-no-input is non-nil, it would be harder to 
justify using the default instead of the first completion in 
minibuffer-force-complete-and-exit, even if the user hasn't made any 
other input.

Re: bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Slawomir Nowaczyk]

On Sun, 17 May 2015 04:07:07 +0300
Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 05/16/2015 05:23 PM, Stefan Monnier wrote:
> >> It seems the easiest way to resolve this is to always make the default
> >> the first item, even when it's not in the collection passed to
> >> `completing-read'.
> >
> > But if the user hit C-., that should change the default.
> 
> Why? Suppose I've pressed C-. a few times, didn't find what I wanted, typed a few chars (didn't find what I wanted there either), erased them, then resigned to my fate and pressed RET. Why would the end selection be the result of those random C-. presses, rather than the default specified by the caller?

How is emacs to know if the user has "resigned to his fate" or "found
the item he wants"?

> On the other hand, we could even call the complaint that started this bug "working as intended". Because C-j is bound to minibuffer-force-complete-and-exit, and its docstring promises it will use the first of the matches. The default isn't even mentioned (the current implementation is against the spec in this regard).

Current behaviour definitely doesn't agree with the spec. Whether it's
"working as intended" I don't know -- but I definitely don't like it.

> But if we wanted to keep that behavior, we could devise a more direct way for a completing-read-function to tell minibuffer that some input has been performed. (eq (minibuffer-prompt-end) (point-max)) apparently doesn't cut it.

I agree.

> Or rather whether any completions are visible. Because if icomplete-show-matches-on-no-input is non-nil, it would be harder to justify using the default instead of the first completion in minibuffer-force-complete-and-exit, even if the user hasn't made any other input.

FWIW, my current workaround is:
  (define-key icomplete-minibuffer-map (kbd "C-.") (lambda () "" (interactive) (progn (setq minibuffer-default nil) (icomplete-forward-completions))))
This is very ugly, but does what I want it to do.

-- 
 Best wishes,
   Slawomir Nowaczyk
     ( Slawomir.Nowaczyk@hh.se )

A hundred thousand lemmings can't be wrong.  -- famous last words

Re: bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Stephen Leake]

Slawomir Nowaczyk <Slawomir.Nowaczyk@hh.se> writes:

> On Sun, 17 May 2015 04:07:07 +0300
> Dmitry Gutov <dgutov@yandex.ru> wrote:
>
>> On 05/16/2015 05:23 PM, Stefan Monnier wrote:
>> >> It seems the easiest way to resolve this is to always make the default
>> >> the first item, even when it's not in the collection passed to
>> >> `completing-read'.
>> >
>> > But if the user hit C-., that should change the default.
>> 
>> Why? Suppose I've pressed C-. a few times, didn't find what I
>> wanted, typed a few chars (didn't find what I wanted there either),
>> erased them, then resigned to my fate and pressed RET. Why would the
>> end selection be the result of those random C-. presses, rather than
>> the default specified by the caller?
>
> How is emacs to know if the user has "resigned to his fate" or "found
> the item he wants"?

It needs two different key bindings: C-ret => use the default, ret =>
use current completed selection.

Also need a third key M-ret => use the current minibuffer input, without
completion (useful for renaming files to a non-existing name). Although
this can be combined with C-ret; if input, use that with no completion;
if no input, use the default.

I have a workaround to get ret:

(defun sal-icomplete-force-and-exit ()
  "Select the current completion."
  (interactive)
  ;; we _don't_ want the default specified by read-buffer! (reopen bug#17545)
  (let ((minibuffer-default nil))
    (minibuffer-force-complete-and-exit)))

-- 
-- Stephe

Re: bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Dmitry Gutov]

On 05/18/2015 10:41 PM, Stephen Leake wrote:

> It needs two different key bindings: C-ret => use the default, ret =>
> use current completed selection.

We already have two bindings: RET and C-j.

> Also need a third key M-ret => use the current minibuffer input, without
> completion (useful for renaming files to a non-existing name). Although
> this can be combined with C-ret; if input, use that with no completion;
> if no input, use the default.

That is what RET does.

Although we might want to swap these bindings, considering what these 
keys do in ido-mode, as well as in electric-indent-mode.

Re: bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Stefan Monnier]

> Why? Suppose I've pressed C-. a few times, didn't find what I wanted,
> typed a few chars (didn't find what I wanted there either), erased them,
> then resigned to my fate and pressed RET. Why would the end selection be the
> result of those random C-. presses, rather than the default specified by
> the caller?

AFAIK the issue is that we should return the entry that icomplete
display(s|ed).


        Stefan

Re: bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Dmitry Gutov]

On 05/18/2015 07:45 PM, Stefan Monnier wrote:

> AFAIK the issue is that we should return the entry that icomplete
> display(s|ed).

Well, minibuffer.el doesn't have the necessary information. How about this?

diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index ee28112..b1894ca 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -149,16 +149,26 @@ icompletion is occurring."
  (defvar icomplete-minibuffer-map
    (let ((map (make-sparse-keymap)))
      (define-key map [?\M-\t] 'minibuffer-force-complete)
-    (define-key map [?\C-j]  'minibuffer-force-complete-and-exit)
+    (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-and-exit ()
+  "Complete the minibuffer and exit.
+Use the first of the matches if there are any displayed, and use
+the default otherwise."
+  (interactive)
+  (if (or icomplete-show-matches-on-no-input
+          (> (icomplete--field-end) (icomplete--field-beg)))
+      (minibuffer-force-complete-and-exit)
+    (minibuffer-complete-and-exit)))
+
  (defun icomplete-forward-completions ()
    "Step forward completions by one entry.
  Second entry becomes the first and can be selected with
-`minibuffer-force-complete-and-exit'."
+`icomplete-force-complete-and-exit'."
    (interactive)
    (let* ((beg (icomplete--field-beg))
           (end (icomplete--field-end))
@@ -171,7 +181,7 @@ Second entry becomes the first and can be selected with
  (defun icomplete-backward-completions ()
    "Step backward completions by one entry.
  Last entry becomes the first and can be selected with
-`minibuffer-force-complete-and-exit'."
+`icomplete-force-complete-and-exit'."
    (interactive)
    (let* ((beg (icomplete--field-beg))
           (end (icomplete--field-end))
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 538bd97..60b89b6 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1241,16 +1241,12 @@ scroll the window of possible completions."
  (defun minibuffer-force-complete-and-exit ()
    "Complete the minibuffer with first of the matches and exit."
    (interactive)
-  (if (and (eq (minibuffer-prompt-end) (point-max))
-           minibuffer-default)
-      ;; Use the provided default if there's one (bug#17545).
-      (minibuffer-complete-and-exit)
-    (minibuffer-force-complete)
-    (completion--complete-and-exit
-     (minibuffer-prompt-end) (point-max) #'exit-minibuffer
-     ;; If the previous completion completed to an element which fails
-     ;; test-completion, then we shouldn't exit, but that should be rare.
-     (lambda () (minibuffer-message "Incomplete")))))
+  (minibuffer-force-complete)
+  (completion--complete-and-exit
+   (minibuffer-prompt-end) (point-max) #'exit-minibuffer
+   ;; If the previous completion completed to an element which fails
+   ;; test-completion, then we shouldn't exit, but that should be rare.
+   (lambda () (minibuffer-message "Incomplete"))))

  (defun minibuffer-force-complete (&optional start end)
    "Complete the minibuffer to an exact match.

Re: bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Stefan Monnier]

>> AFAIK the issue is that we should return the entry that icomplete
>> display(s|ed).
> Well, minibuffer.el doesn't have the necessary information.

Currently no.  But we could arrange for icomplete.el to pass that info
to minibuffer.el.

> How about this?

Sounds OK acceptable as well.


        Stefan

Re: bug#17545: 24.4.50; icomplete conflicts with minibuffer default

[Dmitry Gutov]

On 05/19/2015 04:38 AM, Stefan Monnier wrote:

> Currently no.  But we could arrange for icomplete.el to pass that info
> to minibuffer.el.

I see no obvious place to do it.

Further, now that we have an icomplete-specific command, we can try to 
bring it closer to Ido, in command-independent way.

For instance, in ido-find-file RET enters the directory (expands it in 
the input), instead of using it as a match to return.

We could try to choose whether C-j in icomplete-mode returns the current 
completion or just expands it, using the same logic that chooses whether 
to pass `finished' or `exact' to the exit-function in 
minibuffer-completion-help.

> Sounds OK acceptable as well.

Installed.