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

path/filepath: Join documentation inconsistencies with path.Join #35655

Open
pam4 opened this issue Nov 18, 2019 · 7 comments
Open

path/filepath: Join documentation inconsistencies with path.Join #35655

pam4 opened this issue Nov 18, 2019 · 7 comments
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@pam4
Copy link

pam4 commented Nov 18, 2019

This is a follow-up to #29875, which is closed.

Problems:

  1. The fix was only applied to path.Join docs, but filepath.Join has the same problem and should be fixed too.
  2. The new wording drops the sentence "all empty strings are ignored", leaving no clue that path.Join("", "foo") results in "foo" rather than "/foo".

New:

Join joins the argument's path elements into a single path, separating them with slashes. The result is Cleaned. However, if the argument list is empty or all its elements are empty, Join returns an empty string.

Old:

Join joins any number of path elements into a single path, adding a separating slash if necessary. The result is Cleaned; in particular, all empty strings are ignored.

I think it should be clear that Join can return the empty string, but it should be also clear that empty strings at the beginning don't make the path absolute.

While I'm at it, I'm also confused by path.Dir:

Dir returns all but the last element of path, typically the path's directory. After dropping the final element using Split, the path is Cleaned and trailing slashes are removed. If the path is empty, Dir returns ".". If the path consists entirely of slashes followed by non-slash bytes, Dir returns a single slash. In any other case, the returned path does not end in a slash.

It should probably read "If the path consists entirely of slashes possibly followed by non-slash bytes" or "If the path consists entirely of slashes followed by zero or more non-slash bytes", otherwise the last sentence ("In any other case ...") is false.

@toothrot toothrot changed the title path, path/filepath: Join documentation inconsistencies path/filepath: Join documentation inconsistencies with path.Join Nov 18, 2019
@toothrot toothrot added this to the Backlog milestone Nov 18, 2019
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 18, 2019
@toothrot
Copy link
Contributor

/cc @erutherford who worked on the updated documentation and @robpike, who owns path.

@erutherford
Copy link
Contributor

erutherford commented Nov 18, 2019

Sorry, I meant to respond to this over the weekend, but ended up being sick instead :)

I think it should be clear that Join can return the empty string, but it should be also clear that empty strings at the beginning don't make the path absolute.

You're right this could have been clearer. Any leading empty strings are ignored in path.Join. I can take a look at updating this later today and look at updating filepath.Join as well.

While I'm at it, I'm also confused by path.Dir:

I can take a look at updating this too.

@gopherbot
Copy link

Change https://golang.org/cl/207797 mentions this issue: path: minor changes to improve documentation for Join

@pam4
Copy link
Author

pam4 commented Nov 19, 2019

@erutherford, thanks! No hurry, I'm never sure if people see my replies on closed issues...

@erutherford
Copy link
Contributor

Ok @toothrot and @pam4, I've got a CL up for the path.Join and filepath.Join changes.

@pam4
Copy link
Author

pam4 commented Nov 19, 2019

@erutherford, "are" repetition typo in "Any leading empty elements are are ignored".

Your wording SGTM, but consider even just "Empty elements are ignored", since non leading ones don't make any difference after Clean.

@erutherford
Copy link
Contributor

erutherford commented Nov 19, 2019

@pam4 thanks for catching that, I've corrected the error and adopted your suggestion.

gopherbot pushed a commit that referenced this issue Nov 20, 2019
Reworking the comments in path to call out how leading
empty elements are treated. Also updating filepath.Join
since it shared much of the wording from path.Join.

Updates #35655

Change-Id: I5b15c5d36e9d19831ed39e6bcc7f2fd6c1330033
Reviewed-on: https://go-review.googlesource.com/c/go/+/207797
Reviewed-by: Rob Pike <r@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants