{"thread":{"id":"40980","subject":"[PATCH v6] blame: add support for --[no-]progress option","startedAt":"2015-12-12T23:57:05Z","lastAt":"2015-12-13T00:40:24Z","messageCount":8,"participants":["Edmundo Carmona Antoranz","Eric Sunshine"],"isPatch":true,"patchVersion":6,"patchTotal":null},"messages":[{"id":"274320","messageId":"1449964625-27737-1-git-send-email-eantoranz@gmail.com","threadId":"40980","inReplyTo":null,"subject":"[PATCH v6] blame: add support for --[no-]progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-12-12T23:57:05Z","receivedAt":"2015-12-12T23:57:05Z","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 \nSigned-off-by: Edmundo Carmona Antoranz \n---\n Documentation/blame-options.txt | 7 +++++++\n Documentation/git-blame.txt | 3 ++-\n builtin/blame.c | 35 +++++++++++++++++++++++++++++++----\n 3 files changed, 40 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex 760eab7..ef642b9 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. Progress information won't be displayed if using\n+\t`--porcelain` or `--incremental`.\n+\n -M||::\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 ] [-S ] [-M] [-C] [-C] [-C] [--since=]\n-\t [--abbrev=] [ | --contents | --reverse ] [--] \n+\t [--progress] [--abbrev=] [ | --contents | --reverse ]\n+\t [--] \n \n DESCRIPTION\n -----------\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 2afe828..be5d73d 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 [] [] [] [--] \");\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,9 @@ 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+\tif (pi.progress)\n+\t\tstop_progress(&pi.progress);\n }\n \n static const char *format_time(unsigned long time, const char *tz_str,\n@@ -2520,6 +2538,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 +2574,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 +2599,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 +2855,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":"274321","messageId":"CAOc6etYBaiAhEjZTn2knCe+HywjqFN5qhQtHFBwRAC2r_iBsJQ@mail.gmail.com","threadId":"40980","inReplyTo":"1449964625-27737-1-git-send-email-eantoranz@gmail.com","subject":"Re: [PATCH v6] blame: add support for --[no-]progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-12-13T00:01:05Z","receivedAt":"2015-12-13T00:01:05Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Sat, Dec 12, 2015 at 5:57 PM, Edmundo Carmona Antoranz\n wrote:\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\nI think I took care of all the comments from Eric gracefully. Hope\nthat block is ok for detection of mutually exclusivity related to\n--progress.\n\nCheers!\n"},{"id":"274322","messageId":"CAPig+cQq2Y0m0UJVCMb-9B8qrGNXV7ecf5hDETdgX5w4oUuAvw@mail.gmail.com","threadId":"40980","inReplyTo":"1449964625-27737-1-git-send-email-eantoranz@gmail.com","subject":"Re: [PATCH v6] blame: add support for --[no-]progress option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-12-13T00:17:12Z","receivedAt":"2015-12-13T00:17:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Dec 12, 2015 at 6:57 PM, Edmundo Carmona Antoranz\n 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 \n> Signed-off-by: Edmundo Carmona Antoranz \n> ---\n\nRight here below the \"---\" line would be a good place to explain what\nchanged since the previous version. As an aid for reviewers, it's also\nhelpful to provide a link to the previous round, like this[1].\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/281677\n\n> Documentation/blame-options.txt | 7 +++++++\n> Documentation/git-blame.txt | 3 ++-\n> builtin/blame.c | 35 +++++++++++++++++++++++++++++++----\n> 3 files changed, 40 insertions(+), 5 deletions(-)\n>\n> diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\n> @@ -69,6 +69,13 @@ include::line-range-format.txt[]\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. Progress information won't be displayed if using\n> + `--porcelain` or `--incremental`.\n\nThe actual implementation (below) actively forbids --progress with\n--porcelain or --incremental, so the final sentence is misleading.\nPerhaps say instead that \"--progress is incompatible with --porcelain\nand --incremental\".\n\nMore below...\n\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> @@ -127,6 +129,11 @@ struct origin {\n> +struct progress_info {\n> + struct progress *progress;\n> + int blamed_lines;\n> +};\n> +\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,9 @@ static void assign_blame(struct scoreboard *sb, int opt)\n> if (DEBUG) /* sanity */\n> sanity_check_refcnt(sb);\n> }\n> +\n> + if (pi.progress)\n> + stop_progress(&pi.progress);\n\nAs noted in the v5 review[2], stop_progress() itself handles NULL\n'struct progress' gracefully, so the 'if (pi.progress)' conditional is\nunnecessary, thus the code can be simplified further to merely:\n\n stop_progress(&pi.progress);\n\nOther than this nit, the implementation is now much cleaner and easier\nto follow.\n\n[2]: http://article.gmane.org/gmane.comp.version-control.git/282150\n\n> }\n>\n> static const char *format_time(unsigned long time, const char *tz_str,\n> @@ -2555,6 +2574,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 +2599,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\nThe 'show_progress = 0' seems unnecessary. What if you did something\nlike this instead?\n\n if (show_progress > 0 && (incremental ||\n (output_option & OUTPUT_PORCELAIN)))\n die(\"--progress can't be used with...\");\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"},{"id":"274323","messageId":"CAOc6etar5DU0w_RFRgUAzK39R43cP=eSwT_=0u2NbgKhdPi8Sg@mail.gmail.com","threadId":"40980","inReplyTo":"CAPig+cQq2Y0m0UJVCMb-9B8qrGNXV7ecf5hDETdgX5w4oUuAvw@mail.gmail.com","subject":"Re: [PATCH v6] blame: add support for --[no-]progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-12-13T00:30:24Z","receivedAt":"2015-12-13T00:30:24Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Sat, Dec 12, 2015 at 6:17 PM, Eric Sunshine wrote:\n> Right here below the \"---\" line would be a good place to explain what\n> changed since the previous version. As an aid for reviewers, it's also\n> helpful to provide a link to the previous round, like this[1].\n>\n> [1]: http://thread.gmane.org/gmane.comp.version-control.git/281677\n>\n\nOk... learning the tricks.\n\n>> @@ -69,6 +69,13 @@ include::line-range-format.txt[]\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. Progress information won't be displayed if using\n>> + `--porcelain` or `--incremental`.\n>\n> The actual implementation (below) actively forbids --progress with\n> --porcelain or --incremental, so the final sentence is misleading.\n> Perhaps say instead that \"--progress is incompatible with --porcelain\n> and --incremental\".\n>\n> More below...\n>\n\nAbsolutely right.... didn't reflect the 'policy change' in the\ndocumentation. Will update for next patch version.\n\n>> +\n>> + if (pi.progress)\n>> + stop_progress(&pi.progress);\n>\n> As noted in the v5 review[2], stop_progress() itself handles NULL\n> 'struct progress' gracefully, so the 'if (pi.progress)' conditional is\n> unnecessary, thus the code can be simplified further to merely:\n>\n> stop_progress(&pi.progress);\n>\n\nYou are right!\n\n>\n> The 'show_progress = 0' seems unnecessary. What if you did something\n> like this instead?\n>\n> if (show_progress > 0 && (incremental ||\n> (output_option & OUTPUT_PORCELAIN)))\n> die(\"--progress can't be used with...\");\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\nBecause, if the user didn't provide --[no-]progress option, then the\nvalue in show_progress will move forward being -1 and then in\nassign_blame, there will be progress output if you chose --incremental\nor porcelain. So, if you chose --incremental or porcelain, we better\nset the value to 0 to make sure there will be _no_ progress. Agree?\n\nCheers!\n"},{"id":"274324","messageId":"CAOc6etYAbT8Yc3NwB4Rh1dShjDQB_PR2K-HvXpx6dTcB=YaB8w@mail.gmail.com","threadId":"40980","inReplyTo":"CAOc6etar5DU0w_RFRgUAzK39R43cP=eSwT_=0u2NbgKhdPi8Sg@mail.gmail.com","subject":"Re: [PATCH v6] blame: add support for --[no-]progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-12-13T00:36:39Z","receivedAt":"2015-12-13T00:36:39Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Sat, Dec 12, 2015 at 6:30 PM, Edmundo Carmona Antoranz\n wrote:\n>>\n>> The 'show_progress = 0' seems unnecessary. What if you did something\n>> like this instead?\n>>\n>> if (show_progress > 0 && (incremental ||\n>> (output_option & OUTPUT_PORCELAIN)))\n>> die(\"--progress can't be used with...\");\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>\n> Because, if the user didn't provide --[no-]progress option, then the\n> value in show_progress will move forward being -1 and then in\n> assign_blame, there will be progress output if you chose --incremental\n> or porcelain. So, if you chose --incremental or porcelain, we better\n> set the value to 0 to make sure there will be _no_ progress. Agree?\n\n\nHmmmm.... if the code in assign_blame changed to this, it would be\npossible to allow the -1 to go through:\n\nif (show_progress > 0)\n pi.progress = start_progress_delay(_(\"Blaming lines\"),\nsb->num_lines, 50, 1);\n\nBut then I think it would be more 'concise' if we had the value set to\n0/1 instead of expecting to see a possible value of -1 there (or\nanywhere else) after progressing if progress will be shown or not in\nthe piece of code we are chatting about.\n\nComments?\n"},{"id":"274325","messageId":"CAPig+cQjWgTR9Sd-CutkbuBa1Czp5YApsYoFXc9HS1H08cno7A@mail.gmail.com","threadId":"40980","inReplyTo":"CAOc6etar5DU0w_RFRgUAzK39R43cP=eSwT_=0u2NbgKhdPi8Sg@mail.gmail.com","subject":"Re: [PATCH v6] blame: add support for --[no-]progress option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-12-13T00:37:01Z","receivedAt":"2015-12-13T00:37:01Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Dec 12, 2015 at 7:30 PM, Edmundo Carmona Antoranz\n wrote:\n> On Sat, Dec 12, 2015 at 6:17 PM, Eric Sunshine wrote:\n>> The 'show_progress = 0' seems unnecessary. What if you did something\n>> like this instead?\n>>\n>> if (show_progress > 0 && (incremental ||\n>> (output_option & OUTPUT_PORCELAIN)))\n>> die(\"--progress can't be used with...\");\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>\n> Because, if the user didn't provide --[no-]progress option, then the\n> value in show_progress will move forward being -1 and then in\n> assign_blame, there will be progress output if you chose --incremental\n> or porcelain. So, if you chose --incremental or porcelain, we better\n> set the value to 0 to make sure there will be _no_ progress. Agree?\n\nYeah, I was thinking of that and had the correct interpretation in\nmind when reading the code, but then blocked it out of my brain for\nsome reason when actually composing the response.\n"},{"id":"274326","messageId":"CAOc6etYCZOA9SOxWp3cKuY0z249jeb5vYAhvbLEohYGeVqi6Bg@mail.gmail.com","threadId":"40980","inReplyTo":"CAPig+cQjWgTR9Sd-CutkbuBa1Czp5YApsYoFXc9HS1H08cno7A@mail.gmail.com","subject":"Re: [PATCH v6] blame: add support for --[no-]progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-12-13T00:40:13Z","receivedAt":"2015-12-13T00:40:13Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Sat, Dec 12, 2015 at 6:37 PM, Eric Sunshine wrote:\n>>\n>> Because, if the user didn't provide --[no-]progress option, then the\n>> value in show_progress will move forward being -1 and then in\n>> assign_blame, there will be progress output if you chose --incremental\n>> or porcelain. So, if you chose --incremental or porcelain, we better\n>> set the value to 0 to make sure there will be _no_ progress. Agree?\n>\n> Yeah, I was thinking of that and had the correct interpretation in\n> mind when reading the code, but then blocked it out of my brain for\n> some reason when actually composing the response.\n\nGood! So, the only things to modify would be:\n- documentation to reflect new policy\n- no need to check for show_progress to ask to finish up struct\nprogress instance.\n\nLet's give some time to allow for more comments before my next patch\nversion.... so, say, 5 minutes.\n"},{"id":"274327","messageId":"CAPig+cQ__eueXdeq=CHUicz727hx-THuJxt20_PXGgo=KzK0xA@mail.gmail.com","threadId":"40980","inReplyTo":"CAOc6etYAbT8Yc3NwB4Rh1dShjDQB_PR2K-HvXpx6dTcB=YaB8w@mail.gmail.com","subject":"Re: [PATCH v6] blame: add support for --[no-]progress option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-12-13T00:40:24Z","receivedAt":"2015-12-13T00:40: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:36 PM, Edmundo Carmona Antoranz\n wrote:\n> On Sat, Dec 12, 2015 at 6:30 PM, Edmundo Carmona Antoranz\n> wrote:\n> Hmmmm.... if the code in assign_blame changed to this, it would be\n> possible to allow the -1 to go through:\n>\n> if (show_progress > 0)\n> pi.progress = start_progress_delay(_(\"Blaming lines\"),\n> sb->num_lines, 50, 1);\n>\n> But then I think it would be more 'concise' if we had the value set to\n> 0/1 instead of expecting to see a possible value of -1 there (or\n> anywhere else) after progressing if progress will be shown or not in\n> the piece of code we are chatting about.\n\nThe name \"show_progress\" does read like a boolean rather than a\ntristate, so making sure its value is 0 or 1 after option processing\n(as your current patch does) is probably the best way to go. I don't\notherwise feel strongly about it.\n"}]}