rust-lang / git2-rs

libgit2 bindings for Rust
https://docs.rs/git2
Apache License 2.0
1.71k stars 389 forks source link

Return `Result<&str, Utf8Error>` instead of `Option<&str>` #857

Open nasso opened 2 years ago

nasso commented 2 years ago

I don't understand why many methods return Option<&str> instead of Result<&str, Utf8Error> when decoding a string.

It hides information that could be used by the caller to form a more meaningful error message, and doesn't prevent anyone from just calling unwrap, expect or ok when they don't care about the error.

Furthermore, it would disambiguate the return value of some methods, such as Reference::symbolic_target, for which None may be returned if the reference is either not symbolic or not a valid utf-8 string. If that method returned Result<Option<&str>, Utf8Error> (or Option<Result<&str, Utf8Error>>, though one can switch between the two with (Option|Result)::transpose), it would make it easier to disambiguate with a single function call.

I couldn't find any past discussion regarding this API design choice, so I'm assuming nobody ever made that suggestion because nobody encountered this issue before. I doubt a change like this, even if it's a breaking change, wouldn't significantly hurt anyone's codebase.