plasma-umass / browsix

Browsix is a Unix-like operating system for the browser.
Other
3.15k stars 181 forks source link

implement: cp #19

Closed anuragagarwal561994 closed 8 years ago

anuragagarwal561994 commented 8 years ago

The cases being covered here includes:

  1. If no argument -> err.
  2. If 1 argument -> err.
  3. For 2 arguments: a. If second argument is not a directory but contains a slash -> err or otherwise will create a new file b. If second argument is an already existing file -> overwrite
  4. It will always check if all the arguments except the last one is a file, if they are not it will print the correct error message.
  5. Last argument for 2 (except for the cases in 3) or more argument has to be a directory or it will report an error or otherwise it will copy the files correctly.

The inspiration of the implementation is taken from the core utilities of GNU.

bpowers commented 8 years ago

Hi @anuragagarwal561994, this is looking real good! I understand you're working on -R, so I'm going to leave this open. Additionally, it would be good to see a new test for cp in /test/test-cp.ts. To ensure that test is run, add a line for it to test/test-all.ts.

One additional thing, due to a limitation of our browser-node implementation, you need to explicitly call process.exit when done (which is why the finished callback is passed to the log function, otherwise the process will appear to hang even though it has successfully completed its work.

bpowers commented 8 years ago

(Addresses #13)

anuragagarwal561994 commented 8 years ago

So well there are still some of the edge cases left which I am going to cover in my next commit. Right now in case where 2 arguments are given and when the second path does not exist:

  1. It can possibly be either an argument not ending in '/' -> to be assumed as file
  2. It can be a assumed as a directory when there is a '/' in the end

It is taking both of the paths as file only.

bpowers commented 8 years ago

hi @anuragagarwal561994! The changes you made for browser-node look great. It sounds like you have an additional commit in-progress? If you have any questions, don't hesitate to reach out on there or IRC!

bpowers commented 8 years ago

this looks great, merging. We can add tests as a followup