Kevin Leung wrote:
> read_pipe() returns "\n". We need to remove it before passing it
> to system().
>
> Signed-off-by: Kevin Leung <kevinlsk@xxxxxxxxx>
If I understand correctly, this is a cosmetic change: os.system()
calls system(3), which uses 'sh -c', which has no problem coping with
an extra newline at the end. So 'need' seems too strong a word.
Still, the change sounds sensible.
> ---
> contrib/fast-import/git-p4 | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 0cef242..04bf4f4 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -732,8 +732,8 @@ class P4Submit(Command):
> if os.environ.has_key("P4EDITOR"):
> editor = os.environ.get("P4EDITOR")
> else:
> - editor = read_pipe("git var GIT_EDITOR")
> - system(editor + " " + fileName)
> + editor = read_pipe("git var GIT_EDITOR").strip()
> + system("%s %s" % (editor, fileName))
>
> response = "y"
> if os.stat(fileName).st_mtime <= mtime:
> --
What is the rationale for the rewritten system() line? I would have
understood a change to
os.spawnlp("sh", "-c", editor + " \"$@\"", fileName)
for better behavior when TMPDIR contains shell metacharacters, but
even this has nothing to do with read_pipe() returning a trailing
newline.
Just my two cents,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html