Re: [PATCH] cg-diff fixed to work with BSD xargs
- From
Junio C Hamano <junkio@cox.net>
- Date
- Aug 30, 2005, 14:20 UTC
- Message-ID
- <7v7je3ld53.fsf@assigned-by-dhcp.cox.net>
- In-Reply-To
- <11253960093915-git-send-email-martin@catalyst.net.nz>
Martin Langhoff <martin@catalyst.net.nz> writes:
Show 5 quoted lines
> 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 <martin@catalyst.net.nz>
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 <junkio@cox.net> ---
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