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

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

 



Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:

> On Thu, Oct 29, 2015 at 9:23 PM, Edmundo Carmona Antoranz
> <eantoranz@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>> -       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);
--
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]