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

Would like path_of #1723

Closed
njaard opened this issue Mar 4, 2020 · 16 comments
Closed

Would like path_of #1723

njaard opened this issue Mar 4, 2020 · 16 comments
Labels
C-enhancement Category: Raise on the bar on expectations

Comments

@njaard
Copy link

njaard commented Mar 4, 2020

Use case

It's common to get a filename from clap with value_of_os and its companion values_of_os. You often want to turn it into a Path in order to pass it to std::fs::File::open().

The only way I've been able to find to turn an OsStr into a Path is with Path::new(), which is a bit wordy. Therefor, it'd be nice be able to get Path objects directly.

Proposed solution

It'd be nice if there were a function like Matches::path_of(name: &str) -> Option<&Path> so that you can get a Path in a single function call.

Alternate names for path_of might be the consistent but less concise value_of_path but that somehow reads grammatically different than value_of_os, not to mention being longer. Therefor, path_of sounds better.

Additionally, you'd want paths_of the way there is values_of_os

Hypothetically, one could also make it reject (at get_matches-time) impossible Paths (on Windows, not all Paths are valid).

Alternative solutions

The standard library could get some sort of convenient shorthand for turning OsStr into Path. I think adding the function into clap serves the purpose better because it self-documents better while keeping std clean and explicit.

@njaard njaard changed the title Would like value_of_path Would like path_of Mar 4, 2020
@CreepySkeleton
Copy link
Contributor

I'm not very excited about adding yet another output format, but we can make it if paths are really so common, perhaps under value_of_as_path and values_of_as_paths names. cc @pksunkara @Dylan-DPC what do you think?

Hypothetically, one could also make it reject (at get_matches-time) impossible Paths (on Windows, not all Paths are valid).

That's definitely up to user. Even Path::from doesn't validate it.

@njaard
Copy link
Author

njaard commented Mar 5, 2020

I'm not very excited about adding yet another output format, but we can make it if paths are really so common, perhaps under value_of_as_path and values_of_as_paths names.

CLI tools of course have filenames as a common use case.

That's definitely up to user. Even Path::from doesn't validate it.

Well, I was just speculating, but naturally if the user doesn't want it validated, they could just use value_of_os.

@CreepySkeleton
Copy link
Contributor

Except, what does "valid path" means?

@njaard
Copy link
Author

njaard commented Mar 5, 2020

Except, what does "valid path" means?

On Windows, a Path may not contain <, >, |, ", ?, *, and outside of drive letters, : according to Wikipedia. (I don't use Windows)

On Linux, I think there are no restrictions other than of course the null termination.

@CreepySkeleton
Copy link
Contributor

This kind of checking would be largely useless. The os facilities would check it anyway (std::fs::* in Rust), so I don't see the point to check it on our side.

Come to think about it, value_of_as_path is a trivial value_of().map(Path::from); I'm very unconvinced about your proposal, no offense. Let's hear out the other maintainers @TeXitoi @pksunkara @Dylan-DPC .

@TeXitoi
Copy link
Contributor

TeXitoi commented Mar 5, 2020

It would be only me, I'd say no. I'm not really convinced by the added feature.

@pksunkara
Copy link
Member

It is definitely trivial. I would consider this one of those things where if more people request it, I wouldn't mind adding it.

@CreepySkeleton
Copy link
Contributor

@pksunkara Which one? The checking or the conversion?

@pksunkara
Copy link
Member

Conversion

@njaard
Copy link
Author

njaard commented Mar 13, 2020

Another compelling argument in favor is that I believe it's more teachable: If a user has an &str, they can pass it directly into File::open, however, OsStr cannot be, it needs to be converted into a Path and not making it easy and automatic acts as another barrier to writing correct code.

@CreepySkeleton
Copy link
Contributor

If a user has an &str, they can pass it directly into File::open, however, OsStr cannot be

Good try, but it can 😮

@njaard
Copy link
Author

njaard commented Mar 18, 2020

If a user has an &str, they can pass it directly into File::open, however, OsStr cannot be

Good try, but it can open_mouth

Doh. Yes because File accepts AsRef<Path>, but when one writes basic application code, I think &Path is more common. At least it is for me.

@CreepySkeleton
Copy link
Contributor

Doh. Yes because File accepts AsRef, but when one writes basic application code, I think &Path is more common. At least it is for me.

Well, the argument was about flattening the learning curve, wasn't it? I've swept through std::fs::* and found that the API uses AsRef<Path> instead of &Path literally everywhere. In fact, you would want to use Path explicitly only when you want to query some property of the Path itself, like extension or is_file.

I see your point about Paths being commonplace, but it could use some acknowledgement. If and when at least two more people come here to ask for it, I'll have become open to accepting such a PR.

@CreepySkeleton
Copy link
Contributor

Closing as inactive. I don't think this feature is of any use, and, evidently, almost nobody wants it.

@Dylan-DPC-zz
Copy link

Please don't close any issues just because nobody commented they needed it. There could always be a future feature request.

@Dylan-DPC-zz Dylan-DPC-zz reopened this Jun 30, 2020
@epage epage added C-enhancement Category: Raise on the bar on expectations and removed T: new feature labels Dec 8, 2021
@epage
Copy link
Member

epage commented Dec 8, 2021

I believe #2683 should supersede this, closing it.

@epage epage closed this as completed Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

No branches or pull requests

6 participants