Merge pull request #1241 from DanielEScherzer/return-results

Return `Result` values rather than converting errors to `None`

Author: weihanglo

examples/cat-file.rs

@@ -89,7 +89,7 @@ fn show_commit(commit: &Commit) {
     }
     show_sig("author", Some(commit.author()));
     show_sig("committer", Some(commit.committer()));
-    if let Some(msg) = commit.message() {
+    if let Ok(msg) = commit.message() {
         println!("\n{}", msg);
     }
 }
@@ -100,7 +100,7 @@ fn show_tag(tag: &Tag) {
     println!("tag {}", tag.name().unwrap());
     show_sig("tagger", tag.tagger());
 
-    if let Some(msg) = tag.message() {
+    if let Ok(Some(msg)) = tag.message() {
         println!("\n{}", msg);
     }
 }

examples/log.rs

@@ -176,7 +176,7 @@ fn run(args: &Args) -> Result<(), Error> {
             if !sig_matches(&commit.committer(), &args.flag_committer) {
                 return None;
             }
-            if !log_message_matches(commit.message(), &args.flag_grep) {
+            if !log_message_matches(commit.message().ok(), &args.flag_grep) {
                 return None;
             }
             Some(Ok(commit))

examples/pull.rs

@@ -56,7 +56,7 @@ fn do_fetch<'a>(
     // Always fetch all tags.
     // Perform a download and also update tips
     fo.download_tags(git2::AutotagOption::All);
-    println!("Fetching {} for repo", remote.name().unwrap());
+    println!("Fetching {} for repo", remote.name().unwrap().unwrap());
     remote.fetch(refs, Some(&mut fo), None)?;
 
     // If there are local objects (we got a thin pack), then tell the user
@@ -90,8 +90,8 @@ fn fast_forward(
     rc: &git2::AnnotatedCommit,
 ) -> Result<(), git2::Error> {
     let name = match lb.name() {
-        Some(s) => s.to_string(),
-        None => String::from_utf8_lossy(lb.name_bytes()).to_string(),
+        Ok(s) => s.to_string(),
+        Err(_) => String::from_utf8_lossy(lb.name_bytes()).to_string(),
     };
     let msg = format!("Fast-Forward: Setting {} to id: {}", name, rc.id());
     println!("{}", msg);

examples/status.rs

@@ -134,7 +134,7 @@ fn show_branch(repo: &Repository, format: &Format) -> Result<(), Error> {
         }
         Err(e) => return Err(e),
     };
-    let head = head.as_ref().and_then(|h| h.shorthand());
+    let head = head.as_ref().and_then(|h| h.shorthand().ok());
 
     if format == &Format::Long {
         println!(

examples/tag.rs

@@ -65,7 +65,7 @@ fn run(args: &Args) -> Result<(), Error> {
     } else if args.flag_list {
         let pattern = args.arg_pattern.as_ref().map(|s| &s[..]).unwrap_or("*");
         for name in repo.tag_names(Some(pattern))?.iter() {
-            let name = name.unwrap();
+            let name = name.expect("Not invalid utf8").expect("Not None");
             let obj = repo.revparse_single(name)?;
 
             if let Some(tag) = obj.as_tag() {
@@ -83,7 +83,7 @@ fn run(args: &Args) -> Result<(), Error> {
 fn print_tag(tag: &Tag, args: &Args) {
     print!("{:<16}", tag.name().unwrap());
     if args.flag_n.is_some() {
-        print_list_lines(tag.message(), args);
+        print_list_lines(tag.message().unwrap(), args);
     } else {
         println!();
     }
@@ -92,7 +92,7 @@ fn print_tag(tag: &Tag, args: &Args) {
 fn print_commit(commit: &Commit, name: &str, args: &Args) {
     print!("{:<16}", name);
     if args.flag_n.is_some() {
-        print_list_lines(commit.message(), args);
+        print_list_lines(commit.message().ok(), args);
     } else {
         println!();
     }

src/blame.rs

@@ -206,10 +206,12 @@ impl<'blame> BlameHunk<'blame> {
     /// The returned message is the summary of the commit, comprising the first
     /// paragraph of the message with whitespace trimmed and squashed.
     ///
-    /// `None` may be returned if an error occurs or if the summary is not valid
-    /// utf-8.
-    pub fn summary(&self) -> Option<&str> {
-        self.summary_bytes().and_then(|s| str::from_utf8(s).ok())
+    /// `Ok(None)` may be returned if there is no summary.
+    pub fn summary(&self) -> Result<Option<&str>, Error> {
+        match self.summary_bytes() {
+            Some(sb) => str::from_utf8(sb).map(|s| Some(s)).map_err(|e| e.into()),
+            None => Ok(None),
+        }
     }
 
     /// Get the short "summary" of the git commit message for the hunk.
@@ -448,7 +450,7 @@ mod tests {
         assert_eq!(hunk.final_start_line(), 1);
         assert_eq!(hunk.path(), Some(Path::new("foo/bar")));
         assert_eq!(hunk.lines_in_hunk(), 0);
-        assert_eq!(hunk.summary(), Some("commit"));
+        assert_eq!(hunk.summary(), Ok(Some("commit")));
         assert!(!hunk.is_boundary());
 
         let blame_buffer = blame.blame_buffer("\n".as_bytes()).unwrap();

src/buf.rs

@@ -5,6 +5,7 @@ use std::str;
 
 use crate::raw;
 use crate::util::Binding;
+use crate::Error;
 
 /// A structure to wrap an intermediate buffer used by libgit2.
 ///
@@ -34,10 +35,8 @@ impl Buf {
     }
 
     /// Attempt to view this buffer as a string slice.
-    ///
-    /// Returns `None` if the buffer is not valid utf-8.
-    pub fn as_str(&self) -> Option<&str> {
-        str::from_utf8(&**self).ok()
+    pub fn as_str(&self) -> Result<&str, Error> {
+        str::from_utf8(&**self).map_err(|e| e.into())
     }
 }
 

src/commit.rs

@@ -63,10 +63,8 @@ impl<'repo> Commit<'repo> {
     ///
     /// The returned message will be slightly prettified by removing any
     /// potential leading newlines.
-    ///
-    /// `None` will be returned if the message is not valid utf-8
-    pub fn message(&self) -> Option<&str> {
-        str::from_utf8(self.message_bytes()).ok()
+    pub fn message(&self) -> Result<&str, Error> {
+        str::from_utf8(self.message_bytes()).map_err(|e| e.into())
     }
 
     /// Get the full message of a commit as a byte slice.
@@ -80,17 +78,18 @@ impl<'repo> Commit<'repo> {
     /// Get the encoding for the message of a commit, as a string representing a
     /// standard encoding name.
     ///
-    /// `None` will be returned if the encoding is not known
-    pub fn message_encoding(&self) -> Option<&str> {
+    /// `Ok(None)` will be returned if the encoding is not known
+    pub fn message_encoding(&self) -> Result<Option<&str>, Error> {
         let bytes = unsafe { crate::opt_bytes(self, raw::git_commit_message_encoding(&*self.raw)) };
-        bytes.and_then(|b| str::from_utf8(b).ok())
+        match bytes {
+            Some(b) => str::from_utf8(b).map(|s| Some(s)).map_err(|e| e.into()),
+            None => Ok(None),
+        }
     }
 
     /// Get the full raw message of a commit.
-    ///
-    /// `None` will be returned if the message is not valid utf-8
-    pub fn message_raw(&self) -> Option<&str> {
-        str::from_utf8(self.message_raw_bytes()).ok()
+    pub fn message_raw(&self) -> Result<&str, Error> {
+        str::from_utf8(self.message_raw_bytes()).map_err(|e| e.into())
     }
 
     /// Get the full raw message of a commit.
@@ -99,10 +98,8 @@ impl<'repo> Commit<'repo> {
     }
 
     /// Get the full raw text of the commit header.
-    ///
-    /// `None` will be returned if the message is not valid utf-8
-    pub fn raw_header(&self) -> Option<&str> {
-        str::from_utf8(self.raw_header_bytes()).ok()
+    pub fn raw_header(&self) -> Result<&str, Error> {
+        str::from_utf8(self.raw_header_bytes()).map_err(|e| e.into())
     }
 
     /// Get an arbitrary header field.
@@ -129,10 +126,12 @@ impl<'repo> Commit<'repo> {
     /// The returned message is the summary of the commit, comprising the first
     /// paragraph of the message with whitespace trimmed and squashed.
     ///
-    /// `None` may be returned if an error occurs or if the summary is not valid
-    /// utf-8.
-    pub fn summary(&self) -> Option<&str> {
-        self.summary_bytes().and_then(|s| str::from_utf8(s).ok())
+    /// `Ok(None)` may be returned if there is no summary
+    pub fn summary(&self) -> Result<Option<&str>, Error> {
+        match self.summary_bytes() {
+            Some(sb) => str::from_utf8(sb).map(|s| Some(s)).map_err(|e| e.into()),
+            None => Ok(None),
+        }
     }
 
     /// Get the short "summary" of the git commit message.
@@ -151,10 +150,12 @@ impl<'repo> Commit<'repo> {
     /// but the first paragraph of the message. Leading and trailing whitespaces
     /// are trimmed.
     ///
-    /// `None` may be returned if an error occurs or if the summary is not valid
-    /// utf-8.
-    pub fn body(&self) -> Option<&str> {
-        self.body_bytes().and_then(|s| str::from_utf8(s).ok())
+    /// `Ok(None)` may be returned if there is no body.
+    pub fn body(&self) -> Result<Option<&str>, Error> {
+        match self.body_bytes() {
+            Some(sb) => str::from_utf8(sb).map(|s| Some(s)).map_err(|e| e.into()),
+            None => Ok(None),
+        }
     }
 
     /// Get the long "body" of the git commit message.
@@ -347,7 +348,7 @@ impl<'repo> std::fmt::Debug for Commit<'repo> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
         let mut ds = f.debug_struct("Commit");
         ds.field("id", &self.id());
-        if let Some(summary) = self.summary() {
+        if let Ok(Some(summary)) = self.summary() {
             ds.field("summary", &summary);
         }
         ds.finish()
@@ -424,12 +425,12 @@ mod tests {
         let head = repo.head().unwrap();
         let target = head.target().unwrap();
         let commit = repo.find_commit(target).unwrap();
-        assert_eq!(commit.message(), Some("initial\n\nbody"));
-        assert_eq!(commit.body(), Some("body"));
+        assert_eq!(commit.message(), Ok("initial\n\nbody"));
+        assert_eq!(commit.body(), Ok(Some("body")));
         assert_eq!(commit.id(), target);
         commit.message_raw().unwrap();
         commit.raw_header().unwrap();
-        commit.message_encoding();
+        commit.message_encoding().expect("Should not be invalid");
         commit.summary().unwrap();
         commit.body().unwrap();
         commit.tree_id();
@@ -441,10 +442,10 @@ mod tests {
             crate::Oid::from_str(tree_header_bytes.as_str().unwrap()).unwrap(),
             commit.tree_id()
         );
-        assert_eq!(commit.author().name(), Some("name"));
-        assert_eq!(commit.author().email(), Some("email"));
-        assert_eq!(commit.committer().name(), Some("name"));
-        assert_eq!(commit.committer().email(), Some("email"));
+        assert_eq!(commit.author().name(), Ok("name"));
+        assert_eq!(commit.author().email(), Ok("email"));
+        assert_eq!(commit.committer().name(), Ok("name"));
+        assert_eq!(commit.committer().email(), Ok("email"));
 
         let sig = repo.signature().unwrap();
         let tree = repo.find_tree(commit.tree_id()).unwrap();
@@ -457,7 +458,7 @@ mod tests {
             .amend(Some("HEAD"), None, None, None, Some("new message"), None)
             .unwrap();
         let new_head = repo.find_commit(new_head).unwrap();
-        assert_eq!(new_head.message(), Some("new message"));
+        assert_eq!(new_head.message(), Ok("new message"));
         new_head.into_object();
 
         repo.find_object(target, None).unwrap().as_commit().unwrap();

src/config.rs

@@ -522,10 +522,8 @@ impl Drop for Config {
 
 impl<'cfg> ConfigEntry<'cfg> {
     /// Gets the name of this entry.
-    ///
-    /// May return `None` if the name is not valid utf-8
-    pub fn name(&self) -> Option<&str> {
-        str::from_utf8(self.name_bytes()).ok()
+    pub fn name(&self) -> Result<&str, Error> {
+        str::from_utf8(self.name_bytes()).map_err(|e| e.into())
     }
 
     /// Gets the name of this entry as a byte slice.
@@ -535,13 +533,11 @@ impl<'cfg> ConfigEntry<'cfg> {
 
     /// Gets the value of this entry.
     ///
-    /// May return `None` if the value is not valid utf-8
-    ///
     /// # Panics
     ///
     /// Panics when no value is defined.
-    pub fn value(&self) -> Option<&str> {
-        str::from_utf8(self.value_bytes()).ok()
+    pub fn value(&self) -> Result<&str, Error> {
+        str::from_utf8(self.value_bytes()).map_err(|e| e.into())
     }
 
     /// Gets the value of this entry as a byte slice.
@@ -683,8 +679,8 @@ mod tests {
         let mut entries = cfg.entries(None).unwrap();
         while let Some(entry) = entries.next() {
             let entry = entry.unwrap();
-            entry.name();
-            entry.value();
+            entry.name().unwrap();
+            entry.value().unwrap();
             entry.level();
         }
     }

src/mailmap.rs

@@ -105,8 +105,8 @@ mod tests {
         let mut mm = t!(Mailmap::new());
 
         let mailmapped_sig = t!(mm.resolve_signature(&sig));
-        assert_eq!(mailmapped_sig.name(), Some(sig_name));
-        assert_eq!(mailmapped_sig.email(), Some(sig_email));
+        assert_eq!(mailmapped_sig.name(), Ok(sig_name));
+        assert_eq!(mailmapped_sig.email(), Ok(sig_email));
 
         t!(mm.add_entry(None, None, None, sig_email));
         t!(mm.add_entry(
@@ -117,8 +117,8 @@ mod tests {
         ));
 
         let mailmapped_sig = t!(mm.resolve_signature(&sig));
-        assert_eq!(mailmapped_sig.name(), Some("real name"));
-        assert_eq!(mailmapped_sig.email(), Some("real@email"));
+        assert_eq!(mailmapped_sig.name(), Ok("real name"));
+        assert_eq!(mailmapped_sig.email(), Ok("real@email"));
     }
 
     #[test]
@@ -128,7 +128,7 @@ mod tests {
 
         let sig = t!(Signature::now("name", "email"));
         let mailmapped_sig = t!(mm.resolve_signature(&sig));
-        assert_eq!(mailmapped_sig.name(), Some("name"));
-        assert_eq!(mailmapped_sig.email(), Some("prøper@emæil"));
+        assert_eq!(mailmapped_sig.name(), Ok("name"));
+        assert_eq!(mailmapped_sig.email(), Ok("prøper@emæil"));
     }
 }