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

feat: add withSymlinks argument back #104

Merged
merged 4 commits into from
Aug 25, 2024

Conversation

SuperchupuDev
Copy link
Contributor

While working on tinyglobby I've noticed there is no way to disable fdir returning the real path of a symlink anymore, due to #81 silently removing support for it (even though the documentation still mentions the feature!).

This PR adds it back, inside an object parameter instead for better API design (see #83 (comment)). It defaults to true for backwards compatibility

Closes #103

@SuperchupuDev
Copy link
Contributor Author

SuperchupuDev commented Aug 5, 2024

hi @thecodrr, any chance you can take a look at this anytime soon? symlinks wont quite work in my library until this (or an equivalent solution) is merged

@thecodrr
Copy link
Owner

thecodrr commented Aug 5, 2024

Relevant commit: cdb201f

@SuperchupuDev
Copy link
Contributor Author

yes, that commit is what this PR aims to implement, but #81 removed the withSymlinks parameter that commit introduced. i can rename this pr to make that clearer

@SuperchupuDev SuperchupuDev changed the title feat: add useRealPaths back feat: add withSymlinks argument back Aug 5, 2024
Copy link
Owner

@thecodrr thecodrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for adding this back!

src/api/functions/resolve-symlink.ts Show resolved Hide resolved
src/api/functions/resolve-symlink.ts Show resolved Hide resolved
src/api/functions/resolve-symlink.ts Outdated Show resolved Hide resolved
SuperchupuDev and others added 2 commits August 5, 2024 17:20
Co-authored-by: Abdullah Atta <thecodrr@protonmail.com>
@SuperchupuDev
Copy link
Contributor Author

SuperchupuDev commented Aug 5, 2024

changing it to lstat makes this test fail as it makes it return non-resolved symlinks
image

@SuperchupuDev
Copy link
Contributor Author

@thecodrr i've reverted the lstat change, it makes behavior the same as if not using withSymlinks at all for some reason. not to worry though as it seems to work as intended with stat

@SuperchupuDev
Copy link
Contributor Author

@thecodrr is there anything blocking the PR?

@thecodrr thecodrr merged commit f425d11 into thecodrr:master Aug 25, 2024
11 checks passed
@SuperchupuDev SuperchupuDev deleted the feat/use-real-paths branch August 25, 2024 09:38
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

Successfully merging this pull request may close these issues.

[Feature] add back crawling by symlink path, not real path
2 participants