Re: [PATCH 4/5] mv: improve overwrite warning

Jeff King [off-list ref] writes:

This message looks overly long to me, but I wanted to match the existing
messages. Another option would be just:

  warning: overwriting 'three/one'

Yes, I think it makes perfect sense to drop the ugly "source=one destination=two"
cruft, both for single-source and multiple-source cases.

On Mon, Dec 12, 2011 at 11:57:31AM -0800, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

This message looks overly long to me, but I wanted to match the existing
messages. Another option would be just:

  warning: overwriting 'three/one'

Yes, I think it makes perfect sense to drop the ugly "source=one destination=two"
cruft, both for single-source and multiple-source cases.

Yeah, the more I look at the message in the patch I sent, the uglier it
gets. Here's a re-rolled 4 and 5 with the nicer format.

-Peff

When we try to "git mv" over an existing file, the error
message is fairly informative:

  $ git mv one two
  fatal: destination exists, source=one, destination=two

When the user forces the overwrite, we give a warning:

  $ git mv -f one two
  warning: destination exists; will overwrite!

This is less informative, but still sufficient in the simple
rename case, as there is only one rename happening.

But when moving files from one directory to another, it
becomes useless:

  $ mkdir three
  $ touch one two three/one
  $ git add .
  $ git mv one two three
  fatal: destination exists, source=one, destination=three/one
  $ git mv -f one two three
  warning: destination exists; will overwrite!

The first message is helpful, but the second one gives us no
clue about what was overwritten. Let's mention the name of
the destination file:

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

Signed-off-by: Jeff King <redacted>
---
 builtin/mv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index ae6c30c..8dd5a45 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c

@@ -177,7 +177,7 @@ 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!"), bad);
+					warning(_("overwriting '%s'"), dst);
 					bad = NULL;
 				} else
 					bad = _("Cannot overwrite");

-- 
1.7.8.13.g74677

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: overwriting '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 <redacted>
---
 builtin/mv.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 8dd5a45..2a144b0 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c

@@ -177,7 +177,8 @@ 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(_("overwriting '%s'"), dst);
+					if (verbose)
+						warning(_("overwriting '%s'"), dst);
 					bad = NULL;
 				} else
 					bad = _("Cannot overwrite");

-- 
1.7.8.13.g74677