Use custom config entry iterator. (#854)

Author: ehuss

src/config.rs

@@ -23,8 +23,39 @@ pub struct ConfigEntry<'cfg> {
 }
 
 /// An iterator over the `ConfigEntry` values of a `Config` structure.
+///
+/// Due to lifetime restrictions, `ConfigEntries` does not implement the
+/// standard [`Iterator`] trait. It provides a [`next`] function which only
+/// allows access to one entry at a time. [`for_each`] is available as a
+/// convenience function.
+///
+/// [`next`]: ConfigEntries::next
+/// [`for_each`]: ConfigEntries::for_each
+///
+/// # Example
+///
+/// ```
+/// // Example of how to collect all entries.
+/// use git2::Config;
+///
+/// let config = Config::new()?;
+/// let iter = config.entries(None)?;
+/// let mut entries = Vec::new();
+/// iter
+///     .for_each(|entry| {
+///         let name = entry.name().unwrap().to_string();
+///         let value = entry.value().unwrap_or("").to_string();
+///         entries.push((name, value))
+///     })?;
+/// for entry in &entries {
+///     println!("{} = {}", entry.0, entry.1);
+/// }
+/// # Ok::<(), git2::Error>(())
+///
+/// ```
 pub struct ConfigEntries<'cfg> {
     raw: *mut raw::git_config_iterator,
+    current: Option<ConfigEntry<'cfg>>,
     _marker: marker::PhantomData<&'cfg Config>,
 }
 
@@ -280,15 +311,18 @@ impl Config {
     /// the variable name: the section and variable parts are lower-cased. The
     /// subsection is left unchanged.
     ///
+    /// Due to lifetime restrictions, the returned value does not implement
+    /// the standard [`Iterator`] trait. See [`ConfigEntries`] for more.
+    ///
     /// # Example
     ///
     /// ```
-    /// # #![allow(unstable)]
     /// use git2::Config;
     ///
     /// let cfg = Config::new().unwrap();
     ///
-    /// for entry in &cfg.entries(None).unwrap() {
+    /// let mut entries = cfg.entries(None).unwrap();
+    /// while let Some(entry) = entries.next() {
     ///     let entry = entry.unwrap();
     ///     println!("{} => {}", entry.name().unwrap(), entry.value().unwrap());
     /// }
@@ -317,6 +351,9 @@ impl Config {
     /// The regular expression is applied case-sensitively on the normalized form of
     /// the variable name: the section and variable parts are lower-cased. The
     /// subsection is left unchanged.
+    ///
+    /// Due to lifetime restrictions, the returned value does not implement
+    /// the standard [`Iterator`] trait. See [`ConfigEntries`] for more.
     pub fn multivar(&self, name: &str, regexp: Option<&str>) -> Result<ConfigEntries<'_>, Error> {
         let mut ret = ptr::null_mut();
         let name = CString::new(name)?;
@@ -550,6 +587,7 @@ impl<'cfg> Binding for ConfigEntries<'cfg> {
     unsafe fn from_raw(raw: *mut raw::git_config_iterator) -> ConfigEntries<'cfg> {
         ConfigEntries {
             raw,
+            current: None,
             _marker: marker::PhantomData,
         }
     }
@@ -558,24 +596,33 @@ impl<'cfg> Binding for ConfigEntries<'cfg> {
     }
 }
 
-// entries are only valid until the iterator is freed, so this impl is for
-// `&'b T` instead of `T` to have a lifetime to tie them to.
-//
-// It's also not implemented for `&'b mut T` so we can have multiple entries
-// (ok).
-impl<'cfg, 'b> Iterator for &'b ConfigEntries<'cfg> {
-    type Item = Result<ConfigEntry<'b>, Error>;
-    fn next(&mut self) -> Option<Result<ConfigEntry<'b>, Error>> {
+impl<'cfg> ConfigEntries<'cfg> {
+    /// Advances the iterator and returns the next value.
+    ///
+    /// Returns `None` when iteration is finished.
+    pub fn next(&mut self) -> Option<Result<&ConfigEntry<'cfg>, Error>> {
         let mut raw = ptr::null_mut();
+        drop(self.current.take());
         unsafe {
             try_call_iter!(raw::git_config_next(&mut raw, self.raw));
-            Some(Ok(ConfigEntry {
+            let entry = ConfigEntry {
                 owned: false,
                 raw,
                 _marker: marker::PhantomData,
-            }))
+            };
+            self.current = Some(entry);
+            Some(Ok(self.current.as_ref().unwrap()))
         }
     }
+
+    /// Calls the given closure for each remaining entry in the iterator.
+    pub fn for_each<F: FnMut(&ConfigEntry<'cfg>)>(mut self, mut f: F) -> Result<(), Error> {
+        while let Some(entry) = self.next() {
+            let entry = entry?;
+            f(entry);
+        }
+        Ok(())
+    }
 }
 
 impl<'cfg> Drop for ConfigEntries<'cfg> {
@@ -628,7 +675,8 @@ mod tests {
         assert_eq!(cfg.get_i64("foo.k3").unwrap(), 2);
         assert_eq!(cfg.get_str("foo.k4").unwrap(), "bar");
 
-        for entry in &cfg.entries(None).unwrap() {
+        let mut entries = cfg.entries(None).unwrap();
+        while let Some(entry) = entries.next() {
             let entry = entry.unwrap();
             entry.name();
             entry.value();
@@ -649,39 +697,42 @@ mod tests {
         cfg.set_multivar("foo.baz", "^$", "oki").unwrap();
 
         // `entries` filters by name
-        let mut entries: Vec<String> = cfg
-            .entries(Some("foo.bar"))
+        let mut entries: Vec<String> = Vec::new();
+        cfg.entries(Some("foo.bar"))
             .unwrap()
-            .into_iter()
-            .map(|entry| entry.unwrap().value().unwrap().into())
-            .collect();
+            .for_each(|entry| entries.push(entry.value().unwrap().to_string()))
+            .unwrap();
         entries.sort();
         assert_eq!(entries, ["baz", "quux", "qux"]);
 
         // which is the same as `multivar` without a regex
-        let mut multivals: Vec<String> = cfg
-            .multivar("foo.bar", None)
+        let mut multivals = Vec::new();
+        cfg.multivar("foo.bar", None)
             .unwrap()
-            .into_iter()
-            .map(|entry| entry.unwrap().value().unwrap().into())
-            .collect();
+            .for_each(|entry| multivals.push(entry.value().unwrap().to_string()))
+            .unwrap();
         multivals.sort();
         assert_eq!(multivals, entries);
 
         // yet _with_ a regex, `multivar` filters by value
-        let mut quxish: Vec<String> = cfg
-            .multivar("foo.bar", Some("qu.*x"))
+        let mut quxish = Vec::new();
+        cfg.multivar("foo.bar", Some("qu.*x"))
             .unwrap()
-            .into_iter()
-            .map(|entry| entry.unwrap().value().unwrap().into())
-            .collect();
+            .for_each(|entry| quxish.push(entry.value().unwrap().to_string()))
+            .unwrap();
         quxish.sort();
         assert_eq!(quxish, ["quux", "qux"]);
 
         cfg.remove_multivar("foo.bar", ".*").unwrap();
 
-        assert_eq!(cfg.entries(Some("foo.bar")).unwrap().count(), 0);
-        assert_eq!(cfg.multivar("foo.bar", None).unwrap().count(), 0);
+        let count = |entries: super::ConfigEntries<'_>| -> usize {
+            let mut c = 0;
+            entries.for_each(|_| c += 1).unwrap();
+            c
+        };
+
+        assert_eq!(count(cfg.entries(Some("foo.bar")).unwrap()), 0);
+        assert_eq!(count(cfg.multivar("foo.bar", None).unwrap()), 0);
     }
 
     #[test]