xilun / cbwin

Launch Windows programs from "Bash on Ubuntu on Windows" (WSL)
Other
327 stars 25 forks source link

Use double quotes to prevent word splitting may be better #25

Open goreliu opened 8 years ago

goreliu commented 8 years ago

When I pass a filename which contains spaces to wcmd/wrun/wstart:

# Use tab to complete filename
$ wstart c:/mine/app/mpv/mpv.exe aa\ bb.mp4

Playing: aa
[file] Cannot open file 'aa': No such file or directory
Failed to open aa.

Playing: bb.mp4
[file] Cannot open file 'bb.mp4': No such file or directory
Failed to open bb.mp4.

or

$ wstart c:/mine/app/mpv/mpv.exe "aa bb.mp4"
Playing: aa
[file] Cannot open file 'aa': No such file or directory
Failed to open aa.

Playing: bb.mp4
[file] Cannot open file 'bb.mp4': No such file or directory
Failed to open bb.mp4.

aa bb.mp4 was splited to two filenames.

# It works, but inconveniently.
$ wrun c:/mine/app/mpv/mpv.exe '"aa bb.mp4"'
Playing: aa bb.mp4

So I wrote a shell function:

vrun() {
    local cmd arg
    cmd="$1"
    shift

    for i in "$@"; do
        arg=$arg' "'$i'"'
    done

    wrun $cmd $arg
}

$ vrun c:/mine/app/mpv/mpv.exe aa\ bb.mp4
Playing: aa bb.mp4

$ vrun c:/mine/app/mpv/mpv.exe "aa bb.mp4"
Playing: aa bb.mp4

But I think wrun/wcmd/wstart should do this work, like this:

diff --git a/caller/wrun.c b/caller/wrun.c
index ed99a21..4dbcff4 100644
--- a/caller/wrun.c
+++ b/caller/wrun.c

@@ -964,8 +981,9 @@ int main(int argc, char *argv[])

     bool sep = false;
     for (int i = 0; i < argc; i++) {
-        if (sep) string_append(&outbash_command, " ");
+        if (sep) string_append(&outbash_command, " \"");
         string_append(&outbash_command, argv[i]);
+        if (sep) string_append(&outbash_command, "\"");
         sep = true;
     }
     string_append(&outbash_command, "\n\n");
@@ -1017,6 +1035,7 @@ int main(int argc, char *argv[])
         terminate_nocore();
     }
xilun commented 8 years ago

One problem is that Win32 escaping/splitting of the command line is not 100% standardized. But I guess by default we could use the MSVC / CommandLineToArgvW way and add an option to stay raw.

xilun commented 8 years ago

Update: when we go through cmd.exe we must also take into account its own parsing rules to do proper escaping.

goreliu commented 8 years ago

I didn't add quotes to the first argument(command), so wrun/wcmd/wstart 'anything' is OK. When we want to use cmd.exe's parsing rules, can run with wrun/wcmd/wstart 'anything', just 1 argument.

Examples:

# 2 arguments, 1: aa bb 2: cc
$ wcmd echo "aa bb" cc
"aa bb" "cc"

# 3 arguments, 1:aa 2:bb 3: cc
$ wcmd echo aa bb cc
"aa" "bb" "cc"

# I don't kown how many arguments there are, pass the string to cmd.exe(or other softwares). Some softwares think `aa bb cc` is 1 argument, others think there are 3 arguments, I don't care.
$ wcmd 'echo aa bb cc'
aa bb cc
xilun commented 8 years ago

This is slightly more tricky than it looks like at first, for example for now with no quoting/escaping at all:

$ wcmd  "foo&bar" 2000
'foo' n’est pas reconnu en tant que commande interne
ou externe, un programme exécutable ou un fichier de commandes.
'bar' n’est pas reconnu en tant que commande interne
ou externe, un programme exécutable ou un fichier de commandes.

A reasonable expectation if we handle quoting and escaping would be that an attempt to run e.g. foo&bar.exe is performed instead of that, but & is interpreted by cmd, so when using cmd we should escape some characters. Then if we do so, the behavior if we remove the "2000" param should not be drastically different.

goreliu commented 8 years ago

If someone run wcmd "foo&bar" 2000, I don't think wcmd should convert foo&bar to foo^&bar, because wcmd "foo&bar" seems same with wcmd "dir&pause". He didn't tell wcmd that foo&bar is a filename. He should run wcmd "foo^&bar" if he has a foo&bar.exe.

But if I want to edit dir&pause with notepad, I will type wcmd notepad "dir&pause". If I want to edit dir with notepad and run pause, I will type wcmd 'notepad dir&pause'. It works well.

goreliu commented 8 years ago

Now:

wcmd a b c    ->    run:cmd /C a b c
wcmd a "b c"    ->    run:cmd /C a b c    # same with wcmd a b c
wcmd a '"b c"'    ->    run:cmd /C a "b c"
wcmd '"a"' '"b"' '"c"'    ->    run:cmd /C "a" "b" "c"
wcmd '"a"' b c    ->    run:cmd /C "a" b c

A better way:

wcmd a b c    ->    run:cmd /C a "b" "c"
wcmd 'a b c'    ->    run:cmd /C a b c
wcmd a "b c"    ->    run:cmd /C a "b c"
wcmd '"a"' b c    ->    run:cmd /C "a" "b" "c"
wcmd '"a" b c'    ->    run:cmd /C "a" b c

And so we shouldn't add " to a(1st argument).

xilun commented 8 years ago

A problem with never quoting/escaping the first param at all is also that e.g. wcmd "c:\Program Files (x86)\Notepad++\notepad++.exe" bla.txt would not work, while oddly wcmd echo toto \& "c:\Program Files (x86)\Notepad++\notepad++.exe" would. I'm looking for something consistent...

goreliu commented 8 years ago

wcmd '"c:\Program Files (x86)\Notepad++\notepad++.exe" bla.txt' works.

xilun commented 8 years ago

But then you can't have your filenames you want to edit escaped, and the double escaping just for the (first) program is nasty. Actually I did a mistake wcmd echo toto \& "c:\Program Files (x86)\Notepad++\notepad++.exe" bla.txt would not launch notepad++ because the '&' would be escaped from cmd. But then I must also think about #27: wstart'ing urls is a big use case and while #27 uses c:/progra~2/opera/launcher.exe it is actually possible to do wstart http://www.google.com/ (<- (edit:) I did not know that was possible), except that for now it fails when you have a '&' in the url, which is quite common.

edit: more precise about what I did not know was possible :)

xilun commented 8 years ago

So for now I more in favor of the general idea of quoting and escaping everything if needed, and nothing if doing for example: wcmd --raw 'notepad&pause', but I'll still think about it a bit before I implement anything.

goreliu commented 8 years ago

The first parameter of start is ["title"]:

> start "a b.txt"
( Open a cmd.exe window with the title "a b.txt"
> start "" "a b.txt"
(Open "a b.txt" file)

So wstart '""' '"a b.txt"' and wstart "" '"http://example.com/?a=1&b=2"' works.

If I modify the code like this:

@@ -958,14 +975,15 @@ int main(int argc, char *argv[])
         string_append(&outbash_command, "\nrun:cmd /C ");
         break;
     case TOOL_WSTART:
-        string_append(&outbash_command, "\nrun:cmd /C start ");
+        string_append(&outbash_command, "\nrun:cmd /C start \"\" \"");
         break;
     }

     bool sep = false;
     for (int i = 0; i < argc; i++) {
-        if (sep) string_append(&outbash_command, " ");
+        if (sep) string_append(&outbash_command, " \"");
         string_append(&outbash_command, argv[i]);
+        if (sep || tool == TOOL_WSTART) string_append(&outbash_command, "\"");
         sep = true;
     }
     string_append(&outbash_command, "\n\n");

wstart "a b.txt" and wstart "http://example.com/?a=1&b=2" works.

xilun commented 8 years ago

The first parameter of start is ["title"]

Yes, only with quotes though, and most of the time useless anyway; except when launching a command line program that does not changes the console title itself. So I plan to give it, at least by default, the same thing as the command. Also, its escaping rules are yet another time different (wtf...) Or maybe I could avoid all that insanity, well more probably only trade it for another, and use ShellExecute for wstart.