unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
@ 2011-11-20 15:58 Michael Albinus
  2011-11-22 17:04 ` Stefan Monnier
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Michael Albinus @ 2011-11-20 15:58 UTC (permalink / raw)
  To: 10085

Start "emacs -Q". Apply "C-x C-f /sudo:: TAB". This results in
"/sudo:sudo:root@". A correct expansion would be "/sudo:root@".

As far as I can see, Tramp's completion functions work properly. The
problem seems to be `completion-pcm--find-all-completions'.

If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
For information about debugging Emacs, please read the file
/home/albinus/src/emacs/etc/DEBUG.


In GNU Emacs 24.0.91.8 (x86_64-unknown-linux-gnu, GTK+ Version 2.24.6)
 of 2011-11-19 on detlef
Windowing system distributor `The X.Org Foundation', version 11.0.11004000
Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Emacs-Lisp

Minor modes in effect:
  shell-dirtrack-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC 
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC <down-mouse-1> 
<mouse-1> <prior> <prior> <down-mouse-1> <mouse-movement> 
<mouse-movement> <drag-mouse-1> <help-echo> <help-echo> 
<down-mouse-1> <mouse-1> q C-x C-f / s u d o : : <tab> 
c <help-echo> <help-echo> <down-mouse-3> <mouse-3> 
<down-mouse-1> <mouse-movement> <mouse-movement> <drag-mouse-1> 
<down-mouse-1> <mouse-1> <escape> x r e p o r t C-x 
u <escape> x r e p o r t <tab> <return>

Recent messages:
Result: ("sudo:sudo:root@")

Result: ("sudo:sudo:root@")

Result: nil
Back to top level.
Continue...
Tramp: Opening connection for root@sudo using sudo...done
let*: Command attempted to use minibuffer while in minibuffer
Undo!

Load-path shadows:
None found.

Features:
(shadow sort mail-extr message rfc822 mml mml-sec mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev
mail-utils gmm-utils mailheader emacsbug derived easy-mmode cl-specs cl
edebug debug vc-bzr find-func help-mode easymenu view tramp-cache
warnings trace tramp-sh tramp tramp-compat auth-source eieio byte-opt
bytecomp byte-compile cconv macroexp assoc gnus-util mm-util mail-prsvr
password-cache shell pcomplete comint ring format-spec advice help-fns
advice-preload tramp-loaddefs regexp-opt time-date tooltip ediff-hook
vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image
fringe lisp-mode register page menu-bar rfn-eshadow timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple
abbrev minibuffer loaddefs button faces cus-face files text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dbusbind
dynamic-setting system-font-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty emacs)





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-20 15:58 bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names Michael Albinus
@ 2011-11-22 17:04 ` Stefan Monnier
  2011-11-22 22:13   ` Michael Albinus
  2011-11-22 19:05 ` Stefan Monnier
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2011-11-22 17:04 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 10085

> Start "emacs -Q". Apply "C-x C-f /sudo:: TAB". This results in
> "/sudo:sudo:root@". A correct expansion would be "/sudo:root@".

I haven't tracked this down yet, but while trying to I bumped into the
following problem:

   emacs -Q
   (completion-try-completion "/sudo::" 'completion-file-name-table nil t)
   C-g at the prompt
   C-x C-f /sudo:: TAB

and at that point Emacs is frozen solid.  The backtrace (shown after my
sig) shows that rfn-eshadow (run with inhibit-quit) ends up waiting for
a process that never replies.

Can you look into it, while I try to solve the other problem?


        Stefan


Lisp Backtrace:
"accept-process-output" (0xbfff6874)
"byte-code" (0xbfff6b28)
"tramp-accept-process-output" (0xbfff7094)
"tramp-wait-for-regexp" (0xbfff7444)
"tramp-wait-for-output" (0xbfff77f4)
"tramp-send-command" (0xbfff7b94)
"tramp-sh-handle-expand-file-name" (0xbfff7f44)
"apply" (0xbfff8094)
"tramp-sh-file-name-handler" (0xbfff8424)
"apply" (0xbfff8574)
"byte-code" (0xbfff8828)
"tramp-file-name-handler" (0xbfff8dc8)
"expand-file-name" (0xbfff8f64)
"apply" (0xbfff90b4)
"tramp-completion-run-real-handler" (0xbfff9454)
"tramp-completion-file-name-handler" (0xbfff97e8)
"expand-file-name" (0xbfff99a8)
"tramp-handle-file-remote-p" (0xbfff9d44)
"apply" (0xbfff9e94)
"tramp-sh-file-name-handler" (0xbfffa224)
"apply" (0xbfffa384)
"byte-code" (0xbfffa638)
"tramp-file-name-handler" (0xbfffabd4)
"file-remote-p" (0xbfffaf74)
"apply" (0xbfffb0c4)
"tramp-completion-run-real-handler" (0xbfffb464)
"tramp-completion-file-name-handler" (0xbfffb7f4)
"file-remote-p" (0xbfffbb94)
"byte-code" (0xbfffbe38)
"tramp-rfn-eshadow-update-overlay" (0xbfffc47c)
"run-hooks" (0xbfffc564)
"byte-code" (0xbfffc818)
"rfn-eshadow-update-overlay" (0xbfffcdc8)
"read-from-minibuffer" (0xbfffd66c)
"completing-read-default" (0xbfffda2c)
"completing-read" (0xbfffdb98)
"read-file-name-default" (0xbfffdf5c)
"read-file-name" (0xbfffe304)
"find-file-read-args" (0xbfffe6a4)
"byte-code" (0xbfffe948)
"call-interactively" (0xbfffed18)





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-20 15:58 bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names Michael Albinus
  2011-11-22 17:04 ` Stefan Monnier
@ 2011-11-22 19:05 ` Stefan Monnier
  2011-11-22 21:55   ` Michael Albinus
  2012-03-30 18:28 ` Stefan Monnier
  2016-04-23 20:08 ` bug#10085: Tramp method completions Live System User
  3 siblings, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2011-11-22 19:05 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 10085

> Start "emacs -Q". Apply "C-x C-f /sudo:: TAB". This results in
> "/sudo:sudo:root@". A correct expansion would be "/sudo:root@".

> As far as I can see, Tramp's completion functions work properly.
> The problem seems to be `completion-pcm--find-all-completions'.

I think the patch below fixes it.  The problem is that PCM assumes that
a field separator cannot appear within a field.  In "/sudo::" the first
/ is a field separator, and the last ":" is also a field separator, but
the first ":" isn't.

Sadly the fix could have some detrimental impact on performance, and
I think that the overall problem is linked to an oddity of Tramp's
completion:

If you do "C-x C-f /sudo ?" you get "/sudo:" but if you
do "C-x C-f /sudo: ?" you get "sudo:root@".  So fundamentally, the : of
"/sudo:" acts a field separator, so (completion-boundaries "/sudo:") should
probably return (6 . 0) rather than (1 . 0), and then "C-x C-f /sudo: ?"
should list '("root@" ":") rather than '("sudo:root@").

Related inconsistency from a trace of C-x C-f /sud: TAB:
   1 -> completion-file-name-table: string="/sud:" pred=file-exists-p action=(boundaries . "")
   1 <- completion-file-name-table: (boundaries 5)
   [...]
   ======================================================================
   1 -> completion-file-name-table: string="/sudo:" pred=file-exists-p action=(boundaries . "")
   1 <- completion-file-name-table: (boundaries 1)


        Stefan


=== modified file 'lisp/minibuffer.el'
*** lisp/minibuffer.el	2011-11-19 09:18:31 +0000
--- lisp/minibuffer.el	2011-11-22 18:45:38 +0000
***************
*** 2458,2464 ****
                    (between nil))
                ;; Eliminate submatches that don't end with the separator.
                (dolist (submatch (prog1 suball (setq suball ())))
!                 (when (eq sep (aref submatch (1- (length submatch))))
                    (push submatch suball)))
                (when suball
                  ;; Update the boundaries and corresponding pattern.
--- 2458,2474 ----
                    (between nil))
                ;; Eliminate submatches that don't end with the separator.
                (dolist (submatch (prog1 suball (setq suball ())))
!                 (when (and (eq sep (aref submatch (1- (length submatch))))
! 			   ;; The `sep' check is an optimization, but we need
! 			   ;; to check that submatch really introduces
! 			   ;; a new field.  E.g. When completing "/sudo::",
! 			   ;; prefix="/sudo:" and submatch is "sudo:" which
! 			   ;; matches `sep' but is not sufficient (we'd need
! 			   ;; "sudo::" or "sudo:foo@bar:" to get back to the
! 			   ;; field that we're trying to complete).
! 			   (let ((match (concat subprefix submatch)))
! 			     (eq (length match)
! 				 (completion-boundaries match table pred ""))))
                    (push submatch suball)))
                (when suball
                  ;; Update the boundaries and corresponding pattern.






^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-22 19:05 ` Stefan Monnier
@ 2011-11-22 21:55   ` Michael Albinus
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Albinus @ 2011-11-22 21:55 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 10085

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

>> Start "emacs -Q". Apply "C-x C-f /sudo:: TAB". This results in
>> "/sudo:sudo:root@". A correct expansion would be "/sudo:root@".
>
>> As far as I can see, Tramp's completion functions work properly.
>> The problem seems to be `completion-pcm--find-all-completions'.

> I think the patch below fixes it.

Yes, it does.

> The problem is that PCM assumes that
> a field separator cannot appear within a field.  In "/sudo::" the first
> / is a field separator, and the last ":" is also a field separator, but
> the first ":" isn't.

Unfortunately, I don't know anything about the completion machinery in
minibuffer.el. Maybe I should learn about. Is there documentation which
explains the concept of completion-table?

And is there something Tramp could do? It shall know the boundaries, and
it could provide such information more precisely.

> If you do "C-x C-f /sudo ?" you get "/sudo:" but if you
> do "C-x C-f /sudo: ?" you get "sudo:root@".  So fundamentally, the : of
> "/sudo:" acts a field separator, so (completion-boundaries "/sudo:") should
> probably return (6 . 0) rather than (1 . 0), and then "C-x C-f /sudo: ?"
> should list '("root@" ":") rather than '("sudo:root@").
>
> Related inconsistency from a trace of C-x C-f /sud: TAB:
>    1 -> completion-file-name-table: string="/sud:" pred=file-exists-p action=(boundaries . "")
>    1 <- completion-file-name-table: (boundaries 5)
>    [...]
>    ======================================================================
>    1 -> completion-file-name-table: string="/sudo:" pred=file-exists-p action=(boundaries . "")
>    1 <- completion-file-name-table: (boundaries 1)

I'ld like to check it if I could understand what's behind.

>         Stefan

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-22 17:04 ` Stefan Monnier
@ 2011-11-22 22:13   ` Michael Albinus
  2011-11-22 23:05     ` Stefan Monnier
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2011-11-22 22:13 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 10085

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

>> Start "emacs -Q". Apply "C-x C-f /sudo:: TAB". This results in
>> "/sudo:sudo:root@". A correct expansion would be "/sudo:root@".
>
> I haven't tracked this down yet, but while trying to I bumped into the
> following problem:
>
>    emacs -Q
>    (completion-try-completion "/sudo::" 'completion-file-name-table nil t)

I get

Debugger entered--Lisp error: (wrong-type-argument integerp t)
  completion-try-completion("/sudo::" completion-file-name-table nil t)
  eval((completion-try-completion "/sudo::" (quote completion-file-name-table) nil t) nil)
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp nil nil)

>    C-g at the prompt
>    C-x C-f /sudo:: TAB
>
> and at that point Emacs is frozen solid.  The backtrace (shown after my
> sig) shows that rfn-eshadow (run with inhibit-quit) ends up waiting for
> a process that never replies.
>
> Can you look into it, while I try to solve the other problem?

`expand-file-name' seems to access the remote host. This can be
suppressed by let-binding of `non-essential' to t.

Could you, please, try this patch:

--8<---------------cut here---------------start------------->8---
=== modified file 'lisp/net/tramp.el'
--- lisp/net/tramp.el	2011-11-17 09:09:20 +0000
+++ lisp/net/tramp.el	2011-11-22 22:04:16 +0000
@@ -1609,7 +1609,9 @@
   (ignore-errors
     (let ((end (or (tramp-compat-funcall
 		    'overlay-end (symbol-value 'rfn-eshadow-overlay))
-		   (tramp-compat-funcall 'minibuffer-prompt-end))))
+		   (tramp-compat-funcall 'minibuffer-prompt-end)))
+	  ;; We do not want to send any remote command.
+	  (non-essential t))
       (when
 	  (file-remote-p
 	   (tramp-compat-funcall
--8<---------------cut here---------------end--------------->8---

>         Stefan

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-22 22:13   ` Michael Albinus
@ 2011-11-22 23:05     ` Stefan Monnier
  2011-11-23  6:43       ` Michael Albinus
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2011-11-22 23:05 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 10085

>>> Start "emacs -Q". Apply "C-x C-f /sudo:: TAB". This results in
>>> "/sudo:sudo:root@". A correct expansion would be "/sudo:root@".
>> 
>> I haven't tracked this down yet, but while trying to I bumped into the
>> following problem:
>> 
>> emacs -Q
>> (completion-try-completion "/sudo::" 'completion-file-name-table nil t)
                                                                       ^^^
                                                                       7
Sorry!

> `expand-file-name' seems to access the remote host. This can be
> suppressed by let-binding of `non-essential' to t.

I think adding non-essential to rfn-eshadow.el would be even more
correct (but it doesn't fix the underlying problem).


        Stefan





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-22 23:05     ` Stefan Monnier
@ 2011-11-23  6:43       ` Michael Albinus
  2011-11-23 14:04         ` Stefan Monnier
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2011-11-23  6:43 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 10085

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

>> `expand-file-name' seems to access the remote host. This can be
>> suppressed by let-binding of `non-essential' to t.
>
> I think adding non-essential to rfn-eshadow.el would be even more
> correct (but it doesn't fix the underlying problem).

Shall I submit such a patch towards rfn-eshadow.el, for the sake of the
upcoming release?

Honestly, I do not understand in detail the "underlying problem". I
guess we need to find out, what are separators in Tramp wrt completion
tables, and how to handle them. Note, that I'm re-introducing multi-hops
in Tramp these days (outside the Emacs trunk), which introduces another
separator "|". Likely, the "underlying problem" will be more evident then.

>         Stefan

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-23  6:43       ` Michael Albinus
@ 2011-11-23 14:04         ` Stefan Monnier
  2011-11-23 20:28           ` Michael Albinus
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2011-11-23 14:04 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 10085

>>> `expand-file-name' seems to access the remote host. This can be
>>> suppressed by let-binding of `non-essential' to t.
>> I think adding non-essential to rfn-eshadow.el would be even more
>> correct (but it doesn't fix the underlying problem).
> Shall I submit such a patch towards rfn-eshadow.el, for the sake of the
> upcoming release?

You can even install it now.

> Honestly, I do not understand in detail the "underlying problem".

Simple completion tables have (all-completions STRING TABLE) return
a list whose elements all have STRING as a prefix.

More complex completion tables such as file-name completion instead
return a list whose elements have only a suffix of STRING as a prefix.
Until Emacs-23, completion tables could do this without telling anyone,
which restricted the kind of completion styles that could be implemented
(e.g. partial-completion-mode provided partial-completion for files in
an ad-hoc way, whereas Emacs-23 provides partial-completion for files
without using file-specific knowledge).
Emacs-23 solved this issue by adding a new method to completion tables,
called completion-boundaries which lets the completion table announce
which part of STRING will be the prefix of the all-completions result.

So, completion of "/foo/bar" is divided into fields "foo" and "bar" with
"/" as separators, and (completion-boundaries "/foo/bar") returns 5 to
indicate that "/foo/" will be stripped from all-completions's output,
leaving only "bar" as a prefix of all returned elements.

As I have shown with the trace, Tramp treats "/sud:" and "/sudo:"
differently, making the ":" a boundary separator in the first case but
not in the second.  It's not strictly incorrect, but it is inconsistent
and makes it harder for the minibuffer.el code to behave well.

> I guess we need to find out, what are separators in Tramp wrt
> completion tables, and how to handle them.

The user finds out, the implementer decides.


        Stefan





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-23 14:04         ` Stefan Monnier
@ 2011-11-23 20:28           ` Michael Albinus
  2011-11-24  2:10             ` Stefan Monnier
  2016-04-27 14:16             ` Stefan Monnier
  0 siblings, 2 replies; 19+ messages in thread
From: Michael Albinus @ 2011-11-23 20:28 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 10085

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

>> Shall I submit such a patch towards rfn-eshadow.el, for the sake of the
>> upcoming release?
>
> You can even install it now.

Done.

> As I have shown with the trace, Tramp treats "/sud:" and "/sudo:"
> differently, making the ":" a boundary separator in the first case but
> not in the second.  It's not strictly incorrect, but it is inconsistent
> and makes it harder for the minibuffer.el code to behave well.

Tramp does not know of programmed completion and pcm style completion;
all what it knows is file-name-all-completions. What else could Tramp
do?

>> I guess we need to find out, what are separators in Tramp wrt
>> completion tables, and how to handle them.
>
> The user finds out, the implementer decides.

Oh. You haven't seen Tramp's heuristic to determine, whether completion
of "/sudo" means the method or the user or the host (in fact, it is all).

>         Stefan

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-23 20:28           ` Michael Albinus
@ 2011-11-24  2:10             ` Stefan Monnier
  2016-04-27 14:16             ` Stefan Monnier
  1 sibling, 0 replies; 19+ messages in thread
From: Stefan Monnier @ 2011-11-24  2:10 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 10085

>>> Shall I submit such a patch towards rfn-eshadow.el, for the sake of the
>>> upcoming release?
>> You can even install it now.
> Done.

Thanks.

>> As I have shown with the trace, Tramp treats "/sud:" and "/sudo:"
>> differently, making the ":" a boundary separator in the first case but
>> not in the second.  It's not strictly incorrect, but it is inconsistent
>> and makes it harder for the minibuffer.el code to behave well.
> Tramp does not know of programmed completion and pcm style completion;
> all what it knows is file-name-all-completions.  What else could Tramp do?

Ah, right, Tramp doesn't control it directly but only indirectly.
So maybe the bug is in completion-file-name-table, or in the interaction
between it and Tramp.


        Stefan





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-20 15:58 bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names Michael Albinus
  2011-11-22 17:04 ` Stefan Monnier
  2011-11-22 19:05 ` Stefan Monnier
@ 2012-03-30 18:28 ` Stefan Monnier
  2012-03-31 12:36   ` Michael Albinus
  2016-04-23 20:08 ` bug#10085: Tramp method completions Live System User
  3 siblings, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2012-03-30 18:28 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 10085

> Start "emacs -Q".  Apply "C-x C-f /sudo:: TAB".  This results in
> "/sudo:sudo:root@". A correct expansion would be "/sudo:root@".

Still trying to figure out what's really going on inside Tramp.
Something I bumped into along the way (still doesn't quite explain it) is:

  (let ((non-essential t))   (file-name-nondirectory "/sudo:")) -> "sudo:"
  (let ((non-essential nil)) (file-name-nondirectory "/sudo:")) -> ""

I think it's wrong for those two to return different values.
`non-essential' should prevent connecting to the remote host, but here
neither operation needs to contact the remote host.  And of course,
things are made worse because it's not only `non-essential' that
controls the behavior but also `last-input-event' which makes debugging
that much more .... interesting.


        Stefan





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2012-03-30 18:28 ` Stefan Monnier
@ 2012-03-31 12:36   ` Michael Albinus
  2012-03-31 15:29     ` Stefan Monnier
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2012-03-31 12:36 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 10085

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

> Still trying to figure out what's really going on inside Tramp.
> Something I bumped into along the way (still doesn't quite explain it) is:
>
>   (let ((non-essential t))   (file-name-nondirectory "/sudo:")) -> "sudo:"
>   (let ((non-essential nil)) (file-name-nondirectory "/sudo:")) -> ""
>
> I think it's wrong for those two to return different values.

That's an error, and must be fixed, for sure. The correct result must be "".

I could apply a simple fix in `tramp-find-foreign-file-name-handler',
but this breaks other functionality. Grrrr.

I fear it will take some days, until I have puzzled it out.

> `non-essential' should prevent connecting to the remote host, but here
> neither operation needs to contact the remote host.  And of course,
> things are made worse because it's not only `non-essential' that
> controls the behavior but also `last-input-event' which makes debugging
> that much more .... interesting.

Hmm, yes. This is a heuristic for older Emacsen, which don't give Tramp
a `non-essential' value. Maybe I shall change it, that in case
`non-essential' does exist, no other check is needed. But I don't know,
whether all completion packages outside core Emacs care about
`non-essential' already. We shall postpone this change to 24.2, at least.

>         Stefan

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2012-03-31 12:36   ` Michael Albinus
@ 2012-03-31 15:29     ` Stefan Monnier
  0 siblings, 0 replies; 19+ messages in thread
From: Stefan Monnier @ 2012-03-31 15:29 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 10085

>> Still trying to figure out what's really going on inside Tramp.
>> Something I bumped into along the way (still doesn't quite explain it) is:
>> (let ((non-essential t))   (file-name-nondirectory "/sudo:")) -> "sudo:"
>> (let ((non-essential nil)) (file-name-nondirectory "/sudo:")) -> ""
>> I think it's wrong for those two to return different values.
> That's an error, and must be fixed, for sure. The correct result must be "".

Actually, I like "/sudo:" better, because the completion code gets
caught off-guard when all-completions of "/sudo:" includes "sudo:root@"
but all-completions of "/s" only include "/sudo:"; whereas it is
accustomed to situations where all-completions of "/s" includes "/sudo:"
and all-completions of "sudo:" includes "root@".


        Stefan





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: Tramp method completions
  2011-11-20 15:58 bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names Michael Albinus
                   ` (2 preceding siblings ...)
  2012-03-30 18:28 ` Stefan Monnier
@ 2016-04-23 20:08 ` Live System User
  2016-04-24  8:22   ` Michael Albinus
  3 siblings, 1 reply; 19+ messages in thread
From: Live System User @ 2016-04-23 20:08 UTC (permalink / raw)
  To: 10085; +Cc: Michael Albinus, Stefan Monnier

Hi,

I reported this bug to the tramp-devel mailing list and was told that
this bug was already reported as Bug#10085.

This bug is still present in GNU Emacs 25.0.92.1

Should I open up a new bug report and have Bug#10085 merged
with it so that this old bug (initially reported in November, 2011).
may receive renewed attention?  Or should I just add my bug report
to Bug#10085 as I am now doing?

Here is my bug report below.

Thanks.

I'm having a problem with Tramp and completion.

1. I type C-x C-f (find-file)
   I see "Find file: ~/ in the echo area
2. I type /ssh: and TAB
   I see as completion candidates

Click on a completion to select it.
In this buffer, type RET to select the completion near point.

Possible completions are:
ssh:127.0.0.1:
ssh:localhost.localdomain:
ssh:localhost:

3. I type : so the prompt now displays:

    Find file: ~//ssh::


4. I press TAB and see the error:

Tramp: Opening connection for ssh using ssh...failed


Now the prompt says:

  Find file: ~//ssh:ssh:

and, of course, if I were to press another TAB it would yields
a "[No match]" message

5. If I then remove the ~/ and second ssh from prompt so that it
   now displays:

     Find file: /ssh::

  and press TAB, the completion candidates now yield:

Click on a completion to select it.
In this buffer, type RET to select the completion near point.

Possible completions are:
ssh:127.0.0.1: 	ssh:localhost.localdomain:
ssh:localhost: 	ssh:ssh:



Why was ssh:ssh added as a completion canidate?

This probably why the prompt now shows:

  Find file: /ssh:ssh

6. If I then remove the second ssh: so that prompt shows:

     Find file: /ssh::

  and press TAB, I get the error:

Tramp: Opening connection for ssh using ssh...failed

 and the prompt returns back to:

  Find file: /ssh:ssh:


It appears that once Tramp gets ahold of ~// it doesn't
let go.

This is into contrast to not invoking Tramp.

Consider:

 C-x C-f

and the prompt

 Find file: ~/

and typing /etc so that the prompt is now:

  Find file: ~//etc

Pressing TAB yields

  Find file: ~//etc/

and another TAB shows the files in /etc as completion
canndidates.

The ~/ here (and in Tramp after step 2 above) is greyed out.

Also, it appears that

   ~//ssh:: TAB
   ~//ssh:localhost: TAB

behave diffently as it appears that ~//ssh:localhost: TAB works as I
expect whereas ~//ssh:: TAB does not.

Perhaps :: doesn't imply localhost?


Thanks.







^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: Tramp method completions
  2016-04-23 20:08 ` bug#10085: Tramp method completions Live System User
@ 2016-04-24  8:22   ` Michael Albinus
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Albinus @ 2016-04-24  8:22 UTC (permalink / raw)
  To: Live System User; +Cc: 10085, Stefan Monnier

Live System User <nyc4bos@aol.com> writes:

> Hi,

Hi,

> Should I open up a new bug report and have Bug#10085 merged
> with it so that this old bug (initially reported in November, 2011).
> may receive renewed attention?  Or should I just add my bug report
> to Bug#10085 as I am now doing?

I doubt that opening a new bug report will add more drive to this problem.

Stefan, your last statement was that "maybe the bug is in
completion-file-name-table, or in the interaction between it and Tramp".
Do you have more information now? Could Tramp change something in order
to get this fixed?

> Thanks.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2011-11-23 20:28           ` Michael Albinus
  2011-11-24  2:10             ` Stefan Monnier
@ 2016-04-27 14:16             ` Stefan Monnier
  2016-04-27 18:37               ` Michael Albinus
  1 sibling, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2016-04-27 14:16 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 10085

> Tramp does not know of programmed completion and pcm style completion;
> all what it knows is file-name-all-completions.
> What else could Tramp do?

Tramp has control via file-name-directory.

>>> I guess we need to find out, what are separators in Tramp wrt
>>> completion tables, and how to handle them.
>> The user finds out, the implementer decides.
> Oh. You haven't seen Tramp's heuristic to determine, whether completion
> of "/sudo" means the method or the user or the host (in fact, it is all).

;-)

OK, here are some inconsistencies I found just now in emacs-25:

    ELISP> (completion-boundaries "/sudo:" #'completion-file-name-table nil "")
    (6 . 0)
    
    ELISP> (let ((non-essential t)) (completion-boundaries "/sudo:" #'completion-file-name-table nil ""))
    (1 . 0)

In the above the first answer looks good to me.
The second looks wrong: it should be the same as the first.
This is controlled by Tramp via (file-name-directory "/sudo:").

    ELISP> (all-completions "/sudo:" #'completion-file-name-table)
    *** Eval error ***  Host name must not match method "sudo"
    ELISP> (let ((non-essential t)) (all-completions "/sudo:" #'completion-file-name-table))
    ("sudo:root@")

The first answer above looks wrong (there's no reason for Tramp to
assume that "/sudo:" uses "sudo" as a host name, and indeed in the
second case it correctly interprets "sudo" as a method rather than
a host name).

Assuming we fix the completion-boundaries to be (6 . 0) the second
answer (which comes from `file-name-all-completions "" "/sudo:") should
be ("root@").

Fixing those inconsistencies should fix bug#10085.


        Stefan





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2016-04-27 14:16             ` Stefan Monnier
@ 2016-04-27 18:37               ` Michael Albinus
  2016-04-27 19:16                 ` Stefan Monnier
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2016-04-27 18:37 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 10085

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

Hi Stefan,

>> Tramp does not know of programmed completion and pcm style completion;
>> all what it knows is file-name-all-completions.
>> What else could Tramp do?
>
> Tramp has control via file-name-directory.
>
> OK, here are some inconsistencies I found just now in emacs-25:
>
>     ELISP> (completion-boundaries "/sudo:" #'completion-file-name-table nil "")
>     (6 . 0)
>     
>     ELISP> (let ((non-essential t)) (completion-boundaries "/sudo:"
>     ELISP> #'completion-file-name-table nil ""))
>     (1 . 0)
>
> In the above the first answer looks good to me.
> The second looks wrong: it should be the same as the first.
> This is controlled by Tramp via (file-name-directory "/sudo:").

That's true, there is a bug:

(file-name-directory "/sudo:") => "/sudo:"
(let ((non-essential t)) (file-name-directory "/sudo:")) => "/"

The second answer is wrong, indeed. I will check what's up. 

>     ELISP> (all-completions "/sudo:" #'completion-file-name-table)
>     *** Eval error ***  Host name must not match method "sudo"
>     ELISP> (let ((non-essential t)) (all-completions "/sudo:"
>     ELISP> #'completion-file-name-table))
>     ("sudo:root@")
>
> The first answer above looks wrong (there's no reason for Tramp to
> assume that "/sudo:" uses "sudo" as a host name, and indeed in the
> second case it correctly interprets "sudo" as a method rather than
> a host name).

Hmm, "/sudo:" is a valid remote file name. It uses as method the value
of `tramp-default-host', and as host the string between "/" and
":". That's how it is specified. How shall Tramp know from the
syntactical point of view, that "sudo" is meant as method? It cannot,
unless somebody tells it to Tramp, for example by let-binding
`non-essential'.

> Assuming we fix the completion-boundaries to be (6 . 0) the second
> answer (which comes from `file-name-all-completions "" "/sudo:") should
> be ("root@").
>
> Fixing those inconsistencies should fix bug#10085.

I see. I will check what could be done on Tramp side. Whatever I'll do,
it will go into master.

(Being busy just now, it might take the weekend to work on this. Sorry.)

>         Stefan

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2016-04-27 18:37               ` Michael Albinus
@ 2016-04-27 19:16                 ` Stefan Monnier
  2016-05-02  7:19                   ` Michael Albinus
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2016-04-27 19:16 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 10085

> Hmm, "/sudo:" is a valid remote file name. It uses as method the value
> of `tramp-default-host', and as host the string between "/" and
> ":".  That's how it is specified.

Where/who specifies it?  AFAIK *you* specify it, so you can change it.
More specifically, I think it's OK to say:

   /sudo:      is matched as /<method>:
   /sudo:foo   is matched as /<method>:foo
   /sudo:foo/  is matched as /<host>:foo/

As you know, I'd even be happy to do

   /sudo:foo/  is matched as /<method>:foo/

and force the user to write

   /ssh:sudo:
or
   /sudo.domain:

when she wants to access a host whose name is the same as an existing method.

But IIUC you want to preserve the old /<host>: syntax a bit more
closely, so I think

   /sudo:      is matched as /<method>:
   /sudo:foo   is matched as /<method>:foo
   /sudo:foo/  is matched as /<host>:foo/

might be an acceptable middle ground.

> How shall Tramp know from the syntactical point of view, that "sudo"
> is meant as method?

By fiat ;-)

> It cannot, unless somebody tells it to Tramp, for example by
> let-binding `non-essential'.

IIUC you'd rather skip this part of the discussion, so I won't get into
it ;-)

> Whatever I'll do, it will go into master.

Sounds good to me.


        Stefan





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names
  2016-04-27 19:16                 ` Stefan Monnier
@ 2016-05-02  7:19                   ` Michael Albinus
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Albinus @ 2016-05-02  7:19 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 10085-done

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

>> Hmm, "/sudo:" is a valid remote file name. It uses as method the value
>> of `tramp-default-host', and as host the string between "/" and
>> ":".  That's how it is specified.
>
> Where/who specifies it?  AFAIK *you* specify it, so you can change it.

This was specified even before I've entered the Tramp camp, back in 2002.
And I'm very conservative in changing the interface.

Anyway, I've fixed in Tramp the implementation for `file-name-as-directory',
`file-name-directory' and `file-name-nondirectory' in completion mode.
This solves this bug, I'm closing it.

Thanks for pointing me into the right direction with `file-name-directory'!

>         Stefan

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-05-02  7:19 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-20 15:58 bug#10085: 24.0.91; completion-pcm--find-all-completions returns wrong remote file names Michael Albinus
2011-11-22 17:04 ` Stefan Monnier
2011-11-22 22:13   ` Michael Albinus
2011-11-22 23:05     ` Stefan Monnier
2011-11-23  6:43       ` Michael Albinus
2011-11-23 14:04         ` Stefan Monnier
2011-11-23 20:28           ` Michael Albinus
2011-11-24  2:10             ` Stefan Monnier
2016-04-27 14:16             ` Stefan Monnier
2016-04-27 18:37               ` Michael Albinus
2016-04-27 19:16                 ` Stefan Monnier
2016-05-02  7:19                   ` Michael Albinus
2011-11-22 19:05 ` Stefan Monnier
2011-11-22 21:55   ` Michael Albinus
2012-03-30 18:28 ` Stefan Monnier
2012-03-31 12:36   ` Michael Albinus
2012-03-31 15:29     ` Stefan Monnier
2016-04-23 20:08 ` bug#10085: Tramp method completions Live System User
2016-04-24  8:22   ` Michael Albinus

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).