{"thread":{"id":"40981","subject":"[PATCH v7] blame: add support for --[no-]progress option","startedAt":"2015-12-13T00:51:03Z","lastAt":"2015-12-16T07:47:24Z","messageCount":2,"participants":["Edmundo Carmona Antoranz","Eric Sunshine"],"isPatch":true,"patchVersion":7,"patchTotal":null},"messages":[{"id":"274328","messageId":"1449967863-31194-1-git-send-email-eantoranz@gmail.com","threadId":"40981","inReplyTo":null,"subject":"[PATCH v7] blame: add support for --[no-]progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-12-13T00:51:03Z","receivedAt":"2015-12-13T00:51:03Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"--progress can't be used with --incremental or\n porcelain formats.\n\ngit-annotate inherits the option as well\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n---\n Documentation/blame-options.txt |  7 +++++++\n Documentation/git-blame.txt     |  3 ++-\n builtin/blame.c                 | 34 ++++++++++++++++++++++++++++++----\n 3 files changed, 39 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex 760eab7..02cb684 100644\n--- a/Documentation/blame-options.txt\n+++ b/Documentation/blame-options.txt\n@@ -69,6 +69,13 @@ include::line-range-format.txt[]\n \tiso format is used. For supported values, see the discussion\n \tof the --date option at linkgit:git-log[1].\n \n+--[no-]progress::\n+\tProgress status is reported on the standard error stream\n+\tby default when it is attached to a terminal. This flag\n+\tenables progress reporting even if not attached to a\n+\tterminal. Can't use `--progress` together with `--porcelain`\n+\tor `--incremental`.\n+\n -M|<num>|::\n \tDetect moved or copied lines within a file. When a commit\n \tmoves or copies a block of lines (e.g. the original file\ndiff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\nindex e6e947c..ba54175 100644\n--- a/Documentation/git-blame.txt\n+++ b/Documentation/git-blame.txt\n@@ -10,7 +10,8 @@ SYNOPSIS\n [verse]\n 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]\n \t    [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]\n-\t    [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>] [--] <file>\n+\t    [--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>]\n+\t    [--] <file>\n \n DESCRIPTION\n -----------\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 2afe828..e78ca09 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -28,6 +28,7 @@\n #include \"line-range.h\"\n #include \"line-log.h\"\n #include \"dir.h\"\n+#include \"progress.h\"\n \n static char blame_usage[] = N_(\"git blame [<options>] [<rev-opts>] [<rev>] [--] <file>\");\n \n@@ -50,6 +51,7 @@ static int incremental;\n static int xdl_opts;\n static int abbrev = -1;\n static int no_whole_file_rename;\n+static int show_progress;\n \n static struct date_mode blame_date_mode = { DATE_ISO8601 };\n static size_t blame_date_width;\n@@ -127,6 +129,11 @@ struct origin {\n \tchar path[FLEX_ARRAY];\n };\n \n+struct progress_info {\n+\tstruct progress *progress;\n+\tint blamed_lines;\n+};\n+\n static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,\n \t\t      xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)\n {\n@@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin *suspect, int repeat)\n  * The blame_entry is found to be guilty for the range.\n  * Show it in incremental output.\n  */\n-static void found_guilty_entry(struct blame_entry *ent)\n+static void found_guilty_entry(struct blame_entry *ent,\n+\t\t\t   struct progress_info *pi)\n {\n \tif (incremental) {\n \t\tstruct origin *suspect = ent->suspect;\n@@ -1758,6 +1766,8 @@ static void found_guilty_entry(struct blame_entry *ent)\n \t\twrite_filename_info(suspect->path);\n \t\tmaybe_flush_or_die(stdout, \"stdout\");\n \t}\n+\tpi->blamed_lines += ent->num_lines;\n+\tdisplay_progress(pi->progress, pi->blamed_lines);\n }\n \n /*\n@@ -1768,6 +1778,11 @@ static void assign_blame(struct scoreboard *sb, int opt)\n {\n \tstruct rev_info *revs = sb->revs;\n \tstruct commit *commit = prio_queue_get(&sb->commits);\n+\tstruct progress_info pi = { NULL, 0 };\n+\n+\tif (show_progress)\n+\t\tpi.progress = start_progress_delay(_(\"Blaming lines\"),\n+\t\t                                   sb->num_lines, 50, 1);\n \n \twhile (commit) {\n \t\tstruct blame_entry *ent;\n@@ -1809,7 +1824,7 @@ static void assign_blame(struct scoreboard *sb, int opt)\n \t\t\tsuspect->guilty = 1;\n \t\t\tfor (;;) {\n \t\t\t\tstruct blame_entry *next = ent->next;\n-\t\t\t\tfound_guilty_entry(ent);\n+\t\t\t\tfound_guilty_entry(ent, &pi);\n \t\t\t\tif (next) {\n \t\t\t\t\tent = next;\n \t\t\t\t\tcontinue;\n@@ -1825,6 +1840,8 @@ static void assign_blame(struct scoreboard *sb, int opt)\n \t\tif (DEBUG) /* sanity */\n \t\t\tsanity_check_refcnt(sb);\n \t}\n+\n+\tstop_progress(&pi.progress);\n }\n \n static const char *format_time(unsigned long time, const char *tz_str,\n@@ -2520,6 +2537,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL('b', NULL, &blank_boundary, N_(\"Show blank SHA-1 for boundary commits (Default: off)\")),\n \t\tOPT_BOOL(0, \"root\", &show_root, N_(\"Do not treat root commits as boundaries (Default: off)\")),\n \t\tOPT_BOOL(0, \"show-stats\", &show_stats, N_(\"Show work cost statistics\")),\n+\t\tOPT_BOOL(0, \"progress\", &show_progress, N_(\"Force progress reporting\")),\n \t\tOPT_BIT(0, \"score-debug\", &output_option, N_(\"Show output score for blame entries\"), OUTPUT_SHOW_SCORE),\n \t\tOPT_BIT('f', \"show-name\", &output_option, N_(\"Show original filename (Default: auto)\"), OUTPUT_SHOW_NAME),\n \t\tOPT_BIT('n', \"show-number\", &output_option, N_(\"Show original linenumber (Default: off)\"), OUTPUT_SHOW_NUMBER),\n@@ -2555,6 +2573,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \n \tsave_commit_buffer = 0;\n \tdashdash_pos = 0;\n+\tshow_progress = -1;\n \n \tparse_options_start(&ctx, argc, argv, prefix, options,\n \t\t\t    PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0);\n@@ -2579,6 +2598,13 @@ parse_done:\n \tDIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);\n \targc = parse_options_end(&ctx);\n \n+\tif (incremental || (output_option & OUTPUT_PORCELAIN)) {\n+\t\tif (show_progress > 0)\n+\t\t\tdie(\"--progress can't be used with --incremental or porcelain formats\");\n+\t\tshow_progress = 0;\n+\t} else if (show_progress < 0)\n+\t\tshow_progress = isatty(2);\n+\n \tif (0 < abbrev)\n \t\t/* one more abbrev length is needed for the boundary commit */\n \t\tabbrev++;\n@@ -2828,11 +2854,11 @@ parse_done:\n \n \tread_mailmap(&mailmap, NULL);\n \n+\tassign_blame(&sb, opt);\n+\n \tif (!incremental)\n \t\tsetup_pager();\n \n-\tassign_blame(&sb, opt);\n-\n \tfree(final_commit_name);\n \n \tif (incremental)\n-- \n2.6.2\n"},{"id":"274555","messageId":"CAPig+cTFsZCowqNxmNtr1za+O6KjZmqJBZLGmUFd77rHmD+5pQ@mail.gmail.com","threadId":"40981","inReplyTo":"1449967863-31194-1-git-send-email-eantoranz@gmail.com","subject":"Re: [PATCH v7] blame: add support for --[no-]progress option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-12-16T07:47:24Z","receivedAt":"2015-12-16T07:47:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Dec 12, 2015 at 7:51 PM, Edmundo Carmona Antoranz\n<eantoranz@gmail.com> wrote:\n> --progress can't be used with --incremental or\n>  porcelain formats.\n>\n> git-annotate inherits the option as well\n>\n> Helped-by: Eric Sunshine <sunshine@sunshineco.com>\n> Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n> ---\n\nThis version seems to address all the issues raised in my reviews of\nprevious rounds, and the new code is more concise, well-formed, and\neasier to follow than earlier attempts. Thanks.\n\nFor the sake of other reviewers, recent previous versions and reviews are here:\n\nv6: http://thread.gmane.org/gmane.comp.version-control.git/282305\nv5: http://thread.gmane.org/gmane.comp.version-control.git/281677\n\n> diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\n> index 760eab7..02cb684 100644\n> --- a/Documentation/blame-options.txt\n> +++ b/Documentation/blame-options.txt\n> @@ -69,6 +69,13 @@ include::line-range-format.txt[]\n>         iso format is used. For supported values, see the discussion\n>         of the --date option at linkgit:git-log[1].\n>\n> +--[no-]progress::\n> +       Progress status is reported on the standard error stream\n> +       by default when it is attached to a terminal. This flag\n> +       enables progress reporting even if not attached to a\n> +       terminal. Can't use `--progress` together with `--porcelain`\n> +       or `--incremental`.\n> +\n>  -M|<num>|::\n>         Detect moved or copied lines within a file. When a commit\n>         moves or copies a block of lines (e.g. the original file\n> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\n> index e6e947c..ba54175 100644\n> --- a/Documentation/git-blame.txt\n> +++ b/Documentation/git-blame.txt\n> @@ -10,7 +10,8 @@ SYNOPSIS\n>  [verse]\n>  'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]\n>             [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]\n> -           [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>] [--] <file>\n> +           [--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>]\n> +           [--] <file>\n>\n>  DESCRIPTION\n>  -----------\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 2afe828..e78ca09 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -28,6 +28,7 @@\n>  #include \"line-range.h\"\n>  #include \"line-log.h\"\n>  #include \"dir.h\"\n> +#include \"progress.h\"\n>\n>  static char blame_usage[] = N_(\"git blame [<options>] [<rev-opts>] [<rev>] [--] <file>\");\n>\n> @@ -50,6 +51,7 @@ static int incremental;\n>  static int xdl_opts;\n>  static int abbrev = -1;\n>  static int no_whole_file_rename;\n> +static int show_progress;\n>\n>  static struct date_mode blame_date_mode = { DATE_ISO8601 };\n>  static size_t blame_date_width;\n> @@ -127,6 +129,11 @@ struct origin {\n>         char path[FLEX_ARRAY];\n>  };\n>\n> +struct progress_info {\n> +       struct progress *progress;\n> +       int blamed_lines;\n> +};\n> +\n>  static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,\n>                       xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)\n>  {\n> @@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin *suspect, int repeat)\n>   * The blame_entry is found to be guilty for the range.\n>   * Show it in incremental output.\n>   */\n> -static void found_guilty_entry(struct blame_entry *ent)\n> +static void found_guilty_entry(struct blame_entry *ent,\n> +                          struct progress_info *pi)\n>  {\n>         if (incremental) {\n>                 struct origin *suspect = ent->suspect;\n> @@ -1758,6 +1766,8 @@ static void found_guilty_entry(struct blame_entry *ent)\n>                 write_filename_info(suspect->path);\n>                 maybe_flush_or_die(stdout, \"stdout\");\n>         }\n> +       pi->blamed_lines += ent->num_lines;\n> +       display_progress(pi->progress, pi->blamed_lines);\n>  }\n>\n>  /*\n> @@ -1768,6 +1778,11 @@ static void assign_blame(struct scoreboard *sb, int opt)\n>  {\n>         struct rev_info *revs = sb->revs;\n>         struct commit *commit = prio_queue_get(&sb->commits);\n> +       struct progress_info pi = { NULL, 0 };\n> +\n> +       if (show_progress)\n> +               pi.progress = start_progress_delay(_(\"Blaming lines\"),\n> +                                                  sb->num_lines, 50, 1);\n>\n>         while (commit) {\n>                 struct blame_entry *ent;\n> @@ -1809,7 +1824,7 @@ static void assign_blame(struct scoreboard *sb, int opt)\n>                         suspect->guilty = 1;\n>                         for (;;) {\n>                                 struct blame_entry *next = ent->next;\n> -                               found_guilty_entry(ent);\n> +                               found_guilty_entry(ent, &pi);\n>                                 if (next) {\n>                                         ent = next;\n>                                         continue;\n> @@ -1825,6 +1840,8 @@ static void assign_blame(struct scoreboard *sb, int opt)\n>                 if (DEBUG) /* sanity */\n>                         sanity_check_refcnt(sb);\n>         }\n> +\n> +       stop_progress(&pi.progress);\n>  }\n>\n>  static const char *format_time(unsigned long time, const char *tz_str,\n> @@ -2520,6 +2537,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>                 OPT_BOOL('b', NULL, &blank_boundary, N_(\"Show blank SHA-1 for boundary commits (Default: off)\")),\n>                 OPT_BOOL(0, \"root\", &show_root, N_(\"Do not treat root commits as boundaries (Default: off)\")),\n>                 OPT_BOOL(0, \"show-stats\", &show_stats, N_(\"Show work cost statistics\")),\n> +               OPT_BOOL(0, \"progress\", &show_progress, N_(\"Force progress reporting\")),\n>                 OPT_BIT(0, \"score-debug\", &output_option, N_(\"Show output score for blame entries\"), OUTPUT_SHOW_SCORE),\n>                 OPT_BIT('f', \"show-name\", &output_option, N_(\"Show original filename (Default: auto)\"), OUTPUT_SHOW_NAME),\n>                 OPT_BIT('n', \"show-number\", &output_option, N_(\"Show original linenumber (Default: off)\"), OUTPUT_SHOW_NUMBER),\n> @@ -2555,6 +2573,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>\n>         save_commit_buffer = 0;\n>         dashdash_pos = 0;\n> +       show_progress = -1;\n>\n>         parse_options_start(&ctx, argc, argv, prefix, options,\n>                             PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0);\n> @@ -2579,6 +2598,13 @@ parse_done:\n>         DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);\n>         argc = parse_options_end(&ctx);\n>\n> +       if (incremental || (output_option & OUTPUT_PORCELAIN)) {\n> +               if (show_progress > 0)\n> +                       die(\"--progress can't be used with --incremental or porcelain formats\");\n> +               show_progress = 0;\n> +       } else if (show_progress < 0)\n> +               show_progress = isatty(2);\n> +\n>         if (0 < abbrev)\n>                 /* one more abbrev length is needed for the boundary commit */\n>                 abbrev++;\n> @@ -2828,11 +2854,11 @@ parse_done:\n>\n>         read_mailmap(&mailmap, NULL);\n>\n> +       assign_blame(&sb, opt);\n> +\n>         if (!incremental)\n>                 setup_pager();\n>\n> -       assign_blame(&sb, opt);\n> -\n>         free(final_commit_name);\n>\n>         if (incremental)\n> --\n> 2.6.2\n"}]}