[PATCH] checkout: refactor of --progress option

[PATCH] checkout: refactor of --progress option

[Edmundo Carmona Antoranz]

- removed static variable show_progress
- processing if progress will be shown or not right
  after running parse_options()

Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
 builtin/checkout.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e28c36b..10e2b42 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);
-- 
2.6.1

Re: [PATCH] checkout: refactor of --progress option

[Edmundo Carmona Antoranz]

On Sat, Oct 31, 2015 at 10:57 AM, Edmundo Carmona Antoranz
<eantoranz@gmail.com> wrote:
> +       /*
> +        * 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));
> +

I did a very small adjustment there from what I had sent yesterday.

It was (opts.show_progress >= 0 || isatty(2))

It was unnecessary to include the case of a 0 because it was discarded
by the second item on the outer short-circuited if:
opts_show_progress.

Let me know of your feedback and thanks.

Re: [PATCH] checkout: refactor of --progress option

[Edmundo Carmona Antoranz]

On Sat, Oct 31, 2015 at 11:10 AM, Edmundo Carmona Antoranz
<eantoranz@gmail.com> wrote:
> by the second item on the outer short-circuited if:

I meant: ...short-circuited _and operator_, sorry.

Re: [PATCH] checkout: refactor of --progress option

[Edmundo Carmona Antoranz]

Disregard this patch.