ices-tools-prod / icesTAF

Functions to support the ICES Transparent Assessment Framework
GNU General Public License v3.0
5 stars 7 forks source link

cp function drops files #3

Closed einarhjorleifsson closed 6 years ago

einarhjorleifsson commented 6 years ago

function cp does not check if directory exists. results in file to "vanish" if argument move = TRUE. example:

cp(from = "README.md", to = "nonexistentfolder", move = TRUE)
arni-magnusson commented 6 years ago

Thanks for the feedback! The to argument can either be a filename (existing or non-existing) or a directory name (existing). With move=TRUE, the function becomes similar to the shell command (Linux mv, Windows move) to move/rename files. So in the example above, the README.md file doesn't vanish but is renamed to nonexistentfolder. That behavior in itself seems logical and harmless. All file commands can be vehicles in harmful accidents, but it's not clear whether the cp() function could or should be made safer. There is already an inherent check, and if the to argument is not an existing directory, it will act as a filename.

einarhjorleifsson commented 6 years ago

was thinking of:

writeLines(text = "txt1", con = "txt1.txt")
writeLines(text = "txt2", con = "txt2.txt")
icesTAF::cp(from = "*.txt", to = "existentfolder", move = TRUE)

writeLines(text = "txt1", con = "txt1.txt")
writeLines(text = "txt2", con = "txt2.txt")
icesTAF::cp(from = "*.txt", to = "nonexistentfolder", move = TRUE)

e

arni-magnusson commented 6 years ago

This is true, losing files sucks - let's add some seatbelts.

To prevent accidental loss of files, two safeguards are now enforced when move=TRUE:

  1. When moving files, the to argument must either have a filename extension or be an existing directory.
  2. When moving many files to one destination, the to argument must be an existing directory.

If these conditions do not hold, no files are changed and an error is returned.

See commit b49a727.