Skip to content
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

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

Open
andrewkoung opened this issue Apr 5, 2019 · 4 comments

Comments

@andrewkoung
Copy link
Contributor

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
Copy link
Member

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

@andrewkoung
Copy link
Contributor Author

andrewkoung commented Apr 6, 2019

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
Copy link
Member

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 humphd changed the title Refactor find_node() to have a parameter to make deferencing optional Add find_symlink_node() to allow l* methods (lstat, lchown, etc) to use find_node like functionality Apr 7, 2019
@humphd
Copy link
Contributor

humphd commented Apr 7, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants