Zoltan Klinger <zoltan.klinger@xxxxxxxxx> writes:
> When --prompt option is set, git-difftool displays a prompt for each
> modified file to be viewed in an external diff program. At that point it
> could be useful to display a counter and the total number of files in
> the diff queue.
>
> Below is the current difftool prompt for the first of 5 modified files:
> Viewing: 'diff.c'
> Launch 'vimdiff' [Y/n]:
>
> Consider the modified prompt:
> Viewing (1/5): 'diff.c'
> Launch 'vimdiff' [Y/n]:
>
> (1) Modify run_external_diff() function in diff.c to pass a counter and
> the total number of files in the diff queue to the external program.
>
> (2) Modify git-difftool--helper.sh script to display the counter and the
> diff queue count values in the difftool prompt.
>
> (3) Update git.txt documentation
>
> (4) Update t4020-diff-external.sh test script
>
> Signed-off-by: Zoltan Klinger <zoltan.klinger@xxxxxxxxx>
> ---
> Documentation/git.txt | 6 +++++-
> diff.c | 14 +++++++++++++-
> git-difftool--helper.sh | 8 +++++---
> t/t4020-diff-external.sh | 27 +++++++++++++++++++++------
> 4 files changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index b73a24a..d8241bb 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -785,9 +785,10 @@ Git Diffs
> When the environment variable 'GIT_EXTERNAL_DIFF' is set, the
> program named by it is called, instead of the diff invocation
> described above. For a path that is added, removed, or modified,
> - 'GIT_EXTERNAL_DIFF' is called with 7 parameters:
> + 'GIT_EXTERNAL_DIFF' is called with 9 parameters:
>
> path old-file old-hex old-mode new-file new-hex new-mode
> + counter total
As the "git difftool" is not the only thing that reads using the
GIT_EXTERNAL_DIFF mechanism (it is for general consumption by end
user scripts), I wonder how/if this change breaks existing scripts.
I do recall writing an EXTERNAL_DIFF script myself that began by
switching on $# (i.e. the number of arguments) to check the state of
the given path, like this:
case $# in
1)
... handle unmerged path ...
;;
7)
... handle comparison ...
;;
*)
die "Unexpected number of arguments to $0: $#"
;;
esac
which will be broken by this change. Updating such scripts is
trivial but that does not change the fact that this change is
forcing an unnecessary work on our users to adjust their scripts
that have been working perfectly fine. So I think this, if we were
to apply, may need a compatibility warning in large flashing red
letters in the release notes.
> +
> where:
>
> @@ -795,6 +796,9 @@ where:
> contents of <old|new>,
> <old|new>-hex:: are the 40-hexdigit SHA-1 hashes,
> <old|new>-mode:: are the octal representation of the file modes.
> + counter:: is a numeric value incremented by one for every modified
> + file
> + total:: is the total number of modified files
> +
> The file parameters can point at the user's working file
> (e.g. `new-file` in "git-diff-files"), `/dev/null` (e.g. `old-file`
> diff --git a/diff.c b/diff.c
> index e34bf97..938f00a 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -37,6 +37,7 @@ static int diff_stat_graph_width;
> static int diff_dirstat_permille_default = 30;
> static struct diff_options default_diff_options;
> static long diff_algorithm;
> +static int diff_display_counter = 1;
There should be a place somewhere, e.g. diff_setup_done(), to reset
this counter to 0 (and the site that uses the variable should
pre-increment it instead of relying the initial value being 1), so
that a single program could later run the diff machinery more than
once for different set of files. This counter may actually belong
to diff_options, just like existing "found_changes" and
"found_follow" fields are there to keep track of state of the diff
machinery per invocation.
Having said all that, the fact that the current arrangement since we
introduced GIT_EXTERNAL_DIFF mechanism does not tell how many paths
there are in the output is indeed bad. If a script that uses
GIT_EXTERNAL_DIFF wants to first collect all the paths and the
parameters and then show everything in a single UI, such a script
may want to (1) start collecting the paths and args to a persistent
place (e.g. starting a GUI diff daemon for the first path it gets,
or starting a new temporary file), (2) keep collecting the paths and
args, and then (3) after collecting all, present diff for all paths
it obtained, but it is impossible because there is no cue when a
series of external diff calls starts or ends.
And this "counter/total" mechanism could be one possible solution to
it (another possibility is to make an extra dummy call to signal the
end, perhaps with no parameters---the one that is collecting can
then know how many paths there are and which one is the Nth path).
--
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