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

Exposes Path.expand_dot/1 as a public function #6486

Closed
wants to merge 1 commit into from

Conversation

fredwu
Copy link
Contributor

@fredwu fredwu commented Aug 20, 2017

Hi all,

I think Path.expand_dot/1 is a very useful function for resolving paths, without needing to worry about the added path manipulation from Path.expand/1-2.

This PR merely makes the function public, with no change whatsoever to its implementation.

What do you think?

@josevalim
Copy link
Member

The issue is that expanding only the dots without the full path can lead to incorrect results. Even after full expansion it can be an issue. Do you have an use case for this?

@fredwu
Copy link
Contributor Author

fredwu commented Aug 20, 2017

@josevalim That's true, hence I've added in the doc that this function does not take into account the absolute or relative nature of the path itself.

The use case for me is to resolve a relative link (link B) in a given page path (Link A):

Link A: foo/bar
Link B: ../baz

And I'm expecting foo/baz to be the correct output.

I intend to use it to resolve paths in my web crawler: https://github.com/fredwu/crawler/blob/e19de1500485c2808a42545be8f8d6ba1bbfbacd/lib/crawler/linker/path_builder.ex (it would replace the current use of path |> Path.expand |> Path.relative_to(cwd).

fredwu added a commit to fredwu/crawler that referenced this pull request Aug 20, 2017
It’s copy-pasted from Elixir’s stdlib for now, with a PR (elixir-lang/elixir#6486)
@josevalim
Copy link
Member

To be very honest, I don't think you should use the functions in Path to handle non-file system paths because we will always make assumptions based on those. I would say it is preferrable for you to go with something specific for URIs then.

@fredwu
Copy link
Contributor Author

fredwu commented Aug 20, 2017

It's a tricky one as the use case for me isn't strictly URI-related either. A lot of the resolving are needed to translate URI into relative paths that are usable offline, the resolving bit therefore is related to the file-system in this case, but in a "controlled environment" where not expanding it to absolute path still gives correct result.

For now I've lifted the expand_dot/1 function so I could use it in my lib. It would be nice for this helpful function to be available though. 😅

@fredwu
Copy link
Contributor Author

fredwu commented Aug 20, 2017

Thanks @josevalim! I'll close this PR now. :)

@fredwu fredwu closed this Aug 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants