Stabilize std::path::Path::ancestors

[teiesti]

Closes #48581

r? @BurntSushi

[BurntSushi]

r? @alexcrichton

[alexcrichton]

@rfcbot fcp merge

Thanks for the PR @teiesti! This all looks good to me, let's see what the libs team thinks

[rfcbot]

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams:

• @Kimundi
• @SimonSapin
• @alexcrichton
• @aturon
• @dtolnay
• @sfackler
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[Mark-Simulacrum]

Did the decision around calling this ancestors vs. parents get settled? parents seems more appropriate to me since it's more readily searchable/recognizable with relation to the existing parent method.

[teiesti]

There has been a long lasting debate around the naming issue in the initial PR (#48420). The discussion was then postponed to the tracking issue/stabilization in order to get this feature working on nightly. There has indeed never been a deliberate decision but the discussion immediately ebbed away after the feature was merged.

When writing the stabilization PR, I consciously decided not to bring up this issue again in order to see if there's still need for further debate. @Mark-Simulacrum: Thank You for mentioning this, because this shows me that we should indeed talk about this again!

Here is a list of arguments already raised:

Pro ancestors:

1. Cargo has a similar method called ancestors.
2. It might be weird that an iterator called Ancestor yields the current path. It is more reasonable that the current path is a "null ancestor" than a "null parent".
3. The name parents might be misleading.
4. Changing the name would break existing code in nightly.

Pro parents:

1. When sorted, parents comes right after parent. It is therefore grouped with its more specific counterpart within the documentation. (This argument is ruled out by the fact that the standard library is not sorted by name but by the order in which functions appear in the source code. It is therefore possible to put ancestors right after parents.)
2. parents is more easily searchable/recognizable, given you are familiar with parent.

There have been arguments against both, ancestors and parents, but nobody came up with an idea that was clearly superior. Therefore, I suggest that we limit the decision to the choice between these two.

I, personally, have a slight tendency towards ancestors but in the end I don't really care about the name as long as it is one of the two. I do care about a fast stabilization because the functionality is highly useful for me.

In order to have both, fast stabilization and a good decision about the name, I propose to announce some kind of a final comment period about the name. This would give anyone the chance to raise new arguments. After that, @rust-lang/libs could decide.

[Mark-Simulacrum]

Just wanted to check that it was brought up before stabilization, but looks like there has been debate about the topic already -- I'm fine with ancestors per the arguments you linked to.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[dtolnay]

I have only read @teiesti's recap of the arguments and not the original tracking issue, but I prefer ancestors over parents for a reason that hasn't been listed so far. An iterator called parents implies to me that a path can have multiple parents. In reality a path has at most one parent. There is no transitive property where a parent of my parent would be considered my parent. In contrast, it seems easy to accept that an ancestor of my ancestor would be considered my ancestor, and thus ancestors would be an iterator over the chain of ancestors.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

[teiesti]

@alexcrichton, what about merging this now?

(The final comment period is complete. No concern was listed. The naming debate was - as I understand - finally settled by @dtolnay's comment. Time is running short for having this stabilized in 1.28.0.)

[dtolnay]

@bors r+

[bors]

📌 Commit 19aa79f has been approved by dtolnay

[bors]

🔒 Merge conflict

[teiesti]

I've rebased this branch. Can we try again?

[dtolnay]

Thank you @teiesti.

@bors r+

[bors]

📌 Commit 65d119c has been approved by dtolnay

[bors]

⌛ Testing commit 65d119c with merge 5230979...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: dtolnay
Pushing 5230979 to master...