Should Utf8PathBuf::from_path_buf return a real error?

[EverlastingBugstopper]

Hi!

I just opened a PR that introduces camino to the tool I'm working on, and I seem to be relying on Utf8PathBuf::from_path_buf quite a bit when interacting with std::path::PathBufs coming from places like std::env::current_dir.

I'd love to just use ? like I would any other fallible function that returns a Result type, but it seems that Utf8PathBuf::from_path_buf returns a Result<Utf8PathBuf, PathBuf> which requires me to write .map_err every time I call it.

Am I doing something wrong here? Is there a better way to do these conversions that I'm missing? If not, it seems like that Result type should implement std::error::Error and possibly wrap the old PathBuf up in it.

Love the library! Thanks for all of the work you've put in to make it a reality 😄

[sunshowers]

Hey, thanks for the report and for checking out camino!

So I went back and forth on the definition of this for a while, eventually settling on matching OsString::into_string (which is morally equivalent to this method). However, I do see the argument for having a proper error type with the option of converting it into a PathBuf. It's too late to change the definition of from_path_buf itself now, but one could in principle add a different method -- perhaps a TryFrom<PathBuf> for Utf8PathBuf impl -- which returns an error type. What do you think? Would love some more feedback about this approach.

For your use case, do you think you could write your own freestanding function (or extension trait) for now which wraps Utf8PathBuf::from_path_buf + map_err into your own error type?

[EverlastingBugstopper]

I actually reached for try_from initially so I think that makes total sense!

For your use case, do you think you could write your own freestanding function (or extension trait) for now which wraps Utf8PathBuf::from_path_buf + map_err into your own error type?

Mmmmaaaaayyybbeee? The problem is that we have a workspace with a bunch of different subcrates that all have their own thiserror enums. I've added an error type to that enum, but still need to call map_err to create that other error type. Not sure how I would do it in a more seamless way. (It totally works as is so I'll probably just leave it until a possible try_from appears.

[sunshowers]

Mmmmaaaaayyybbeee? The problem is that we have a workspace with a bunch of different subcrates that all have their own thiserror enums. I've added an error type to that enum, but still need to call map_err to create that other error type. Not sure how I would do it in a more seamless way. (It totally works as is so I'll probably just leave it until a possible try_from appears.

Ah, hmm, yeah. One way might be to define a crate + error type that sits at the bottom of your dependency graph, have it return an error, then use thiserror's automatic From conversion. But that's probably messier than just solving it in camino.

I do agree that solving this is probably worth it, and I'm inclined to pursue the TryFrom solution -- I think it'll likely work. I'll leave this open for a bit for more feedback from other users.

[sunshowers]

This will be fixed in 1.0.3, which is about to go out to crates.io. Thanks for the report!

[sunshowers]

And released :)