Re: [PATCH/RFC] checkout: print something when checking out paths

[Junio C Hamano <gitster@pobox.com>]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Since the purpose of printing this is to help disambiguate. Only do it
> when "--" is missing (the actual reason though is many tests check
> empty stderr to see that no error is raised and I'm too lazy to fix
> all the test cases).

Heh, honesty is good but in this particular case the official reason
alone would make perfect sense, too ;-)

As with progress output, shouldn't this automatically be turned off
when the standard error stream goes to non tty, as the real purpose
of printing is to help the user sitting in front of the terminal and
interacting with the command?

And by this, I do not mean to say that "When -- is missing" can go
away.  I agree that "--" is a clear sign that the user knows what
s/he is doing---to overwrite the paths in the working tree that
match the pathspec.

> @@ -371,17 +374,27 @@ static int checkout_paths(const struct checkout_opts *opts,
>  		struct cache_entry *ce = active_cache[pos];
>  		if (ce->ce_flags & CE_MATCHED) {
>  			if (!ce_stage(ce)) {
> -				errs |= checkout_entry(ce, &state, NULL);
> +				errs |= checkout_entry(ce, &state,
> +						       NULL, &nr_checkouts);
>  				continue;

As we count inside checkout_entry(), we might not actually write
this out when we know that the working tree file is up to date
already but we do not increment in that case either, so we keep
track of the accurate count of "updates", not path matches (i.e. we
are not reporting "we made sure this many paths are up to date in
the working tree"; instead we are reporting how many paths we needed
to overwrite in the working tree).

>  			}
>  			if (opts->writeout_stage)
> -				errs |= checkout_stage(opts->writeout_stage, ce, pos, &state);
> +				errs |= checkout_stage(opts->writeout_stage,
> +						       ce, pos,
> +						       &state, &nr_checkouts);

Likewike.

>  			else if (opts->merge)
> -				errs |= checkout_merged(pos, &state);
> +				errs |= checkout_merged(pos, &state,
> +							&nr_checkouts);

Likewise.

>  			pos = skip_same_name(ce, pos) - 1;
>  		}
>  	}
> -	errs |= finish_delayed_checkout(&state);
> +	errs |= finish_delayed_checkout(&state, &nr_checkouts);
> +
> +	if (opts->count_checkout_paths)
> +		fprintf_ln(stderr, Q_("%d path has been updated",
> +				      "%d paths have been updated",
> +				      nr_checkouts),
> +			   nr_checkouts);

This one may want to be protected behind isatty(2).  Or the default
value of count_checkout_paths could be tweakedbased on isatty(2).

> @@ -1064,6 +1077,7 @@ static int parse_branchname_arg(int argc, const char **argv,
>  		has_dash_dash = 1; /* case (3) or (1) */
>  	else if (dash_dash_pos >= 2)
>  		die(_("only one reference expected, %d given."), dash_dash_pos);
> +	opts->count_checkout_paths = !opts->quiet && !has_dash_dash;

i.e. "&& isatty(2)" as well.

Of course, "--[no-]count-paths" could be added to override this.