change!: Repository::branch_remote_name() returns reference::remote::Name. (#450)

That way it's made clear the remote can also be a URL, while rejecting
illformed UTF8. The latter isn't valid for remote names anyway as these
only support a very limited character set.

Note that this error currently is degenerated, making it appear if the
remote name doesn't exists if illformed UTF-8 is found in what appears
to be a symbolic ref.

Author: Byron

git-repository/src/reference/remote.rs

@@ -1,7 +1,8 @@
 use std::borrow::Cow;
+use std::convert::TryInto;
 
 use crate::{
-    bstr::{BStr, ByteSlice, ByteVec},
+    bstr::{BStr, ByteSlice},
     remote, Reference,
 };
 
@@ -14,20 +15,60 @@ pub enum Name<'repo> {
     Url(Cow<'repo, BStr>),
 }
 
-impl Name<'_> {
-    /// Return this instance as a symbolic name, if it is one.
-    pub fn as_symbol(&self) -> Option<&str> {
-        match self {
-            Name::Symbol(n) => n.as_ref().into(),
-            Name::Url(_) => None,
+mod name {
+    use super::Name;
+    use crate::bstr::{BStr, ByteSlice, ByteVec};
+    use std::borrow::Cow;
+    use std::convert::TryFrom;
+
+    impl Name<'_> {
+        /// Obtain the name as string representation.
+        pub fn as_bstr(&self) -> &BStr {
+            match self {
+                Name::Symbol(v) => v.as_ref().into(),
+                Name::Url(v) => v.as_ref(),
+            }
+        }
+
+        /// Return this instance as a symbolic name, if it is one.
+        pub fn as_symbol(&self) -> Option<&str> {
+            match self {
+                Name::Symbol(n) => n.as_ref().into(),
+                Name::Url(_) => None,
+            }
+        }
+
+        /// Return this instance as url, if it is one.
+        pub fn as_url(&self) -> Option<&BStr> {
+            match self {
+                Name::Url(n) => n.as_ref().into(),
+                Name::Symbol(_) => None,
+            }
+        }
+    }
+
+    impl<'a> TryFrom<Cow<'a, BStr>> for Name<'a> {
+        type Error = Cow<'a, BStr>;
+
+        fn try_from(name: Cow<'a, BStr>) -> Result<Self, Self::Error> {
+            if name.contains(&b'/') {
+                Ok(Name::Url(name))
+            } else {
+                match name {
+                    Cow::Borrowed(n) => n.to_str().ok().map(Cow::Borrowed).ok_or(name),
+                    Cow::Owned(n) => Vec::from(n)
+                        .into_string()
+                        .map_err(|err| Cow::Owned(err.into_vec().into()))
+                        .map(Cow::Owned),
+                }
+                .map(Name::Symbol)
+            }
         }
     }
 
-    /// Return this instance as url, if it is one.
-    pub fn as_url(&self) -> Option<&BStr> {
-        match self {
-            Name::Url(n) => n.as_ref().into(),
-            Name::Symbol(_) => None,
+    impl<'a> AsRef<BStr> for Name<'a> {
+        fn as_ref(&self) -> &BStr {
+            self.as_bstr()
         }
     }
 }
@@ -56,17 +97,7 @@ impl<'repo> Reference<'repo> {
             })
             .flatten()
             .or_else(|| config.string("branch", Some(name), "remote"))
-            .and_then(|name| {
-                if name.contains(&b'/') {
-                    Some(Name::Url(name))
-                } else {
-                    match name {
-                        Cow::Borrowed(n) => n.to_str().ok().map(Cow::Borrowed),
-                        Cow::Owned(n) => Vec::from(n).into_string().ok().map(Cow::Owned),
-                    }
-                    .map(Name::Symbol)
-                }
-            })
+            .and_then(|name| name.try_into().ok())
     }
 
     /// Like [`remote_name(…)`][Self::remote_name()], but configures the returned `Remote` with additional information like

git-repository/src/repository/config.rs

@@ -83,8 +83,6 @@ mod branch {
     use git_ref::FullNameRef;
     use git_validate::reference::name::Error as ValidateNameError;
 
-    use crate::bstr::BStr;
-
     impl crate::Repository {
         /// Return a set of unique short branch names for which custom configuration exists in the configuration,
         /// if we deem them [trustworthy][crate::open::Options::filter_config_section()].
@@ -114,12 +112,15 @@ mod branch {
         /// Returns the unvalidated name of the remote associated with the given `short_branch_name`,
         /// typically `main` instead of `refs/heads/main`.
         /// In some cases, the returned name will be an URL.
-        /// Returns `None` if the remote was not found.
+        /// Returns `None` if the remote was not found or if the name contained illformed UTF-8.
         ///
         /// See also [Reference::remote_name()][crate::Reference::remote_name()] for a more typesafe version
         /// to be used when a `Reference` is available.
-        pub fn branch_remote_name(&self, short_branch_name: &str) -> Option<Cow<'_, BStr>> {
-            self.config.resolved.string("branch", Some(short_branch_name), "remote")
+        pub fn branch_remote_name(&self, short_branch_name: &str) -> Option<crate::reference::remote::Name<'_>> {
+            self.config
+                .resolved
+                .string("branch", Some(short_branch_name), "remote")
+                .and_then(|name| name.try_into().ok())
         }
     }
 }