path.lua documentation #487
Conversation
jlaurens
commented
Sep 8, 2024
jlaurens
commented
- Typos fixed
- Consistent function description: always refer to an action (no more "Returns").
- Capitalized first letter of sentences
- "file path" → "path"
- "file part" → "base part" (cf basename)
- more precision
- more @Usage: see common_prefix and splitpath
- Consistent function description: always refer to an action. - Capitalized first letter of sentences - "file path" → "path" - "file part" → "base part" (cf basename) - more precision
There was a problem hiding this comment.
Thanks for taking the time to contribute. Some of this looks good and cleaning up the docs is certainly valuable. There are some small nitpicks in review comments, but also more generally I think all the changes that are stuffing "Get..." in at the beginning need to be reviewed. Many of them are more confusing than the original. The ones replacing "Return" could be unacceptable if that matches other docs we have, but only if it is used that way consistently elsewhere. I think at least the other verbs it replaces ("Change...", "Normalize...", etc.) need to be restored.
| @@ -71,7 +71,7 @@ path.rmdir = function(d) | |||
| return ok, err, code | |||
| end | |||
|
|
|||
| --- Gets attributes. | |||
| --- Get file system attributes. | |||
There was a problem hiding this comment.
| --- Get file system attributes. | |
| --- Get file attributes. |
This isn't querying the underlying file system attributes, just the file attributes.
| -- if there's no extension part, the second value will be empty | ||
| -- @string P A file path | ||
| --- Get the root and extension parts of a path. | ||
| -- If some part is missing, the corresponding value will be empty. |
There was a problem hiding this comment.
Both ways make sense, but this edit looses the tip off that if only one part is found it is assumed to be the base, not an extension. This is perhaps intuitive, but less explicit docs doesn't seem good to me. Maybe we can come up with wording that makes this more explicit.
| function path.dirname(P) | ||
| assert_string(1,P) | ||
| local p1 = path.splitpath(P) | ||
| return p1 | ||
| end | ||
|
|
||
| --- return the file part of a path | ||
| -- @string P A file path | ||
| --- Get the base part of a path. |
There was a problem hiding this comment.
This might actually be more confusing to some people. I suggest a wording that includes both labeling for clarify, perhaps:
| --- Get the base part of a path. | |
| --- Get the base part (filename segment) of a path. |
| @@ -380,12 +385,12 @@ function path.join(p1,p2,...) | |||
| return p1..p2 | |||
| end | |||
|
|
|||
| --- normalize the case of a pathname. On Unix, this returns the path unchanged, | |||
| --- Get a path by normalizing the case of a path. On Unix, this returns the path unchanged, | |||
There was a problem hiding this comment.
| --- Get a path by normalizing the case of a path. On Unix, this returns the path unchanged, | |
| --- Normalize the case of a path. On Unix, this returns the path unchanged, |
| @@ -398,7 +403,7 @@ function path.normcase(P) | |||
| end | |||
| end | |||
|
|
|||
| --- normalize a path name. | |||
| --- Get a path by normalizing a path. | |||
There was a problem hiding this comment.
Just fix the case on the original, this is less clear than the original. It's just stuffing words into a word salad, not clarifying what is happening.
