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

Add Path::with_replaced_extension as a alias of Path::with_extension and deprecate Path::with_extension for cleaner function name #515

Closed
anatawa12 opened this issue Dec 28, 2024 · 2 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@anatawa12
Copy link

Proposal

Problem statement

The Path::with_extension has unclear name if the function will replace the extension of the path or add the extension to the path.

This is because both adding and replacing the extension is likely, and "with" preposition can be used to express replace / substitute and addition.

Motivating examples or use cases

use std::path::Path;

let some_path = Path::new("foo.txt");

some_path.with_extension("gz"); // unclear if foo.txt.gz or foo.gz from source code

some_path.with_replaced_extension("gz"); // it's cleaner that will be foo.gz

In real world, there are some bug cases that used with_extension to add an extension to the path by mistake.

Solution sketch

Add Path::with_replaced_extension as a alias of Path::with_extension and deprecate Path::with_extension in the later version.

Alternatives

Keep Path::with_extension as is.
With Path::with_added_extension added (tracked in rust-lang/rust#127292), IDE will suggest both Path::with_extension and Path::with_added_extension when user types Path::with_ext so it will be easier to choose the right function, but this doesn't solve the problem on reading the source code.

Just add Path::with_replaced_extension without deprecating Path::with_extension, but this may confuse the user on which function to use.

Links and related work

Possible Drawbacks

With the deprecation of Path::with_extension, tons of existing code will face the deprecation warning and needs to be replaced with Path::with_replaced_extension.
However, with the deprecation people may found some other places should use Path::with_added_extension instead of Path::with_extension so it may be a good opportunity to fix the issue.

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@anatawa12 anatawa12 added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Dec 28, 2024
@joshtriplett
Copy link
Member

joshtriplett commented Jan 7, 2025

We discussed this in today's @rust-lang/libs-api. We felt it would be too disruptive to deprecate with_extension. However, we'd be happy to see a documentation change pointing people to with_added_extension. (Note that there are use cases for both: going from base.tar to base.tar.zst wants with_added_extension, but going from base.c to base.o wants with_extension.)

We've started an FCP to stabilize with_added_extension. If, after that has been stable for a while, the confusion continues to be a problem, we'd be happy to reopen and reconsider this.

@joshtriplett joshtriplett closed this as not planned Won't fix, can't repro, duplicate, stale Jan 7, 2025
@tgross35
Copy link

tgross35 commented Jan 7, 2025

Link to that relevant tracking issue, since I can't be the only one who couldn't find it via search (since the feature is called path_add_extension) rust-lang/rust#127292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

3 participants