Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el (read-file-name-default): Bind `non-essential' to `t'.

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el (read-file-name-default): Bind `non-essential' to `t'.

[Stefan Monnier]

>   * minibuffer.el (read-file-name-default): Bind `non-essential' to `t'.
  
This is wrong.  At the time it happens, it is the main focus of the
user's interaction, so it is very much essential.


        Stefan

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el (read-file-name-default): Bind `non-essential' to `t'.

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>   * minibuffer.el (read-file-name-default): Bind `non-essential' to `t'.
>
> This is wrong.  At the time it happens, it is the main focus of the
> user's interaction, so it is very much essential.

`non-essential' is meant to protect user interaction from undesired
events like connection establishing, including password request
etc. Usually, it is the task of the several file name completion
packages to care. Ido and icomplete, for example, behave like this.

However, without the patch in minibuffer, it still happens. This is due
to a `file-exists-p' call somewhere in the completion.

Maybe there is a better place to bind `non-essential'. But I still don't
understand all the completion code :-(

Here's a recipe to reproduce the problem (revert the patch in
minibuffer.el first):

emacs -Q -l icomplete -f icomplete-mode
C-x C-f /ssh:localhost:/ <TAB>

When quitting the password request, there's the backtrace

Debugger entered--Lisp error: (quit)
  signal(quit nil)
  byte-code("\302^H!\210\303    @       A\"\207" [vec err tramp-cleanup signal] 3)
  byte-code("\306^H!\307^H\310\311#\312 !r\313^H!q\210`)^Z^Y^[\211^\\203)^@\314\f!\203)^@\315\f!\316>\204E^@\317\320^H\311\"!^M@\232\203E^@\321\322 ^MA\"\32$
  tramp-maybe-open-connection(["ssh" nil "localhost" "/" nil])
  tramp-send-command(["ssh" nil "localhost" "/" nil] "echo \\\"`getconf PATH 2>/dev/null`\\\" 2>/dev/null; echo tramp_exit_status $?")
  tramp-send-command-and-check(["ssh" nil "localhost" "/" nil] "echo \\\"`getconf PATH 2>/dev/null`\\\"")
  tramp-send-command-and-read(["ssh" nil "localhost" "/" nil] "echo \\\"`getconf PATH 2>/dev/null`\\\"" noerror)
  tramp-get-remote-path(["ssh" nil "localhost" "/" nil])
  tramp-get-remote-perl(["ssh" nil "localhost" "/" nil])
  tramp-sh-handle-file-name-all-completions("" "/ssh:localhost:/")
  apply(tramp-sh-handle-file-name-all-completions ("" "/ssh:localhost:/"))
  tramp-sh-file-name-handler(file-name-all-completions "" "/ssh:localhost:/")
  apply(tramp-sh-file-name-handler file-name-all-completions ("" "/ssh:localhost:/"))
  byte-code("\300\301\215\207" [suppress (apply foreign operation args)] 2)
  #[nil "^HK\306^Y\211^Z<\203\240^@\n@\307=\203\240^@\306^[\310\311!\203\"^@\312\311\313N@!\202\221^@\314\315!\2039^@\316\315!\2044^@\317\315!\205\221^@\315$
  funcall(#[nil "^HK\306^Y\211^Z<\203\240^@\n@\307=\203\240^@\306^[\310\311!\203\"^@\312\311\313N@!\202\221^@\314\315!\2039^@\316\315!\2044^@\317\315!\205\2$
  tramp-file-name-handler(file-name-all-completions "" "/ssh:localhost:/")
  file-name-all-completions("" "/ssh:localhost:/")
  tramp-handle-file-name-completion("" "/ssh:localhost:/" file-exists-p)
  apply(tramp-handle-file-name-completion ("" "/ssh:localhost:/" file-exists-p))
  tramp-sh-file-name-handler(file-name-completion "" "/ssh:localhost:/" file-exists-p)
  apply(tramp-sh-file-name-handler file-name-completion ("" "/ssh:localhost:/" file-exists-p))
  byte-code("\300\301\215\207" [suppress (apply foreign operation args)] 2)
  #[nil "^HK\306^Y\211^Z<\203\240^@\n@\307=\203\240^@\306^[\310\311!\203\"^@\312\311\313N@!\202\221^@\314\315!\2039^@\316\315!\2044^@\317\315!\205\221^@\315$
  funcall(#[nil "^HK\306^Y\211^Z<\203\240^@\n@\307=\203\240^@\306^[\310\311!\203\"^@\312\311\313N@!\202\221^@\314\315!\2039^@\316\315!\2044^@\317\315!\205\2$
  tramp-file-name-handler(file-name-completion "" "/ssh:localhost:/" file-exists-p)
  file-name-completion("" "/ssh:localhost:/" file-exists-p)
  #[0 "\302\306=\203^H^@\307\207\310\300\311^[^\^]\312^^^_\313^M\f^K#,\203)^@\314\315\316\317\320 \"\300\321\311O\301\302%\207\302\242\322=\203M^@\323\300!G$
  funcall(#[0 "\302\306=\203^H^@\307\207\310\300\311^[^\^]\312^^^_\313^M\f^K#,\203)^@\314\315\316\317\320 \"\300\321\311O\301\302%\207\302\242\322=\203M^@\3$
  completion-file-name-table("/ssh:localhost:/" file-exists-p nil)
  try-completion("/ssh:localhost:/" completion-file-name-table file-exists-p)
  completion-basic-try-completion("/ssh:localhost:/" completion-file-name-table file-exists-p 16)
  #[257 "\300^A^M\2368\301@\302@\303\304@$\207" [1 ("/ssh:localhost:/") (completion-file-name-table) file-exists-p (16) completion-styles-alist] 6 "\n\n(fn $
  #[0 "\302\300\301@\301\211@A\240\210@!\240\207" [#[257 "\300^A^M\2368\301@\302@\303\304@$\207" [1 ("/ssh:localhost:/") (completion-file-name-table) file-e$
  funcall(#[0 "\302\300\301@\301\211@A\240\210@!\240\207" [#[257 "\300^A^M\2368\301@\302@\303\304@$\207" [1 ("/ssh:localhost:/") (completion-file-name-table$
  completion--some(#[257 "\300^A^M\2368\301@\302@\303\304@$\207" [1 ("/ssh:localhost:/") (completion-file-name-table) file-exists-p (16) completion-styles-a$
  completion--nth-completion(1 "/ssh:localhost:/" read-file-name-internal file-exists-p 16 (metadata (category . file) (completion--unquote-requote . t)))
  completion-try-completion("/ssh:localhost:/" read-file-name-internal file-exists-p 16 (metadata (category . file) (completion--unquote-requote . t)))
  completion--do-completion()
  minibuffer-complete()
  call-interactively(minibuffer-complete nil nil)
  read-from-minibuffer("Find file: " "~/src/emacs/" (keymap (keymap (32)) keymap (10 . minibuffer-complete-and-exit) (13 . minibuffer-complete-and-exit) key$
  completing-read-default("Find file: " read-file-name-internal file-exists-p confirm-after-completion "~/src/emacs/" file-name-history "~/src/emacs/" nil)
  completing-read("Find file: " read-file-name-internal file-exists-p confirm-after-completion "~/src/emacs/" file-name-history "~/src/emacs/")
  read-file-name-default("Find file: " nil "~/src/emacs/" confirm-after-completion nil nil)
  read-file-name("Find file: " nil "~/src/emacs/" confirm-after-completion)
  find-file-read-args("Find file: " confirm-after-completion)
  byte-code("\300\301\302 \"\207" [find-file-read-args "Find file: " confirm-nonexistent-file-or-buffer] 3)
  call-interactively(find-file nil nil)

>         Stefan

Best regards, Michael.

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el (read-file-name-default): Bind `non-essential' to `t'.

[Stefan Monnier]

> Here's a recipe to reproduce the problem (revert the patch in
> minibuffer.el first):
> emacs -Q -l icomplete -f icomplete-mode
> C-x C-f /ssh:localhost:/ <TAB>

I don't see any problem here.  Tramp requests a password, as expected
and as it should (since the user hit TAB, she clearly wants completion).


        Stefan

RE: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Drew Adams]

> > Here's a recipe to reproduce the problem (revert the patch in
> > minibuffer.el first):
> > emacs -Q -l icomplete -f icomplete-mode
> > C-x C-f /ssh:localhost:/ <TAB>
> 
> I don't see any problem here.  Tramp requests a password, as expected
> and as it should (since the user hit TAB, she clearly wants 
> completion).

If I can chime in about the use case, though not about the implementation/fix -

I think that for most users most of the time, the completions shown by Icomplete
act mainly as background help.  They are not "essential", and most users (or at
least some users) would not want to be interrupted when entering a file name
just to provide some password because their minibuffer input so far was
recognized by Tramp/ange-ftp as possibly corresponding to a remote file.

A user who does want Tramp to prompt for a password need only hit TAB to cause
that.  IOW, I think there is a difference in how much a user is actively
requesting completion between hitting TAB and just letting Icomplete precomplete
the current input.

This is only about Icomplete preventing _its_ preparation of completion
suggestions from prompting the user to access a remote file system.

And if we think that some users do want Tramp to kick in during Icomplete, then
we can provide a user option (e.g. in icomplete.el) to control the behavior.

In sum, I think that most of the time an Icomplete user does not want Icomplete
to be so eager/assertive/aggressive.  (It is a bit like Eldoc.)

Again, I am not speaking about the implementation or about things other than
Icomplete.

If I had to guess about Ido, for instance, it would be that users of Ido might
typically want Tramp to interrupt for a password.

But users should in general be able to ignore Icomplete's suggestions - it
should be possible to treat Icomplete as background noise.  It would be OK for
that to be controlled by a user option.

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Stefan Monnier]

> I think that for most users most of the time, the completions shown by
> Icomplete act mainly as background help.  They are not "essential",
> and most users (or at

AFAICT, the behavior in the test-case is the same with or
without icomplete, so this is not about icomplete (and icomplete
already sets non-essential, which indeed is the right thing to do).


        Stefan

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el (read-file-name-default): Bind `non-essential' to `t'.

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Here's a recipe to reproduce the problem (revert the patch in
>> minibuffer.el first):
>> emacs -Q -l icomplete -f icomplete-mode
>> C-x C-f /ssh:localhost:/ <TAB>
>
> I don't see any problem here.  Tramp requests a password, as expected
> and as it should (since the user hit TAB, she clearly wants completion).

As Drew said, it might disturb users. It wouldn't be necessary, if the
default check wouldn't be `file-exsists-p', which provokes the password
request. Something like

(or (file-remote-p fn) (file-exists-p fn))

should be better (again, I don't know where to change).

Or we spend a customer option for this, which will be the bound value
of `non-essential' in `read-file-name-default'.

>         Stefan

Best regards, Michael.

RE: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Drew Adams]

> > I think that for most users most of the time, the 
> > completions shown by Icomplete act mainly as background help.
> > They are not "essential",
> 
> AFAICT, the behavior in the test-case is the same with or
> without icomplete, so this is not about icomplete (and icomplete
> already sets non-essential, which indeed is the right thing to do).

Right.  Actually, it was I who reported the problem to Michael, which was this:

Without my hitting TAB to ask for completion, Icomplete was prompting for a
password as soon as I typed the `:' here:

 Find file in other window: /foo.bar.com:
                                        ^ This provoked prompt

It is normal and desirable for Tramp to ask for a password if the user hits TAB
and a connection has not yet been opened to the given host.  But not when the
user just types self-inserting chars such as `:'.  That was the point.

(Dunno whether it is relevant, but I am also using FTP as my default Tramp
method.)

Again, if some users do want Icomplete to be more eager and try to also provide
remote matches for connections that have not yet been opened, then let's add a
user option for this.

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el (read-file-name-default): Bind `non-essential' to `t'.

[Stefan Monnier]

>>> Here's a recipe to reproduce the problem (revert the patch in
>>> minibuffer.el first):
>>> emacs -Q -l icomplete -f icomplete-mode
>>> C-x C-f /ssh:localhost:/ <TAB>
>> I don't see any problem here.  Tramp requests a password, as expected
>> and as it should (since the user hit TAB, she clearly wants completion).
> As Drew said, it might disturb users. It wouldn't be necessary, if the
> default check wouldn't be `file-exsists-p', which provokes the password
> request. Something like

> (or (file-remote-p fn) (file-exists-p fn))

> should be better (again, I don't know where to change).

Why?  The user wants to get the list of completions (that's why she hit
TAB), so she really wants to connect.  Not connecting (as we currently
do) is a bug.


        Stefan

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el (read-file-name-default): Bind `non-essential' to `t'.

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Why?  The user wants to get the list of completions (that's why she hit
> TAB), so she really wants to connect.  Not connecting (as we currently
> do) is a bug.

Well, after reading Drew's use case again, I've reverted that patch.

>         Stefan

Best regards, Michael.

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Stefan Monnier]

>  Find file in other window: /foo.bar.com:
>                                         ^ This provoked prompt

Yes, this is a bug, indeed.

> It is normal and desirable for Tramp to ask for a password if the user
> hits TAB and a connection has not yet been opened to the given host.

Agreed.  I still don't understand why this would require changing the
non-icomplete code to fix.


        Stefan

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>  Find file in other window: /foo.bar.com:
>>                                         ^ This provoked prompt
>
> Yes, this is a bug, indeed.

It looks like a bug. But I cannot reproduce it. Scenario:

emacs -Q

(require 'icomplete)
(setq icomplete-with-completion-tables
      (append icomplete-with-completion-tables
      '(read-file-name-internal)))
(icomplete-mode 1)

C-x C-f /ssh:localhost:
                      ^ no problem

                       <TAB>
                       ^ a new connection is established

That's expected. Drew, could you, please, check again when the recent
Windows binaries arrive?

>         Stefan

Best regards, Michael.

RE: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Drew Adams]

> It looks like a bug. But I cannot reproduce it. Scenario:
> 
> emacs -Q
> (require 'icomplete)
> (setq icomplete-with-completion-tables
>       (append icomplete-with-completion-tables
>       '(read-file-name-internal)))
> (icomplete-mode 1)
> 
> C-x C-f /ssh:localhost:
>                       ^ no problem
>                        <TAB>
>                        ^ a new connection is established
> That's expected.

Yes, that's what I would expect.

1. I'm on Windows.  This is missing at the beginning of the recipe:

M-x load-file c:/path/to/cygwin-mount.el
M-x load-file c:/path/to/setup-cygwin.el

2. And you left this out, Michael:

(setq tramp-default-method "ftp")

3. And the last step is this (no ssh):

C-x C-f /foobar.baztoto.com:
                           ^ Tramp asks for the password

(At that point, even C-g won't exit the minibuffer, presumably because Icomplete
does its thing again on post-command-hook (?).  I need to do C-] to exit.)

That's using 24.1 (the release).

> Drew, could you, please, check again when the recent
> Windows binaries arrive?

Will do.  Hopefully that will be in a week or so.  Thanks for working on this.

In the latest version I have, nothing at all happens when I type `:' or when I
then type TAB after the `:' - GNU Emacs 24.1.50.1 (i386-mingw-nt5.1.2600) of
2012-06-18 on MARVIN.

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Michael Albinus]

"Drew Adams" <drew.adams@oracle.com> writes:

> 1. I'm on Windows.  This is missing at the beginning of the recipe:
>
> M-x load-file c:/path/to/cygwin-mount.el
> M-x load-file c:/path/to/setup-cygwin.el

Nothing I could reproduce, I'm not on Windows.

> 2. And you left this out, Michael:
>
> (setq tramp-default-method "ftp")
>
> 3. And the last step is this (no ssh):
>
> C-x C-f /foobar.baztoto.com:
>                            ^ Tramp asks for the password
>

OK, again on Ubuntu:

src/emacs -Q

(require 'icomplete)
(setq icomplete-with-completion-tables
      (append icomplete-with-completion-tables
      '(read-file-name-internal)))
(icomplete-mode 1)
(setq tramp-default-method "ftp")

C-x C-f /foobar.baztoto.com:
                           ^ no problem so far              

                            <TAB>
                            ^ nothing happens

                            / 
                            ^ I get an error:

Error in post-command-hook (#[nil \300\301!\207 [run-hooks
icomplete-post-command-hook] 2]): (error "Command attempted to use
minibuffer while in minibuffer")

But that's another story. Problem in icomplete?

> In the latest version I have, nothing at all happens when I type `:' or when I
> then type TAB after the `:' - GNU Emacs 24.1.50.1 (i386-mingw-nt5.1.2600) of
> 2012-06-18 on MARVIN.

Yes, that's due to the patch to minibuffer.el, which I have reverted
meanwhile.

Best regards, Michael.

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Stefan Monnier]

>> 1. I'm on Windows.  This is missing at the beginning of the recipe:
>> 
>> M-x load-file c:/path/to/cygwin-mount.el
>> M-x load-file c:/path/to/setup-cygwin.el

> Nothing I could reproduce, I'm not on Windows.

>> 2. And you left this out, Michael:
>> 
>> (setq tramp-default-method "ftp")
>> 
>> 3. And the last step is this (no ssh):
>> 
>> C-x C-f /foobar.baztoto.com:
>> ^ Tramp asks for the password
>> 

> OK, again on Ubuntu:

> src/emacs -Q

> (require 'icomplete)
> (setq icomplete-with-completion-tables
>       (append icomplete-with-completion-tables
>       '(read-file-name-internal)))
> (icomplete-mode 1)
> (setq tramp-default-method "ftp")

> C-x C-f /foobar.baztoto.com:
>                            ^ no problem so far              

>                             <TAB>
>                             ^ nothing happens

>                             / 
>                             ^ I get an error:

> Error in post-command-hook (#[nil \300\301!\207 [run-hooks
> icomplete-post-command-hook] 2]): (error "Command attempted to use
> minibuffer while in minibuffer")

> But that's another story. Problem in icomplete?

That "Command attempted to use minibuffer while in minibuffer" is
because icomplete triggered a Tramp connection which tried to use
the minibuffer to get a password.
So if you add (setq enable-recursive-minibuffers t) to the above recipe
you can reproduce the problem.

I don't think Tramp should attempt a connection in that case since
(according to M-x trace-function RET icomplete-completions) the password
prompt happens during the call to icomplete-completions which binds
non-essential to t.


        Stefan

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> C-x C-f /foobar.baztoto.com:
>>                            ^ no problem so far              
>
>>                             <TAB>
>>                             ^ nothing happens
>
>>                             / 
>>                             ^ I get an error:
>
>> Error in post-command-hook (#[nil \300\301!\207 [run-hooks
>> icomplete-post-command-hook] 2]): (error "Command attempted to use
>> minibuffer while in minibuffer")
>
>> But that's another story. Problem in icomplete?
>
> That "Command attempted to use minibuffer while in minibuffer" is
> because icomplete triggered a Tramp connection which tried to use
> the minibuffer to get a password.
> So if you add (setq enable-recursive-minibuffers t) to the above recipe
> you can reproduce the problem.
>
> I don't think Tramp should attempt a connection in that case since
> (according to M-x trace-function RET icomplete-completions) the password
> prompt happens during the call to icomplete-completions which binds
> non-essential to t.

Seems that `ange-ftp-get-passwd' should also care `non-essential'. I'll
work on it, tomorrow. A first attempt with a trivial patch did not
succeed :-(

>         Stefan

Best regards, Michael.

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> I don't think Tramp should attempt a connection in that case since
>> (according to M-x trace-function RET icomplete-completions) the password
>> prompt happens during the call to icomplete-completions which binds
>> non-essential to t.
>
> Seems that `ange-ftp-get-passwd' should also care `non-essential'. I'll
> work on it, tomorrow. A first attempt with a trivial patch did not
> succeed :-(

Should be fixed now, as rev 108665. Drew, could you please check on
Windows with the next binaries?

>>         Stefan

Best regards, Michael.

RE: [Emacs-diffs] /srv/bzr/emacs/trunk r108646: * minibuffer.el(read-file-name-default): Bind `non-essential' to `t'.

[Drew Adams]

> Should be fixed now, as rev 108665. Drew, could you please check on
> Windows with the next binaries?

Will do.  Thanks.