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

Clarify Path::extension() semantics in docs abstract #102058

Merged
merged 1 commit into from
Sep 21, 2022

Conversation

mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Sep 20, 2022

State up-front and center what shape the returned extension will have, without making the user read through the description and examples.

This is a doc-only change. There are no changes to the API contract and the clarification is in line with what was already stated/promised in the existing doc text - just clarified, summarized, and served bright and early.

Rationale: Various frameworks and libraries for different platforms have their different conventions as to whether an "extension" is ".ext" or just "ext" and anyone that's had to deal with this ambiguity in the past is always double- or triple-checking to make sure the function call returns an extension that matches the expected semantics. Offer the answer to this important question right off the bat instead of making them dig to find it.

@rustbot label +A-docs

State up-front and center what shape the returned extension will have, without
making the user read through the description and examples.

Rationale: Various frameworks and libraries for different platforms have their
different conventions as to whether an "extension" is ".ext" or just "ext" and
anyone that's had to deal with this ambiguity in the past is always double- or
triple-checking to make sure the function call returns an extension that matches
the expected semantics. Offer the answer to this important question right off
the bat instead of making them dig to find it.
@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Sep 20, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 20, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-highfive
Copy link
Collaborator

r? @thomcc

(rust-highfive has picked a reviewer for you, use r? to override)

@rustbot rustbot added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Sep 20, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2022
@thomcc
Copy link
Member

thomcc commented Sep 20, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 20, 2022

📌 Commit c291d2a has been approved by thomcc

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 20, 2022
@mqudsi
Copy link
Contributor Author

mqudsi commented Sep 20, 2022

Thanks, @thomcc.

@the8472
Copy link
Member

the8472 commented Sep 20, 2022

Offer the answer to this important question right off the bat instead of making them dig to find it.

But there are other important questions. E.g. is .tar.gz the extension or just .gz? What makes this question more important than others?

@thomcc
Copy link
Member

thomcc commented Sep 20, 2022

I'd approve additional clarifications like that, but I don't think they block this one.

@mqudsi
Copy link
Contributor Author

mqudsi commented Sep 20, 2022

@the8472 pedantically, whether or not an "extension" is ".ext" or "ext" affects each and every file and each and every extension while "tar.gz" or "gz" affects only cases where you are matching against a compound extension or a file with potentially a compound extension. Developers that get the first one wrong will get the wrong answer every single time, while developers not taking into account the relatively fringe case of compound extensions will have broken code some fraction of the time.

Less pedantically and on a more personal/subjective level, as best as I can recollect, every api I've worked with treated the extension of "foo.tar.gz" to be ".gz" (or "gz" as the case may be) so it's not a question that I personally found as pressing to answer (it's also IMHO the only logical result because any file may have a dot/period in the non-ext part of its name/path).

@the8472
Copy link
Member

the8472 commented Sep 20, 2022

People have at least asked for such methods, e.g. in #86319 so if they existed then we'd have to disambiguate the difference which I think would be more important than mentioning whether the dot is included or not.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 21, 2022
Clarify Path::extension() semantics in docs abstract

State up-front and center what shape the returned extension will have, without making the user read through the description and examples.

This is a doc-only change. There are no changes to the API contract and the clarification is in line with what was already stated/promised in the existing doc text - just clarified, summarized, and served bright and early.

Rationale: Various frameworks and libraries for different platforms have their different conventions as to whether an "extension" is ".ext" or just "ext" and anyone that's had to deal with this ambiguity in the past is always double- or triple-checking to make sure the function call returns an extension that matches the expected semantics. Offer the answer to this important question right off the bat instead of making them dig to find it.

`@rustbot` label +A-docs
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 21, 2022
Clarify Path::extension() semantics in docs abstract

State up-front and center what shape the returned extension will have, without making the user read through the description and examples.

This is a doc-only change. There are no changes to the API contract and the clarification is in line with what was already stated/promised in the existing doc text - just clarified, summarized, and served bright and early.

Rationale: Various frameworks and libraries for different platforms have their different conventions as to whether an "extension" is ".ext" or just "ext" and anyone that's had to deal with this ambiguity in the past is always double- or triple-checking to make sure the function call returns an extension that matches the expected semantics. Offer the answer to this important question right off the bat instead of making them dig to find it.

``@rustbot`` label +A-docs
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#89891 (`alloc`: add unstable cfg features `no_rc` and `no_sync`)
 - rust-lang#101995 (Add another example for `uN::carrying_mul`)
 - rust-lang#102031 (Adding ignore fuchsia tests for Backtrace, ErrorKind cases)
 - rust-lang#102041 (Improve `-Zmeta-stats` some more)
 - rust-lang#102045 (fix ConstProp handling of written_only_inside_own_block_locals)
 - rust-lang#102058 (Clarify Path::extension() semantics in docs abstract)
 - rust-lang#102059 (Use rebind instead of dummy binder in `SameTypeModuloInfer` relation)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 77d063b into rust-lang:master Sep 21, 2022
@rustbot rustbot added this to the 1.66.0 milestone Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants