bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

 emacs -Q
 M-! cd d:\gnu TAB

(A directory 'd:/gnu' exists on my disk.)

After pressing the TAB key, the minibuffer displays this:

  cd d:\/gnu/

whereas the expected result is

  cd d:/gnu/

This is a regression from Emacs 23.4, where I do get "d:/gnu/".


In GNU Emacs 24.0.95.1 (i386-mingw-nt5.1.2600)
 of 2012-04-26 on HOME-C4E4A596F7
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (3.4)'

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: ENU
  value of $XMODIFIERS: nil
  locale-coding-system: cp1255
  default enable-multibyte-characters: t

Major mode: Mail

Minor modes in effect:
  shell-dirtrack-mode: t
  diff-auto-refine-mode: t
  flyspell-mode: t
  desktop-save-mode: t
  show-paren-mode: t
  display-time-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
  temp-buffer-resize-mode: t
  line-number-mode: t
  abbrev-mode: t

Recent input:
o r SPC E m a c s SPC 2 4 . 2 . SPC SPC T h e SPC d 
i f f s SPC a r e SPC b v e l o w <C-left> <right> 
<right> <backspace> <M-right> , SPC i f SPC y o u SPC 
w a n t SPC t o SPC <M-backspace> <M-backspace> d o 
n ' t C-y C-x C-x SPC <M-right> w a i t . <return> 
<return> C-u M-! c d d : \ <backspace> <backspace> 
<backspace> SPC d : \ g n <tab> <backspace> <backspace> 
<backspace> <backspace> <backspace> <backspace> \ g 
n u \ e m a c s <backspace> <backspace> <backspace> 
<backspace> <backspace> b z r \ e m a c s \ t r u n 
k SPC & & S-SPC b z r SPC d i f f SPC - c - 1 - <backspace> 
<return> <next> <next> <prior> <next> <prior> <prior> 
<up> <up> <up> <return> <up> <left> <left> <up> <up> 
<down> <C-left> <C-left> <C-right> <C-right> <C-right> 
SPC o n SPC t h e SPC t r u n k <right> ( <C-right> 
<C-right> <C-right> <C-right> ) M-q <down> <right> 
<C-home> C-z C-z C-z C-z C-z C-z C-z C-z C-z C-z C-z 
C-z C-z C-z C-z C-z C-z C-z C-z C-z C-z C-z C-z C-z 
C-z C-z C-z C-z C-z C-z C-z C-z C-z C-z C-z <C-home> 
C-z C-z C-z C-z C-z C-z C-z C-z C-z C-c C-s <help-echo> 
<help-echo> <help-echo> <switch-frame> d <help-echo> 
<help-echo> C-x C-s <switch-frame> <switch-frame> <help-echo> 
<switch-frame> <switch-frame> <help-echo> <help-echo> 
<switch-frame> <help-echo> <help-echo> <help-echo> 
<switch-frame> M-x r e p o r t C-g M-! c d SPC d : 
\ g n u <tab> C-SPC <C-left> <C-left> <C-left> M-w 
C-g M-x r e p o r t - e m <tab> <return>

Recent messages:
Mark set [2 times]
Sending...
Added to d:/usr/eli/rmail/SENT.MAIL
Sending email 
Sending email done
Sending...done
No following nondeleted message
Saving file d:/usr/eli/rmail/INBOX...
Wrote d:/usr/eli/rmail/INBOX [2 times]
Quit
Quit

Load-path shadows:
None found.

Features:
(shadow emacsbug pcmpl-unix shell add-log rmailout network-stream
starttls tls smtpmail auth-source eieio assoc gnus-util password-cache
dabbrev multi-isearch quail help-mode view mailalias sendmail dired-x
dired tcl nxml-uchnm rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid
rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util
rng-pttrn nxml-ns nxml-mode nxml-outln nxml-rap nxml-util nxml-glyph
nxml-enc xmltok sgml-mode face-remap org-wl org-w3m org-vm org-rmail
org-mhe org-mew org-irc org-jsinfo org-infojs org-html org-exp ob-exp
org-exp-blocks find-func org-agenda org-info org-gnus org-docview
org-bibtex bibtex org-bbdb org byte-opt warnings bytecomp byte-compile
cconv macroexp advice help-fns advice-preload ob-emacs-lisp ob-tangle
ob-ref ob-lob ob-table org-footnote org-src ob-comint ob-keys ob
ob-eval org-pcomplete pcomplete org-list org-faces org-compat
org-entities org-macs cal-menu calendar cal-loaddefs noutline outline
arc-mode archive-mode diff-mode conf-mode newcomment sh-script
executable generic jka-compr make-mode texinfo ld-script flyspell
parse-time vc-cvs autorevert gud easy-mmode comint ansi-color ring
info cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align
cc-engine cc-vars cc-defs regexp-opt vc-bzr rmailsum qp rmailmm
message format-spec rfc822 mml mml-sec mm-decode mm-bodies mm-encode
mailabbrev gmm-utils mailheader mail-parse rfc2231 rmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils desktop server
filecache mairix cus-edit easymenu cus-start cus-load wid-edit
saveplace midnight ispell generic-x paren battery time time-date
tooltip ediff-hook vc-hooks lisp-float-type mwheel dos-w32 disp-table
ls-lisp w32-win w32-vars 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 multi-tty
emacs)

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

Ping!

I think this is a serious bug that should be fixed before Emacs 24.1
is released.

If Someone™ tells me where in the completion code to look for the
possible cause(s), I could try debugging this myself.

>  emacs -Q
>  M-! cd d:\gnu TAB
> 
> (A directory 'd:/gnu' exists on my disk.)
> 
> After pressing the TAB key, the minibuffer displays this:
> 
>   cd d:\/gnu/
> 
> whereas the expected result is
> 
>   cd d:/gnu/
> 
> This is a regression from Emacs 23.4, where I do get "d:/gnu/".
> 
> 
> In GNU Emacs 24.0.95.1 (i386-mingw-nt5.1.2600)
>  of 2012-04-26 on HOME-C4E4A596F7
> Windowing system distributor `Microsoft Corp.', version 5.1.2600
> Configured using:
>  `configure --with-gcc (3.4)'

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> If Someone™ tells me where in the completion code to look for the
> possible cause(s), I could try debugging this myself.

The black magic occurs in `comint--complete-file-name-data' in
comint.el.  You could first try to evaluate

  (completion-file-name-table "d:\gnu" nil t)

and see if that does something funny.

Hopefully Stefan chimes in soon; personally, I find the completion code
almost impossible to debug.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: 11348@debbugs.gnu.org
> Date: Fri, 04 May 2012 22:29:41 +0800
> 
> The black magic occurs in `comint--complete-file-name-data' in
> comint.el.  You could first try to evaluate
> 
>   (completion-file-name-table "d:\gnu" nil t)
> 
> and see if that does something funny.

Thanks, I will take a look.

> personally, I find the completion code almost impossible to debug.

Then I'm in good company ;-)

Perhaps Stefan could at some point add some documentation about the
internals, that would allow mere mortals such as myself debug the
completion code.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Drew Adams]

> > personally, I find the completion code almost impossible to debug.
> 
> Then I'm in good company ;-)
> 
> Perhaps Stefan could at some point add some documentation about the
> internals, that would allow mere mortals such as myself debug the
> completion code.

+1

 - mere and mortal Drew

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> The black magic occurs in `comint--complete-file-name-data' in
>> comint.el.  You could first try to evaluate
>> 
>>   (completion-file-name-table "d:\gnu" nil t)
>> 
>> and see if that does something funny.
>
> Thanks, I will take a look.

FWIW, here is my impression.  Emacs 23.4 has the following code in
`comint-dynamic-complete-as-filename':

   (let ((file (concat (file-name-as-directory directory) completion)))
     ;; Insert completion.  Note that the completion string
     ;; may have a different case than what's in the prompt,
     ;; if read-file-name-completion-ignore-case is non-nil,
     (delete-region filename-beg filename-end)
     (if filedir (insert (comint-quote-filename filedir)))

If I'm not mistaken, this is the part that turns d:\gnu into d:/gnu.

Emacs 24 uses the completion-at-point mechanism.  The equivalent code
responsible for replacing the quoted prefix is (IIUC) the `prefixes'
stuff in comint--complete-file-name-data, around comint.el:3178.
That's probably where the bug lies.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: 11348@debbugs.gnu.org
> Date: Fri, 04 May 2012 22:29:41 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > If Someone™ tells me where in the completion code to look for the
> > possible cause(s), I could try debugging this myself.
> 
> The black magic occurs in `comint--complete-file-name-data' in
> comint.el.

That function doesn't get called when I use the recipe to reproduce
the problem.  At least instrumenting it with Edebug doesn't activate
Edebug inside that function.

> You could first try to evaluate
> 
>   (completion-file-name-table "d:\gnu" nil t)
> 
> and see if that does something funny.

It produces "gnu/", which looks correct to me.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> The black magic occurs in `comint--complete-file-name-data' in
>> comint.el.
>
> That function doesn't get called when I use the recipe to reproduce
> the problem.  At least instrumenting it with Edebug doesn't activate
> Edebug inside that function.

Whoops.  Then maybe the problem is in pcomplete-completions-at-point,
which is even more alien territory for me.  As an experiment, could you
try removing pcomplete-completions-at-point from
shell-dynamic-complete-functions and see if there is anything different?

(pcomplete-completions-at-point was added to shell-d-c-f in Emacs 24.)

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: 11348@debbugs.gnu.org
> Date: Fri, 04 May 2012 23:46:31 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> The black magic occurs in `comint--complete-file-name-data' in
> >> comint.el.
> >
> > That function doesn't get called when I use the recipe to reproduce
> > the problem.  At least instrumenting it with Edebug doesn't activate
> > Edebug inside that function.
> 
> Whoops.  Then maybe the problem is in pcomplete-completions-at-point,

I see in pcomplete-completions-at-point that typing "M-! cd d:\gnu TAB"
causes pcomplete-stub on entry to pcomplete-completions-at-point to get
the value "d:gnu", whereas if I type "M-! cd d:/gnu TAB", the value of
pcomplete-stub is "d:/gnu".

Sounds like whoever computes pcomplete-stub is the culprit: they treat
the backslash as an escape character (or so it seems).

> As an experiment, could you try removing
> pcomplete-completions-at-point from shell-dynamic-complete-functions
> and see if there is anything different?

This works better, it produces "cd d:\gnu/ ", which is ugly, but
correct.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Stefan Monnier]

>  M-! cd d:\gnu TAB
> (A directory 'd:/gnu' exists on my disk.)
> After pressing the TAB key, the minibuffer displays this:
>   cd d:\/gnu/

Duh, I somehow assumed it was on the trunk (seeing how it was reported
soon after I modified code on the trunk in this area).

IIUC from the rest of the thread, the problem is in the pcomplete code
(it affects `cd' but not `toto').  That's bad because the pcomplete code
is even trickier than the rest.

This said, based on your description, the problem may simply come from
shell.el's setting of pcomplete-arg-quote-list which tells pcomplete
that \ is an escape char.

I.e. does the patch below fix the problem?

> This works better, it produces "cd d:\gnu/ ", which is ugly, but
> correct.

Which part is ugly?  The \, the /, or the use of a mix of them?

> Perhaps Stefan could at some point add some documentation about the
> internals, that would allow mere mortals such as myself debug the
> completion code.

I'd love to, but I'm much too deeply in it to know what needs more
documentation, so fire away your questions and I'll reply with
comments&docstrings.


        Stefan


=== modified file 'lisp/shell.el'
--- lisp/shell.el	2012-02-28 08:17:21 +0000
+++ lisp/shell.el	2012-05-04 17:57:59 +0000
@@ -429,7 +429,7 @@
   (set (make-local-variable 'pcomplete-parse-arguments-function)
        #'shell-parse-pcomplete-arguments)
   (set (make-local-variable 'pcomplete-arg-quote-list)
-       (append "\\ \t\n\r\"'`$|&;(){}[]<>#" nil))
+       shell-delimiter-argument-list)
   (set (make-local-variable 'pcomplete-termination-string)
        (cond ((not comint-completion-addsuffix) "")
              ((stringp comint-completion-addsuffix)

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 11348@debbugs.gnu.org
> Date: Fri, 04 May 2012 13:59:43 -0400
> 
> This said, based on your description, the problem may simply come from
> shell.el's setting of pcomplete-arg-quote-list which tells pcomplete
> that \ is an escape char.
> 
> I.e. does the patch below fix the problem?

No, I still get "d:\/gnu/".

> > This works better, it produces "cd d:\gnu/ ", which is ugly, but
> > correct.
> 
> Which part is ugly?  The \, the /, or the use of a mix of them?

The mix.

> > Perhaps Stefan could at some point add some documentation about the
> > internals, that would allow mere mortals such as myself debug the
> > completion code.
> 
> I'd love to, but I'm much too deeply in it to know what needs more
> documentation, so fire away your questions and I'll reply with
> comments&docstrings.

A useful beginning would be some overview of the design and
description of the control and data flow in several popular use-cases.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Stefan Monnier]

>> This said, based on your description, the problem may simply come from
>> shell.el's setting of pcomplete-arg-quote-list which tells pcomplete
>> that \ is an escape char.
>> I.e. does the patch below fix the problem?
> No, I still get "d:\/gnu/".

>> > This works better, it produces "cd d:\gnu/ ", which is ugly, but
>> > correct.
>> Which part is ugly?  The \, the /, or the use of a mix of them?

> The mix.

>> > Perhaps Stefan could at some point add some documentation about the
>> > internals, that would allow mere mortals such as myself debug the
>> > completion code.
>> I'd love to, but I'm much too deeply in it to know what needs more
>> documentation, so fire away your questions and I'll reply with
>> comments&docstrings.
> A useful beginning would be some overview of the design and

AFAIK that's in the lispref, but clearly that's not sufficient for you,
so please be more specific.

> description of the control and data flow in several popular use-cases.

Not sure what that could look like.  Would the following be helpful?

For completion--file-name-table, after hitting TAB, here's the
general way it is supposed to work (seen from the completion-table):
- the `metadata' method is called, so the caller can know which
  completion styles should be used, as well as whether escaping/quoting
  should take place.
- because file-names in the minibuffer are quoted (the unquoting
  replaces $$ with $ and expands envvars), which is evidenced by the
  fact that the completion-table is defined with
  completion-table-with-quoting, the text to be completed is unquoted
  and the (quoting)completion table is replaced by the "plain"
  completion table (the details of how this is done is internal to
  completion-table-with-quoting).
- the completion goes on in the simpler unquoted world of file names
  (using the completion-file-name-table).
- each completion style is attempted in sequence, and can use the
  `try-completion' method for simple prefix-based completion, as well as
  `all-completions' and `completion-boundaries' methods for more complex
  styles (the `completion-boundaries' method indicates which part of
  the completed string is *not* included in `all-completions'; in the
  case of file-name the part that's not included is the directory part).
  The returned completion is accompanied with some information about
  where point should go.
- once a style returns a valid completion, that completion is re-quoted
  (because of the use of completion-table-with-quoting) and the
  corresponding position of point in the quoted string is computed.


        Stefan

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Drew Adams]

> > description of the control and data flow in several popular 
> > use-cases.
> 
> Not sure what that could look like.  Would the following be helpful?
> 
> For completion--file-name-table... <helpful info snipped> ...

Yes, this kind of thing is helpful.  I think, at least for what you just wrote,
that simply adding it to the source file would be a good way to communicate it.

Some other info might be more directed to users of the functions concerned, and
so could be added to the Elisp manual.  But for purposes of understanding the
code it should be sufficient to comment the file with similar text to what you
wrote.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 11348@debbugs.gnu.org
> Date: Fri, 04 May 2012 19:32:41 -0400
> 
> >> > Perhaps Stefan could at some point add some documentation about the
> >> > internals, that would allow mere mortals such as myself debug the
> >> > completion code.
> >> I'd love to, but I'm much too deeply in it to know what needs more
> >> documentation, so fire away your questions and I'll reply with
> >> comments&docstrings.
> > A useful beginning would be some overview of the design and
> 
> AFAIK that's in the lispref

Where exactly?  The only place I found is the "Completion" section and
what's under it.  If this is what you meant, then this part of the
manual documents how to _use_ the APIs; it does not describe the
design and the internals.  And neither should it, IMO: the lipsref
manual is meant for Lisp programmers, not for Emacs maintainers.  If
any place in that manual is suitable for the kind of information I'm
looking for, it's the "Internals" appendix.  Commentary in the code is
an even better place.

As for lispref, some parts of what's written need to be clarified.
For example, in "Programmed Completion":

    `(boundaries . SUFFIX)'
          This specifies a `completion-boundaries' operation.  The
          function should return `(boundaries START . END)', where
          START is the position of the beginning boundary in the
          specified string, and END is the position of the end boundary
          in SUFFIX.

This should at least include a cross-reference to where
completion-boundaries are described; without that, it's not even clear
what "boundaries" are being referenced here and what exactly is
SUFFIX.

    `metadata'
          This specifies a request for information about the state of
          the current completion.  The function should return an alist,
          as described below.  The alist may contain any number of
          elements.

Does the last sentence mean that not all of the elements "described
below" should always be returned?  If so, when to return which ones?

  `category'
       The value should be a symbol describing what kind of text the
       completion function is trying to complete.  If the symbol matches
       one of the keys in `completion-category-overrides', the usual
       completion behavior is overridden.  *Note Completion Variables::.

A list of available categories is missing here.  Also, this begs the
question "what will the caller do with the category?", so you should
at least say something about that.

  `annotation-function'
       The value should be a function for "annotating" completions.  The
       function should take one argument, STRING, which is a possible
       completion.  It should return a string, which is displayed after
       the completion STRING in the `*Completions*' buffer.

This is not clear at all.  What are "annotations" in this context, and
what are the possible uses by the caller of the annotations returned
by the function?  The reference to *Completions* buffer is no more
than a hint, and falls short of clarifying the issue (I couldn't
figure out what is "displayed after the completion STRING").  Also,
are there any standard functions that can be reused for this? if so,
name them.

  `cycle-sort-function'
       The value should be a function for sorting completions, when
       `completion-cycle-threshold' is non-`nil' and the user is cycling
       through completion alternatives.  *Note Completion Options:
       (emacs)Completion Options.  Its argument list and return value are
       the same as for `display-sort-function'.

Again, if there are standard cycling functions available, name them.

> > description of the control and data flow in several popular use-cases.
> 
> Not sure what that could look like.  Would the following be helpful?

Yes, very.  A list of completion predicates used for completing on the
most popular objects (files, buffers, shell commands, etc.) would also
be extremely useful.

> For completion--file-name-table, after hitting TAB, here's the
> general way it is supposed to work (seen from the completion-table):
                                                    ^^^^^^^^^^^^^^^^
What is that "completion-table" you refer to here?

> - the `metadata' method is called, so the caller can know which
>   completion styles should be used, as well as whether escaping/quoting
>   should take place.

How does the caller know about escaping/quoting from metadata?

Thanks.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Stefan Monnier]

>> This said, based on your description, the problem may simply come from
>> shell.el's setting of pcomplete-arg-quote-list which tells pcomplete
>> that \ is an escape char.
>> I.e. does the patch below fix the problem?
> No, I still get "d:\/gnu/".

I installed the patch below, which seems to fix this specific problem
(according to my testing under Wine ;-)

>> Which part is ugly?  The \, the /, or the use of a mix of them?
> The mix.

Yes, I think I agree.  I'll have to think about it some more.


        Stefan


=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2012-05-04 10:26:36 +0000
+++ lisp/ChangeLog	2012-05-05 04:16:59 +0000
@@ -1,3 +1,10 @@
+2012-05-05  Stefan Monnier  <monnier@iro.umontreal.ca>
+
+	* shell.el (shell-completion-vars): Set pcomplete-arg-quote-list like
+	shell-delimiter-argument-list.
+	(shell-parse-pcomplete-arguments):
+	Obey pcomplete-arg-quote-list (bug#11348).
+
 2012-05-04  Chong Yidong  <cyd@gnu.org>
 
 	* select.el (xselect--encode-string): Always use utf-8 for TEXT on

=== modified file 'lisp/shell.el'
--- lisp/shell.el	2012-02-28 08:17:21 +0000
+++ lisp/shell.el	2012-05-05 04:13:57 +0000
@@ -393,8 +393,11 @@
             (goto-char (match-end 0))
             (cond
              ((match-beginning 3)       ;Backslash escape.
-              (push (if (= (match-beginning 3) (match-end 3))
-                        "\\" (match-string 3))
+              (push (cond
+                     ((null pcomplete-arg-quote-list)
+                      (goto-char (match-beginning 3)) "\\")
+                     ((= (match-beginning 3) (match-end 3)) "\\")
+                     (t (match-string 3)))
                     arg))
              ((match-beginning 2)       ;Double quote.
               (push (replace-regexp-in-string
@@ -429,7 +432,7 @@
   (set (make-local-variable 'pcomplete-parse-arguments-function)
        #'shell-parse-pcomplete-arguments)
   (set (make-local-variable 'pcomplete-arg-quote-list)
-       (append "\\ \t\n\r\"'`$|&;(){}[]<>#" nil))
+       shell-delimiter-argument-list)
   (set (make-local-variable 'pcomplete-termination-string)
        (cond ((not comint-completion-addsuffix) "")
              ((stringp comint-completion-addsuffix)

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 11348@debbugs.gnu.org
> Date: Sat, 05 May 2012 00:20:17 -0400
> 
> >> This said, based on your description, the problem may simply come from
> >> shell.el's setting of pcomplete-arg-quote-list which tells pcomplete
> >> that \ is an escape char.
> >> I.e. does the patch below fix the problem?
> > No, I still get "d:\/gnu/".
> 
> I installed the patch below, which seems to fix this specific problem
> (according to my testing under Wine ;-)

I'm not sure why it worked for you, because it still doesn't for me.
Are you applying the changes to the emacs-24 branch?  Because that's
what I do, this bug being against Emacs 24.0.96 and a regression from
Emacs 23.4.

According to my debugging inside shell-parse-pcomplete-arguments, what
happens there is that this fragment

          (while (looking-at
                  (eval-when-compile
                    (concat
                     "\\(?:[^\s\t\n\\\"']+"
                     "\\|'\\([^']*\\)'?"
                     "\\|\"\\(\\(?:[^\"\\]\\|\\\\.\\)*\\)\"?"
                     "\\|\\\\\\(\\(?:.\\|\n\\)?\\)\\)")))

decides that \g is an escape sequence.  (Btw, what's the purpose of
using eval-when-compile here?)  Therefore, the value of args at the
end of the loop is

   ("nu" "g" "d:")

After this line:

          (push (mapconcat #'identity (nreverse arg) "") args)))

it becomes

   ("d:" "g" "nu")

and the result of the function is therefore

   (("cd" "d:gnu") 16 19)

By contrast, if I type "M-! cd d:/gnu TAB", the results are,
correspondingly,

   ("d:/gnu")

and

   (("cd" "d:/gnu") 16 19)

IOW, the problem is that shell-parse-pcomplete-arguments removes the
backslash in "d:\gnu", because the last alternative in the above
regexp treats backslashes as escape characters, which on MS-DOS and
MS-Windows is true (for shell commands) only when the backslash
precedes a quote character (").

Thanks.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> I installed the patch below, which seems to fix this specific problem
>> (according to my testing under Wine ;-)
>
> I'm not sure why it worked for you, because it still doesn't for me.

I also still see the bug, running under Wine.

> IOW, the problem is that shell-parse-pcomplete-arguments removes the
> backslash in "d:\gnu", because the last alternative in the above
> regexp treats backslashes as escape characters, which on MS-DOS and
> MS-Windows is true (for shell commands) only when the backslash
> precedes a quote character (").

How about something like the following?  (Works for me on Wine with your
test case, but I don't know if it breaks quoting.)


=== modified file 'lisp/shell.el'
*** lisp/shell.el	2012-05-05 04:18:49 +0000
--- lisp/shell.el	2012-05-07 07:59:33 +0000
***************
*** 397,402 ****
--- 397,408 ----
                       ((null pcomplete-arg-quote-list)
                        (goto-char (match-beginning 3)) "\\")
                       ((= (match-beginning 3) (match-end 3)) "\\")
+ 		     ;; On Windows, the backslash is an escape
+ 		     ;; character only if it precedes a quote char.
+ 		     ((and (memq system-type
+ 				 '(ms-dos windows-nt darwin cygwin))
+ 			   (not (equal (match-string 3) "\"")))
+ 		      (concat "/" (match-string 3)))
                       (t (match-string 3)))
                      arg))
               ((match-beginning 2)       ;Double quote.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  11348@debbugs.gnu.org
> Date: Mon, 07 May 2012 16:01:15 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> I installed the patch below, which seems to fix this specific problem
> >> (according to my testing under Wine ;-)
> >
> > I'm not sure why it worked for you, because it still doesn't for me.
> 
> I also still see the bug, running under Wine.
> 
> > IOW, the problem is that shell-parse-pcomplete-arguments removes the
> > backslash in "d:\gnu", because the last alternative in the above
> > regexp treats backslashes as escape characters, which on MS-DOS and
> > MS-Windows is true (for shell commands) only when the backslash
> > precedes a quote character (").
> 
> How about something like the following?  (Works for me on Wine with your
> test case, but I don't know if it breaks quoting.)
> 
> 
> === modified file 'lisp/shell.el'
> *** lisp/shell.el	2012-05-05 04:18:49 +0000
> --- lisp/shell.el	2012-05-07 07:59:33 +0000
> ***************
> *** 397,402 ****
> --- 397,408 ----
>                        ((null pcomplete-arg-quote-list)
>                         (goto-char (match-beginning 3)) "\\")
>                        ((= (match-beginning 3) (match-end 3)) "\\")
> + 		     ;; On Windows, the backslash is an escape
> + 		     ;; character only if it precedes a quote char.
> + 		     ((and (memq system-type
> + 				 '(ms-dos windows-nt darwin cygwin))
> + 			   (not (equal (match-string 3) "\"")))
> + 		      (concat "/" (match-string 3)))
>                        (t (match-string 3)))
>                       arg))
>                ((match-beginning 2)       ;Double quote.
> 

This fixes the recipe in the original bug report (I get d:\gnu/), but
again fails in this variant:

  M-! cd "d:\gnu TAB

It produces "d:\/gnu/ (with the leading quote) instead of "d:\gnu/

If I type

  M-! cd "d:/gnu TAB

I get "d:/gnu/, as expected.

Thanks.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Stefan Monnier]

> I'm not sure why it worked for you, because it still doesn't for me.
> Are you applying the changes to the emacs-24 branch?  Because that's
> what I do, this bug being against Emacs 24.0.96 and a regression from
> Emacs 23.4.

Yes, I tried it with the 24.0.96 pretest.   Hmm...

> According to my debugging inside shell-parse-pcomplete-arguments, what
> happens there is that this fragment

>           (while (looking-at
>                   (eval-when-compile
>                     (concat
>                      "\\(?:[^\s\t\n\\\"']+"
>                      "\\|'\\([^']*\\)'?"
>                      "\\|\"\\(\\(?:[^\"\\]\\|\\\\.\\)*\\)\"?"
>                      "\\|\\\\\\(\\(?:.\\|\n\\)?\\)\\)")))

> decides that \g is an escape sequence.

No, it does match "\g" but then the new code:

              (push (cond
                     ((null pcomplete-arg-quote-list)
                      (goto-char (match-beginning 3)) "\\")
                     ((= (match-beginning 3) (match-end 3)) "\\")
                     (t (match-string 3)))
                    arg))

sees that pcomplete-arg-quote-list is nil, pushes "\\" and moves
backward 1 char (to right before the "g") so you end up with
("gnu" "\\" "d:").

Can you check to see why this is not happening?

> (Btw, what's the purpose of using eval-when-compile here?)

To avoid calling `concat' at run-time (especially within the loop).
In older Emacsen, `concat' was marked as pure so the byte-compiler would
evaluate it at compile-time, but someone complained that it's not
strictly correct because he wanted that (concat <foo>) is not `eq' to
(concat <foo>).  I actually think this change was a mistake and would
rather mark `concat' as pure again.


        Stefan

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Drew Adams]

> > (Btw, what's the purpose of using eval-when-compile here?)
> 
> To avoid calling `concat' at run-time (especially within the loop).
> In older Emacsen, `concat' was marked as pure so the 
> byte-compiler would evaluate it at compile-time, but someone
> complained that it's not strictly correct because he wanted that
> (concat <foo>) is not `eq' to (concat <foo>).  I actually think
> this change was a mistake and would rather mark `concat' as pure again.

That sounds like the kind of explanation that would help readers as a comment in
the code.  If someone as attentive as Eli has to ask this then it might not hurt
to inform readers generally.

(Ignore my suggestion if inappropriate - I'm not following the thread etc.)

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Chong Yidong]

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

> No, it does match "\g" but then the new code:
>
>               (push (cond
>                      ((null pcomplete-arg-quote-list)
>                       (goto-char (match-beginning 3)) "\\")
>                      ((= (match-beginning 3) (match-end 3)) "\\")
>                      (t (match-string 3)))
>                     arg))
>
> sees that pcomplete-arg-quote-list is nil, pushes "\\" and moves
> backward 1 char (to right before the "g") so you end up with
> ("gnu" "\\" "d:").
>
> Can you check to see why this is not happening?

?? Why should pcomplete-arg-quote-list be nil?  In that same change, you
set it to shell-delimiter-argument-list, which is a non-empty list.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Stefan Monnier]

>> No, it does match "\g" but then the new code:
>> 
>> (push (cond
>> ((null pcomplete-arg-quote-list)
>> (goto-char (match-beginning 3)) "\\")
>> ((= (match-beginning 3) (match-end 3)) "\\")
>> (t (match-string 3)))
>> arg))
>> 
>> sees that pcomplete-arg-quote-list is nil, pushes "\\" and moves
>> backward 1 char (to right before the "g") so you end up with
>> ("gnu" "\\" "d:").
>> 
>> Can you check to see why this is not happening?

> ?? Why should pcomplete-arg-quote-list be nil?  In that same change, you
> set it to shell-delimiter-argument-list, which is a non-empty list.

Duh! I see I committed the wrong version.  I've just installed the patch
below which should hopefully fix this one for good this time.


        Stefan


=== modified file 'lisp/shell.el'
--- lisp/shell.el	2012-05-05 04:18:49 +0000
+++ lisp/shell.el	2012-05-08 00:24:43 +0000
@@ -432,7 +432,7 @@
   (set (make-local-variable 'pcomplete-parse-arguments-function)
        #'shell-parse-pcomplete-arguments)
   (set (make-local-variable 'pcomplete-arg-quote-list)
-       shell-delimiter-argument-list)
+       comint-file-name-quote-list)
   (set (make-local-variable 'pcomplete-termination-string)
        (cond ((not comint-completion-addsuffix) "")
              ((stringp comint-completion-addsuffix)

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  11348@debbugs.gnu.org
> Date: Mon, 07 May 2012 20:27:36 -0400
> 
> >> No, it does match "\g" but then the new code:
> >> 
> >> (push (cond
> >> ((null pcomplete-arg-quote-list)
> >> (goto-char (match-beginning 3)) "\\")
> >> ((= (match-beginning 3) (match-end 3)) "\\")
> >> (t (match-string 3)))
> >> arg))
> >> 
> >> sees that pcomplete-arg-quote-list is nil, pushes "\\" and moves
> >> backward 1 char (to right before the "g") so you end up with
> >> ("gnu" "\\" "d:").
> >> 
> >> Can you check to see why this is not happening?
> 
> > ?? Why should pcomplete-arg-quote-list be nil?  In that same change, you
> > set it to shell-delimiter-argument-list, which is a non-empty list.
> 
> Duh! I see I committed the wrong version.  I've just installed the patch
> below which should hopefully fix this one for good this time.

Thanks.  This fixes the original recipe.  But the following minor
variation (which works correctly in Emacs 23.3) still misfires:

  M-! cd "d:\gnu TAB

I get "d:\/gnu/ after I press TAB above.

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Stefan Monnier]

> Thanks.  This fixes the original recipe.  But the following minor
> variation (which works correctly in Emacs 23.3) still misfires:
>   M-! cd "d:\gnu TAB
> I get "d:\/gnu/ after I press TAB above.

The patch below (which I just installed) might fix it.


        Stefan


=== modified file 'lisp/shell.el'
--- lisp/shell.el	2012-05-08 00:27:13 +0000
+++ lisp/shell.el	2012-05-09 17:18:21 +0000
@@ -400,8 +400,9 @@
                      (t (match-string 3)))
                     arg))
              ((match-beginning 2)       ;Double quote.
-              (push (replace-regexp-in-string
-                     "\\\\\\(.\\)" "\\1" (match-string 2))
+              (push (if (null pcomplete-arg-quote-list) (match-string 2)
+                      (replace-regexp-in-string
+                       "\\\\\\(.\\)" "\\1" (match-string 2)))
                     arg))
              ((match-beginning 1)       ;Single quote.
               (push (match-string 1) arg))

bug#11348: 24.0.95; TAB-completion in shell-command produces d:\/foo on MS-Windows

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: cyd@gnu.org, 11348@debbugs.gnu.org
> Date: Wed, 09 May 2012 13:22:30 -0400
> 
> > Thanks.  This fixes the original recipe.  But the following minor
> > variation (which works correctly in Emacs 23.3) still misfires:
> >   M-! cd "d:\gnu TAB
> > I get "d:\/gnu/ after I press TAB above.
> 
> The patch below (which I just installed) might fix it.

It does, thank!  I think this bug can be closed now.