Oops the CI did catch errors, not sure why I did not see those tests locally my bad

Im not sure why but locally I can't get to trigger those errors, looking closer into it it seems to silently fail

Thank you! Looks good to me, I agree that this is much safer+saner behaviour than before. Just going to discuss with the rest of the team before merging.

If you have time, would you mind documenting this breaking change in the blog post/release notes for the upcoming v0.71 release? nushell/nushell.github.io#647

I think we generally like this change but it might be good to add some more tests. @sholderbach had some ideas/requests for testing, @sholderbach do you mind elaborating here?

I just had a thought… maybe not in this PR, but do you think you could add a similar flag for save, that if absent stops overwrites when it also doesn't have -a? I looked up the zsh NOCLOBBER setting, and not only does it protect mv, but also piping to a file (Nushell equivalent being save).

Nice! @rgwood , yes I can PR the change in the 0.71 release branch, I guess this should be listed in "breaking changes" ?

also piping to a file (Nushell equivalent being save).

This sound like something I can handle! Can you open a dedicated issue ?

I like the change and being more defensive is definitely a good default. GNU coreutils mv file fileexists will allow overwriting regular files but they don't seem to document their rules publicly. (The comments in #6920 hint to me that after landing this change we might still want to provide the people who want to live dangerous with a config option to go silent in some common cases)

The only question for testing I would have, are we safely prohibiting overwriting non-empty directories after this change? Generally I think in this case for testing it is more important that we test, that we don't cause unintended data loss for the users. I think for files you got us covered.

Nice!
I'll check this case and if it's needs an extra test later today!

After trying to come up with a new test I'm actually unsure what you meant Sholder?
mv to an existing folder moves it inside it, I'm not sure of the case you had in mind.

@melMass you are correct, all other cases I was wondering about (moving a folder on a file etc.) are covered by tests and protected.
So I am happy to merge it and get ready for user feedback.
Would you mind adding the breaking change to the release notes branch of our documentation/blog?
https://github.com/nushell/nushell.github.io/blob/release-notes-0.71/blog/2022-11-08-nushell-0.71.md

Would you mind adding the breaking change to the release notes branch of our documentation/blog?

@sholderbach I usually leave that out to better PR people 😅 as english is not my main language, but I drafted it here:
https://github.com/nushell/nushell.github.io/pull/653/files

Thanks! Looks perfectly understandable! and people on the internet will be happy to correct any mistakes :P

"saner" default
This is not a "standard" expectation users that want this behavior

I'm a bit confused by this PR. UNIX and Linux behavior has always been for commands such as mv and cp to overwrite destination files with the same name.

Every zsh guide I've seen tells you to turn the NOCLOBBER setting on (which is the equivalent to this) so if that isn't demand, I don't know what is.

For people who've been using Unix/Linux for a long time, this is very unexpected behavior. There is another argument against this default: for people whose jobs require them to use command-line interfaces in critical production contexts, it is important that they do NOT get accustomed to NOCLOBBER behavior.

As @sholderbach mentioned above, I think we really want a config option controlling this, since it's a bit awkward in Nushell currently to override a default while retaining control over the rest of the command, i.e. there's no way to do the equivalent of

def mv(**kwargs):
    stdlib.mv(force=True, **kwargs)

Well, rm does have those two flags and one config setting that control trashing vs. erasing… so it's not unprecedented…

Also we should bring cp into line with mv -- currently cp clobbers by default and lacks a --force option.

Well, rm does have those two flags and one config setting that control trashing vs. erasing

I didn't quite follow what you were saying here.

Y'know, this:

  rm: {
    always_trash: true # always act as if -t was given. Can be overridden with -p
  }