std docs: edit `PathBuf::set_file_name` example #109540

marcospb19 · 2023-03-23T20:32:01Z

To make explicit that set_file_name might replace or remove the
extension, not just the file stem.

Also edit docs for Path::with_file_name, which calls set_file_name.

rustbot · 2023-03-23T20:32:09Z

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-03-23T20:32:12Z

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

marcospb19 · 2023-03-23T21:08:51Z

force-pushed because I called it "file name" instead of "file stem" in the commit msg.

JohnTitor · 2023-04-14T19:00:56Z

library/std/src/path.rs

@@ -2562,7 +2567,8 @@ impl Path {
    /// ```
    /// use std::path::{Path, PathBuf};
    ///
-    /// let path = Path::new("/tmp/foo.txt");
+    /// let path = Path::new("/tmp/foo.png");


You don't have to change the extension at all.

Hi John, the point of changing the extension here is to explicitly show that the subsequent call is overwriting the extension.

Before: foo.txt -> bar.txt.
After : foo.png -> bar.txt.

It's now clear that txt was overwritten.

Note that the extension is unrelated to the "file name". Methods for it don't care about the extension at all, and the current doc should tell it already by using foo.txt and var. And overwriting is explained by bar.txt.

And overwriting is explained by bar.txt.

I agree, people should/could be able to understand overwriting by the bar.txt part.

Although I can't find a real reason to revert this 3-letter change.

JohnTitor · 2023-04-14T19:01:15Z

library/std/src/path.rs

-    /// buf.set_file_name("bar");
-    /// assert!(buf == PathBuf::from("/bar"));
+    ///
+    /// buf.set_file_name("foo.txt");
+    /// assert!(buf == PathBuf::from("/foo.txt"));
    /// assert!(buf.file_name().is_some());
-    /// buf.set_file_name("baz.txt");
-    /// assert!(buf == PathBuf::from("/baz.txt"));
+    ///
+    /// buf.set_file_name("bar.txt");
+    /// assert!(buf == PathBuf::from("/bar.txt"));
+    ///
+    /// buf.set_file_name("baz");
+    /// assert!(buf == PathBuf::from("/baz"));


This shouldn't be related to with_file_name and should be dropped.

Ok, I changed the PR title, description, and commit message to better indicate that this is about PathBuf::set_file_name than with_file_name (but also changes with_file_name because that's a shorthand for the other method).

I can split it on different PRs but I don't think that's necessary.

To make explicit that set_file_name might replace or remove the extension, not just the file stem.

The current doc doesn't mention stem and extension at all, and it also tells that it could overwrite everything by using bar and baz.txt.

The current doc doesn't mention stem and extension at all

Exactly! That's the point of the PR, make it explicit.

Would you prefer if this is explained in the method description instead of reflected in the examples?

and it also tells that it could overwrite everything by using bar and baz.txt

I disagree, this is not stated as clearly as you're describing.

I'm trying to make it explicit because I was bitten by it recently.

This is ultimately a question of "What is the filename?", and to explain it from scratch would be over-explaining it. We indeed have an explanation for stem but file name and extension are common things (I believe) and it seems less important here.

Well, this is very subjective.

What you might see as "over-explaining" some simple/common concepts, I'm seeing as "filling the gaps" in the documentation for something that is non-trivial.

It's reasonable for people to assume "file name" is actually the "file stem", and that .set_file_name() doesn't overwrite the extension, so I'm being explicit here.

to show this method might replace or remove the extension, not just the file stem also edit docs of `Path::with_file_name` because it calls `PathBuf::set_file_name`

marcospb19 · 2023-04-16T21:01:11Z

I think my changes would be helpful to other people reading the docs, so I'd like to keep them.

Let's wait for @m-ou-se and see what she thinks.

m-ou-se · 2023-05-01T12:48:32Z

@bors r+ rollup

bors · 2023-05-01T12:48:34Z

📌 Commit 4eb4e10 has been approved by m-ou-se

It is now in the queue for this repository.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#109540 (std docs: edit `PathBuf::set_file_name` example) - rust-lang#110093 (Add 64-bit `time_t` support on 32-bit glibc Linux to `set_times`) - rust-lang#110987 (update wasi_clock_time_api ref.) - rust-lang#111038 (Leave promoteds untainted by errors when borrowck fails) - rust-lang#111042 (Add `#[no_coverage]` to the test harness's `fn main`) - rust-lang#111057 (Make sure the implementation of TcpStream::as_raw_fd is fully inlined) - rust-lang#111065 (Explicitly document how Send and Sync relate to references) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned m-ou-se Mar 23, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 23, 2023

marcospb19 changed the title ~~edit Path::with_file_name example~~ std docs: edit Path::with_file_name example Mar 23, 2023

marcospb19 marked this pull request as draft March 23, 2023 20:52

marcospb19 force-pushed the edit-Path-with_file_name-example branch from 1166f51 to cb2e29f Compare March 23, 2023 20:59

marcospb19 marked this pull request as ready for review March 23, 2023 21:00

marcospb19 force-pushed the edit-Path-with_file_name-example branch from cb2e29f to 8f63abf Compare March 23, 2023 21:08

JohnTitor requested changes Apr 14, 2023

View reviewed changes

edit docs of PathBuf::set_file_name

4eb4e10

to show this method might replace or remove the extension, not just the file stem also edit docs of `Path::with_file_name` because it calls `PathBuf::set_file_name`

marcospb19 changed the title ~~std docs: edit Path::with_file_name example~~ std docs: edit PathBuf::set_file_name example Apr 14, 2023

marcospb19 force-pushed the edit-Path-with_file_name-example branch from 8f63abf to 4eb4e10 Compare April 14, 2023 21:09

marcospb19 requested a review from JohnTitor April 14, 2023 21:14

jyn514 added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` labels Apr 26, 2023

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 May 1, 2023

matthiaskrgr mentioned this pull request May 1, 2023

Rollup of 7 pull requests #111066

Merged

bors merged commit 4da8a7a into rust-lang:master May 2, 2023

rustbot added this to the 1.71.0 milestone May 2, 2023

marcospb19 deleted the edit-Path-with_file_name-example branch May 2, 2023 03:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

std docs: edit `PathBuf::set_file_name` example #109540

std docs: edit `PathBuf::set_file_name` example #109540

marcospb19 commented Mar 23, 2023 •

edited

Loading

rustbot commented Mar 23, 2023

rustbot commented Mar 23, 2023

marcospb19 commented Mar 23, 2023

JohnTitor Apr 14, 2023

marcospb19 Apr 14, 2023

JohnTitor Apr 14, 2023 •

edited

Loading

marcospb19 Apr 16, 2023

JohnTitor Apr 14, 2023

marcospb19 Apr 14, 2023 •

edited

Loading

JohnTitor Apr 14, 2023

marcospb19 Apr 14, 2023

marcospb19 Apr 14, 2023

JohnTitor Apr 14, 2023

marcospb19 Apr 16, 2023

marcospb19 commented Apr 16, 2023

m-ou-se commented May 1, 2023

bors commented May 1, 2023

std docs: edit PathBuf::set_file_name example #109540

std docs: edit PathBuf::set_file_name example #109540

Conversation

marcospb19 commented Mar 23, 2023 • edited Loading

rustbot commented Mar 23, 2023

rustbot commented Mar 23, 2023

marcospb19 commented Mar 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnTitor Apr 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcospb19 Apr 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcospb19 commented Apr 16, 2023

m-ou-se commented May 1, 2023

bors commented May 1, 2023

std docs: edit `PathBuf::set_file_name` example #109540

std docs: edit `PathBuf::set_file_name` example #109540

marcospb19 commented Mar 23, 2023 •

edited

Loading

JohnTitor Apr 14, 2023 •

edited

Loading

marcospb19 Apr 14, 2023 •

edited

Loading