From: Jonathan Nieder <jrnieder@gmail.com>
Date: Tue, 27 Mar 2012 04:49:57 GMT
Subject: Re: [PATCH] Use SHELL_PATH to fork commands in run_command.c:prepare_shell_cmd
Message-ID: <20120327044957.GB22547@burratino>
In-Reply-To: <20120327042620.GA22547@burratino>

Jonathan Nieder wrote:

[...]
> 	The cause: that test (and some subsequent ones) sets the pager

	The cause: that test and some subsequent ones set the pager ...

(The text has subject/verb agreement problems if the reader chooses to
follow the parentheses.  Removing parens, s/sets/set to spare the reader
the choice.)

> 	command used by git log to "sed s/^/foo:/ >actual" which is fine
> 	in a POSIX-compliant sh, but not in Solaris' sh.  The

	                   ... to "sed s/^/foo:/ >actual", which is fine
	in a POSIX-compliant sh but not in Solaris's sh.

(missing comma before "which"; extra comma before "but").

[...]
> 	a. instead of launching a shell on its own, run_command could
> 	   call system() or popen().  This launches a POSIX shell on
> 	   Solaris as long as _POSIX_SOURCE is defined.

Git uses _XOPEN_SOURCE so it's clearer to say that instead.

[...]
> 	c. the run_command machinery could use the same SHELL_PATH
> 	   shell that is used by the git filter-branch script and in
> 	   all script's #! lines.

Punctuation: script's should be scripts'.

Sorry for the noise.