{"thread":{"id":"40673","subject":"[PATCH] checkout: add --progress option","startedAt":"2015-10-30T01:23:06Z","lastAt":"2015-11-01T16:50:28Z","messageCount":16,"participants":["Edmundo Carmona Antoranz","Junio C Hamano","Eric Sunshine","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"272510","messageId":"1446168186-4730-1-git-send-email-eantoranz@gmail.com","threadId":"40673","inReplyTo":null,"subject":"[PATCH] checkout: add --progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-10-30T01:23:06Z","receivedAt":"2015-10-30T01:23:06Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"Under normal circumstances, and like other git commands,\ngit checkout will write progress info to stderr if\nattached to a terminal. This option allows progress\nto be forced even if not using a terminal. Also,\nprogress can be skipped if using option --no-progress.\n\nSigned-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n---\n Documentation/git-checkout.txt |  6 ++++++\n builtin/checkout.c             | 17 ++++++++++++++++-\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex e269fb1..93ba35a 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -107,6 +107,12 @@ OPTIONS\n --quiet::\n \tQuiet, suppress feedback messages.\n \n+--progress::\n+\tProgress status is reported on the standard error stream\n+\tby default when it is attached to a terminal, unless -q\n+\tis specified. This flag forces progress status even if the\n+\tstandard error stream is not directed to a terminal.\n+\n -f::\n --force::\n \tWhen switching branches, proceed even if the index or the\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex bc703c0..e28c36b 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -27,6 +27,8 @@ static const char * const checkout_usage[] = {\n \tNULL,\n };\n \n+static int option_progress = -1;\n+\n struct checkout_opts {\n \tint patch_mode;\n \tint quiet;\n@@ -417,7 +419,19 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,\n \topts.reset = 1;\n \topts.merge = 1;\n \topts.fn = oneway_merge;\n-\topts.verbose_update = !o->quiet && isatty(2);\n+\t/**\n+\t * Rules to display progress:\n+\t * -q is selected\n+\t *      no verbiage\n+\t * -q is _not_ selected and --no-progress _is_ selected,\n+\t *      progress will be skipped\n+\t * -q is _not_ selected and --progress _is_ selected,\n+\t *      progress will be printed to stderr\n+\t * -q is _not_ selected and --progress is 'undefined'\n+\t *      progress will be printed to stderr _if_ working on a terminal\n+\t */\n+\topts.verbose_update = !o->quiet && (option_progress > 0 ||\n+\t\t\t\t\t   (option_progress < 0 && isatty(2)));\n \topts.src_index = &the_index;\n \topts.dst_index = &the_index;\n \tparse_tree(tree);\n@@ -1156,6 +1170,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\t\t\tN_(\"second guess 'git checkout <no-such-branch>'\")),\n \t\tOPT_BOOL(0, \"ignore-other-worktrees\", &opts.ignore_other_worktrees,\n \t\t\t N_(\"do not check if another worktree is holding the given ref\")),\n+\t\tOPT_BOOL(0, \"progress\", &option_progress, N_(\"force progress reporting\")),\n \t\tOPT_END(),\n \t};\n \n-- \n2.6.1\n"},{"id":"272534","messageId":"xmqq611omgoo.fsf@gitster.mtv.corp.google.com","threadId":"40673","inReplyTo":"1446168186-4730-1-git-send-email-eantoranz@gmail.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-30T17:04:07Z","receivedAt":"2015-10-30T17:04:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:\n\n> Under normal circumstances, and like other git commands,\n> git checkout will write progress info to stderr if\n> attached to a terminal. This option allows progress\n> to be forced even if not using a terminal. Also,\n> progress can be skipped if using option --no-progress.\n>\n> Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n> ---\n>  Documentation/git-checkout.txt |  6 ++++++\n>  builtin/checkout.c             | 17 ++++++++++++++++-\n>  2 files changed, 22 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\n> index e269fb1..93ba35a 100644\n> --- a/Documentation/git-checkout.txt\n> +++ b/Documentation/git-checkout.txt\n> @@ -107,6 +107,12 @@ OPTIONS\n>  --quiet::\n>  \tQuiet, suppress feedback messages.\n>  \n> +--progress::\n> +\tProgress status is reported on the standard error stream\n> +\tby default when it is attached to a terminal, unless -q\n> +\tis specified. This flag forces progress status even if the\n> +\tstandard error stream is not directed to a terminal.\n> +\n\nUnlike some other codepaths like pack-objects (hence \"fetch\"), \"git\ncheckout\" uses start_progress_delay() to avoid showing the progress\nwhen the operation finishes quickly.  I do not think the --progress\noption should \"force\" the output in such a case, but the text reads\nas if that is what happens.\n\nProbably it is not a big deal, though.\n\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index bc703c0..e28c36b 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -27,6 +27,8 @@ static const char * const checkout_usage[] = {\n>  \tNULL,\n>  };\n>  \n> +static int option_progress = -1;\n> +\n>  struct checkout_opts {\n>  \tint patch_mode;\n>  \tint quiet;\n> @@ -417,7 +419,19 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,\n>  \topts.reset = 1;\n>  \topts.merge = 1;\n>  \topts.fn = oneway_merge;\n> -\topts.verbose_update = !o->quiet && isatty(2);\n> +\t/**\n> +\t * Rules to display progress:\n> +\t * -q is selected\n> +\t *      no verbiage\n\nAll the other say \"progress will be...\"; this only confuses readers\nif \"verbiage\" is referring to the same \"progress\" or it is something\nelse (and if so at least two important things are left unsaid: (1)\nif \"progress\" is also part of \"verbiage\", and (2) what kind of\noutput you are squelching).\n\nAnd I cannot quite tell which among these possibilities (and there\nmay be others) you meant.\n\n> +\t * -q is _not_ selected and --no-progress _is_ selected,\n> +\t *      progress will be skipped\n> +\t * -q is _not_ selected and --progress _is_ selected,\n> +\t *      progress will be printed to stderr\n> +\t * -q is _not_ selected and --progress is 'undefined'\n> +\t *      progress will be printed to stderr _if_ working on a terminal\n> +\t */\n> +\topts.verbose_update = !o->quiet && (option_progress > 0 ||\n> +\t\t\t\t\t   (option_progress < 0 && isatty(2)));\n>  \topts.src_index = &the_index;\n>  \topts.dst_index = &the_index;\n>  \tparse_tree(tree);\n> @@ -1156,6 +1170,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  \t\t\t\tN_(\"second guess 'git checkout <no-such-branch>'\")),\n>  \t\tOPT_BOOL(0, \"ignore-other-worktrees\", &opts.ignore_other_worktrees,\n>  \t\t\t N_(\"do not check if another worktree is holding the given ref\")),\n> +\t\tOPT_BOOL(0, \"progress\", &option_progress, N_(\"force progress reporting\")),\n>  \t\tOPT_END(),\n>  \t};\n\nI see some existing commands say \"show progress\" and some others say\n\"force progress reporting\".  At some point we may want to pick one\nand use it consistently (but that is not on a topic for this change).\n\nThanks.\n"},{"id":"272541","messageId":"CAPig+cTvRs_O8uY9_SrZZCf95Lraon8knkQUDgm0Lke3pve01A@mail.gmail.com","threadId":"40673","inReplyTo":"1446168186-4730-1-git-send-email-eantoranz@gmail.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-10-30T18:12:54Z","receivedAt":"2015-10-30T18:12:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Oct 29, 2015 at 9:23 PM, Edmundo Carmona Antoranz\n<eantoranz@gmail.com> wrote:\n> Under normal circumstances, and like other git commands,\n> git checkout will write progress info to stderr if\n> attached to a terminal. This option allows progress\n> to be forced even if not using a terminal. Also,\n> progress can be skipped if using option --no-progress.\n>\n> Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n> ---\n> -       opts.verbose_update = !o->quiet && isatty(2);\n> +       /**\n> +        * Rules to display progress:\n> +        * -q is selected\n> +        *      no verbiage\n> +        * -q is _not_ selected and --no-progress _is_ selected,\n> +        *      progress will be skipped\n> +        * -q is _not_ selected and --progress _is_ selected,\n> +        *      progress will be printed to stderr\n> +        * -q is _not_ selected and --progress is 'undefined'\n> +        *      progress will be printed to stderr _if_ working on a terminal\n> +        */\n> +       opts.verbose_update = !o->quiet && (option_progress > 0 ||\n> +                                          (option_progress < 0 && isatty(2)));\n\nDoes this logic also need to be applied to the other instance where\nisatty() is consulted in merge_working_tree()?\n"},{"id":"272557","messageId":"xmqqh9l8kxa3.fsf@gitster.mtv.corp.google.com","threadId":"40673","inReplyTo":"CAPig+cTvRs_O8uY9_SrZZCf95Lraon8knkQUDgm0Lke3pve01A@mail.gmail.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-30T18:48:36Z","receivedAt":"2015-10-30T18:48:36Z","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> On Thu, Oct 29, 2015 at 9:23 PM, Edmundo Carmona Antoranz\n> <eantoranz@gmail.com> wrote:\n>> Under normal circumstances, and like other git commands,\n>> git checkout will write progress info to stderr if\n>> attached to a terminal. This option allows progress\n>> to be forced even if not using a terminal. Also,\n>> progress can be skipped if using option --no-progress.\n>>\n>> Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n>> ---\n>> -       opts.verbose_update = !o->quiet && isatty(2);\n>> +       /**\n>> +        * Rules to display progress:\n>> +        * -q is selected\n>> +        *      no verbiage\n>> +        * -q is _not_ selected and --no-progress _is_ selected,\n>> +        *      progress will be skipped\n>> +        * -q is _not_ selected and --progress _is_ selected,\n>> +        *      progress will be printed to stderr\n>> +        * -q is _not_ selected and --progress is 'undefined'\n>> +        *      progress will be printed to stderr _if_ working on a terminal\n>> +        */\n>> +       opts.verbose_update = !o->quiet && (option_progress > 0 ||\n>> +                                          (option_progress < 0 && isatty(2)));\n>\n> Does this logic also need to be applied to the other instance where\n> isatty() is consulted in merge_working_tree()?\n\nThis makes me wonder if option_progress and o->quiet change once\nparse_options() finished doing its job.  If we know that these two\nwill never change once parse_options() is done, then we can\nintroduce a variable \"show_progress\", assign the value of that\nvariable to opts.verbose_update in these places and then arrange\nthat variable is set to the combination of quiet, option_progress\nand isatty(2) just once after parse_options().\n\nThat is, something like this on top of Edmundo's patch.\n\n builtin/checkout.c | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex e28c36b..53d7c49 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -27,7 +27,7 @@ static const char * const checkout_usage[] = {\n \tNULL,\n };\n \n-static int option_progress = -1;\n+static int show_progress = 1;\n \n struct checkout_opts {\n \tint patch_mode;\n@@ -430,8 +430,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,\n \t * -q is _not_ selected and --progress is 'undefined'\n \t *      progress will be printed to stderr _if_ working on a terminal\n \t */\n-\topts.verbose_update = !o->quiet && (option_progress > 0 ||\n-\t\t\t\t\t   (option_progress < 0 && isatty(2)));\n+\topts.verbose_update = show_progress;\n \topts.src_index = &the_index;\n \topts.dst_index = &the_index;\n \tparse_tree(tree);\n@@ -515,7 +514,7 @@ static int merge_working_tree(const struct checkout_opts *opts,\n \t\ttopts.update = 1;\n \t\ttopts.merge = 1;\n \t\ttopts.gently = opts->merge && old->commit;\n-\t\ttopts.verbose_update = !opts->quiet && isatty(2);\n+\t\ttopts.verbose_update = show_progress;\n \t\ttopts.fn = twoway_merge;\n \t\tif (opts->overwrite_ignore) {\n \t\t\ttopts.dir = xcalloc(1, sizeof(*topts.dir));\n@@ -1143,6 +1142,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \tstruct branch_info new;\n \tchar *conflict_style = NULL;\n \tint dwim_new_local_branch = 1;\n+\tint option_progress = -1;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&opts.quiet, N_(\"suppress progress reporting\")),\n \t\tOPT_STRING('b', NULL, &opts.new_branch, N_(\"branch\"),\n@@ -1187,6 +1187,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, checkout_usage,\n \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n \n+\tshow_progress = (!opts.quiet &&\n+\t\t\t (0 < option_progress || (option_progress < 0 && isatty(2))));\n+\n \tif (conflict_style) {\n \t\topts.merge = 1; /* implied */\n \t\tgit_xmerge_config(\"merge.conflictstyle\", conflict_style, NULL);\n"},{"id":"272560","messageId":"CAPig+cSw7+-vfsWtfP2y4eNijuWdauWe-HfgbOvUt1vYFUEjAg@mail.gmail.com","threadId":"40673","inReplyTo":"xmqqh9l8kxa3.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-10-30T19:04:02Z","receivedAt":"2015-10-30T19:04:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 30, 2015 at 2:48 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>> On Thu, Oct 29, 2015 at 9:23 PM, Edmundo Carmona Antoranz\n>> <eantoranz@gmail.com> wrote:\n>>> +       opts.verbose_update = !o->quiet && (option_progress > 0 ||\n>>> +                                          (option_progress < 0 && isatty(2)));\n>>\n>> Does this logic also need to be applied to the other instance where\n>> isatty() is consulted in merge_working_tree()?\n>\n> This makes me wonder if option_progress and o->quiet change once\n> parse_options() finished doing its job.  If we know that these two\n> will never change once parse_options() is done, then we can\n> introduce a variable \"show_progress\", assign the value of that\n> variable to opts.verbose_update in these places and then arrange\n> that variable is set to the combination of quiet, option_progress\n> and isatty(2) just once after parse_options().\n>\n> That is, something like this on top of Edmundo's patch.\n>\n> @@ -27,7 +27,7 @@ static const char * const checkout_usage[] = {\n> -static int option_progress = -1;\n> +static int show_progress = 1;\n> @@ -430,8 +430,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,\n> -       opts.verbose_update = !o->quiet && (option_progress > 0 ||\n> -                                          (option_progress < 0 && isatty(2)));\n> +       opts.verbose_update = show_progress;\n> @@ -515,7 +514,7 @@ static int merge_working_tree(const struct checkout_opts *opts,\n> -               topts.verbose_update = !opts->quiet && isatty(2);\n> +               topts.verbose_update = show_progress;\n> @@ -1143,6 +1142,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n> +       int option_progress = -1;\n> @@ -1187,6 +1187,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>         argc = parse_options(argc, argv, prefix, options, checkout_usage,\n>                              PARSE_OPT_KEEP_DASHDASH);\n>\n> +       show_progress = (!opts.quiet &&\n> +                        (0 < option_progress || (option_progress < 0 && isatty(2))));\n\nYep, I was thinking that too. Also, Edmundo's explanatory comment\nabout how --quiet, --progress, and isatty() interact would move down\nto this location (if it's deemed worth keeping).\n"},{"id":"272569","messageId":"20151030193151.GB5336@sigill.intra.peff.net","threadId":"40673","inReplyTo":"xmqqh9l8kxa3.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-30T19:31:51Z","receivedAt":"2015-10-30T19:31:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 30, 2015 at 11:48:36AM -0700, Junio C Hamano wrote:\n\n> > Does this logic also need to be applied to the other instance where\n> > isatty() is consulted in merge_working_tree()?\n> \n> This makes me wonder if option_progress and o->quiet change once\n> parse_options() finished doing its job.  If we know that these two\n> will never change once parse_options() is done, then we can\n> introduce a variable \"show_progress\", assign the value of that\n> variable to opts.verbose_update in these places and then arrange\n> that variable is set to the combination of quiet, option_progress\n> and isatty(2) just once after parse_options().\n> \n> That is, something like this on top of Edmundo's patch.\n\nYeah, I like splitting it out like this.\n\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index e28c36b..53d7c49 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -27,7 +27,7 @@ static const char * const checkout_usage[] = {\n>  \tNULL,\n>  };\n>  \n> -static int option_progress = -1;\n> +static int show_progress = 1;\n\nYou don't need this initialization, right? Looks like we assign\nunconditionally to it in cmd_checkout.\n\n> @@ -1143,6 +1142,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  \tstruct branch_info new;\n>  \tchar *conflict_style = NULL;\n>  \tint dwim_new_local_branch = 1;\n> +\tint option_progress = -1;\n>  \tstruct option options[] = {\n>  \t\tOPT__QUIET(&opts.quiet, N_(\"suppress progress reporting\")),\n>  \t\tOPT_STRING('b', NULL, &opts.new_branch, N_(\"branch\"),\n> @@ -1187,6 +1187,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  \targc = parse_options(argc, argv, prefix, options, checkout_usage,\n>  \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n>  \n> +\tshow_progress = (!opts.quiet &&\n> +\t\t\t (0 < option_progress || (option_progress < 0 && isatty(2))));\n> +\n\nI sometimes find it confusing when there are two variables with very\nsimilar meanings (option_progress and show_progress here). I wonder if\nwe could use one variable, like:\n\n  static int show_progress = -1;\n  ...\n  OPT_BOOL(0, \"progress\", &show_progress, ...);\n  ...\n  parse_options(...);\n  if (show_progress < 0) {\n\tif (opts.quiet)\n\t\tshow_progress = 0;\n\telse\n\t\tshow_progress = isatty(2);\n  }\n\nThat somehow is much clearer to me, especially around the behavior of\n\"-q --progress\". Mine does the opposite of what you wrote above, but I\nthink it makes more sense.\n\nI can live with it either way, though. :)\n\n-Peff\n"},{"id":"272572","messageId":"xmqq4mh8kv0e.fsf@gitster.mtv.corp.google.com","threadId":"40673","inReplyTo":"20151030193151.GB5336@sigill.intra.peff.net","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-30T19:37:37Z","receivedAt":"2015-10-30T19:37:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I sometimes find it confusing when there are two variables with very\n> similar meanings (option_progress and show_progress here). I wonder if\n> we could use one variable, like:\n>\n>   static int show_progress = -1;\n>   ...\n>   OPT_BOOL(0, \"progress\", &show_progress, ...);\n>   ...\n>   parse_options(...);\n>   if (show_progress < 0) {\n> \tif (opts.quiet)\n> \t\tshow_progress = 0;\n> \telse\n> \t\tshow_progress = isatty(2);\n>   }\n>\n> That somehow is much clearer to me, especially around the behavior of\n> \"-q --progress\". Mine does the opposite of what you wrote above, but I\n> think it makes more sense.\n>\n> I can live with it either way, though. :)\n\nActually, using a single variable is my preference.  In this case I\nwanted to illustrate that the value parsed by parse_options() does\nnot have to be the one that is used in the final location deep in\nthe callchain for educational purposes, and I found it clearer to\nuse two separate variables in the illustration.\n"},{"id":"272601","messageId":"CAOc6etYiqH8bvnCD_9hedzDW6fhknXLGesM6dX7S9DBB_r-9zA@mail.gmail.com","threadId":"40673","inReplyTo":"xmqq4mh8kv0e.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-10-31T00:58:46Z","receivedAt":"2015-10-31T00:58:46Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Fri, Oct 30, 2015 at 1:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Actually, using a single variable is my preference.  In this case I\n> wanted to illustrate that the value parsed by parse_options() does\n> not have to be the one that is used in the final location deep in\n> the callchain for educational purposes, and I found it clearer to\n> use two separate variables in the illustration.\n\n>From all your feedback, I think I got a working example not using the\nstatic variable, also reworked the wording to explain in what cases\nprogress will be skipped.\n\nSo... on top of my patch, I think this sums it up:\n\n builtin/checkout.c | 33 ++++++++++++++++-----------------\n1 file changed, 16 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex e28c36b..a2e5cd3 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -27,8 +27,6 @@ static const char * const checkout_usage[] = {\n       NULL,\n};\n\n-static int option_progress = -1;\n-\nstruct checkout_opts {\n       int patch_mode;\n       int quiet;\n@@ -39,6 +37,7 @@ struct checkout_opts {\n       int overwrite_ignore;\n       int ignore_skipworktree;\n       int ignore_other_worktrees;\n+       int show_progress;\n\n       const char *new_branch;\n       const char *new_branch_force;\n@@ -406,6 +405,17 @@ static void describe_detached_head(const char\n*msg, struct commit *commit)\n       strbuf_release(&sb);\n}\n\n+/*\n+ * Any of these conditions will make progress output be skipped:\n+ * - selected --quiet\n+ * - selected --no-progress\n+ * - (didn't select --progress nor --no-progress) and not working on a terminal\n+ */\n+static int verbose_update(const struct checkout_opts *o)\n+{\n+       return !o->quiet && o->show_progress && (o->show_progress >= 0\n|| isatty(2));\n+}\n+\nstatic int reset_tree(struct tree *tree, const struct checkout_opts *o,\n                     int worktree, int *writeout_error)\n{\n@@ -419,19 +429,7 @@ static int reset_tree(struct tree *tree, const\nstruct checkout_opts *o,\n       opts.reset = 1;\n       opts.merge = 1;\n       opts.fn = oneway_merge;\n-       /**\n-        * Rules to display progress:\n-        * -q is selected\n-        *      no verbiage\n-        * -q is _not_ selected and --no-progress _is_ selected,\n-        *      progress will be skipped\n-        * -q is _not_ selected and --progress _is_ selected,\n-        *      progress will be printed to stderr\n-        * -q is _not_ selected and --progress is 'undefined'\n-        *      progress will be printed to stderr _if_ working on a terminal\n-        */\n-       opts.verbose_update = !o->quiet && (option_progress > 0 ||\n-                                          (option_progress < 0 && isatty(2)));\n+       opts.verbose_update = verbose_update(o);\n       opts.src_index = &the_index;\n       opts.dst_index = &the_index;\n       parse_tree(tree);\n@@ -515,7 +513,7 @@ static int merge_working_tree(const struct\ncheckout_opts *opts,\n               topts.update = 1;\n               topts.merge = 1;\n               topts.gently = opts->merge && old->commit;\n-               topts.verbose_update = !opts->quiet && isatty(2);\n+               topts.verbose_update = verbose_update(opts);\n               topts.fn = twoway_merge;\n               if (opts->overwrite_ignore) {\n                       topts.dir = xcalloc(1, sizeof(*topts.dir));\n@@ -1170,7 +1168,7 @@ int cmd_checkout(int argc, const char **argv,\nconst char *prefix)\n                               N_(\"second guess 'git checkout\n<no-such-branch>'\")),\n               OPT_BOOL(0, \"ignore-other-worktrees\",\n&opts.ignore_other_worktrees,\n                        N_(\"do not check if another worktree is\nholding the given ref\")),\n-               OPT_BOOL(0, \"progress\", &option_progress, N_(\"force\nprogress reporting\")),\n+               OPT_BOOL(0, \"progress\", &opts.show_progress, N_(\"force\nprogress reporting\")),\n               OPT_END(),\n       };\n\n@@ -1178,6 +1176,7 @@ int cmd_checkout(int argc, const char **argv,\nconst char *prefix)\n       memset(&new, 0, sizeof(new));\n       opts.overwrite_ignore = 1;\n       opts.prefix = prefix;\n+       opts.show_progress = -1;\n\n       gitmodules_config();\n       git_config(git_checkout_config, &opts);\n"},{"id":"272602","messageId":"CAOc6etZ+MeSuVDF6W4pefsxDDee3Dx3DikGxNA_X8-Qh3R9ZQQ@mail.gmail.com","threadId":"40673","inReplyTo":"CAOc6etYiqH8bvnCD_9hedzDW6fhknXLGesM6dX7S9DBB_r-9zA@mail.gmail.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-10-31T01:08:12Z","receivedAt":"2015-10-31T01:08:12Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"One line got wrapped around, hope that's not a problem to understand\nthe whole concept.\n\n> +/*\n> + * Any of these conditions will make progress output be skipped:\n> + * - selected --quiet\n> + * - selected --no-progress\n> + * - (didn't select --progress nor --no-progress) and not working on a terminal\n> + */\n> +static int verbose_update(const struct checkout_opts *o)\n> +{\n> +       return !o->quiet && o->show_progress && (o->show_progress >= 0\n> || isatty(2));\n> +}\n> +\n"},{"id":"272603","messageId":"CAOc6etYCzBYpf+7p8p3=zQun7bYXYVc-codoUf5Abcq+hAz8cA@mail.gmail.com","threadId":"40673","inReplyTo":"CAOc6etYiqH8bvnCD_9hedzDW6fhknXLGesM6dX7S9DBB_r-9zA@mail.gmail.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-10-31T02:11:21Z","receivedAt":"2015-10-31T02:11:21Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"Ok.... this is like the previous patch (on top of my patch) but does\npost_processing right after parse_options and so there's no need for\nthe function I had introduced before. Hope it can get your blessings\nso I can send-email the _second_ patch for this feature (following the\none that has already being merged into pu, right?).\n\n builtin/checkout.c | 32 +++++++++++++++-----------------\n1 file changed, 15 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex e28c36b..9e78835 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -27,8 +27,6 @@ static const char * const checkout_usage[] = {\n       NULL,\n};\n\n-static int option_progress = -1;\n-\nstruct checkout_opts {\n       int patch_mode;\n       int quiet;\n@@ -39,6 +37,7 @@ struct checkout_opts {\n       int overwrite_ignore;\n       int ignore_skipworktree;\n       int ignore_other_worktrees;\n+       int show_progress;\n\n       const char *new_branch;\n       const char *new_branch_force;\n@@ -419,19 +418,7 @@ static int reset_tree(struct tree *tree, const\nstruct checkout_opts *o,\n       opts.reset = 1;\n       opts.merge = 1;\n       opts.fn = oneway_merge;\n-       /**\n-        * Rules to display progress:\n-        * -q is selected\n-        *      no verbiage\n-        * -q is _not_ selected and --no-progress _is_ selected,\n-        *      progress will be skipped\n-        * -q is _not_ selected and --progress _is_ selected,\n-        *      progress will be printed to stderr\n-        * -q is _not_ selected and --progress is 'undefined'\n-        *      progress will be printed to stderr _if_ working on a terminal\n-        */\n-       opts.verbose_update = !o->quiet && (option_progress > 0 ||\n-                                          (option_progress < 0 && isatty(2)));\n+       opts.verbose_update = o->show_progress;\n       opts.src_index = &the_index;\n       opts.dst_index = &the_index;\n       parse_tree(tree);\n@@ -515,7 +502,7 @@ static int merge_working_tree(const struct\ncheckout_opts *opts,\n               topts.update = 1;\n               topts.merge = 1;\n               topts.gently = opts->merge && old->commit;\n-               topts.verbose_update = !opts->quiet && isatty(2);\n+               topts.verbose_update = opts->show_progress;\n               topts.fn = twoway_merge;\n               if (opts->overwrite_ignore) {\n                       topts.dir = xcalloc(1, sizeof(*topts.dir));\n@@ -1170,7 +1157,7 @@ int cmd_checkout(int argc, const char **argv,\nconst char *prefix)\n                               N_(\"second guess 'git checkout\n<no-such-branch>'\")),\n               OPT_BOOL(0, \"ignore-other-worktrees\",\n&opts.ignore_other_worktrees,\n                        N_(\"do not check if another worktree is\nholding the given ref\")),\n-               OPT_BOOL(0, \"progress\", &option_progress, N_(\"force\nprogress reporting\")),\n+               OPT_BOOL(0, \"progress\", &opts.show_progress, N_(\"force\nprogress reporting\")),\n               OPT_END(),\n       };\n\n@@ -1178,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv,\nconst char *prefix)\n       memset(&new, 0, sizeof(new));\n       opts.overwrite_ignore = 1;\n       opts.prefix = prefix;\n+       opts.show_progress = -1;\n\n       gitmodules_config();\n       git_config(git_checkout_config, &opts);\n@@ -1187,6 +1175,16 @@ int cmd_checkout(int argc, const char **argv,\nconst char *prefix)\n       argc = parse_options(argc, argv, prefix, options, checkout_usage,\n                            PARSE_OPT_KEEP_DASHDASH);\n\n+       /*\n+        * Final processing of show_progress\n+        * Any of these 3 conditions will make progress output be skipped:\n+        * - selected --quiet\n+        * - selected --no-progress\n+        * - didn't select --progress and not working on a terminal\n+        */\n+       opts.show_progress = !opts.quiet && opts.show_progress &&\n+                            (opts.show_progress >= 0 || isatty(2));\n+\n       if (conflict_style) {\n               opts.merge = 1; /* implied */\n               git_xmerge_config(\"merge.conflictstyle\", conflict_style, NULL);\n\nOn Fri, Oct 30, 2015 at 6:58 PM, Edmundo Carmona Antoranz\n<eantoranz@gmail.com> wrote:\n> On Fri, Oct 30, 2015 at 1:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Actually, using a single variable is my preference.  In this case I\n>> wanted to illustrate that the value parsed by parse_options() does\n>> not have to be the one that is used in the final location deep in\n>> the callchain for educational purposes, and I found it clearer to\n>> use two separate variables in the illustration.\n>\n> From all your feedback, I think I got a working example not using the\n> static variable, also reworked the wording to explain in what cases\n> progress will be skipped.\n>\n> So... on top of my patch, I think this sums it up:\n>\n>  builtin/checkout.c | 33 ++++++++++++++++-----------------\n> 1 file changed, 16 insertions(+), 17 deletions(-)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index e28c36b..a2e5cd3 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -27,8 +27,6 @@ static const char * const checkout_usage[] = {\n>        NULL,\n> };\n>\n> -static int option_progress = -1;\n> -\n> struct checkout_opts {\n>        int patch_mode;\n>        int quiet;\n> @@ -39,6 +37,7 @@ struct checkout_opts {\n>        int overwrite_ignore;\n>        int ignore_skipworktree;\n>        int ignore_other_worktrees;\n> +       int show_progress;\n>\n>        const char *new_branch;\n>        const char *new_branch_force;\n> @@ -406,6 +405,17 @@ static void describe_detached_head(const char\n> *msg, struct commit *commit)\n>        strbuf_release(&sb);\n> }\n>\n> +/*\n> + * Any of these conditions will make progress output be skipped:\n> + * - selected --quiet\n> + * - selected --no-progress\n> + * - (didn't select --progress nor --no-progress) and not working on a terminal\n> + */\n> +static int verbose_update(const struct checkout_opts *o)\n> +{\n> +       return !o->quiet && o->show_progress && (o->show_progress >= 0\n> || isatty(2));\n> +}\n> +\n> static int reset_tree(struct tree *tree, const struct checkout_opts *o,\n>                      int worktree, int *writeout_error)\n> {\n> @@ -419,19 +429,7 @@ static int reset_tree(struct tree *tree, const\n> struct checkout_opts *o,\n>        opts.reset = 1;\n>        opts.merge = 1;\n>        opts.fn = oneway_merge;\n> -       /**\n> -        * Rules to display progress:\n> -        * -q is selected\n> -        *      no verbiage\n> -        * -q is _not_ selected and --no-progress _is_ selected,\n> -        *      progress will be skipped\n> -        * -q is _not_ selected and --progress _is_ selected,\n> -        *      progress will be printed to stderr\n> -        * -q is _not_ selected and --progress is 'undefined'\n> -        *      progress will be printed to stderr _if_ working on a terminal\n> -        */\n> -       opts.verbose_update = !o->quiet && (option_progress > 0 ||\n> -                                          (option_progress < 0 && isatty(2)));\n> +       opts.verbose_update = verbose_update(o);\n>        opts.src_index = &the_index;\n>        opts.dst_index = &the_index;\n>        parse_tree(tree);\n> @@ -515,7 +513,7 @@ static int merge_working_tree(const struct\n> checkout_opts *opts,\n>                topts.update = 1;\n>                topts.merge = 1;\n>                topts.gently = opts->merge && old->commit;\n> -               topts.verbose_update = !opts->quiet && isatty(2);\n> +               topts.verbose_update = verbose_update(opts);\n>                topts.fn = twoway_merge;\n>                if (opts->overwrite_ignore) {\n>                        topts.dir = xcalloc(1, sizeof(*topts.dir));\n> @@ -1170,7 +1168,7 @@ int cmd_checkout(int argc, const char **argv,\n> const char *prefix)\n>                                N_(\"second guess 'git checkout\n> <no-such-branch>'\")),\n>                OPT_BOOL(0, \"ignore-other-worktrees\",\n> &opts.ignore_other_worktrees,\n>                         N_(\"do not check if another worktree is\n> holding the given ref\")),\n> -               OPT_BOOL(0, \"progress\", &option_progress, N_(\"force\n> progress reporting\")),\n> +               OPT_BOOL(0, \"progress\", &opts.show_progress, N_(\"force\n> progress reporting\")),\n>                OPT_END(),\n>        };\n>\n> @@ -1178,6 +1176,7 @@ int cmd_checkout(int argc, const char **argv,\n> const char *prefix)\n>        memset(&new, 0, sizeof(new));\n>        opts.overwrite_ignore = 1;\n>        opts.prefix = prefix;\n> +       opts.show_progress = -1;\n>\n>        gitmodules_config();\n>        git_config(git_checkout_config, &opts);\n"},{"id":"272630","messageId":"xmqqk2q3hrbl.fsf@gitster.mtv.corp.google.com","threadId":"40673","inReplyTo":"CAOc6etYCzBYpf+7p8p3=zQun7bYXYVc-codoUf5Abcq+hAz8cA@mail.gmail.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-31T17:37:50Z","receivedAt":"2015-10-31T17:37:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:\n\n> +       /*\n> +        * Final processing of show_progress\n> +        * Any of these 3 conditions will make progress output be skipped:\n> +        * - selected --quiet\n> +        * - selected --no-progress\n> +        * - didn't select --progress and not working on a terminal\n> +        */\n> +       opts.show_progress = !opts.quiet && opts.show_progress &&\n> +                            (opts.show_progress >= 0 || isatty(2));\n> +\n\nI do find what Peff showed us a lot easier to follow.\n\n\tif (opts.show_progress < 0) {\n\t\tif (opts.quiet)\n\t\t\topts.show_progress = 0;\n\t\telse\n\t\t\topts.show_progress = isatty(2);\n\t}\n"},{"id":"272632","messageId":"CAOc6eta7_0RfBUngtMg5ZAEUvjuPVgZ20ESgnbJK=--h53k+Tw@mail.gmail.com","threadId":"40673","inReplyTo":"xmqqk2q3hrbl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-10-31T17:42:19Z","receivedAt":"2015-10-31T17:42:19Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Sat, Oct 31, 2015 at 11:37 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> I do find what Peff showed us a lot easier to follow.\n>\n>         if (opts.show_progress < 0) {\n>                 if (opts.quiet)\n>                         opts.show_progress = 0;\n>                 else\n>                         opts.show_progress = isatty(2);\n>         }\n>\n\nOk.... let me rewrite it that way. Other than that, the other things are ok?\n"},{"id":"272634","messageId":"CAOc6etapqKNAXxNycAx9g9VTJXnnCxuXj5eU7giKvZUy5s8X3Q@mail.gmail.com","threadId":"40673","inReplyTo":"CAOc6eta7_0RfBUngtMg5ZAEUvjuPVgZ20ESgnbJK=--h53k+Tw@mail.gmail.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-10-31T18:14:39Z","receivedAt":"2015-10-31T18:14:39Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"I just noticed something interesting.\n\nOn Sat, Oct 31, 2015 at 11:42 AM, Edmundo Carmona Antoranz\n<eantoranz@gmail.com> wrote:\n> On Sat, Oct 31, 2015 at 11:37 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> I do find what Peff showed us a lot easier to follow.\n>>\n>>         if (opts.show_progress < 0) {\n>>                 if (opts.quiet)\n>>                         opts.show_progress = 0;\n>>                 else\n>>                         opts.show_progress = isatty(2);\n>>         }\n>>\n>\n> Ok.... let me rewrite it that way. Other than that, the other things are ok?\n\nIn Peff's implementation I think he uses -1 as --no-progress, 1 as\n--progress and 0 as undefined, right?\n\nIn my implementation I'm using -1 as undefined and 0 as --no-progress.\nWhat would be the standard approach? From what I can see on\nparse_options's behavior, if you select --no-progress, the variable\nends up with a 0, which makes me think I'm using the right approach.\n\nEnd result with my assumptions would be:\n\n        if (opts.show_progress) {\n                /* user selected --progress or didn't specify */\n                if (opts.quiet) {\n                        opts.show_progress = 0;\n                } else if (opts.show_progress < 0) {\n                        opts.show_progress = isatty(2);\n                }\n        }\n"},{"id":"272638","messageId":"20151031200702.GA4115@sigill.intra.peff.net","threadId":"40673","inReplyTo":"CAOc6etapqKNAXxNycAx9g9VTJXnnCxuXj5eU7giKvZUy5s8X3Q@mail.gmail.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-31T20:07:02Z","receivedAt":"2015-10-31T20:07:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 31, 2015 at 12:14:39PM -0600, Edmundo Carmona Antoranz wrote:\n\n> On Sat, Oct 31, 2015 at 11:42 AM, Edmundo Carmona Antoranz\n> <eantoranz@gmail.com> wrote:\n> > On Sat, Oct 31, 2015 at 11:37 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> >> I do find what Peff showed us a lot easier to follow.\n> >>\n> >>         if (opts.show_progress < 0) {\n> >>                 if (opts.quiet)\n> >>                         opts.show_progress = 0;\n> >>                 else\n> >>                         opts.show_progress = isatty(2);\n> >>         }\n> >>\n> >\n> > Ok.... let me rewrite it that way. Other than that, the other things are ok?\n> \n> In Peff's implementation I think he uses -1 as --no-progress, 1 as\n> --progress and 0 as undefined, right?\n\nI didn't mean to, though I don't promise I didn't send something buggy.\nIt looks right to me, though:\n\n  if (opts.show_progress < 0) { /* if the user didn't say... */\n          if (opts.quiet)\n\t\topts.show_progress = 0; /* quiet means \"no progress\" */\n\t  else\n\t\topts.show_progress = isatty(2); /* returns 0/1 bool */\n  }\n\n> In my implementation I'm using -1 as undefined and 0 as --no-progress.\n> What would be the standard approach?\n\nThat's standard. And \"1\" is \"--progress\".\n\n> From what I can see on\n> parse_options's behavior, if you select --no-progress, the variable\n> ends up with a 0, which makes me think I'm using the right approach.\n> \n> End result with my assumptions would be:\n> \n>         if (opts.show_progress) {\n>                 /* user selected --progress or didn't specify */\n>                 if (opts.quiet) {\n>                         opts.show_progress = 0;\n>                 } else if (opts.show_progress < 0) {\n>                         opts.show_progress = isatty(2);\n>                 }\n>         }\n\nThe difference between mine and yours is that in mine, \"--progress\"\ntrumps \"--quiet\", whereas it is the other way around in yours. I don't\nknow if it is a huge deal, but mine makes more sense to me (because\n\"--progress\" is more specific than \"--quiet\", which might silence other\nmessages, too).\n\n-Peff\n"},{"id":"272641","messageId":"CAPig+cQ+-k_eAm+EMEHqAyQ1P-AjNXQwmyPKqm=fD3vZ1chTLA@mail.gmail.com","threadId":"40673","inReplyTo":"CAOc6etYCzBYpf+7p8p3=zQun7bYXYVc-codoUf5Abcq+hAz8cA@mail.gmail.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-01T00:45:46Z","receivedAt":"2015-11-01T00:45:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 30, 2015 at 10:11 PM, Edmundo Carmona Antoranz\n<eantoranz@gmail.com> wrote:\n> Ok.... this is like the previous patch (on top of my patch) but does\n> post_processing right after parse_options and so there's no need for\n> the function I had introduced before. Hope it can get your blessings\n> so I can send-email the _second_ patch for this feature (following the\n> one that has already being merged into pu, right?).\n\nPatches in 'next' are pretty much set in stone, and those in 'master'\ndefinitely are, but 'pu' is volatile, so just send the entire patch\nrevised, tagged v2 (or v3, etc.), rather than sending a patch to fix\nwhat is in 'pu', and Junio will replace the previous version with the\nnew one.\n\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index e28c36b..9e78835 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -27,8 +27,6 @@ static const char * const checkout_usage[] = {\n>        NULL,\n> };\n>\n> -static int option_progress = -1;\n> -\n> struct checkout_opts {\n>        int patch_mode;\n>        int quiet;\n> @@ -39,6 +37,7 @@ struct checkout_opts {\n>        int overwrite_ignore;\n>        int ignore_skipworktree;\n>        int ignore_other_worktrees;\n> +       int show_progress;\n>\n>        const char *new_branch;\n>        const char *new_branch_force;\n> @@ -419,19 +418,7 @@ static int reset_tree(struct tree *tree, const\n> struct checkout_opts *o,\n>        opts.reset = 1;\n>        opts.merge = 1;\n>        opts.fn = oneway_merge;\n> -       /**\n> -        * Rules to display progress:\n> -        * -q is selected\n> -        *      no verbiage\n> -        * -q is _not_ selected and --no-progress _is_ selected,\n> -        *      progress will be skipped\n> -        * -q is _not_ selected and --progress _is_ selected,\n> -        *      progress will be printed to stderr\n> -        * -q is _not_ selected and --progress is 'undefined'\n> -        *      progress will be printed to stderr _if_ working on a terminal\n> -        */\n> -       opts.verbose_update = !o->quiet && (option_progress > 0 ||\n> -                                          (option_progress < 0 && isatty(2)));\n> +       opts.verbose_update = o->show_progress;\n>        opts.src_index = &the_index;\n>        opts.dst_index = &the_index;\n>        parse_tree(tree);\n> @@ -515,7 +502,7 @@ static int merge_working_tree(const struct\n> checkout_opts *opts,\n>                topts.update = 1;\n>                topts.merge = 1;\n>                topts.gently = opts->merge && old->commit;\n> -               topts.verbose_update = !opts->quiet && isatty(2);\n> +               topts.verbose_update = opts->show_progress;\n>                topts.fn = twoway_merge;\n>                if (opts->overwrite_ignore) {\n>                        topts.dir = xcalloc(1, sizeof(*topts.dir));\n> @@ -1170,7 +1157,7 @@ int cmd_checkout(int argc, const char **argv,\n> const char *prefix)\n>                                N_(\"second guess 'git checkout\n> <no-such-branch>'\")),\n>                OPT_BOOL(0, \"ignore-other-worktrees\",\n> &opts.ignore_other_worktrees,\n>                         N_(\"do not check if another worktree is\n> holding the given ref\")),\n> -               OPT_BOOL(0, \"progress\", &option_progress, N_(\"force\n> progress reporting\")),\n> +               OPT_BOOL(0, \"progress\", &opts.show_progress, N_(\"force\n> progress reporting\")),\n>                OPT_END(),\n>        };\n>\n> @@ -1178,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv,\n> const char *prefix)\n>        memset(&new, 0, sizeof(new));\n>        opts.overwrite_ignore = 1;\n>        opts.prefix = prefix;\n> +       opts.show_progress = -1;\n>\n>        gitmodules_config();\n>        git_config(git_checkout_config, &opts);\n> @@ -1187,6 +1175,16 @@ int cmd_checkout(int argc, const char **argv,\n> const char *prefix)\n>        argc = parse_options(argc, argv, prefix, options, checkout_usage,\n>                             PARSE_OPT_KEEP_DASHDASH);\n>\n> +       /*\n> +        * Final processing of show_progress\n> +        * Any of these 3 conditions will make progress output be skipped:\n> +        * - selected --quiet\n> +        * - selected --no-progress\n> +        * - didn't select --progress and not working on a terminal\n> +        */\n> +       opts.show_progress = !opts.quiet && opts.show_progress &&\n> +                            (opts.show_progress >= 0 || isatty(2));\n> +\n>        if (conflict_style) {\n>                opts.merge = 1; /* implied */\n>                git_xmerge_config(\"merge.conflictstyle\", conflict_style, NULL);\n"},{"id":"272673","messageId":"CAOc6etY5Gh+njnWjtA3MYgnu+LCR-x75UCNOdRDw4+ReEbhVLQ@mail.gmail.com","threadId":"40673","inReplyTo":"CAPig+cQ+-k_eAm+EMEHqAyQ1P-AjNXQwmyPKqm=fD3vZ1chTLA@mail.gmail.com","subject":"Re: [PATCH] checkout: add --progress option","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2015-11-01T16:50:28Z","receivedAt":"2015-11-01T16:50:28Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Sat, Oct 31, 2015 at 6:45 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> Patches in 'next' are pretty much set in stone, and those in 'master'\n> definitely are, but 'pu' is volatile, so just send the entire patch\n> revised, tagged v2 (or v3, etc.), rather than sending a patch to fix\n> what is in 'pu', and Junio will replace the previous version with the\n> new one.\n>\n\nWill do!\n"}]}