Re: [PATCH 5/5] mv: be quiet about overwriting

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 12, 2011, 19:59 UTC
Message-ID: <7vk46136iv.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20111212075407.GE17532@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 21 quoted lines

> When a user asks us to force a mv and overwrite the
> destination, we print a warning. However, since a typical
> use would be:
>
>   $ git mv one two
>   fatal: destination exists, source=one, destination=two
>   $ git mv -f one two
>   warning: destination exists (will overwrite), source=one, destination=two
>
> this warning is just noise. We already know we're
> overwriting; that's why we gave -f!
>
> This patch silences the warning unless "--verbose" is given.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> You could perhaps argue that it is useful in the case of moving multiple
> files into a directory (since it tells you _which_ files were
> overwritten). We could turn the warning on in that case, but I'm
> inclined to leave it. If the user cares about this information, they can
> use "-v" along with "-f".

Makes sense, but I also think even under verbose mode we should avoid the future tense. I.e. something like this:

    $ git mv -v -f one two
    warning: overwriting two
    $ git mv -v -f one two three
    warning: overwriting three/one

Show 19 quoted lines

>  builtin/mv.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/mv.c b/builtin/mv.c
> index c9ecb03..b6e7e4f 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -177,8 +177,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  				 * check both source and destination
>  				 */
>  				if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
> -					warning(_("%s (will overwrite), source=%s, destination=%s"),
> -						bad, src, dst);
> +					if (verbose)
> +						warning(_("%s (will overwrite), source=%s, destination=%s"),
> +							bad, src, dst);
>  					bad = NULL;
>  				} else
>  					bad = _("Cannot overwrite");