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

Attach symlink property to Vinyl objects when passed through symlink method #249

Closed
phated opened this issue Jun 23, 2017 · 6 comments
Closed

Comments

@phated
Copy link
Member

phated commented Jun 23, 2017

As per @erikkemperman's comments in #246, we should be attaching the .symlink property in other scenarios (other than just src(..., { resolveSymlinks: false })). The most obvious of these is when the file passes through the symlink method and a symlink is created (I think this can be done in symlink's prepare stream to reduce some path history hackiness).

@erikkemperman
Copy link
Member

That's exactly what I'd done earlier but never got around to submitting: attach symlink property before changing file.path in prepare, and then use that instead of file.history[-2].

@phated
Copy link
Member Author

phated commented Jun 23, 2017

Feel free to submit it if you get a chance. I'm not tackling this currently.

@erikkemperman
Copy link
Member

Will do, as soon as I find some time!

@erikkemperman
Copy link
Member

@phated I think the file.symlink property should be absolute, like file.path, even for relative links. Does that sound right?

@erikkemperman
Copy link
Member

Hm on second thought I'm not sure: src just attaches the result of fs.readLink for symlinks, which might be relative.

erikkemperman added a commit to erikkemperman/vinyl-fs that referenced this issue Jun 23, 2017
erikkemperman added a commit to erikkemperman/vinyl-fs that referenced this issue Jun 23, 2017
erikkemperman added a commit to erikkemperman/vinyl-fs that referenced this issue Jun 24, 2017
@phated phated closed this as completed in a39aeb8 Jun 26, 2017
@erikkemperman
Copy link
Member

Thanks for review and merge! Sorry about the bogus refs above, guess I shouldn't include issue ids until the commit is final.

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

No branches or pull requests

2 participants