Re: [PATCH] cg-diff fixed to work with BSD xargs

Martin Langhoff [off-list ref] writes:

Calls to cg-diff without filename parameters were dependent on GNU xargs
traits. BSD xargs is hardcoded to do --no-run-if-empty -- so if the filter
is effectively empty we avoid calling xargs.

Signed-off-by: Martin Langhoff <redacted>

The fix you did and the reason you stated why you did it in your
commit log message makes perfect sense (I think you missed the
other call to git-diff-tree at the end which is done the same
way with xargs, though); what I write below is not a complaint
to your patch.

But the code you are fixing looks to me like it is already
somewhat obsolete, even if it is still working.  The munging of
user-given paths into $filter temporary file was necessary only
because older git-diff-* family did not work from anywhere but
the top-level directory; they do, thanks to Linus' enhancements,
these days.

I think it is time to start updating Cogito to take advantage of
the modern core.  I do not do Porcelains, but here is my stab at
it.

------------
[PATCH] Redo cg-diff without its own "relative path" support.

It used to be that you had to do "relative path" by hand if you
wanted to work from a subdirectory, but some commands, notably
git-diff-* family, from the modern core knows how to do that
themselves, so take advantage of that.

Signed-off-by: Junio C Hamano <redacted>
---

    jc: The upstream maintainer is welcome to take it, but this
    patch is not intended for immediate inclusion.  I am sure
    there are corner cases I overlooked, without knowing the
    subtleties of what tree-id is supposed to do, for example.
    Setting _git_repo_unneeded upfront to forcibly disable the
    relative path support is another thing I am not proud about;
    there would probably be a better way which I did not find
    only because I did not look closely enough.

cd /opt/packrat/playpen/public/in-place/git/git.pasky/
git diff HEAD

diff --git a/cg-diff b/cg-diff
--- a/cg-diff
+++ b/cg-diff

@@ -39,6 +39,7 @@
 
 USAGE="cg-diff [-c] [-m] [-p] [-r FROM_ID[:TO_ID]] [FILE]..."
 
+_git_repo_unneeded=t
 . ${COGITO_LIB}cg-Xlib || exit 1
 
 

@@ -136,17 +137,11 @@ if [ "$mergebase" ]; then
 fi
 
 
-filter=$(mktemp -t gitdiff.XXXXXX)
-[ "$_git_relpath" -a ! "$ARGS" ] && echo "$_git_relpath" >>$filter
-for file in "${ARGS[@]}"; do
-	echo "${_git_relpath}$file" >>$filter
-done
-
 if [ "$id2" = " " ]; then
 	if [ "$id1" != " " ]; then
-		tree=$(tree-id "$id1") || exit 1
+		tree=$(git-rev-parse --verify "$id1") || exit 1
 	else
-		tree=$(tree-id) || exit 1
+		tree=$(git-rev-parse --verify --default HEAD) || exit 1
 	fi
 
 	# Ensure to only diff modified files

@@ -155,21 +150,18 @@ if [ "$id2" = " " ]; then
 	# FIXME: Update ret based on what did we match. And take "$@"
 	# to account after all.
 	ret=
-	cat $filter | xargs git-diff-cache -r -p $tree | colorize | pager
-
-	rm $filter
+	git-diff-cache -r -p $tree "${ARGS[@]}" | colorize | pager
 
 	[ "$ret" ] && die "no files matched"
 	exit $ret
 fi
 
 
-id1=$(tree-id "$id1") || exit 1
-id2=$(tree-id "$id2") || exit 1
+id1=$(git-rev-parse --verify --default HEAD "$id1") || exit 1
+id2=$(git-rev-parse --verify --default HEAD "$id2") || exit 1
 
 [ "$id1" = "$id2" ] && die "trying to diff $id1 against itself"
 
-cat $filter | xargs git-diff-tree -r -p $id1 $id2 | colorize | pager
+git-diff-tree -r -p $id1 $id2 "${ARGS[@]}" | colorize | pager
 
-rm $filter
 exit 0

Dear diary, on Tue, Aug 30, 2005 at 04:20:40PM CEST, I got a letter
where Junio C Hamano [off-list ref] told me that...

But the code you are fixing looks to me like it is already
somewhat obsolete, even if it is still working.  The munging of
user-given paths into $filter temporary file was necessary only
because older git-diff-* family did not work from anywhere but
the top-level directory; they do, thanks to Linus' enhancements,
these days.

I think it is time to start updating Cogito to take advantage of
the modern core.  I do not do Porcelains, but here is my stab at
it.

Thanks for the patch - it'd be actually very nice to get rid of the
complexity. But more than that, I want to be consistent - so do all the
GIT core commands work in subdirectories by now, or only a subset of
them? I'd hate to have half of the scripts stay in the subdirectory
and half use the Cogito's old relpath logic, so it's either everything
or anything from my POV.

Thanks,

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.