Ok.... this is like the previous patch (on top of my patch) but does post_processing right after parse_options and so there's no need for the function I had introduced before. Hope it can get your blessings so I can send-email the _second_ patch for this feature (following the one that has already being merged into pu, right?). builtin/checkout.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index e28c36b..9e78835 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -27,8 +27,6 @@ static const char * const checkout_usage[] = { NULL, }; -static int option_progress = -1; - struct checkout_opts { int patch_mode; int quiet; @@ -39,6 +37,7 @@ struct checkout_opts { int overwrite_ignore; int ignore_skipworktree; int ignore_other_worktrees; + int show_progress; const char *new_branch; const char *new_branch_force; @@ -419,19 +418,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o, opts.reset = 1; opts.merge = 1; opts.fn = oneway_merge; - /** - * Rules to display progress: - * -q is selected - * no verbiage - * -q is _not_ selected and --no-progress _is_ selected, - * progress will be skipped - * -q is _not_ selected and --progress _is_ selected, - * progress will be printed to stderr - * -q is _not_ selected and --progress is 'undefined' - * progress will be printed to stderr _if_ working on a terminal - */ - opts.verbose_update = !o->quiet && (option_progress > 0 || - (option_progress < 0 && isatty(2))); + opts.verbose_update = o->show_progress; opts.src_index = &the_index; opts.dst_index = &the_index; parse_tree(tree); @@ -515,7 +502,7 @@ static int merge_working_tree(const struct checkout_opts *opts, topts.update = 1; topts.merge = 1; topts.gently = opts->merge && old->commit; - topts.verbose_update = !opts->quiet && isatty(2); + topts.verbose_update = opts->show_progress; topts.fn = twoway_merge; if (opts->overwrite_ignore) { topts.dir = xcalloc(1, sizeof(*topts.dir)); @@ -1170,7 +1157,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) N_("second guess 'git checkout <no-such-branch>'")), OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees, N_("do not check if another worktree is holding the given ref")), - OPT_BOOL(0, "progress", &option_progress, N_("force progress reporting")), + OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")), OPT_END(), }; @@ -1178,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) memset(&new, 0, sizeof(new)); opts.overwrite_ignore = 1; opts.prefix = prefix; + opts.show_progress = -1; gitmodules_config(); git_config(git_checkout_config, &opts); @@ -1187,6 +1175,16 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, checkout_usage, PARSE_OPT_KEEP_DASHDASH); + /* + * Final processing of show_progress + * Any of these 3 conditions will make progress output be skipped: + * - selected --quiet + * - selected --no-progress + * - didn't select --progress and not working on a terminal + */ + opts.show_progress = !opts.quiet && opts.show_progress && + (opts.show_progress >= 0 || isatty(2)); + if (conflict_style) { opts.merge = 1; /* implied */ git_xmerge_config("merge.conflictstyle", conflict_style, NULL); On Fri, Oct 30, 2015 at 6:58 PM, Edmundo Carmona Antoranz <eantoranz@xxxxxxxxx> wrote: > On Fri, Oct 30, 2015 at 1:37 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> >> Actually, using a single variable is my preference. In this case I >> wanted to illustrate that the value parsed by parse_options() does >> not have to be the one that is used in the final location deep in >> the callchain for educational purposes, and I found it clearer to >> use two separate variables in the illustration. > > From all your feedback, I think I got a working example not using the > static variable, also reworked the wording to explain in what cases > progress will be skipped. > > So... on top of my patch, I think this sums it up: > > builtin/checkout.c | 33 ++++++++++++++++----------------- > 1 file changed, 16 insertions(+), 17 deletions(-) > > diff --git a/builtin/checkout.c b/builtin/checkout.c > index e28c36b..a2e5cd3 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -27,8 +27,6 @@ static const char * const checkout_usage[] = { > NULL, > }; > > -static int option_progress = -1; > - > struct checkout_opts { > int patch_mode; > int quiet; > @@ -39,6 +37,7 @@ struct checkout_opts { > int overwrite_ignore; > int ignore_skipworktree; > int ignore_other_worktrees; > + int show_progress; > > const char *new_branch; > const char *new_branch_force; > @@ -406,6 +405,17 @@ static void describe_detached_head(const char > *msg, struct commit *commit) > strbuf_release(&sb); > } > > +/* > + * Any of these conditions will make progress output be skipped: > + * - selected --quiet > + * - selected --no-progress > + * - (didn't select --progress nor --no-progress) and not working on a terminal > + */ > +static int verbose_update(const struct checkout_opts *o) > +{ > + return !o->quiet && o->show_progress && (o->show_progress >= 0 > || isatty(2)); > +} > + > static int reset_tree(struct tree *tree, const struct checkout_opts *o, > int worktree, int *writeout_error) > { > @@ -419,19 +429,7 @@ static int reset_tree(struct tree *tree, const > struct checkout_opts *o, > opts.reset = 1; > opts.merge = 1; > opts.fn = oneway_merge; > - /** > - * Rules to display progress: > - * -q is selected > - * no verbiage > - * -q is _not_ selected and --no-progress _is_ selected, > - * progress will be skipped > - * -q is _not_ selected and --progress _is_ selected, > - * progress will be printed to stderr > - * -q is _not_ selected and --progress is 'undefined' > - * progress will be printed to stderr _if_ working on a terminal > - */ > - opts.verbose_update = !o->quiet && (option_progress > 0 || > - (option_progress < 0 && isatty(2))); > + opts.verbose_update = verbose_update(o); > opts.src_index = &the_index; > opts.dst_index = &the_index; > parse_tree(tree); > @@ -515,7 +513,7 @@ static int merge_working_tree(const struct > checkout_opts *opts, > topts.update = 1; > topts.merge = 1; > topts.gently = opts->merge && old->commit; > - topts.verbose_update = !opts->quiet && isatty(2); > + topts.verbose_update = verbose_update(opts); > topts.fn = twoway_merge; > if (opts->overwrite_ignore) { > topts.dir = xcalloc(1, sizeof(*topts.dir)); > @@ -1170,7 +1168,7 @@ int cmd_checkout(int argc, const char **argv, > const char *prefix) > N_("second guess 'git checkout > <no-such-branch>'")), > OPT_BOOL(0, "ignore-other-worktrees", > &opts.ignore_other_worktrees, > N_("do not check if another worktree is > holding the given ref")), > - OPT_BOOL(0, "progress", &option_progress, N_("force > progress reporting")), > + OPT_BOOL(0, "progress", &opts.show_progress, N_("force > progress reporting")), > OPT_END(), > }; > > @@ -1178,6 +1176,7 @@ int cmd_checkout(int argc, const char **argv, > const char *prefix) > memset(&new, 0, sizeof(new)); > opts.overwrite_ignore = 1; > opts.prefix = prefix; > + opts.show_progress = -1; > > gitmodules_config(); > git_config(git_checkout_config, &opts); -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html