filerjs / filer

Node-like file system for browsers
BSD 2-Clause "Simplified" License
617 stars 154 forks source link

Add find_symlink_node() to allow l* methods (lstat, lchown, etc) to use find_node like functionality #749

Open andrewkoung opened 5 years ago

andrewkoung commented 5 years ago

Currently the find_node() method in implementation.js will attempt to dereference to find the root node. It would be a good idea to have an option to not dereference for situations where you need to work on symlinks.

modeswitch commented 5 years ago

Can you give an example of why you would want to do this?

andrewkoung commented 5 years ago

Created this issue on behalf of @humphd, but I'm currently trying to implement lchown(). Though I'm trying to get the metadata for a symbolic link, currently the find_node() method will try to de-reference and get the metadata for a regular link.

modeswitch commented 5 years ago

My preference is to add a new function find_symlink_node instead of adding logic to find_node. Adding new functionality to find_node will make it more difficult to comprehend (it's already difficult) and more difficult to reason about when debugging. In this case I think two comprehensible functions is better than one function that tries to be too clever. You also have the benefit of making your improvements without disturbing other code that relies on find_node.

Have a look at the code for lstat; It also operates on symlinks. You can probably lift that logic out into file_symlink_node and rewrite lstat to use it as well.

humphd commented 5 years ago

Good idea. Morphed this to be about adding a new function for the non-dereferencing case.