Skip to content

Fix "mv allows moving a directory into itself"#2619

Merged
sophiajt merged 22 commits intonushell:mainfrom
luccasmmg:main
Oct 1, 2020
Merged

Fix "mv allows moving a directory into itself"#2619
sophiajt merged 22 commits intonushell:mainfrom
luccasmmg:main

Conversation

@luccasmmg
Copy link
Copy Markdown
Contributor

@luccasmmg luccasmmg commented Sep 30, 2020

Fixes #2612

Old behaviour allowed this, and just deleted the file

mkdir a
mkdir a/b
mv a a/b

New behaviour

mkdir a
mkdir a/b
mv a a/b
mv a a/b
^ cannot move "a" to itself

There was also a second problem related to the first, in other shells, if i try mv with more than one source, it just ignores the destination file. So that one file will not break the whole process. I decided to add this behavior. It now works like this.

mkdir foo bar fizz
mv * fizz
ls fizz
foo bar

I have two questions now
1 - Should i give some type of warning that some files will not be moved(just like the classic mv command), and if so how could i give this warning without stopping the process?
2 - I decided to make the "sources" variable mutable and use a filter to remove the error files, i could also do this checking in the for loop that moves the files(Line 501), but i thought this way would make the code less readable given that there would be a piece of code that does two different things at the same time. Was that the best decision?

@sophiajt
Copy link
Copy Markdown
Contributor

sophiajt commented Oct 1, 2020

Looks good!

@sophiajt sophiajt merged commit 6606119 into nushell:main Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mv allows moving a directory into itself

3 participants