[GRASS-dev] Re: [GRASSGUI] Re: [grass-addons] r659 - trunk/grassaddons/gui/gui_modules

[Michael Barton]

This is helpful. But I will wait to make any changes until this is worked
out--since it looks like we will also need to redefine the original command
string (i.e., to make it a proper list) prior to calling cmd.Command.

Michael


On 5/17/07 8:56 AM, "Glynn Clements" <glynn at gclements.plus.com> wrote:

> 
> Martin Landa wrote:
> 
>> I am not quite sure if this patch OK...
>> 
>> -           self.module = subprocess.Popen(self.cmd, shell=True,
>> +           cmdList = cmd.split(' ')
>> +           self.module = subprocess.Popen(args=cmdList, shell=False,
>>                                            stdin=subprocess.PIPE,
>>                                            stdout=subprocess.PIPE,
>>                                            stderr=subprocess.PIPE,
> 
> Nope. In fact, it's a good example of why the interface is wrong
> (think about how it will behave if you have a space in an argument).
> 
> The problem isn't the implementation, it's the interface.
> 
> Let me put it another way: which of the following is correct?
> 
> 1. int main(char *cmd)
> 2. int main(int argc, char **argv)
> 
> The correct answer is #2. A command isn't a string, it's a list of
> strings, and that is the interface which you need to provide.
> 
> Taking a list of strings, combining them to form a single string, then
> splitting the list into multiple strings will introduce errors, i.e.
> the list you end up with won't always be the original list.
> 
> The existing interface splits them according to the shell's syntax,
> which has several problems; not least of which is that /bin/sh and
> cmd.exe have radically different syntax, meaning that you would often
> need to write separate Unix/Windows versions of any code which calls
> cmd.Command().
> 
> You're proposed patch is worse, in that it becomes entirely impossible
> to pass an argument which contains a space (these are quite common for
> filenames on Windows).
> 
> The only realistic solution is not to combine them in the first place.
> IOW, the cmd parameter should be a list, and Popen should be called
> as:
> 
> self.module = subprocess.Popen(args=self.cmd,
> 
> You then need to change everything which calls cmd.Command() to pass a
> list, not a string.

__________________________________________
Michael Barton, Professor of Anthropology
School of Human Evolution & Social Change
Center for Social Dynamics & Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton