{"thread":{"id":"29134","subject":"git 1.7.7.3: BUG - please make git mv -f quiet","startedAt":"2011-12-11T21:22:37Z","lastAt":"2011-12-12T21:54:42Z","messageCount":16,"participants":["Jari Aalto","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"180835","messageId":"877h226bxe.fsf@picasso.cante.net","threadId":"29134","inReplyTo":null,"subject":"git 1.7.7.3: BUG - please make git mv -f quiet","fromName":"Jari Aalto","fromEmail":"jari.aalto@cante.net","sentAt":"2011-12-11T21:22:37Z","receivedAt":"2011-12-11T21:22:37Z","isPatch":false,"sender":{"key":"jari.aalto@cante.net","avatar":"https://avatars.githubusercontent.com/u/34601?v=4"},"body":"\nEvery time I do:\n\n    git mv -f FROM TO\n\nGit displays:\n\n    warning: destination exists; will overwrite!\n\nPlease don't display anything other than errors (no write permission....).\n\nThe \"-f\" is like with mv(1), cp(1); there is nothing than can be done\nafterwards, so the message is redundant and obstructing.\n\nIf messages are needed, please add option:\n\n    -v, --verbose\n\nJari\n"},{"id":"180896","messageId":"20111212074503.GB16511@sigill.intra.peff.net","threadId":"29134","inReplyTo":"877h226bxe.fsf@picasso.cante.net","subject":"[PATCH 0/5] mixed bag of minor \"git mv\" fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T07:45:03Z","receivedAt":"2011-12-12T07:45:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 11, 2011 at 11:22:37PM +0200, Jari Aalto wrote:\n\n> Every time I do:\n> \n>     git mv -f FROM TO\n> \n> Git displays:\n> \n>     warning: destination exists; will overwrite!\n> \n> Please don't display anything other than errors (no write permission....).\n> \n> The \"-f\" is like with mv(1), cp(1); there is nothing than can be done\n> afterwards, so the message is redundant and obstructing.\n\nI'm inclined to agree. Outputting a warning just because we did what the\nuser asked us to is unnecessarily chatty.\n\nWhen I looked into it, though, it seems that \"git mv\" is somewhat\nneglected, and this trival one-line patch turned into a 5-patch series\nof fixes.\n\n  [1/5]: docs: mention \"-k\" for both forms of \"git mv\"\n  [2/5]: mv: honor --verbose flag\n  [3/5]: mv: make non-directory destination error more clear\n  [4/5]: mv: improve overwrite warning\n  [5/5]: mv: be quiet about overwriting\n\n-Peff\n"},{"id":"180898","messageId":"20111212075031.GA17532@sigill.intra.peff.net","threadId":"29134","inReplyTo":"20111212074503.GB16511@sigill.intra.peff.net","subject":"[PATCH 1/5] docs: mention \"-k\" for both forms of \"git mv\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T07:50:31Z","receivedAt":"2011-12-12T07:50:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The \"git mv\" synopsis shows two forms: renaming a file, and\nmoving files into a directory. They can both make use of the\n\"-k\" flag to ignore errors, so mention it in both places.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI can kind of see the rationale for the original content. Using \"-k\" is\na lot more useful if you are actually doing multiple renames, so it\nmakes more sense in the second form. But it is still useful in the first\nform as a shorthand for \"git mv 2>/dev/null || true\".\n\nI actually would rather just see:\n\n  git mv [options] <source> <destination>\n  git mv [options] <source>... <destination>\n\nbut if we are going to go that route, we should probably decide on a\nstyle and convert all of the descriptions at the same time.\n\n Documentation/git-mv.txt |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt\nindex b8db373..4be7a71 100644\n--- a/Documentation/git-mv.txt\n+++ b/Documentation/git-mv.txt\n@@ -15,7 +15,7 @@ DESCRIPTION\n -----------\n This script is used to move or rename a file, directory or symlink.\n \n- git mv [-f] [-n] <source> <destination>\n+ git mv [-f] [-n] [-k] <source> <destination>\n  git mv [-f] [-n] [-k] <source> ... <destination directory>\n \n In the first form, it renames <source>, which must exist and be either\n-- \n1.7.8.13.g74677\n"},{"id":"180899","messageId":"20111212075124.GB17532@sigill.intra.peff.net","threadId":"29134","inReplyTo":"20111212074503.GB16511@sigill.intra.peff.net","subject":"[PATCH 2/5] mv: honor --verbose flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T07:51:24Z","receivedAt":"2011-12-12T07:51:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The code for a verbose flag has been here since \"git mv\" was\nconverted to C many years ago, but actually getting the \"-v\"\nflag from the command line was accidentally lost in the\ntransition.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis has been broken since 2006, so I guess nobody really cares. But\nit's simple to fix.\n\n Documentation/git-mv.txt |    8 ++++++--\n builtin/mv.c             |    1 +\n 2 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt\nindex 4be7a71..e3c8448 100644\n--- a/Documentation/git-mv.txt\n+++ b/Documentation/git-mv.txt\n@@ -15,8 +15,8 @@ DESCRIPTION\n -----------\n This script is used to move or rename a file, directory or symlink.\n \n- git mv [-f] [-n] [-k] <source> <destination>\n- git mv [-f] [-n] [-k] <source> ... <destination directory>\n+ git mv [-v] [-f] [-n] [-k] <source> <destination>\n+ git mv [-v] [-f] [-n] [-k] <source> ... <destination directory>\n \n In the first form, it renames <source>, which must exist and be either\n a file, symlink or directory, to <destination>.\n@@ -40,6 +40,10 @@ OPTIONS\n --dry-run::\n \tDo nothing; only show what would happen\n \n+-v::\n+--verbose::\n+\tReport the names of files as they are moved.\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 5efe6c5..11abaf5 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -59,6 +59,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \tint i, newfd;\n \tint verbose = 0, show_only = 0, force = 0, ignore_errors = 0;\n \tstruct option builtin_mv_options[] = {\n+\t\tOPT__VERBOSE(&verbose, \"be verbose\"),\n \t\tOPT__DRY_RUN(&show_only, \"dry run\"),\n \t\tOPT__FORCE(&force, \"force move/rename even if target exists\"),\n \t\tOPT_BOOLEAN('k', NULL, &ignore_errors, \"skip move/rename errors\"),\n-- \n1.7.8.13.g74677\n"},{"id":"180900","messageId":"20111212075136.GC17532@sigill.intra.peff.net","threadId":"29134","inReplyTo":"20111212074503.GB16511@sigill.intra.peff.net","subject":"[PATCH 3/5] mv: make non-directory destination error more clear","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T07:51:36Z","receivedAt":"2011-12-12T07:51:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If you try to \"git mv\" multiple files onto another\nnon-directory file, you confusingly get the \"usage\" message:\n\n  $ touch one two three\n  $ git add .\n  $ git mv one two three\n  usage: git mv [options] <source>... <destination>\n  [...]\n\n>From the user's perspective, that makes no sense. They just\ngave parameters that exactly match that usage!\n\nThis behavior dates back to the original C version of \"git\nmv\", which had a usage message like:\n\n  usage: git mv (<source> <destination> | <source>...  <destination>)\n\nThis was slightly less confusing, because it at least\nmentions that there are two ways to invoke (but it still\nisn't clear why what the user provided doesn't work).\n\nInstead, let's show an error message like:\n\n  $ git mv one two three\n  fatal: destination 'three' is not a directory\n\nWe could leave the usage message in place, too, but it\ndoesn't actually help here. It contains no hints that there\nare two forms, nor that multi-file form requires that the\nendpoint be a directory. So it just becomes useless noise\nthat distracts from the real error.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/mv.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 11abaf5..ae6c30c 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -94,7 +94,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\tdestination = copy_pathspec(dest_path[0], argv, argc, 1);\n \t} else {\n \t\tif (argc != 1)\n-\t\t\tusage_with_options(builtin_mv_usage, builtin_mv_options);\n+\t\t\tdie(\"destination '%s' is not a directory\", dest_path[0]);\n \t\tdestination = dest_path;\n \t}\n \n-- \n1.7.8.13.g74677\n"},{"id":"180901","messageId":"20111212075227.GD17532@sigill.intra.peff.net","threadId":"29134","inReplyTo":"20111212074503.GB16511@sigill.intra.peff.net","subject":"[PATCH 4/5] mv: improve overwrite warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T07:52:27Z","receivedAt":"2011-12-12T07:52:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we try to \"git mv\" over an existing file, the error\nmessage is fairly informative:\n\n  $ git mv one two\n  fatal: destination exists, source=one, destination=two\n\nWhen the user forces the overwrite, we give a warning:\n\n  $ git mv -f one two\n  warning: destination exists; will overwrite!\n\nThis is less informative, but still sufficient in the simple\nrename case, as there is only one rename happening.\n\nBut when moving files from one directory to another, it\nbecomes useless:\n\n  $ mkdir three\n  $ touch one two three/one\n  $ git add .\n  $ git mv one two three\n  fatal: destination exists, source=one, destination=three/one\n  $ git mv -f one two three\n  warning: destination exists; will overwrite!\n\nThe first message is helpful, but the second one gives us no\nclue about what was overwritten. Instead, let's mirror the\nfirst form more closely, with:\n\n  $ git mv -f one two three\n  warning: destination exists (will overwrite), source=one, destination=three/one\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis message looks overly long to me, but I wanted to match the existing\nmessages. Another option would be just:\n\n  warning: overwriting 'three/one'\n\n builtin/mv.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex ae6c30c..c9ecb03 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -177,7 +177,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\t\t * check both source and destination\n \t\t\t\t */\n \t\t\t\tif (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {\n-\t\t\t\t\twarning(_(\"%s; will overwrite!\"), bad);\n+\t\t\t\t\twarning(_(\"%s (will overwrite), source=%s, destination=%s\"),\n+\t\t\t\t\t\tbad, src, dst);\n \t\t\t\t\tbad = NULL;\n \t\t\t\t} else\n \t\t\t\t\tbad = _(\"Cannot overwrite\");\n-- \n1.7.8.13.g74677\n"},{"id":"180902","messageId":"20111212075407.GE17532@sigill.intra.peff.net","threadId":"29134","inReplyTo":"20111212074503.GB16511@sigill.intra.peff.net","subject":"[PATCH 5/5] mv: be quiet about overwriting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T07:54:07Z","receivedAt":"2011-12-12T07:54:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When a user asks us to force a mv and overwrite the\ndestination, we print a warning. However, since a typical\nuse would be:\n\n  $ git mv one two\n  fatal: destination exists, source=one, destination=two\n  $ git mv -f one two\n  warning: destination exists (will overwrite), source=one, destination=two\n\nthis warning is just noise. We already know we're\noverwriting; that's why we gave -f!\n\nThis patch silences the warning unless \"--verbose\" is given.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nYou could perhaps argue that it is useful in the case of moving multiple\nfiles into a directory (since it tells you _which_ files were\noverwritten). We could turn the warning on in that case, but I'm\ninclined to leave it. If the user cares about this information, they can\nuse \"-v\" along with \"-f\".\n\n builtin/mv.c |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex c9ecb03..b6e7e4f 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -177,8 +177,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\t\t * check both source and destination\n \t\t\t\t */\n \t\t\t\tif (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {\n-\t\t\t\t\twarning(_(\"%s (will overwrite), source=%s, destination=%s\"),\n-\t\t\t\t\t\tbad, src, dst);\n+\t\t\t\t\tif (verbose)\n+\t\t\t\t\t\twarning(_(\"%s (will overwrite), source=%s, destination=%s\"),\n+\t\t\t\t\t\t\tbad, src, dst);\n \t\t\t\t\tbad = NULL;\n \t\t\t\t} else\n \t\t\t\t\tbad = _(\"Cannot overwrite\");\n-- \n1.7.8.13.g74677\n"},{"id":"180957","messageId":"7v1us94lg7.fsf@alter.siamese.dyndns.org","threadId":"29134","inReplyTo":"20111212075031.GA17532@sigill.intra.peff.net","subject":"Re: [PATCH 1/5] docs: mention \"-k\" for both forms of \"git mv\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-12T19:52:08Z","receivedAt":"2011-12-12T19:52:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I actually would rather just see:\n>\n>   git mv [options] <source> <destination>\n>   git mv [options] <source>... <destination>\n>\n> but if we are going to go that route, we should probably decide on a\n> style and convert all of the descriptions at the same time.\n\nAlso most commands when they use these multi-line synopsis style to\ndifferenciate different invocation contexts do take different set of\noptions for different contexts, so we would need to update the option\ndescriptions to say \"this option only makes sense in this context\", etc.\n\n>  Documentation/git-mv.txt |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt\n> index b8db373..4be7a71 100644\n> --- a/Documentation/git-mv.txt\n> +++ b/Documentation/git-mv.txt\n> @@ -15,7 +15,7 @@ DESCRIPTION\n>  -----------\n>  This script is used to move or rename a file, directory or symlink.\n>  \n> - git mv [-f] [-n] <source> <destination>\n> + git mv [-f] [-n] [-k] <source> <destination>\n>   git mv [-f] [-n] [-k] <source> ... <destination directory>\n>  \n>  In the first form, it renames <source>, which must exist and be either\n"},{"id":"180958","messageId":"7vwra136tj.fsf@alter.siamese.dyndns.org","threadId":"29134","inReplyTo":"20111212075124.GB17532@sigill.intra.peff.net","subject":"Re: [PATCH 2/5] mv: honor --verbose flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-12T19:53:28Z","receivedAt":"2011-12-12T19:53:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> The code for a verbose flag has been here since \"git mv\" was\n> converted to C many years ago, but actually getting the \"-v\"\n> flag from the command line was accidentally lost in the\n> transition.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> This has been broken since 2006, so I guess nobody really cares. But\n> it's simple to fix.\n\nHeh. It means nobody exercised the codepaths that are inside \"if (verbose)\",\nso it may uncover old bugs, no?\n\n>  Documentation/git-mv.txt |    8 ++++++--\n>  builtin/mv.c             |    1 +\n>  2 files changed, 7 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt\n> index 4be7a71..e3c8448 100644\n> --- a/Documentation/git-mv.txt\n> +++ b/Documentation/git-mv.txt\n> @@ -15,8 +15,8 @@ DESCRIPTION\n>  -----------\n>  This script is used to move or rename a file, directory or symlink.\n>  \n> - git mv [-f] [-n] [-k] <source> <destination>\n> - git mv [-f] [-n] [-k] <source> ... <destination directory>\n> + git mv [-v] [-f] [-n] [-k] <source> <destination>\n> + git mv [-v] [-f] [-n] [-k] <source> ... <destination directory>\n>  \n>  In the first form, it renames <source>, which must exist and be either\n>  a file, symlink or directory, to <destination>.\n> @@ -40,6 +40,10 @@ OPTIONS\n>  --dry-run::\n>  \tDo nothing; only show what would happen\n>  \n> +-v::\n> +--verbose::\n> +\tReport the names of files as they are moved.\n> +\n>  GIT\n>  ---\n>  Part of the linkgit:git[1] suite\n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 5efe6c5..11abaf5 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -59,6 +59,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n>  \tint i, newfd;\n>  \tint verbose = 0, show_only = 0, force = 0, ignore_errors = 0;\n>  \tstruct option builtin_mv_options[] = {\n> +\t\tOPT__VERBOSE(&verbose, \"be verbose\"),\n>  \t\tOPT__DRY_RUN(&show_only, \"dry run\"),\n>  \t\tOPT__FORCE(&force, \"force move/rename even if target exists\"),\n>  \t\tOPT_BOOLEAN('k', NULL, &ignore_errors, \"skip move/rename errors\"),\n"},{"id":"180959","messageId":"7vsjkp36pt.fsf@alter.siamese.dyndns.org","threadId":"29134","inReplyTo":"20111212075136.GC17532@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] mv: make non-directory destination error more clear","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-12T19:55:42Z","receivedAt":"2011-12-12T19:55:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Instead, let's show an error message like:\n>\n>   $ git mv one two three\n>   fatal: destination 'three' is not a directory\n\nMakes perfect sense.\n\n> We could leave the usage message in place, too, but it\n> doesn't actually help here. It contains no hints that there\n> are two forms, nor that multi-file form requires that the\n> endpoint be a directory. So it just becomes useless noise\n> that distracts from the real error.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  builtin/mv.c |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 11abaf5..ae6c30c 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -94,7 +94,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n>  \t\tdestination = copy_pathspec(dest_path[0], argv, argc, 1);\n>  \t} else {\n>  \t\tif (argc != 1)\n> -\t\t\tusage_with_options(builtin_mv_usage, builtin_mv_options);\n> +\t\t\tdie(\"destination '%s' is not a directory\", dest_path[0]);\n>  \t\tdestination = dest_path;\n>  \t}\n"},{"id":"180960","messageId":"7vobvd36ms.fsf@alter.siamese.dyndns.org","threadId":"29134","inReplyTo":"20111212075227.GD17532@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] mv: improve overwrite warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-12T19:57:31Z","receivedAt":"2011-12-12T19:57:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> This message looks overly long to me, but I wanted to match the existing\n> messages. Another option would be just:\n>\n>   warning: overwriting 'three/one'\n\nYes, I think it makes perfect sense to drop the ugly \"source=one destination=two\"\ncruft, both for single-source and multiple-source cases.\n\n>  builtin/mv.c |    3 ++-\n>  1 files changed, 2 insertions(+), 1 deletions(-)\n>\n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index ae6c30c..c9ecb03 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -177,7 +177,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n>  \t\t\t\t * check both source and destination\n>  \t\t\t\t */\n>  \t\t\t\tif (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {\n> -\t\t\t\t\twarning(_(\"%s; will overwrite!\"), bad);\n> +\t\t\t\t\twarning(_(\"%s (will overwrite), source=%s, destination=%s\"),\n> +\t\t\t\t\t\tbad, src, dst);\n>  \t\t\t\t\tbad = NULL;\n>  \t\t\t\t} else\n>  \t\t\t\t\tbad = _(\"Cannot overwrite\");\n"},{"id":"180961","messageId":"7vk46136iv.fsf@alter.siamese.dyndns.org","threadId":"29134","inReplyTo":"20111212075407.GE17532@sigill.intra.peff.net","subject":"Re: [PATCH 5/5] mv: be quiet about overwriting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-12T19:59:52Z","receivedAt":"2011-12-12T19:59:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> When a user asks us to force a mv and overwrite the\n> destination, we print a warning. However, since a typical\n> use would be:\n>\n>   $ git mv one two\n>   fatal: destination exists, source=one, destination=two\n>   $ git mv -f one two\n>   warning: destination exists (will overwrite), source=one, destination=two\n>\n> this warning is just noise. We already know we're\n> overwriting; that's why we gave -f!\n>\n> This patch silences the warning unless \"--verbose\" is given.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> You could perhaps argue that it is useful in the case of moving multiple\n> files into a directory (since it tells you _which_ files were\n> overwritten). We could turn the warning on in that case, but I'm\n> inclined to leave it. If the user cares about this information, they can\n> use \"-v\" along with \"-f\".\n\nMakes sense, but I also think even under verbose mode we should avoid the\nfuture tense.  I.e. something like this:\n\n    $ git mv -v -f one two\n    warning: overwriting two\n    $ git mv -v -f one two three\n    warning: overwriting three/one\n\n>  builtin/mv.c |    5 +++--\n>  1 files changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index c9ecb03..b6e7e4f 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -177,8 +177,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n>  \t\t\t\t * check both source and destination\n>  \t\t\t\t */\n>  \t\t\t\tif (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {\n> -\t\t\t\t\twarning(_(\"%s (will overwrite), source=%s, destination=%s\"),\n> -\t\t\t\t\t\tbad, src, dst);\n> +\t\t\t\t\tif (verbose)\n> +\t\t\t\t\t\twarning(_(\"%s (will overwrite), source=%s, destination=%s\"),\n> +\t\t\t\t\t\t\tbad, src, dst);\n>  \t\t\t\t\tbad = NULL;\n>  \t\t\t\t} else\n>  \t\t\t\t\tbad = _(\"Cannot overwrite\");\n"},{"id":"180975","messageId":"20111212214516.GC9754@sigill.intra.peff.net","threadId":"29134","inReplyTo":"7vwra136tj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/5] mv: honor --verbose flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T21:45:16Z","receivedAt":"2011-12-12T21:45:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 12, 2011 at 11:53:28AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > The code for a verbose flag has been here since \"git mv\" was\n> > converted to C many years ago, but actually getting the \"-v\"\n> > flag from the command line was accidentally lost in the\n> > transition.\n> >\n> > Signed-off-by: Jeff King <peff@peff.net>\n> > ---\n> > This has been broken since 2006, so I guess nobody really cares. But\n> > it's simple to fix.\n> \n> Heh. It means nobody exercised the codepaths that are inside \"if (verbose)\",\n> so it may uncover old bugs, no?\n\nI thought that at first, too, but actually there is only one code path\ncurrently enabled by \"verbose\", and it is to print \"Renaming ...\". You\ncan also exercise that code path with \"--dry-run\" (and the whole path\nconsists of only a single printf, so hopefully we didn't manage to\nsqueeze any bugs in there).\n\nOnce upon a time, the verbose flag was passed on to add_file_to_index,\nbut that was dropped when the code switched to using\nrename_index_entry_at in 81dc230 (git-mv: Keep moved index entries\ninact, 2008-07-21).\n\n-Peff\n"},{"id":"180977","messageId":"20111212215238.GD9754@sigill.intra.peff.net","threadId":"29134","inReplyTo":"7vobvd36ms.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/5] mv: improve overwrite warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T21:52:39Z","receivedAt":"2011-12-12T21:52:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 12, 2011 at 11:57:31AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > This message looks overly long to me, but I wanted to match the existing\n> > messages. Another option would be just:\n> >\n> >   warning: overwriting 'three/one'\n> \n> Yes, I think it makes perfect sense to drop the ugly \"source=one destination=two\"\n> cruft, both for single-source and multiple-source cases.\n\nYeah, the more I look at the message in the patch I sent, the uglier it\ngets. Here's a re-rolled 4 and 5 with the nicer format.\n\n-Peff\n"},{"id":"180978","messageId":"20111212215417.GA18310@sigill.intra.peff.net","threadId":"29134","inReplyTo":"20111212215238.GD9754@sigill.intra.peff.net","subject":"[PATCHv2 4/5] mv: improve overwrite warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T21:54:17Z","receivedAt":"2011-12-12T21:54:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we try to \"git mv\" over an existing file, the error\nmessage is fairly informative:\n\n  $ git mv one two\n  fatal: destination exists, source=one, destination=two\n\nWhen the user forces the overwrite, we give a warning:\n\n  $ git mv -f one two\n  warning: destination exists; will overwrite!\n\nThis is less informative, but still sufficient in the simple\nrename case, as there is only one rename happening.\n\nBut when moving files from one directory to another, it\nbecomes useless:\n\n  $ mkdir three\n  $ touch one two three/one\n  $ git add .\n  $ git mv one two three\n  fatal: destination exists, source=one, destination=three/one\n  $ git mv -f one two three\n  warning: destination exists; will overwrite!\n\nThe first message is helpful, but the second one gives us no\nclue about what was overwritten. Let's mention the name of\nthe destination file:\n\n  $ git mv -f one two three\n  warning: overwriting 'three/one'\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/mv.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex ae6c30c..8dd5a45 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -177,7 +177,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\t\t * check both source and destination\n \t\t\t\t */\n \t\t\t\tif (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {\n-\t\t\t\t\twarning(_(\"%s; will overwrite!\"), bad);\n+\t\t\t\t\twarning(_(\"overwriting '%s'\"), dst);\n \t\t\t\t\tbad = NULL;\n \t\t\t\t} else\n \t\t\t\t\tbad = _(\"Cannot overwrite\");\n-- \n1.7.8.13.g74677\n"},{"id":"180979","messageId":"20111212215442.GB18310@sigill.intra.peff.net","threadId":"29134","inReplyTo":"20111212215238.GD9754@sigill.intra.peff.net","subject":"[PATCHv2 5/5] mv: be quiet about overwriting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T21:54:42Z","receivedAt":"2011-12-12T21:54:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When a user asks us to force a mv and overwrite the\ndestination, we print a warning. However, since a typical\nuse would be:\n\n  $ git mv one two\n  fatal: destination exists, source=one, destination=two\n  $ git mv -f one two\n  warning: overwriting 'two'\n\nthis warning is just noise. We already know we're\noverwriting; that's why we gave -f!\n\nThis patch silences the warning unless \"--verbose\" is given.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/mv.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 8dd5a45..2a144b0 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -177,7 +177,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\t\t * check both source and destination\n \t\t\t\t */\n \t\t\t\tif (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {\n-\t\t\t\t\twarning(_(\"overwriting '%s'\"), dst);\n+\t\t\t\t\tif (verbose)\n+\t\t\t\t\t\twarning(_(\"overwriting '%s'\"), dst);\n \t\t\t\t\tbad = NULL;\n \t\t\t\t} else\n \t\t\t\t\tbad = _(\"Cannot overwrite\");\n-- \n1.7.8.13.g74677\n"}]}