nimbosa / meld-installer

Automatically exported from code.google.com/p/meld-installer
0 stars 0 forks source link

Problem with arguments with spaces #39

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
It seems the executable wrapper handles some arguments with spaces in them 
incorrectly. I originally thought it was a TortoiseHg bug so these command 
lines are from launching Meld using `thg vdiff` (for the exact test setup, see 
the original bug report at 
https://bitbucket.org/tortoisehg/thg/issue/3439/windows-buggy-argument-passing-f
or-file).

Replacing meld/bin/meld with this Python code:

  import sys
  sys.stdout = open("C:\\Users\\fk\\meldpy-args.txt", "w")
  print sys.argv
  sys.stdout.close()

shows that the Python program is handed incorrect arguments:

  ['C:\\Program Files (x86)\\Meld\\meld\\bin\\meld', '-a', '--label=[non-existant]@-1:000000000000', 'c:\\users\\fk\\appdata\\local\\temp\\thg.4p_bts\\empty', '--label=space', 'test.txt', 'C:\\Users\\fk\\space-test\\space test.txt']

Note the '--label=space', 'test.exe' bit which should be a single argument, but 
is passed to the Python program as two.

However, replacing meld/meld.exe with a simple C++ program:

  #include <fstream>
  using namespace std;

  int main(int argc, char* argv[]) {
      ofstream args("C:/Users/fk/args.txt");
      args << "# args " << argc << endl;
      for (int i = 0; i < argc; i++) {
          args << argv[i] << endl;
      }
      args.close();
      return 0;
  }

shows that meld.exe received correct arguments:

  # args 6
  C:\Program Files (x86)\Meld\meld\meld.exe
  -a
  --label=[non-existant]@-1:000000000000
  c:\users\fk\appdata\local\temp\thg.uedml8\empty
  --label=space test.txt
  C:\Users\fk\space-test\space test.txt

Original issue reported on code.google.com by fkrul...@gmail.com on 30 Oct 2013 at 1:41

GoogleCodeExporter commented 9 years ago
I'm thinking it'd be simplest to just pass along the parameters exactly as we 
encounter them and don't try to be smart about quoting the arguments.  Please 
give the attached file a try.

Original comment by keeganw...@gmail.com on 31 Oct 2013 at 2:08

Attachments:

GoogleCodeExporter commented 9 years ago
Nope, now all arguments with spaces are split up instead of just some of them. 
Meld sees this and just stops because it has too many arguments:

    ['C:\\Program Files (x86)\\Meld\\meld\\bin\\meld', '-a', '--label=[non-existant]@-1:000000000000', 'c:\\users\\fk\\appdata\\local\\temp\\thg.nbncmo\\empty', '--label=space', 'test.txt', 'C:\\Users\\fk\\space-test\\space', 'test.txt']

Original comment by fkrul...@gmail.com on 31 Oct 2013 at 11:03

GoogleCodeExporter commented 9 years ago
In that case, I don't understand what it is I should be doing.  Are you saying 
I should tokenize out arguments by "--" demarked labels?  That wouldn't make 
sense to do because I'd be interpreting something on top of what the shell 
does.  Shouldn't it be up to the caller to quote appropriately?  Sorry, I'm not 
understanding.

Original comment by keeganw...@gmail.com on 31 Oct 2013 at 5:54

GoogleCodeExporter commented 9 years ago
Well, it's not a caller thing. Running this (nonsensical) command from 
Powershell:

  .\meld --label='space test' INSTALL --label='space 2' 'Makefile  111'

shows exactly the same behaviour:

  ['C:\\Program Files (x86)\\Meld\\meld\\bin\\meld', '--label=space', 'test', 'INSTALL', '--label=space', '2', 'Makefile  111']

I don't really know how command line quoting and passing exactly works on 
Windows or how much the shells or even CreateProcess interpret what they get. 
But it seems whatever gets passed to your wrapper script has all quote 
characters stripped out, so even those after the --label= part. From what I 
could gather from the wrapper script (if there is any consistent logic behind 
the Autohotkey language, it continues to elude me), you're adding quotes to any 
arguments apart from those starting with '-'; but since these don't have any 
internal quotes anymore (so they're "--label=space test" now), they're not 
properly quoted at all and end up as two arguments. The solution seems to be to 
quote *everything*. I've tried it; removed the ' && firstChar != "-"' condition 
from both 'if' statements. Python doesn't mind that the switches suddenly have 
quotes around them. I guess they're filtered out by some win32 function way 
before then.

God, discussing the quoting details of strings in text is hard.

Original comment by fkrul...@gmail.com on 31 Oct 2013 at 11:22

GoogleCodeExporter commented 9 years ago
OK.  I think I see what's going on here.  I think I found a way to get 
AutoHotkey to properly get the parameters (although it took some finagling):

; store parameters in a string
commandLine := DllCall("GetCommandLine" , "Str")
StringReplace, commandLine, commandLine, "%A_ScriptFullPath%", %A_Tab%, All
StringSplit, commandLineArray, commandLine, %A_Tab%,
params = %commandLineArray2%

Please give the attached exe a try.

Original comment by keeganw...@gmail.com on 3 Nov 2013 at 10:26

Attachments:

GoogleCodeExporter commented 9 years ago
Edit: never mind, the solution I was looking at works with ahk as a script, but 
doesn't work as an exe.  I'll keep experimenting.

Original comment by keeganw...@gmail.com on 3 Nov 2013 at 11:29

GoogleCodeExporter commented 9 years ago
I just needed to use a different string replacement
StringReplace, commandLine, commandLine, meld.exe, %A_Tab%, All

Give this one a try please.

Original comment by keeganw...@gmail.com on 3 Nov 2013 at 11:36

Attachments:

GoogleCodeExporter commented 9 years ago
So I ran my trusty test command:

  .\meld --label="space test" Makefile --label="space test 2" "INSTALL 111"

sys.argv was this:

  ['C:\\Program Files (x86)\\Meld\\meld\\bin\\meld', '  --label=space', 'test Makefile --label=space', 'test', '2 INSTALL', '111']

That's... different, I suppose? I'd like to point out that my solution of 
quoting everything (see attached patch) seems to work just fine.

Original comment by fkrul...@gmail.com on 4 Nov 2013 at 12:21

Attachments:

GoogleCodeExporter commented 9 years ago
I know.  I just thought it'd be better to just keep the arguments the way they 
were originally rather than introducing quotes that weren't originally there 
(especially if that's behavior that specific to Python 2 and won't work when we 
move to Python 3).  But maybe there's just no way to do that.  Let me look a 
bit more tonight and if I don't come up with anything I'll probably just give 
up and do as you propose.  I originally chose AutoHotkey because it was easy to 
use.  Maybe I need to re-evaluate that choice.

Original comment by keeganw...@gmail.com on 4 Nov 2013 at 6:05

GoogleCodeExporter commented 9 years ago
I think I found something that does what I'm looking for.  Please give it one 
more try.

Original comment by keeganw...@gmail.com on 4 Nov 2013 at 10:36

Attachments:

GoogleCodeExporter commented 9 years ago
Yep, this one works fine, from what I can see.

Still, as for adding quotes: I just had a quick look over Python's subprocess 
module. The rules for building a Windows command line are actually in the docs 
(http://docs.python.org/3/library/subprocess.html?highlight=subprocess#notes), 
but checking the source code, they also indiscriminately add quotes to any 
argument that contains spaces (or tabs, for that matter). On the other hand, 
there is also a disclaimer that while these are the rules used by the MS C 
runtime, applications may choose to interpret the command line differently 
(which is internally just a single string on Windows). On the mutated third 
hand, there's probably only a handful of programs that don't use the default 
parsing on Windows.
Long story short: the quotes will be processed and eaten up by the C runtime 
before `main()` runs, except for programs that don't use the C runtime where 
adding quotes may be a problem. Anyway, that's just for future reference on the 
subject.

Original comment by fkrul...@gmail.com on 5 Nov 2013 at 1:25

GoogleCodeExporter commented 9 years ago

Original comment by keeganw...@gmail.com on 6 Nov 2013 at 12:45

GoogleCodeExporter commented 9 years ago

Original comment by keeganw...@gmail.com on 7 Nov 2013 at 1:21