Merge branch 'clone'

Author: Byron

README.md

@@ -52,7 +52,10 @@ Please see _'Development Status'_ for a listing of all crates and their capabili
     * **remote**    
         * [x] **refs** - list all references available on the remote based on the current remote configuration.
         * [x] **ref-map** - show how remote references relate to their local tracking branches as mapped by refspecs.
-    * **fetch** - fetch the current remote or the given one, optionally just as dry-run.    
+    * [x] **fetch** - fetch the current remote or the given one, optionally just as dry-run.
+    * **clone**
+        * [ ] initialize a new **bare** repository and fetch all objects.
+        * [ ] initialize a new repository, fetch all objects and checkout the main worktree.
     * **credential**
         * [x] **fill/approve/reject** - The same as `git credential`, but implemented in Rust, calling helpers only when from trusted configuration.
     * **free** - no git repository necessary

etc/check-package-size.sh

@@ -57,6 +57,6 @@ echo "in root: gitoxide CLI"
 (enter git-odb && indent cargo diet -n --package-size-limit 120KB)
 (enter git-protocol && indent cargo diet -n --package-size-limit 50KB)
 (enter git-packetline && indent cargo diet -n --package-size-limit 35KB)
-(enter git-repository && indent cargo diet -n --package-size-limit 185KB)
+(enter git-repository && indent cargo diet -n --package-size-limit 200KB)
 (enter git-transport && indent cargo diet -n --package-size-limit 55KB)
 (enter gitoxide-core && indent cargo diet -n --package-size-limit 90KB)

git-config/src/file/access/mutate.rs

@@ -2,6 +2,7 @@ use std::borrow::Cow;
 
 use git_features::threading::OwnShared;
 
+use crate::file::SectionBodyIdsLut;
 use crate::{
     file::{self, rename_section, write::ends_with_newline, MetadataFilter, SectionId, SectionMut},
     lookup,
@@ -11,7 +12,7 @@ use crate::{
 
 /// Mutating low-level access methods.
 impl<'event> File<'event> {
-    /// Returns an mutable section with a given `name` and optional `subsection_name`, _if it exists_.
+    /// Returns the last mutable section with a given `name` and optional `subsection_name`, _if it exists_.
     pub fn section_mut<'a>(
         &'a mut self,
         name: impl AsRef<str>,
@@ -29,7 +30,16 @@ impl<'event> File<'event> {
             .expect("BUG: Section did not have id from lookup")
             .to_mut(nl))
     }
-    /// Returns an mutable section with a given `name` and optional `subsection_name`, _if it exists_, or create a new section.
+
+    /// Return the mutable section identified by `id`, or `None` if it didn't exist.
+    ///
+    /// Note that `id` is stable across deletions and insertions.
+    pub fn section_mut_by_id<'a>(&'a mut self, id: SectionId) -> Option<SectionMut<'a, 'event>> {
+        let nl = self.detect_newline_style_smallvec();
+        self.sections.get_mut(&id).map(|s| s.to_mut(nl))
+    }
+
+    /// Returns the last mutable section with a given `name` and optional `subsection_name`, _if it exists_, or create a new section.
     pub fn section_mut_or_create_new<'a>(
         &'a mut self,
         name: impl AsRef<str>,
@@ -182,13 +192,39 @@ impl<'event> File<'event> {
             .ok()?
             .rev()
             .next()?;
-        self.section_order.remove(
-            self.section_order
-                .iter()
-                .position(|v| *v == id)
-                .expect("known section id"),
-        );
-        self.sections.remove(&id)
+        self.remove_section_by_id(id)
+    }
+
+    /// Remove the section identified by `id` if it exists and return it, or return `None` if no such section was present.
+    ///
+    /// Note that section ids are unambiguous even in the face of removals and additions of sections.
+    pub fn remove_section_by_id(&mut self, id: SectionId) -> Option<file::Section<'event>> {
+        self.section_order
+            .remove(self.section_order.iter().position(|v| *v == id)?);
+        let section = self.sections.remove(&id)?;
+        let lut = self
+            .section_lookup_tree
+            .get_mut(&section.header.name)
+            .expect("lookup cache still has name to be deleted");
+        for entry in lut {
+            match section.header.subsection_name.as_deref() {
+                Some(subsection_name) => {
+                    if let SectionBodyIdsLut::NonTerminal(map) = entry {
+                        if let Some(ids) = map.get_mut(subsection_name) {
+                            ids.remove(ids.iter().position(|v| *v == id).expect("present"));
+                            break;
+                        }
+                    }
+                }
+                None => {
+                    if let SectionBodyIdsLut::Terminal(ids) = entry {
+                        ids.remove(ids.iter().position(|v| *v == id).expect("present"));
+                        break;
+                    }
+                }
+            }
+        }
+        Some(section)
     }
 
     /// Removes the section with `name` and `subsection_name` that passed `filter`, returning the removed section

git-config/src/file/access/read_only.rs

@@ -1,9 +1,10 @@
-use std::{borrow::Cow, convert::TryFrom, iter::FromIterator};
+use std::{borrow::Cow, convert::TryFrom};
 
 use bstr::BStr;
 use git_features::threading::OwnShared;
 use smallvec::SmallVec;
 
+use crate::file::SectionId;
 use crate::{
     file,
     file::{
@@ -206,6 +207,25 @@ impl<'event> File<'event> {
         })
     }
 
+    /// Similar to [`sections_by_name()`][Self::sections_by_name()], but returns an identifier for this section as well to allow
+    /// referring to it unambiguously even in the light of deletions.
+    #[must_use]
+    pub fn sections_and_ids_by_name<'a>(
+        &'a self,
+        name: &'a str,
+    ) -> Option<impl Iterator<Item = (&file::Section<'event>, SectionId)> + '_> {
+        self.section_ids_by_name(name).ok().map(move |ids| {
+            ids.map(move |id| {
+                (
+                    self.sections
+                        .get(&id)
+                        .expect("section doesn't have id from from lookup"),
+                    id,
+                )
+            })
+        })
+    }
+
     /// Gets all sections that match the provided `name`, ignoring any subsections, and pass the `filter`.
     #[must_use]
     pub fn sections_by_name_and_filter<'a>(
@@ -258,6 +278,11 @@ impl<'event> File<'event> {
         self.section_order.iter().map(move |id| &self.sections[id])
     }
 
+    /// Return an iterator over all sections and their ids, in order of occurrence in the file itself.
+    pub fn sections_and_ids(&self) -> impl Iterator<Item = (&file::Section<'event>, SectionId)> + '_ {
+        self.section_order.iter().map(move |id| (&self.sections[id], *id))
+    }
+
     /// Return an iterator over all sections along with non-section events that are placed right after them,
     /// in order of occurrence in the file itself.
     ///
@@ -296,6 +321,6 @@ impl<'event> File<'event> {
     }
 
     pub(crate) fn detect_newline_style_smallvec(&self) -> SmallVec<[u8; 2]> {
-        SmallVec::from_iter(self.detect_newline_style().iter().copied())
+        self.detect_newline_style().as_ref().into()
     }
 }

git-config/src/file/mod.rs

@@ -110,7 +110,7 @@ impl AddAssign<usize> for Size {
 /// words, it's possible that a section may have an ID of 3 but the next section
 /// has an ID of 5 as 4 was deleted.
 #[derive(PartialEq, Eq, Hash, Copy, Clone, PartialOrd, Ord, Debug)]
-pub(crate) struct SectionId(pub(crate) usize);
+pub struct SectionId(pub(crate) usize);
 
 /// All section body ids referred to by a section name.
 ///

git-config/src/file/mutable/section.rs

@@ -95,7 +95,7 @@ impl<'a, 'event> SectionMut<'a, 'event> {
             Some((key_range, value_range)) => {
                 let value_range = value_range.unwrap_or(key_range.end - 1..key_range.end);
                 let range_start = value_range.start;
-                let ret = self.remove_internal(value_range);
+                let ret = self.remove_internal(value_range, false);
                 self.section
                     .body
                     .0
@@ -109,7 +109,7 @@ impl<'a, 'event> SectionMut<'a, 'event> {
     pub fn remove(&mut self, key: impl AsRef<str>) -> Option<Cow<'event, BStr>> {
         let key = Key::from_str_unchecked(key.as_ref());
         let (key_range, _value_range) = self.key_and_value_range_by(&key)?;
-        Some(self.remove_internal(key_range))
+        Some(self.remove_internal(key_range, true))
     }
 
     /// Adds a new line event. Note that you don't need to call this unless
@@ -231,17 +231,33 @@ impl<'a, 'event> SectionMut<'a, 'event> {
     }
 
     /// Performs the removal, assuming the range is valid.
-    fn remove_internal(&mut self, range: Range<usize>) -> Cow<'event, BStr> {
-        self.section
-            .body
-            .0
-            .drain(range)
-            .fold(Cow::Owned(BString::default()), |mut acc, e| {
+    fn remove_internal(&mut self, range: Range<usize>, fix_whitespace: bool) -> Cow<'event, BStr> {
+        let events = &mut self.section.body.0;
+        if fix_whitespace
+            && events
+                .get(range.end)
+                .map_or(false, |ev| matches!(ev, Event::Newline(_)))
+        {
+            events.remove(range.end);
+        }
+        let value = events
+            .drain(range.clone())
+            .fold(Cow::Owned(BString::default()), |mut acc: Cow<'_, BStr>, e| {
                 if let Event::Value(v) | Event::ValueNotDone(v) | Event::ValueDone(v) = e {
                     acc.to_mut().extend(&**v);
                 }
                 acc
-            })
+            });
+        if fix_whitespace
+            && range
+                .start
+                .checked_sub(1)
+                .and_then(|pos| events.get(pos))
+                .map_or(false, |ev| matches!(ev, Event::Whitespace(_)))
+        {
+            events.remove(range.start - 1);
+        }
+        value
     }
 }
 

git-config/src/parse/section/header.rs

@@ -41,9 +41,13 @@ impl<'a> Header<'a> {
     }
 }
 
+/// Return true if `name` is valid as subsection name, like `origin` in `[remote "origin"]`.
+pub fn is_valid_subsection(name: &BStr) -> bool {
+    name.find_byteset(b"\n\0").is_none()
+}
+
 fn validated_subsection(name: Cow<'_, BStr>) -> Result<Cow<'_, BStr>, Error> {
-    name.find_byteset(b"\n\0")
-        .is_none()
+    is_valid_subsection(name.as_ref())
         .then(|| name)
         .ok_or(Error::InvalidSubSection)
 }
@@ -55,6 +59,24 @@ fn validated_name(name: Cow<'_, BStr>) -> Result<Cow<'_, BStr>, Error> {
         .ok_or(Error::InvalidName)
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn empty_header_names_are_legal() {
+        assert!(Header::new("", None).is_ok(), "yes, git allows this, so do we");
+    }
+
+    #[test]
+    fn empty_header_sub_names_are_legal() {
+        assert!(
+            Header::new("remote", Some("".into())).is_ok(),
+            "yes, git allows this, so do we"
+        );
+    }
+}
+
 impl Header<'_> {
     ///Return true if this is a header like `[legacy.subsection]`, or false otherwise.
     pub fn is_legacy(&self) -> bool {

git-config/src/parse/section/mod.rs

@@ -160,10 +160,10 @@ mod types {
     generate_case_insensitive!(
         Name,
         name,
-        "Valid names consist alphanumeric characters or dashes.",
+        "Valid names consist of alphanumeric characters or dashes.",
         is_valid_name,
         bstr::BStr,
-        "Wrapper struct for section header names, like `includeIf`, since these are case-insensitive."
+        "Wrapper struct for section header names, like `remote`, since these are case-insensitive."
     );
 
     generate_case_insensitive!(

git-config/tests/file/access/mutate.rs

@@ -1,3 +1,51 @@
+mod remove_section {
+    use std::convert::TryFrom;
+
+    #[test]
+    fn removal_of_all_sections_programmatically_with_sections_and_ids_by_name() {
+        let mut file = git_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap();
+        for id in file
+            .sections_and_ids_by_name("core")
+            .expect("2 sections present")
+            .map(|(_, id)| id)
+            .collect::<Vec<_>>()
+        {
+            file.remove_section_by_id(id);
+        }
+        assert!(file.is_void());
+        assert_eq!(file.sections().count(), 0);
+    }
+
+    #[test]
+    fn removal_of_all_sections_programmatically_with_sections_and_ids() {
+        let mut file = git_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap();
+        for id in file.sections_and_ids().map(|(_, id)| id).collect::<Vec<_>>() {
+            file.remove_section_by_id(id);
+        }
+        assert!(file.is_void());
+        assert_eq!(file.sections().count(), 0);
+    }
+
+    #[test]
+    fn removal_is_complete_and_sections_can_be_readded() {
+        let mut file = git_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap();
+        assert_eq!(file.sections().count(), 2);
+
+        let removed = file.remove_section("core", None).expect("removed correct section");
+        assert_eq!(removed.header().name(), "core");
+        assert_eq!(removed.header().subsection_name(), None);
+        assert_eq!(file.sections().count(), 1);
+
+        let removed = file.remove_section("core", Some("name")).expect("found");
+        assert_eq!(removed.header().name(), "core");
+        assert_eq!(removed.header().subsection_name().expect("present"), "name");
+        assert_eq!(file.sections().count(), 0);
+
+        file.section_mut_or_create_new("core", None).expect("creation succeeds");
+        file.section_mut_or_create_new("core", Some("name"))
+            .expect("creation succeeds");
+    }
+}
 mod rename_section {
     use std::{borrow::Cow, convert::TryFrom};
 

git-config/tests/file/mutable/section.rs

@@ -28,6 +28,15 @@ fn section_mut_or_create_new_filter_may_reject_existing_sections() -> crate::Res
     Ok(())
 }
 
+#[test]
+fn section_mut_by_id() {
+    let mut config = multi_value_section();
+    let id = config.sections_and_ids().next().expect("at least one").1;
+    let section = config.section_mut_by_id(id).expect("present");
+    assert_eq!(section.header().name(), "a");
+    assert_eq!(section.header().subsection_name(), None);
+}
+
 mod remove {
     use super::multi_value_section;
 
@@ -49,10 +58,7 @@ mod remove {
         }
 
         assert!(!section.is_void(), "everything is still there");
-        assert_eq!(
-            config.to_string(),
-            "\n        [a]\n            \n            \n            \n            \n            "
-        );
+        assert_eq!(config.to_string(), "\n        [a]\n");
         Ok(())
     }
 }