-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add copy/move commands #39
Conversation
The deno std join doesn't join absolute path with common prefix correctly, I think rust one is more intuitive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Just a few small comments.
src/common.ts
Outdated
rustjoin("/a/b","/a/c") => "/a/b/c" | ||
|
||
instead of: | ||
Deno.path.join("/a/b","/a/c") => "/a/b/a/c" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ummm... weird. Seems like Deno is copying what node does with fs.path
.
src/commands/cp_mv.ts
Outdated
// They only exists to give better error messages. | ||
if (await isDir(from.path)) { | ||
if (flags.recursive) { | ||
if (await fs.exists(to.path) && await isFile(to.path)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of calling fs.exists
here, we could just call Deno.lstat
once.
I have to head out soon, so I'll probably look at this more later tonight or tomorrow. Thanks a lot for all the PRs! |
Sure thanks as well! |
I'm updating this PR just now with main. Give me a few minutes.... |
No description provided.