[ref #140] refactor

git-ref/src/store/file/loose/reference/peel.rs

@@ -29,7 +29,7 @@ impl loose::Reference {
     /// Follow this symbolic reference one level and return the ref it refers to, possibly providing access to `packed` references for lookup.
     ///
     /// Returns `None` if this is not a symbolic reference, hence the leaf of the chain.
-    pub fn peel_one_level<'p>(
+    pub fn follow_symbolic<'p>(
         &self,
         store: &file::Store,
         packed: Option<&'p packed::Buffer>,
@@ -81,20 +81,19 @@ pub mod to_id {
     }
 
     impl loose::Reference {
-        /// Peel this symbolic reference until the end of the chain is reached and an object ID is available,
-        /// possibly providing access to `packed` references for lookup.
+        /// Follow this symbolic reference until the end of the chain is reached and an object ID is available,
+        /// and possibly peel this object until the final target object is revealed.
         ///
         /// If an error occurs this reference remains unchanged.
         pub fn peel_to_id_in_place(
             &mut self,
             store: &file::Store,
             packed: Option<&packed::Buffer>,
         ) -> Result<&oid, Error> {
-            let mut count = 0;
             let mut seen = BTreeSet::new();
             let mut storage;
             let mut cursor = &mut *self;
-            while let Some(next) = cursor.peel_one_level(store, packed) {
+            while let Some(next) = cursor.follow_symbolic(store, packed) {
                 let next_ref = next?;
                 if let crate::Kind::Peeled = next_ref.kind() {
                     match next_ref {
@@ -104,7 +103,7 @@ pub mod to_id {
                             self.name = FullName(p.name.0.to_owned());
                         }
                     };
-                    return Ok(self.target.as_id().expect("it to be present"));
+                    break;
                 }
                 storage = next_ref;
                 cursor = match &mut storage {
@@ -115,9 +114,8 @@ pub mod to_id {
                     return Err(Error::Cycle(store.base.join(cursor.name.to_path())));
                 }
                 seen.insert(cursor.name.clone());
-                count += 1;
                 const MAX_REF_DEPTH: usize = 5;
-                if count == MAX_REF_DEPTH {
+                if seen.len() == MAX_REF_DEPTH {
                     return Err(Error::DepthLimitExceeded {
                         max_depth: MAX_REF_DEPTH,
                     });

git-ref/src/store/file/reference.rs

@@ -76,7 +76,7 @@ impl<'p> Reference<'p> {
         packed: Option<&'p2 packed::Buffer>,
     ) -> Option<Result<Reference<'p2>, crate::store::file::loose::reference::peel::Error>> {
         match self {
-            Reference::Loose(r) => r.peel_one_level(store, packed),
+            Reference::Loose(r) => r.follow_symbolic(store, packed),
             Reference::Packed(p) => packed
                 .and_then(|packed| packed.find(p.name).ok().flatten()) // needed to get data with 'p2 lifetime
                 .and_then(|np| {

git-ref/src/store/file/transaction/mod.rs

@@ -8,7 +8,7 @@ use git_hash::ObjectId;
 /// A function receiving an object id to resolve, returning its decompressed bytes.
 ///
 /// Resolution means to follow tag objects until the end of the chain.
-pub type ObjectResolveFn = dyn FnMut(
+pub type FindObjectFn = dyn FnMut(
     git_hash::ObjectId,
     &mut Vec<u8>,
 ) -> Result<Option<git_object::Kind>, Box<dyn std::error::Error + 'static>>;
@@ -18,10 +18,10 @@ pub enum PackedRefs {
     /// Only propagate deletions of references. This is the default
     DeletionsOnly,
     /// Propagate deletions as well as updates to references which are peeled, that is contain an object id
-    DeletionsAndNonSymbolicUpdates(Box<ObjectResolveFn>),
+    DeletionsAndNonSymbolicUpdates(Box<FindObjectFn>),
     /// Propagate deletions as well as updates to references which are peeled, that is contain an object id. Furthermore delete the
     /// reference which is originally updated if it exists. If it doesn't, the new value will be written into the packed ref right away.
-    DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(Box<ObjectResolveFn>),
+    DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(Box<FindObjectFn>),
 }
 
 impl Default for PackedRefs {

git-ref/src/store/packed/transaction.rs

@@ -1,6 +1,6 @@
 use crate::{
     mutable::Target,
-    store::{file::transaction::ObjectResolveFn, packed, packed::Edit},
+    store::{file::transaction::FindObjectFn, packed, packed::Edit},
     transaction::{Change, RefEdit},
 };
 use std::io::Write;
@@ -42,7 +42,7 @@ impl packed::Transaction {
     pub fn prepare(
         mut self,
         edits: impl IntoIterator<Item = RefEdit>,
-        find: &mut ObjectResolveFn,
+        find: &mut FindObjectFn,
     ) -> Result<Self, prepare::Error> {
         assert!(self.edits.is_none(), "BUG: cannot call prepare(…) more than once");
         let buffer = &self.buffer;

git-ref/tests/file/reference.rs

@@ -60,14 +60,14 @@ mod peel {
         assert_eq!(r.kind(), git_ref::Kind::Symbolic, "there is something to peel");
 
         let nr = git_ref::file::loose::Reference::try_from(
-            r.peel_one_level(&store, None).expect("exists").expect("no failure"),
+            r.follow_symbolic(&store, None).expect("exists").expect("no failure"),
         )
         .expect("loose ref");
         assert!(
             matches!(nr.target.borrow(), git_ref::Target::Peeled(_)),
             "iteration peels a single level"
         );
-        assert!(nr.peel_one_level(&store, None).is_none(), "end of iteration");
+        assert!(nr.follow_symbolic(&store, None).is_none(), "end of iteration");
         assert_eq!(
             nr.target.borrow(),
             git_ref::Target::Peeled(&hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03")),

git-ref/tests/file/transaction/prepare_and_commit/delete.rs

@@ -158,7 +158,7 @@ fn delete_reflog_only_of_symbolic_no_deref() -> crate::Result {
     assert!(main.log_exists(&store), "log is untouched, too");
     assert_eq!(
         main.target,
-        head.peel_one_level(&store, None).expect("a symref")?.target(),
+        head.follow_symbolic(&store, None).expect("a symref")?.target(),
         "head points to main"
     );
     Ok(())
@@ -192,7 +192,7 @@ fn delete_reflog_only_of_symbolic_with_deref() -> crate::Result {
     assert!(!main.log_exists(&store), "log is removed");
     assert_eq!(
         main.target,
-        head.peel_one_level(&store, None).expect("a symref")?.target(),
+        head.follow_symbolic(&store, None).expect("a symref")?.target(),
         "head points to main"
     );
     Ok(())