On Wed, Dec 30, 2009 at 12:05:20AM -0800, Junio C Hamano wrote:
> I stopped promoting the patch after Peff mentioned he was planning a
> rework of textconv, but now I re-read it, I think his rework would be
> orthogonal to the patch.
Actually, I thought I volunteered to produce a patch series converting
textconv and diff.
> Also Peff gives a good hint about borrowing how launch_editor() calls out
> to the shell. I think the codepath we fork&exec user-defined commands
> (not hooks, but filters like smudge/clean/textconv and EDITOR/PAGER that
> take a command line) need to be first enumerated, then we need to see if
> we can refactor what launch_editor() does into a common helper function.
I think the helper function is not too hard. Whereas the patch you
posted earlier actually runs { "sh", "-c", "%s %s" } for textconv, it is
a bit simpler to do { "sh", "-c", "%s \"$@\"", ... } as launch_editor
does. And then we don't have to worry about quoting the arguments (which
your patch gets wrong).
> I felt it was unclear what we would want to do with GIT_EXTERNAL_DIFF,
> diff.external, and diff.<driver>.command trio, but I tend to agree that we
> should run things the same way, honoring $IFS and shell everywhere.
I thought it was left at "maybe for 1.7.0", but perhaps we don't have
time now to issue sufficient warning.
Anyway, I'm working on a series which will hopefully be done in a few
minutes.
-Peff
--
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