Re: Giving command line parameter to textconv command?

From

Junio C Hamano <gitster@pobox.com>

Date

Dec 14, 2009, 23:31 UTC

Message-ID

<7vvdg9ceud.fsf@alter.siamese.dyndns.org>

In-Reply-To

<20091215071735.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> I need this extra script because setting 'nkf -w' for
> textconv like this
>
> 	[diff "eucjp"]
> 		textconv = nkf -w
>
> gives an error.
>
> 	% diff --git a/hello.txt b/hello.txt
> 	index 696acd7..f07aa1a 100644
> 	error: cannot run nkf -w: No such file or directory
> 	error: error running textconv command 'nkf -w'
> 	fatal: unable to read files to diff
>
> Could you fix textconv so that it can be given parameters?

The change to do so looks like this; it has a few side effects:

 - If somebody else were relying on the fact that 'nkf -w' names the
   entire command, it now will run 'nkf' command with '-w' as an argument,
   and it will break such a set-up.  IOW, command that has an IFS white
   space in its path will now need to be quoted from the shell.

   You can see the fallout from this in the damage made to t/ hierarchy in
   the attached patch.

 - You can now use $HOME and other environment variables your shell
   expands when defining your textconv command.

We might need to introduce diff.*.xtextconv or something.

 diff.c                         |    9 ++++-----
 t/t4030-diff-textconv.sh       |    2 +-
 t/t4031-diff-rewrite-binary.sh |    2 +-
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index 08bbd3e..64a1486 100644
--- a/diff.c
+++ b/diff.c
@@ -3760,15 +3760,14 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec,
 		size_t *outsize)
 {
 	struct diff_tempfile *temp;
-	const char *argv[3];
-	const char **arg = argv;
+	const char *argv[4] = { "sh", "-c", NULL, NULL };
 	struct child_process child;
 	struct strbuf buf = STRBUF_INIT;
+	struct strbuf cmd = STRBUF_INIT;
 
 	temp = prepare_temp_file(spec->path, spec);
-	*arg++ = pgm;
-	*arg++ = temp->name;
-	*arg = NULL;
+	strbuf_addf(&cmd, "%s %s", pgm, temp->name);
+	argv[2] = strbuf_detach(&cmd, NULL);
 
 	memset(&child, 0, sizeof(child));
 	child.argv = argv;
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index a3f0897..3468f77 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -48,7 +48,7 @@ test_expect_success 'file is considered binary by plumbing' '
 
 test_expect_success 'setup textconv filters' '
 	echo file diff=foo >.gitattributes &&
-	git config diff.foo.textconv "$PWD"/hexdump &&
+	git config diff.foo.textconv \""$PWD"/hexdump\" &&
 	git config diff.fail.textconv false
 '
 
diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh
index a894c60..e6cb30e 100755
--- a/t/t4031-diff-rewrite-binary.sh
+++ b/t/t4031-diff-rewrite-binary.sh
@@ -54,7 +54,7 @@ chmod +x dump
 
 test_expect_success 'setup textconv' '
 	echo file diff=foo >.gitattributes &&
-	git config diff.foo.textconv "$PWD"/dump
+	git config diff.foo.textconv \""$PWD"/dump\"
 '
 
 test_expect_success 'rewrite diff respects textconv' '