Re: cp/mv: add -n option to not overwrite target (Ubuntu bug #229182)

[Jim Meyering]

Eric Blake <address@hidden> wrote:
> According to Jim Meyering on 1/7/2009 12:27 PM:
>>>> Anyone else have a preference?
>>> Now the option is --no-clobber in attached patch. Anyway the change is
>>> trivial. No problem to change it if there will be a consensus
>>> for --no-overwrite or something else.
>>>
>>
>> Thanks!  Almost done, I think.
>> Unless anyone pipes up about the choice of long option name.
>
> I like --no-clobber, particularly since it is similar to the bash (and
> tcsh) noclobber shell option.
>
>>
>> What do you think about making cp/mv fail if -n and --backup are used 
>> together?
>> Then there would be no risk of misunderstanding.
>
> Seems reasonable to me.
>
>> +++ b/NEWS
>> @@ -4,6 +4,9 @@ GNU coreutils NEWS
> -*- outline -*-
>>
>>  ** New features
>>
>> +  cp and mv accept a new option, --no-clobber (-n): silently refrain
>> +  from overwriting any existing destination file
>
> Missing period.

Thanks for the review.

>> +                                 opened, remove it and try again (The
> -f option\n\
>> +                                 is redundant if the -n option is used.)\n\
>> +  -i, --interactive            prompt before overwrite  (The -i option\n\
>> +                                 overrides a previous -n option.)\n\
>>    -H                           follow command-line symbolic links in
> SOURCE\n\
>>  "), stdout);
>>        fputs (_("\
>> @@ -176,6 +179,8 @@ Mandatory arguments to long options are mandatory
> for short options too.\n\
>>    -L, --dereference            always follow symbolic links in SOURCE\n\
>>  "), stdout);
>>        fputs (_("\
>> +  -n, --no-clobber             do not overwrite an existing file  (The
> -n\n\
>> +                                 option overrides a previous -i option.)\n\
>
> The formatting is not very consistent here (are these full sentences, with
> capitalization, periods, and two spaces, or just phrases?)

It's a little bit unusual to insert text into the option list,
but works for chown --help, so I suggest doing it here, too:

diff --git a/src/mv.c b/src/mv.c
index 635c4e0..d8ca50b 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -1,5 +1,5 @@
 /* mv -- move or rename files
-   Copyright (C) 86, 89, 90, 91, 1995-2008 Free Software Foundation, Inc.
+   Copyright (C) 86, 89, 90, 91, 1995-2009 Free Software Foundation, Inc.

    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -295,13 +295,10 @@ Mandatory arguments to long options are mandatory for 
short options too.\n\
       fputs (_("\
       --backup[=CONTROL]       make a backup of each existing destination 
file\n\
   -b                           like --backup but does not accept an argument\n\
-  -f, --force                  do not prompt before overwriting  (The -f 
option\n\
-                                 overrides any previous -i or -n options.)\n\
-  -i, --interactive            prompt before overwrite  (The -i option\n\
-                                 overrides any previous -f or -n options.)\n\
-  -n, --no-replace             do not overwrite an existing file  (The -n\n\
-                                 option overrides any previous -f or -i\n\
-                                 options.)\n\
+  -f, --force                  do not prompt before overwriting\n\
+  -i, --interactive            prompt before overwrite\n\
+  -n, --no-replace             do not overwrite an existing file\n\
+If you specify more than one of -i, -f, -n, only the final one takes effect.\n\
 "), stdout);
       fputs (_("\
       --strip-trailing-slashes  remove any trailing slashes from each SOURCE\n\

>> +    case 'n':
>> +      x.interactive = I_ALWAYS_NO;
>> +      break;
>
> Does this actually do the right thing, or does it only prevent overwrites
> at spots where, without arguments, a prompt would be issued?  In other
> words, is this particular implementation reinventing the same problems as
> the deprecated --reply=no?

Kamil's test additions appear to cover most cases.
Once -n --backup evokes failure, it'd be nice to exercise that, too.