Tim Henigan <tim.henigan@xxxxxxxxx> writes:
> The 'git difftool' command allows the user to view diffs using an
> external tool. It runs a separate instance of the tool for each
> file in the diff. This makes it tedious to review changes spanning
> multiple files.
>
> The 'git-diffall' script instead prepares temporary directories
> with the files to be compared and launches a single instance of
> the external diff tool to view them (i.e. a directory diff).
>
> The 'diff.tool' or 'merge.tool' configuration variable is used
> to specify which external tool is used.
Shouldn't the primary interface "--extcmd=" also be advertised here?
> diff --git a/contrib/diffall/README b/contrib/diffall/README
> new file mode 100644
> index 0000000..111f3f6
> --- /dev/null
> +++ b/contrib/diffall/README
> @@ -0,0 +1,24 @@
> +The git-diffall script provides a directory based diff mechanism
> +for git. The script relies on the diff.tool configuration option
> +to determine what diff viewer is used.
Same here.
> +This script is compatible with most common forms used to specify a
> +range of revisions to diff:
> +
> + 1. git diffall: shows diff between working tree and staged changes
> + 2. git diffall --cached [<commit>]: shows diff between staged
> + changes and HEAD (or other named commit)
> + 3. git diffall <commit>: shows diff between working tree and named
> + commit
> + 4. git diffall <commit> <commit>: show diff between two named commit
I seem to recall the previous round said "named commits" here...
> + 5. git diffall <commit>..<commit>: same as above
> + 6. git diffall <commit>...<commit>: show the changes on the branch
> + containing and up to the second , starting at a common ancestor
> + of both <commit>
> +
> +Note: all forms take an optional path limiter [--] [<path>*]
Doesn't 3. above make "--" mandatory if you give a pathspec? I.e.
... take an optional pathspec [-- <pathspec>]*
> diff --git a/contrib/diffall/git-diffall b/contrib/diffall/git-diffall
> new file mode 100755
> index 0000000..e00fe89
> --- /dev/null
> +++ b/contrib/diffall/git-diffall
> @@ -0,0 +1,261 @@
> ...
> +# mktemp is not available on all platforms (missing from msysgit)
> +# Use a hard-coded tmp dir if it is not available
> +tmp="$(mktemp -d -t tmp.XXXXXX 2>/dev/null)" || {
> + tmp=/git-diffall-tmp.$$
Missing /tmp in front?
> + mkdir "$tmp" || exit 1
> +}
> + ...
> +while test $# != 0
> +do
> + case "$1" in
> + -h|--h|--he|--hel|--help)
> + usage
> + ;;
> + --cached)
> + compare_staged=1
> + ;;
> + --copy-back)
> + copy_back=1
> + ;;
> + -x|--e|--ex|--ext|--extc|--extcm|--extcmd)
> + if test -z $2
testing $# would be more logical, no?
> + then
> + echo You must specify the tool for use with --extcmd
> + usage
> + else
> + diff_tool=$2
> + shift
> + fi
> + ;;
> + --)
> + path_sep=1
The name path_sep makes one wonder "are we somehow talking about
differences between '/' vs '\' here?" In other parts of the system, this
is typically called "dashdash_seen"; the meaning of it is "expect nothing
but pathspecs from here on", so "expect_pathspec" could be an alternative
name for it.
> ...
> +elif test -n "$compare_staged"
> +then
> + while read name
> + do
> + ls_list=$(git ls-files -- "$name")
Ok, the use of $name is now slightly tightened with the surrounding dq
throughout the script. Better (even though $paths are still prone to
splitting by $IFS).
Thanks, will replace what was queued in 'pu'.
--
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