Re: Bug: git-p4 edit_template() and P4EDITOR w/options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On May 4, 2015, at 3:38 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> 
> Junio C Hamano <gitster@xxxxxxxxx> writes:
> 
> So, use_shell==true codepath grabs that array with "editor", "path"
> and turns it into an equivalent to
> 
> 	sh -c "$EDITOR" "$path"
> 
> when $EDITOR has a "magic" character (including whitespace) in it.
> So what git-p4 does is *not* in line with how we use the environment
> variable.
> 
> Perhaps a single-liner patch like this would work?
> 
> -	system([editor, template_file])
> +	system(["sh", "-c", editor, template_file])
> 

Thanks for the input Junio!

I’ll try it out, but leave the decision about patching  for the maintainers :-)

Though I do notice that their "system()" function does do shell expansion if it’s arg is a string, but not if it’s a list:

def system(cmd):
    expand = isinstance(cmd,basestring)
    if verbose:
        sys.stderr.write("executing %s\n" % str(cmd))
    retcode = subprocess.call(cmd, shell=expand)
    if retcode:
        raise CalledProcessError(retcode, cmd)

and the edit_template() function is passing a list

I had thought about writing a tiny wrapper script as mentioned in that first link you sent, but that felt even more out of line, especially since there are many references on the net to using parameters in EDITOR vars, particularly with mate.

FWIW, my user is all set now, but I’m happy to participate in this conversation if appropriate :-)

Cheers,
-Chris



--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]