Patch 7.3.447

[Bram Moolenaar]

Patch 7.3.447 (after 7.3.446)
Problem: Win32: External commands with "start" do not work.
Solution: Unescape part of the command. (Yasuhiro Matsumoto)
Files: src/os_win32.c

*** ../vim-7.3.446/src/os_win32.c 2012-02-19 18:19:24.000000000 +0100
--- src/os_win32.c 2012-02-21 20:56:51.000000000 +0100
***************
*** 259,264 ****
--- 259,287 ----
}

/*
+ * Unescape characters in "p" that appear in "escaped".
+ */
+ static void
+ unescape_shellxquote(char_u *p, char_u *escaped)
+ {
+ int l = STRLEN(p);
+ int n;
+
+ while (*p != NUL)
+ {
+ if (*p == '^' && vim_strchr(escaped, p[1]) != NULL)
+ mch_memmove(p, p + 1, l--);
+ #ifdef FEAT_MBYTE
+ n = (*mb_ptr2len)(p);
+ #else
+ n = 1;
+ #endif
+ p += n;
+ l -= n;
+ }
+ }
+
+ /*
* Load library "name".
*/
HINSTANCE
***************
*** 3559,3564 ****
--- 3582,3588 ----
garray_T ga;
int delay = 1;
DWORD buffer_off = 0; /* valid bytes in buffer[] */
+ char *p = NULL;

SECURITY_ATTRIBUTES saAttr;

***************
*** 3599,3607 ****
if (options & SHELL_READ)
ga_init2(&ga, 1, BUFLEN);

/* Now, run the command */
CreateProcess(NULL, /* Executable name */
! cmd, /* Command to execute */
NULL, /* Process security attributes */
NULL, /* Thread security attributes */

--- 3623,3640 ----
if (options & SHELL_READ)
ga_init2(&ga, 1, BUFLEN);

+ if (cmd != NULL)
+ {
+ p = (char *)vim_strsave((char_u *)cmd);
+ if (p != NULL)
+ unescape_shellxquote((char_u *)p, p_sxe);
+ else
+ p = cmd;
+ }
+
/* Now, run the command */
CreateProcess(NULL, /* Executable name */
! p, /* Command to execute */
NULL, /* Process security attributes */
NULL, /* Thread security attributes */

***************
*** 3616,3621 ****
--- 3649,3656 ----
&si, /* Startup information */
&pi); /* Process information */

+ if (p != cmd)
+ vim_free(p);

/* Close our unused side of the pipes */
CloseHandle(g_hChildStd_IN_Rd);
***************
*** 3898,4018 ****
else
{
/* we use "command" or "cmd" to start the shell; slow but easy */
! char_u *newcmd;
! long_u cmdlen = (
! #ifdef FEAT_GUI_W32
! (allowPiping && !p_stmp ? 0 : STRLEN(vimrun_path)) +
! #endif
! STRLEN(p_sh) + STRLEN(p_shcf) + STRLEN(cmd) + 10);
!
! newcmd = lalloc(cmdlen, TRUE);
! if (newcmd != NULL)
! {
! char_u *cmdbase = cmd;
!
! /* Skip a leading ", ( and "(. */
! if (*cmdbase == '"' )
! ++cmdbase;
! if (*cmdbase == '(')
! ++cmdbase;
! if ((STRNICMP(cmdbase, "start", 5) == 0) && vim_iswhite(cmdbase[5]))
! {
! STARTUPINFO si;
! PROCESS_INFORMATION pi;
! DWORD flags = CREATE_NEW_CONSOLE;
!
! si.cb = sizeof(si);
! si.lpReserved = NULL;
! si.lpDesktop = NULL;
! si.lpTitle = NULL;
! si.dwFlags = 0;
! si.cbReserved2 = 0;
! si.lpReserved2 = NULL;
!
! cmdbase = skipwhite(cmdbase + 5);
! if ((STRNICMP(cmdbase, "/min", 4) == 0)
! && vim_iswhite(cmdbase[4]))
! {
! cmdbase = skipwhite(cmdbase + 4);
! si.dwFlags = STARTF_USESHOWWINDOW;
! si.wShowWindow = SW_SHOWMINNOACTIVE;
! }
! else if ((STRNICMP(cmdbase, "/b", 2) == 0)
! && vim_iswhite(cmdbase[2]))
! {
! cmdbase = skipwhite(cmdbase + 2);
! flags = CREATE_NO_WINDOW;
! si.dwFlags = STARTF_USESTDHANDLES;
! si.hStdInput = CreateFile("\\\\.\\NUL", // File name
! GENERIC_READ, // Access flags
! 0, // Share flags
! NULL, // Security att.
! OPEN_EXISTING, // Open flags
! FILE_ATTRIBUTE_NORMAL, // File att.
! NULL); // Temp file
! si.hStdOutput = si.hStdInput;
! si.hStdError = si.hStdInput;
! }

! /* When the command is in double quotes, but 'shellxquote' is
! * empty, keep the double quotes around the command.
! * Otherwise remove the double quotes, they aren't needed
! * here, because we don't use a shell to run the command. */
! if (cmdbase > cmd)
! {
! if (STRNCMP(cmd, p_sxq, cmd - cmdbase) != 0)
! {
! STRCPY(newcmd, cmd);
! }
! else
! {
! char_u *p;

! STRCPY(newcmd, cmdbase);
! /* Remove a trailing ", ) and )" if they have a match
! * at the start of the command. */
! p = newcmd + STRLEN(newcmd);
! if (p > newcmd && p[-1] == '"' && *cmd == '"')
! *--p = NUL;
! if (p > newcmd && p[-1] == ')'
! && (*cmd =='(' || cmd[1] == '('))
! *--p = NUL;
! }
! }

! /*
! * Now, start the command as a process, so that it doesn't
! * inherit our handles which causes unpleasant dangling swap
! * files if we exit before the spawned process
! */
! if (CreateProcess(NULL, // Executable name
! newcmd, // Command to execute
! NULL, // Process security attributes
! NULL, // Thread security attributes
! FALSE, // Inherit handles
! flags, // Creation flags
! NULL, // Environment
! NULL, // Current directory
! &si, // Startup information
! &pi)) // Process information
! x = 0;
! else
! {
! x = -1;
#ifdef FEAT_GUI_W32
! EMSG(_("E371: Command not found"));
#endif
- }
- if (si.hStdInput != NULL)
- {
- /* Close the handle to \\.\NUL */
- CloseHandle(si.hStdInput);
- }
- /* Close the handles to the subprocess, so that it goes away */
- CloseHandle(pi.hThread);
- CloseHandle(pi.hProcess);
}
! else
{
#if defined(FEAT_GUI_W32)
if (need_vimrun_warning)
--- 3933,4048 ----
else
{
/* we use "command" or "cmd" to start the shell; slow but easy */
! char_u *cmdbase = cmd;

! /* Skip a leading ", ( and "(. */
! if (*cmdbase == '"' )
! ++cmdbase;
! if (*cmdbase == '(')
! ++cmdbase;
!
! if ((STRNICMP(cmdbase, "start", 5) == 0) && vim_iswhite(cmdbase[5]))
! {
! STARTUPINFO si;
! PROCESS_INFORMATION pi;
! DWORD flags = CREATE_NEW_CONSOLE;
! char_u *p;
!
! si.cb = sizeof(si);
! si.lpReserved = NULL;
! si.lpDesktop = NULL;
! si.lpTitle = NULL;
! si.dwFlags = 0;
! si.cbReserved2 = 0;
! si.lpReserved2 = NULL;
!
! cmdbase = skipwhite(cmdbase + 5);
! if ((STRNICMP(cmdbase, "/min", 4) == 0)
! && vim_iswhite(cmdbase[4]))
! {
! cmdbase = skipwhite(cmdbase + 4);
! si.dwFlags = STARTF_USESHOWWINDOW;
! si.wShowWindow = SW_SHOWMINNOACTIVE;
! }
! else if ((STRNICMP(cmdbase, "/b", 2) == 0)
! && vim_iswhite(cmdbase[2]))
! {
! cmdbase = skipwhite(cmdbase + 2);
! flags = CREATE_NO_WINDOW;
! si.dwFlags = STARTF_USESTDHANDLES;
! si.hStdInput = CreateFile("\\\\.\\NUL", // File name
! GENERIC_READ, // Access flags
! 0, // Share flags
! NULL, // Security att.
! OPEN_EXISTING, // Open flags
! FILE_ATTRIBUTE_NORMAL, // File att.
! NULL); // Temp file
! si.hStdOutput = si.hStdInput;
! si.hStdError = si.hStdInput;
! }
!
! /* Remove a trailing ", ) and )" if they have a match
! * at the start of the command. */
! if (cmdbase > cmd)
! {
! p = cmdbase + STRLEN(cmdbase);
! if (p > cmdbase && p[-1] == '"' && *cmd == '"')
! *--p = NUL;
! if (p > cmdbase && p[-1] == ')'
! && (*cmd =='(' || cmd[1] == '('))
! *--p = NUL;
! }

! /*
! * Unescape characters in shellxescape. This is workaround for
! * /b option. Only redirect character should be unescaped.
! */
! unescape_shellxquote(cmdbase,
! (flags & CREATE_NEW_CONSOLE) ? p_sxe : "<>");

! /*
! * Now, start the command as a process, so that it doesn't
! * inherit our handles which causes unpleasant dangling swap
! * files if we exit before the spawned process
! */
! if (CreateProcess(NULL, // Executable name
! cmdbase, // Command to execute
! NULL, // Process security attributes
! NULL, // Thread security attributes
! FALSE, // Inherit handles
! flags, // Creation flags
! NULL, // Environment
! NULL, // Current directory
! &si, // Startup information
! &pi)) // Process information
! x = 0;
! else
! {
! x = -1;
#ifdef FEAT_GUI_W32
! EMSG(_("E371: Command not found"));
#endif
}
! if (si.hStdInput != NULL)
! {
! /* Close the handle to \\.\NUL */
! CloseHandle(si.hStdInput);
! }
! /* Close the handles to the subprocess, so that it goes away */
! CloseHandle(pi.hThread);
! CloseHandle(pi.hProcess);
! }
! else
! {
! char_u *newcmd;
! long_u cmdlen = (
! #ifdef FEAT_GUI_W32
! (allowPiping && !p_stmp ? 0 : STRLEN(vimrun_path)) +
! #endif
! STRLEN(p_sh) + STRLEN(p_shcf) + STRLEN(cmd) + 10);
!
! newcmd = lalloc(cmdlen, TRUE);
! if (newcmd != NULL)
{
#if defined(FEAT_GUI_W32)
if (need_vimrun_warning)
***************
*** 4038,4045 ****
vim_snprintf((char *)newcmd, cmdlen, "%s %s %s",
p_sh, p_shcf, cmd);
x = mch_system((char *)newcmd, options);
}
- vim_free(newcmd);
}
}

--- 4068,4075 ----
vim_snprintf((char *)newcmd, cmdlen, "%s %s %s",
p_sh, p_shcf, cmd);
x = mch_system((char *)newcmd, options);
+ vim_free(newcmd);
}
}
}

*** ../vim-7.3.446/src/version.c 2012-02-20 22:18:23.000000000 +0100
--- src/version.c 2012-02-21 21:20:05.000000000 +0100
***************
*** 716,717 ****
--- 716,719 ----
{ /* Add new patch number below this line */
+ /**/
+ 447,
/**/

--
From "know your smileys":
:----} You lie like Pinocchio

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[nanashi]

:!start command is not working with /b option,
when cscope is used.

Cscope on Win32
http://iamphet.nm.ru/cscope/index.html

hg clone https://code.google.com/p/vim/
cd vim/src
cscope -b -c -R
gvim7.3.447.exe -u NONE

:!start /min cmd /c echo 1vim_free | cscope -dl -f cscope.out | C:\cygwin-1.7\bin\sed --regexp-extended -e "s/(\S+) (\S+) ([0-9]+)/\1:\3 \2 ^I/" > ./test.txt
/min option is working.

but,
:!start /b cmd /c echo 1vim_free | cscope -dl -f cscope.out | C:\cygwin-1.7\bin\sed --regexp-extended -e "s/(\S+) (\S+) ([0-9]+)/\1:\3 \2 ^I/" > ./test.txt
is not working normally.

[mattn]

ok, I'll look into it later.

[mattn]

> :!start /b cmd /c echo 1vim_free | cscope -dl -f cscope.out | C:\cygwin-1.7\bin\sed --regexp-extended -e "s/(\S+) (\S+) ([0-9]+)/\1:\3 \2 ^I/" > ./test.txt

This is not command which cmd.exe can't treat as couple of commands.
When don't specified /b, it make new 'cmd.exe'. Thus arguments are passed to the cmd.exe as it is.
But /b is specified, all of arguments are not treated correctly.
It need sub-shell.

I wrote patch to fix this problem.

https://raw.github.com/gist/1883709/gistfile1.diff

Below is test cases which you report. This patch is making green currently.

https://gist.github.com/1883728

Thanks.

[nanashi]

thanks.
the /b option did not work in my environment(windows xp).
test 5,7,8 failed.

[mattn]

> the /b option did not work in my environment(windows xp).
> test 5,7,8 failed. 

I tested this on windows xp. :-(

[mattn]

Try to modify sleep "200ms" more large.

[nanashi]

it failed, although it was tried in 30000ms.

[Bram Moolenaar]

Yasuhiro Matsumoto wrote:

Thanks! I do hope we have all corner cases now. At least the number of
complaints has gone down.

--
From "know your smileys":

!-| I-am-a-Cylon-Centurian-with-one-red-eye-bouncing-back-and-forth

[mattn]

please try following.

-------------

$ cat > cmd.c

#include <windows.h>

#include <stdio.h>

int

main(int argc, char* argv[]) {

MessageBox(0, GetCommandLine(), "foo", MB_OK);

return 0;

}

^D

$ c:\mingw\bin\gcc -o cmd.exe cmd.c -luser32

$ gvim -N -u NONE test5.vim

-------------

Then type ":so %", What message is shown? On my environment, following message is shown.

cmd /c ls | nkf -s 

[nanashi]

the result is
cmd /c ls | nkf -s > ./test5.txt

[mattn]

> the result is
> cmd /c ls | nkf -s > ./test5.txt

Ahhh.. Previous patch is crazy broken... Try following.

https://raw.github.com/gist/1885042/gistfile1.diff

[nanashi]

test 5,8 was successful.

test7.vim makes empty file.
:sleep is 30000ms
new cmd.exe's message is
cmd /c echo 1vim_free | cscope -dl -f cscope.out | C:\cygwin\bin\sed --regexp-extended -e "s/(\S+) (\S+) ([0-9]+)/\1:\3 \2 ^I/"

[mattn]

>  test7.vim makes empty file.

Did you run test7.vim under /path/to/src/of/vim7/src ?

If not, result will be empty.

[nanashi]

sorry, sed path was wrong.
all test is green.
thanks very much.

[mattn]

Y(^_^)Y

[Ben Fritz]

On Feb 22, 1:50 am, nanashi <longicalyci...@gmail.com> wrote:
> :!start command is not working with /b option,
> when cscope is used.
>
>

> :!start /min cmd /c echo 1vim_free | cscope -dl -f cscope.out |
> C:\cygwin-1.7\bin\sed --regexp-extended -e "s/(\S+) (\S+) ([0-9]+)/\1:\3 \2
> ^I/" > ./test.txt
> /min option is working.
>
> but,
> :!start /b cmd /c echo 1vim_free | cscope -dl -f cscope.out |
> C:\cygwin-1.7\bin\sed --regexp-extended -e "s/(\S+) (\S+) ([0-9]+)/\1:\3 \2
> ^I/" > ./test.txt
> is not working normally.

I don't know if it's too late, but I wouldn't expect this to work,
based on our previous discussions on cmd.exe quoting.

What you're actually doing here, is:

run cmd /c echo 1vimfree
pipe the result of that whole cmd /c command to cscope
pipe the result to sed
redirect the whole output to test.txt

What you intend is:

run echo | cscope | sed > test.txt all in a cmd /c shell

There are a couple problems here. First of all, you should have quoted
the arg to cmd /c, or escaped the | characters with ^ yourself. True,
Vim can probably fix this (I see a couple later patches which claim
to, but I don't have time to test until maybe later this week or this
weekend). But should it? Does this somehow break some other use-case?
What about using & instead of |?

Second, I'm not sure why it would ever work with /b. :help :!start
says:

2) You can use the /b flag to run console applications without
creating a
console window for them (GUI applications are not affected). But
you
should use this flag only if the application you run doesn't
require any
input. Otherwise it will get an EOF error because its input
stream
(stdin) would be redirected to \\.\NUL (stdout and stderr too).

I've always taken the "stdout and stderr too" bit to mean that I
cannot redirect output to a file if I use /b. Is this not the case?
Have I just been interpreting it wrong even since /b was introduced?
Or maybe this restriction was just something that needed some code to
overcome?

One reason http://vim.wikia.com/wiki/Execute_external_programs_asynchronously_under_Windows
uses /min instead of /b for the extended "async grep" example is to
allow the redirection to work. Does it work with /b if we properly
escape things, potentially using the latest patch?

[Ben Fritz]

On Feb 22, 9:28 am, Ben Fritz <fritzophre...@gmail.com> wrote:
> Second, I'm not sure why it would ever work with /b. :help :!start
> says:
>
>    2) You can use the /b flag to run console applications without
> creating a
>       console window for them (GUI applications are not affected). But
> you
>       should use this flag only if the application you run doesn't
> require any
>       input.  Otherwise it will get an EOF error because its input
> stream
>       (stdin) would be redirected to \\.\NUL (stdout and stderr too).
>
> I've always taken the "stdout and stderr too" bit to mean that I
> cannot redirect output to a file if I use /b. Is this not the case?
> Have I just been interpreting it wrong even since /b was introduced?
> Or maybe this restriction was just something that needed some code to
> overcome?
>

> One reasonhttp://vim.wikia.com/wiki/Execute_external_programs_asynchronously_un...

> uses /min instead of /b for the extended "async grep" example is to
> allow the redirection to work. Does it work with /b if we properly
> escape things, potentially using the latest patch?

Well, I just tried the async grep idea with /b. The only problem (with
7.3.434, so before any of the 'shellxquote' patches) is with the echo
statement in the command (which with /b is never displayed anyway). So
I retract my statement about redirection not working. Can we make the
help clearer? What exactly does that 'stdout and stderr' comment
actually mean?

[mattn]

> Well, I just tried the async grep idea with /b. The only problem (with
> 7.3.434, so before any of the 'shellxquote' patches) is with the echo
> statement in the command (which with /b is never displayed anyway). So
> I retract my statement about redirection not working. Can we make the
> help clearer? What exactly does that 'stdout and stderr' comment
> actually mean?

This is "sub-shell". In unix meanings, \\.\NUL is /dev/null.

(ls > output.txt) 2>&1> /dev/null

As you know, ':!start' is imitation of 'start' command in cmd.exe. So I'm
thinking vim's behavor should be same as cmd's one.
If typing below in cmd.exe, it works correctly.

C:\>start /b cmd /c echo 1vim_free | cscope -dl -f cscope.out | C:\cygwin-1.7\bin\sed --regexp-extended -e "s/(\S+) (\S+) ([0-9]+)/\1:\3 \2

I hope that vim should get same result. And I'm thinking :help is enough to understand behavior currently.

Thanks.

[Ben Fritz]

On Feb 23, 8:01 pm, mattn <mattn...@gmail.com> wrote:
> > What exactly does that 'stdout and stderr' comment
> > actually mean?
>
> This is "sub-shell". In unix meanings, \\.\NUL is /dev/null.
>
> (ls > output.txt) 2>&1> /dev/null
>
> As you know, ':!start' is imitation of 'start' command in cmd.exe. So I'm
> thinking vim's behavor should be same as cmd's one.
> If typing below in cmd.exe, it works correctly.
>

So...you're saying that I can use redirection with :!start /b, if the
redirection is WITHIN a process started by the :!start command?

> C:\>start /b cmd /c echo 1vim_free | cscope -dl -f cscope.out |
> C:\cygwin-1.7\bin\sed --regexp-extended -e "s/(\S+) (\S+) ([0-9]+)/\1:\3 \2
>
> I hope that vim should get same result. And I'm thinking :help is enough to
> understand behavior currently.
>

I don't think the :help is clear enough to understand that right away.
If we can use redirection, we should not say redirection is not
available. We should say that if redirection is needed, you need to
create a new shell within the :!start command.

[Ben Fritz]

On Feb 23, 8:01 pm, mattn <mattn...@gmail.com> wrote:

> As you know, ':!start' is imitation of 'start' command in cmd.exe. So I'm
> thinking vim's behavor should be same as cmd's one.
> If typing below in cmd.exe, it works correctly.
>

I agree. Now, note, with 7.3.456:

C:\>start cmd /k (echo A^^^&B ^& pause)
A new window pops up, "A&B" is echoed at the top, and it says "Press
any key to continue..."

:set shellxquote=(:
:!start cmd /k (echo A^^^&B ^& pause)
A new window pops up, and "A^&B & pause" is echoed at the top.
:!start cmd /k (echo A^&B & pause)
A new window pops up, "A&B" is echoed at the top, and it says "Press
any key to continue..."

:set shellxquote="
:!start cmd /k (echo A^^^&B ^& pause)
A new window pops up, "A&B" is echoed at the top, and it says "Press
any key to continue..."

What should a user expect? That :!start acts like the "start" command
in a cmd.exe shell? Currently, it does not with shellxquote=(. Or
should the user expect that :!start invokes the command with the given
literal arguments as cmd.exe would have passed in after doing its own
processing of escaped characters, etc.? I think conceptually the
former is easier to grasp (and to experiment with outside of Vim to
get right). But, quoting and escaping in cmd.exe is so convoluted (who
knew?) that I suppose either way is acceptable, as long as we provide
an explanation and examples in the :help.

I don't like that shellxquote=" and shellxquote=( have such different
behaviors in this case.

Here's an interesting test:
:!if 1==0 (echo A^&B & pause) else (echo B^&A & echo C^|D & pause)
works as expected
:echo system('if 1==0 (echo A^&B) else (echo B^&A & echo C^|D)')
works as expected
:!start cmd /k (if 1==0 (echo A^&B) else (echo B^&A & echo C^|D))
Works as *intended* with shellxquote=( but not as one might expect if
one tries the same command from a cmd.exe shell
Does NOT work as intended with shellxquote=", but this is to be
expected if the command is compared to a cmd.exe shell.

Escaping the !start command with ^ characters makes it work as
intended and expected for shellxquote=" but breaks shellxquote=(.

Arguably it is easier for the user if we allow them to use mostly the
same string for :!start cmd /c as they do for system() and :!cmd, but
pedantically it's wrong.

Should we just update the help and call it good or try to make :!start
actually behave like "start" from a cmd.exe shell?

[mattn]

Ok, please give me some time to fix.