Skip to content

fix: handle root in join and directory_name - #15

Open
JackoCoolio wants to merge 1 commit into
lpil:mainfrom
JackoCoolio:fix/join-with-root
Open

fix: handle root in join and directory_name#15
JackoCoolio wants to merge 1 commit into
lpil:mainfrom
JackoCoolio:fix/join-with-root

Conversation

@JackoCoolio

Copy link
Copy Markdown

Fixes #9.

Previously, joining an empty string with an absolute path (and vice-versa) would result in a relative path. Now, it returns the absolute path.

As a side-effect, directory_name("/") returns "/", where it used to return an empty string. This matches the behavior of C's dirname("/") and Erlang's filename:dirname("/").

@lpil lpil left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Lovely. Could you update the changelog please 🙏

Previously, `join`ing an empty string with an absolute path (and
vice-versa) would result in a relative path. Now, it returns the
absolute path.

As a side-effect, `directory_name("/")` returns `"/"`, where it used to
return an empty string. This matches the behavior of C's `dirname("/")`
and Erlang's `filename:dirname("/")`.
@JackoCoolio

Copy link
Copy Markdown
Author

Added a "v1.1.3" section to the changelog.

@JackoCoolio

Copy link
Copy Markdown
Author

Hmmm, I initially wrote this to address lustre-labs/dev-tools#173 but I'm realizing now that this alone isn't enough. We'd need join("some/path", "/an/absolute/path") to return "/an/absolute/path".

For reference, that's how Erlang's filename:join/2 and Python's os.path.join work, but NodeJS's require("node:path").join returns "some/path/an/absolute/path". It has a separate resolve function for the alternate behavior.

Do you think we should update join to return "/an/absolute/path" here, or add a new resolve-like function? Of course we could just leave it as is and let clients figure it out lol

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.

Unexpected behavior for filepath.join("/","one")

2 participants