Re: [PATCH v2] difftool: Change prompt to display the number of files in the diff queue

2013-12-04 Thread Junio C Hamano
Eric Sunshine sunsh...@sunshineco.com writes:

 +   setenv (GIT_DIFF_PATH_COUNTER, counterstr.buf, 1);
 +   setenv (GIT_DIFF_PATH_TOTAL, totalstr.buf, 1);
 +
 retval = run_command_v_opt(spawn_arg, RUN_USING_SHELL);

 Would run_command_v_opt_cd_env() be more appropriate than setenv() +
 run_command_v_opt() done here?

Probably (besides, SPs after 'setenv' need to go).

Also, we know total/conter is a decimal integer. On-stack 32-byte
arrays are sufficient and two strbufs are overkill ;-)

 diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
 index 8a30979..4935fc4 100755
 --- a/t/t4020-diff-external.sh
 +++ b/t/t4020-diff-external.sh
 @@ -193,6 +193,22 @@ test_expect_success 'GIT_EXTERNAL_DIFF with more than 
 one changed files' '
 GIT_EXTERNAL_DIFF=echo git diff
  '

 +echo #!$SHELL_PATH external-diff.sh
 +cat  external-diff.sh \EOF
 +echo $GIT_DIFF_PATH_COUNTER of $GIT_DIFF_PATH_TOTAL counter.txt
 +EOF
 +chmod a+x external-diff.sh

 Perhaps write_script()?

Definitely.

Thanks, both.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] difftool: Change prompt to display the number of files in the diff queue

2013-12-03 Thread Zoltan Klinger
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]:

The current GIT_EXTERNAL_DIFF mechanism does not tell the number of
paths in the diff queue nor the current counter. To make this
counter/total info available for GIT_EXTERNAL_DIFF programs without
breaking existing ones:

(1) Modify run_external_diff() function in diff.c to set one environment
variable for a counter and one for the total number of files in the diff
queue. The size of the diff queue is already available in the
diff_queue_struct. For the counter define a new variable in the
diff_options struct and reset it to zero in diff_setup_done() function.
Pre-increment the counter inside the run_external_diff() function.

(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.klin...@gmail.com
---

Reworked the patch to use environment variables instead of command line
arguments for making the counter and total values available to external
scripts. This way existing scripts will still work and can be updated 
later if they want to make use of these two new values.

 Documentation/git.txt|  9 +
 diff.c   | 19 +--
 diff.h   |  2 ++
 git-difftool--helper.sh  |  3 ++-
 t/t4020-diff-external.sh | 16 
 5 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 4448ce2..10939ac 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -806,6 +806,15 @@ temporary file --- it is removed when 'GIT_EXTERNAL_DIFF' 
exits.
 +
 For a path that is unmerged, 'GIT_EXTERNAL_DIFF' is called with 1
 parameter, path.
++
+For each path 'GIT_EXTERNAL_DIFF' is called, two environment variables,
+'GIT_DIFF_PATH_COUNTER' and 'GIT_DIFF_PATH_TOTAL' are set.
+
+'GIT_DIFF_PATH_COUNTER'::
+   A 1-based counter incremented by one for every path.
+
+'GIT_DIFF_PATH_TOTAL'::
+   The total number of paths.
 
 other
 ~
diff --git a/diff.c b/diff.c
index e34bf97..c4078af 100644
--- a/diff.c
+++ b/diff.c
@@ -2899,11 +2899,18 @@ static void run_external_diff(const char *pgm,
  struct diff_filespec *one,
  struct diff_filespec *two,
  const char *xfrm_msg,
- int complete_rewrite)
+ int complete_rewrite,
+ struct diff_options *o)
 {
const char *spawn_arg[10];
int retval;
const char **arg = spawn_arg[0];
+   struct diff_queue_struct *q = diff_queued_diff;
+
+   struct strbuf counterstr = STRBUF_INIT;
+   struct strbuf totalstr = STRBUF_INIT;
+   strbuf_addf(counterstr, %d, ++o-diff_path_counter);
+   strbuf_addf(totalstr, %d, q-nr);
 
if (one  two) {
struct diff_tempfile *temp_one, *temp_two;
@@ -2928,8 +2935,14 @@ static void run_external_diff(const char *pgm,
}
*arg = NULL;
fflush(NULL);
+
+   setenv (GIT_DIFF_PATH_COUNTER, counterstr.buf, 1);
+   setenv (GIT_DIFF_PATH_TOTAL, totalstr.buf, 1);
+
retval = run_command_v_opt(spawn_arg, RUN_USING_SHELL);
remove_tempfile();
+   strbuf_release(counterstr);
+   strbuf_release(totalstr);
if (retval) {
fprintf(stderr, external diff died, stopping at %s.\n, name);
exit(1);
@@ -3042,7 +3055,7 @@ static void run_diff_cmd(const char *pgm,
 
if (pgm) {
run_external_diff(pgm, name, other, one, two, xfrm_msg,
- complete_rewrite);
+ complete_rewrite, o);
return;
}
if (one  two)
@@ -3317,6 +3330,8 @@ void diff_setup_done(struct diff_options *options)
options-output_format = DIFF_FORMAT_NO_OUTPUT;
DIFF_OPT_SET(options, EXIT_WITH_STATUS);
}
+
+options-diff_path_counter = 0;
 }
 
 static int opt_arg(const char *arg, int arg_short, const char *arg_long, int 
*val)
diff --git a/diff.h b/diff.h
index e342325..42bd34c 100644
--- a/diff.h
+++ b/diff.h
@@ -164,6 +164,8 @@ struct diff_options {
diff_prefix_fn_t output_prefix;
int output_prefix_length;
void *output_prefix_data;
+
+   int diff_path_counter;
 };
 
 enum color_diff {
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index b00ed95..7ef36b9 100755
--- a/git-difftool--helper.sh
+++