Implemented chmod command. Github issue 35#48
Implemented chmod command. Github issue 35#48arturadib merged 5 commits intoshelljs:masterfrom brandonramirez:chmod
Conversation
…as no support for empty directories.
…anged to reduce the likelihood of umask issues.
shell.js
Outdated
There was a problem hiding this comment.
since these are not shared among other functions, can we move all constants to either inside _chmod or inside a global object - say PERMS or something? thanks!
|
Superb work Brandon! Have you tested this on Windows? I don't have a VM handy, but I'll try to get one and test it before merging if you haven't already. |
|
By the way, re:compatibility with Windows, we can only be as compliant as the Node API - it's OK to leave the normalization of platforms upstream |
|
I have not tested it on Windows. I'm not entirely sure what the correct I agree that even if it doesn't work on Windows, we should try to test it On Thu, Jan 3, 2013 at 11:00 AM, Artur Adib notifications@github.comwrote:
|
|
I agree with the PERM* constants. I wasn't happy about exposing them to the global namespace (well, module global), but didn't feel like spending the time to come up with a better way. I didn't want them to be in the method for encapsulation purposes. But I should have done better. I will put them into a global CHMOD_PERMS object to at least clean up the namespace. I'll get to that later this evening. |
|
nice, thanks! all merged |
Implemented chmod command. Github issue 35
I have attempted to mimic the behavior of POSIX chmod as much as possible. I used the following resources as references and documented any deviations from the POSIX standard.
http://www.unix.com/tips-tutorials/19060-unix-file-permissions.html (used as a spec for parsing symbolic modes like u+x).
http://www.kernel.org/doc/man-pages/online/pages/man2/chmod.2.html (documented bit positions)
http://linux.die.net/man/1/chmod (functionality and command-line options)
I have included appropriate unit tests and run the code through jshint.