Re: [PATCH] checkout: add --progress option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 30, 2015, 18:48 UTC
- Message-ID
- <xmqqh9l8kxa3.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <CAPig+cTvRs_O8uY9_SrZZCf95Lraon8knkQUDgm0Lke3pve01A@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
Show 27 quoted lines
> On Thu, Oct 29, 2015 at 9:23 PM, Edmundo Carmona Antoranz > <eantoranz@gmail.com> wrote: >> Under normal circumstances, and like other git commands, >> git checkout will write progress info to stderr if >> attached to a terminal. This option allows progress >> to be forced even if not using a terminal. Also, >> progress can be skipped if using option --no-progress. >> >> Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com> >> --- >> - opts.verbose_update = !o->quiet && isatty(2); >> + /** >> + * 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))); > > Does this logic also need to be applied to the other instance where > isatty() is consulted in merge_working_tree()?
This makes me wonder if option_progress and o->quiet change once parse_options() finished doing its job. If we know that these two will never change once parse_options() is done, then we can introduce a variable "show_progress", assign the value of that variable to opts.verbose_update in these places and then arrange that variable is set to the combination of quiet, option_progress and isatty(2) just once after parse_options().
That is, something like this on top of Edmundo's patch.
builtin/checkout.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c index e28c36b..53d7c49 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -27,7 +27,7 @@ static const char * const checkout_usage[] = { NULL, }; -static int option_progress = -1; +static int show_progress = 1; struct checkout_opts { int patch_mode; @@ -430,8 +430,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o, * -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 = show_progress; opts.src_index = &the_index; opts.dst_index = &the_index; parse_tree(tree); @@ -515,7 +514,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 = show_progress; topts.fn = twoway_merge; if (opts->overwrite_ignore) { topts.dir = xcalloc(1, sizeof(*topts.dir)); @@ -1143,6 +1142,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) struct branch_info new; char *conflict_style = NULL; int dwim_new_local_branch = 1; + int option_progress = -1; struct option options[] = { OPT__QUIET(&opts.quiet, N_("suppress progress reporting")), OPT_STRING('b', NULL, &opts.new_branch, N_("branch"), @@ -1187,6 +1187,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, checkout_usage, PARSE_OPT_KEEP_DASHDASH); + show_progress = (!opts.quiet && + (0 < option_progress || (option_progress < 0 && isatty(2)))); + if (conflict_style) { opts.merge = 1; /* implied */ git_xmerge_config("merge.conflictstyle", conflict_style, NULL);