Re: [PATCH] checkout: add --progress option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Edmundo Carmona Antoranz <eantoranz@xxxxxxxxx> writes:

> 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@xxxxxxxxx>
> ---
>  Documentation/git-checkout.txt |  6 ++++++
>  builtin/checkout.c             | 17 ++++++++++++++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index e269fb1..93ba35a 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -107,6 +107,12 @@ OPTIONS
>  --quiet::
>  	Quiet, suppress feedback messages.
>  
> +--progress::
> +	Progress status is reported on the standard error stream
> +	by default when it is attached to a terminal, unless -q
> +	is specified. This flag forces progress status even if the
> +	standard error stream is not directed to a terminal.
> +

Unlike some other codepaths like pack-objects (hence "fetch"), "git
checkout" uses start_progress_delay() to avoid showing the progress
when the operation finishes quickly.  I do not think the --progress
option should "force" the output in such a case, but the text reads
as if that is what happens.

Probably it is not a big deal, though.

> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index bc703c0..e28c36b 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -27,6 +27,8 @@ static const char * const checkout_usage[] = {
>  	NULL,
>  };
>  
> +static int option_progress = -1;
> +
>  struct checkout_opts {
>  	int patch_mode;
>  	int quiet;
> @@ -417,7 +419,19 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
>  	opts.reset = 1;
>  	opts.merge = 1;
>  	opts.fn = oneway_merge;
> -	opts.verbose_update = !o->quiet && isatty(2);
> +	/**
> +	 * Rules to display progress:
> +	 * -q is selected
> +	 *      no verbiage

All the other say "progress will be..."; this only confuses readers
if "verbiage" is referring to the same "progress" or it is something
else (and if so at least two important things are left unsaid: (1)
if "progress" is also part of "verbiage", and (2) what kind of
output you are squelching).

And I cannot quite tell which among these possibilities (and there
may be others) you meant.

> +	 * -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.src_index = &the_index;
>  	opts.dst_index = &the_index;
>  	parse_tree(tree);
> @@ -1156,6 +1170,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_END(),
>  	};

I see some existing commands say "show progress" and some others say
"force progress reporting".  At some point we may want to pick one
and use it consistently (but that is not on a topic for this change).

Thanks.
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]