Roland Kaufmann <rlndkfmn+git@xxxxxxxxx> writes:
> diff --git a/git-dirdiff--helper.sh b/git-dirdiff--helper.sh
> new file mode 100755
> index 0000000..bc0b49d
> --- /dev/null
> +++ b/git-dirdiff--helper.sh
> @@ -0,0 +1,28 @@
> ...
> +# bail out if there is any problems copying
> +set -e
I do not think any of our scripted Porcelains use "set -e"; rather they
try to catch errors and produce messages that are more appropriate for
specific error sites.
> +# check that we are called by git-dirdiff
> +if [ -z $__GIT_DIFF_DIR ]; then
> + echo Error: Do not call $(basename $0) directly 1>&2
> + exit 1
> +fi
(style)
if test -z "$__GIT_DIFF_DIR"
then
echo >&2 "..."
exit 1
fi
If this were to become part of the main Porcelain set, you would probably
source in the git-sh-setup helper near the beginning with
. git-sh-setup
and use "die" instead of "echo && exit 1".
> diff --git a/git-dirdiff.sh b/git-dirdiff.sh
> new file mode 100755
> index 0000000..4e75eda
> --- /dev/null
> +++ b/git-dirdiff.sh
> @@ -0,0 +1,34 @@
> ...
> +# create a temporary directory to hold snapshots of changed files
> +__GIT_DIFF_DIR=$(mktemp --tmpdir -d git-dirdiff.XXXXXX)
> +export __GIT_DIFF_DIR
I do not think any of our scripted Porcelains use "mktemp" especially
"mktemp -d". How portable is it?
> +git diff --raw "$@" > $__GIT_DIFF_DIR/toc
> +
> +# let the helper script accumulate them into the temporary directory
> +cut -f 2- -s $__GIT_DIFF_DIR/toc | while read f; do
> + GIT_EXTERNAL_DIFF=git-dirdiff--helper git --no-pager diff "$@" $f
> +done
(style)
... upstream command ... |
while read f
do
...
done
It also is not clear what could be used in "$@". Obviously you would not
want things like "-U20" and "--stat" fed to the first "list of paths"
phase, but there may be some options you may want to give to the inner
"external diff" thing.
It is not clear to me why you need to grab the list of paths in toc and
iterate over them one by one. IOW, why isn't this sufficient?
# dirdiff--helper will copy the files in the temporary directory
GIT_EXTERNAL_DIFF=git-dirdiff--helper git --no-pager diff "$@"
# Now the temporary old/ and new/ are populated, compare them
git-difftool--helper - .....
Overall, I found it interesting, but I am not convinced yet that this
should be in the set of main Porcelains. It seems to be a hack to work
around the design of the current external diff interface that specifically
targets tools that are about comparing single pair of paths at a time.
If "compare two sets of files, each extracted in its own temporary
directory" turns out to be sufficiently useful thing to do (which I
suspect is true), we would probably want to make it an option to "git
diff" itself, and not a separate git subcommand like "dirdiff". I can see
"git diff" (and possibly even things like "git log -p") populating two
temporary directories (your old/ and new/) and running a custom program
specified by GIT_EXTERNAL_TREEDIFF environment variable, instead of doing
any textual diff generation internally.
I wouldn't mind carrying a polished version of this in contrib/ for a
cycle or two in order to let people try it out and get kinks out of its
design.
For example, how well does this approach work with -M/-C (I do not think
it would do anything useful, but I haven't thought things through). It
would be nice if we gave the hint to the external program that compares
the populated temporary directories how paths in the preimage tree and
postimage tree correspond to each other.
--
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