{"thread":{"id":"35472","subject":"[PATCH v2] difftool: Change prompt to display the number of files in the diff queue","startedAt":"2013-12-04T01:26:27Z","lastAt":"2013-12-04T20:18:18Z","messageCount":3,"participants":["Zoltan Klinger","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"231482","messageId":"1386120387-25300-1-git-send-email-zoltan.klinger@gmail.com","threadId":"35472","inReplyTo":null,"subject":"[PATCH v2] difftool: Change prompt to display the number of files in the diff queue","fromName":"Zoltan Klinger","fromEmail":"zoltan.klinger@gmail.com","sentAt":"2013-12-04T01:26:27Z","receivedAt":"2013-12-04T01:26:27Z","isPatch":true,"sender":{"key":"zoltan.klinger@gmail.com","avatar":"https://avatars.githubusercontent.com/u/95923?v=4"},"body":"When --prompt option is set, git-difftool displays a prompt for each modified\nfile to be viewed in an external diff program. At that point it could be useful\nto display a counter and the total number of files in the diff queue.\n\nBelow is the current difftool prompt for the first of 5 modified files:\nViewing: 'diff.c'\nLaunch 'vimdiff' [Y/n]:\n\nConsider the modified prompt:\nViewing (1/5): 'diff.c'\nLaunch 'vimdiff' [Y/n]:\n\nThe current GIT_EXTERNAL_DIFF mechanism does not tell the number of\npaths in the diff queue nor the current counter. To make this\n\"counter/total\" info available for GIT_EXTERNAL_DIFF programs without\nbreaking existing ones:\n\n(1) Modify run_external_diff() function in diff.c to set one environment\nvariable for a counter and one for the total number of files in the diff\nqueue. The size of the diff queue is already available in the\ndiff_queue_struct. For the counter define a new variable in the\ndiff_options struct and reset it to zero in diff_setup_done() function.\nPre-increment the counter inside the run_external_diff() function.\n\n(2) Modify git-difftool--helper.sh script to display the counter and the diff\nqueue count values in the difftool prompt.\n\n(3) Update git.txt documentation\n\n(4) Update t4020-diff-external.sh test script\n\nSigned-off-by: Zoltan Klinger <zoltan.klinger@gmail.com>\n---\n\nReworked the patch to use environment variables instead of command line\narguments for making the counter and total values available to external\nscripts. This way existing scripts will still work and can be updated \nlater if they want to make use of these two new values.\n\n Documentation/git.txt    |  9 +++++++++\n diff.c                   | 19 +++++++++++++++++--\n diff.h                   |  2 ++\n git-difftool--helper.sh  |  3 ++-\n t/t4020-diff-external.sh | 16 ++++++++++++++++\n 5 files changed, 46 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 4448ce2..10939ac 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -806,6 +806,15 @@ temporary file --- it is removed when 'GIT_EXTERNAL_DIFF' exits.\n +\n For a path that is unmerged, 'GIT_EXTERNAL_DIFF' is called with 1\n parameter, <path>.\n++\n+For each path 'GIT_EXTERNAL_DIFF' is called, two environment variables,\n+'GIT_DIFF_PATH_COUNTER' and 'GIT_DIFF_PATH_TOTAL' are set.\n+\n+'GIT_DIFF_PATH_COUNTER'::\n+\tA 1-based counter incremented by one for every path.\n+\n+'GIT_DIFF_PATH_TOTAL'::\n+\tThe total number of paths.\n \n other\n ~~~~~\ndiff --git a/diff.c b/diff.c\nindex e34bf97..c4078af 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2899,11 +2899,18 @@ static void run_external_diff(const char *pgm,\n \t\t\t      struct diff_filespec *one,\n \t\t\t      struct diff_filespec *two,\n \t\t\t      const char *xfrm_msg,\n-\t\t\t      int complete_rewrite)\n+\t\t\t      int complete_rewrite,\n+\t\t\t      struct diff_options *o)\n {\n \tconst char *spawn_arg[10];\n \tint retval;\n \tconst char **arg = &spawn_arg[0];\n+\tstruct diff_queue_struct *q = &diff_queued_diff;\n+\n+\tstruct strbuf counterstr = STRBUF_INIT;\n+\tstruct strbuf totalstr = STRBUF_INIT;\n+\tstrbuf_addf(&counterstr, \"%d\", ++o->diff_path_counter);\n+\tstrbuf_addf(&totalstr, \"%d\", q->nr);\n \n \tif (one && two) {\n \t\tstruct diff_tempfile *temp_one, *temp_two;\n@@ -2928,8 +2935,14 @@ static void run_external_diff(const char *pgm,\n \t}\n \t*arg = NULL;\n \tfflush(NULL);\n+\n+\tsetenv (\"GIT_DIFF_PATH_COUNTER\", counterstr.buf, 1);\n+\tsetenv (\"GIT_DIFF_PATH_TOTAL\", totalstr.buf, 1);\n+\n \tretval = run_command_v_opt(spawn_arg, RUN_USING_SHELL);\n \tremove_tempfile();\n+\tstrbuf_release(&counterstr);\n+\tstrbuf_release(&totalstr);\n \tif (retval) {\n \t\tfprintf(stderr, \"external diff died, stopping at %s.\\n\", name);\n \t\texit(1);\n@@ -3042,7 +3055,7 @@ static void run_diff_cmd(const char *pgm,\n \n \tif (pgm) {\n \t\trun_external_diff(pgm, name, other, one, two, xfrm_msg,\n-\t\t\t\t  complete_rewrite);\n+\t\t\t\t  complete_rewrite, o);\n \t\treturn;\n \t}\n \tif (one && two)\n@@ -3317,6 +3330,8 @@ void diff_setup_done(struct diff_options *options)\n \t\toptions->output_format = DIFF_FORMAT_NO_OUTPUT;\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n \t}\n+\n+    options->diff_path_counter = 0;\n }\n \n static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)\ndiff --git a/diff.h b/diff.h\nindex e342325..42bd34c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -164,6 +164,8 @@ struct diff_options {\n \tdiff_prefix_fn_t output_prefix;\n \tint output_prefix_length;\n \tvoid *output_prefix_data;\n+\n+\tint diff_path_counter;\n };\n \n enum color_diff {\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex b00ed95..7ef36b9 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -40,7 +40,8 @@ launch_merge_tool () {\n \t# the user with the real $MERGED name before launching $merge_tool.\n \tif should_prompt\n \tthen\n-\t\tprintf \"\\nViewing: '%s'\\n\" \"$MERGED\"\n+\t\tprintf \"\\nViewing (%s/%s): '%s'\\n\" \"$GIT_DIFF_PATH_COUNTER\" \\\n+\t\t\t\"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n \t\tif use_ext_cmd\n \t\tthen\n \t\t\tprintf \"Launch '%s' [Y/n]: \" \\\ndiff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh\nindex 8a30979..4935fc4 100755\n--- a/t/t4020-diff-external.sh\n+++ b/t/t4020-diff-external.sh\n@@ -193,6 +193,22 @@ test_expect_success 'GIT_EXTERNAL_DIFF with more than one changed files' '\n \tGIT_EXTERNAL_DIFF=echo git diff\n '\n \n+echo \"#!$SHELL_PATH\" >external-diff.sh\n+cat >> external-diff.sh <<\\EOF\n+echo $GIT_DIFF_PATH_COUNTER of $GIT_DIFF_PATH_TOTAL >>counter.txt\n+EOF\n+chmod a+x external-diff.sh\n+\n+test_expect_success 'GIT_EXTERNAL_DIFF path counter/total' '\n+\tGIT_EXTERNAL_DIFF=./external-diff.sh git diff &&\n+\techo \"1 of 2\" >expect &&\n+\thead -n 1 counter.txt >actual &&\n+\ttest_cmp expect actual &&\n+\techo \"2 of 2\" >expect &&\n+\ttail -n 1 counter.txt >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' '\n \ttouch file.ext &&\n \tgit add file.ext &&\n-- \n1.8.4.4\n"},{"id":"231483","messageId":"CAPig+cTUB=UbY-j8mtz6QsvJKoA2T41u-wiR7Di596pM3t+k=Q@mail.gmail.com","threadId":"35472","inReplyTo":"1386120387-25300-1-git-send-email-zoltan.klinger@gmail.com","subject":"Re: [PATCH v2] difftool: Change prompt to display the number of files in the diff queue","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-12-04T04:34:58Z","receivedAt":"2013-12-04T04:34:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Dec 3, 2013 at 8:26 PM, Zoltan Klinger <zoltan.klinger@gmail.com> wrote:\n> diff --git a/diff.c b/diff.c\n> index e34bf97..c4078af 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2899,11 +2899,18 @@ static void run_external_diff(const char *pgm,\n>                               struct diff_filespec *one,\n>                               struct diff_filespec *two,\n>                               const char *xfrm_msg,\n> -                             int complete_rewrite)\n> +                             int complete_rewrite,\n> +                             struct diff_options *o)\n>  {\n>         const char *spawn_arg[10];\n>         int retval;\n>         const char **arg = &spawn_arg[0];\n> +       struct diff_queue_struct *q = &diff_queued_diff;\n> +\n> +       struct strbuf counterstr = STRBUF_INIT;\n> +       struct strbuf totalstr = STRBUF_INIT;\n> +       strbuf_addf(&counterstr, \"%d\", ++o->diff_path_counter);\n> +       strbuf_addf(&totalstr, \"%d\", q->nr);\n>\n>         if (one && two) {\n>                 struct diff_tempfile *temp_one, *temp_two;\n> @@ -2928,8 +2935,14 @@ static void run_external_diff(const char *pgm,\n>         }\n>         *arg = NULL;\n>         fflush(NULL);\n> +\n> +       setenv (\"GIT_DIFF_PATH_COUNTER\", counterstr.buf, 1);\n> +       setenv (\"GIT_DIFF_PATH_TOTAL\", totalstr.buf, 1);\n> +\n>         retval = run_command_v_opt(spawn_arg, RUN_USING_SHELL);\n\nWould run_command_v_opt_cd_env() be more appropriate than setenv() +\nrun_command_v_opt() done here?\n\n>         remove_tempfile();\n> +       strbuf_release(&counterstr);\n> +       strbuf_release(&totalstr);\n>         if (retval) {\n>                 fprintf(stderr, \"external diff died, stopping at %s.\\n\", name);\n>                 exit(1);\n> diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh\n> index 8a30979..4935fc4 100755\n> --- a/t/t4020-diff-external.sh\n> +++ b/t/t4020-diff-external.sh\n> @@ -193,6 +193,22 @@ test_expect_success 'GIT_EXTERNAL_DIFF with more than one changed files' '\n>         GIT_EXTERNAL_DIFF=echo git diff\n>  '\n>\n> +echo \"#!$SHELL_PATH\" >external-diff.sh\n> +cat >> external-diff.sh <<\\EOF\n> +echo $GIT_DIFF_PATH_COUNTER of $GIT_DIFF_PATH_TOTAL >>counter.txt\n> +EOF\n> +chmod a+x external-diff.sh\n\nPerhaps write_script()?\n\n> +test_expect_success 'GIT_EXTERNAL_DIFF path counter/total' '\n> +       GIT_EXTERNAL_DIFF=./external-diff.sh git diff &&\n> +       echo \"1 of 2\" >expect &&\n> +       head -n 1 counter.txt >actual &&\n> +       test_cmp expect actual &&\n> +       echo \"2 of 2\" >expect &&\n> +       tail -n 1 counter.txt >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n>  test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' '\n>         touch file.ext &&\n>         git add file.ext &&\n> --\n> 1.8.4.4\n"},{"id":"231537","messageId":"xmqq61r4l69h.fsf@gitster.dls.corp.google.com","threadId":"35472","inReplyTo":"CAPig+cTUB=UbY-j8mtz6QsvJKoA2T41u-wiR7Di596pM3t+k=Q@mail.gmail.com","subject":"Re: [PATCH v2] difftool: Change prompt to display the number of files in the diff queue","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-04T20:18:18Z","receivedAt":"2013-12-04T20:18:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> +       setenv (\"GIT_DIFF_PATH_COUNTER\", counterstr.buf, 1);\n>> +       setenv (\"GIT_DIFF_PATH_TOTAL\", totalstr.buf, 1);\n>> +\n>>         retval = run_command_v_opt(spawn_arg, RUN_USING_SHELL);\n>\n> Would run_command_v_opt_cd_env() be more appropriate than setenv() +\n> run_command_v_opt() done here?\n\nProbably (besides, SPs after 'setenv' need to go).\n\nAlso, we know total/conter is a decimal integer. On-stack 32-byte\narrays are sufficient and two strbufs are overkill ;-)\n\n>> diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh\n>> index 8a30979..4935fc4 100755\n>> --- a/t/t4020-diff-external.sh\n>> +++ b/t/t4020-diff-external.sh\n>> @@ -193,6 +193,22 @@ test_expect_success 'GIT_EXTERNAL_DIFF with more than one changed files' '\n>>         GIT_EXTERNAL_DIFF=echo git diff\n>>  '\n>>\n>> +echo \"#!$SHELL_PATH\" >external-diff.sh\n>> +cat >> external-diff.sh <<\\EOF\n>> +echo $GIT_DIFF_PATH_COUNTER of $GIT_DIFF_PATH_TOTAL >>counter.txt\n>> +EOF\n>> +chmod a+x external-diff.sh\n>\n> Perhaps write_script()?\n\nDefinitely.\n\nThanks, both.\n"}]}