refactor (#450)

Author: Byron

git-refspec/src/spec.rs

@@ -1,6 +1,5 @@
 use crate::types::Push;
 use crate::{Fetch, Instruction, Mode, Operation, RefSpec, RefSpecRef};
-use bstr::BStr;
 
 /// Access
 impl RefSpecRef<'_> {
@@ -11,28 +10,15 @@ impl RefSpecRef<'_> {
 
     /// Transform the state of the refspec into an instruction making clear what to do with it.
     pub fn instruction(&self) -> Instruction<'_> {
-        fn has_pattern(item: &BStr) -> bool {
-            item.contains(&b'*')
-        }
         match self.op {
             Operation::Fetch => match (self.mode, self.src, self.dst) {
                 (Mode::Normal | Mode::Force, Some(src), None) => Instruction::Fetch(Fetch::Only { src }),
-                (Mode::Normal | Mode::Force, Some(src), Some(dst)) if has_pattern(src) => {
-                    Instruction::Fetch(Fetch::AndUpdateMultipleWithGlob {
-                        src,
-                        dst,
-                        allow_non_fast_forward: matches!(self.mode, Mode::Force),
-                    })
-                }
-                (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Fetch(Fetch::AndUpdateSingle {
+                (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Fetch(Fetch::AndUpdate {
                     src,
                     dst,
                     allow_non_fast_forward: matches!(self.mode, Mode::Force),
                 }),
-                (Mode::Negative, Some(src), None) if has_pattern(src) => {
-                    Instruction::Fetch(Fetch::ExcludeMultipleWithGlob { src })
-                }
-                (Mode::Negative, Some(src), None) => Instruction::Fetch(Fetch::ExcludeSingle { src }),
+                (Mode::Negative, Some(src), None) => Instruction::Fetch(Fetch::Exclude { src }),
                 (mode, src, dest) => {
                     unreachable!(
                         "BUG: fetch instructions with {:?} {:?} {:?} are not possible",
@@ -41,7 +27,7 @@ impl RefSpecRef<'_> {
                 }
             },
             Operation::Push => match (self.mode, self.src, self.dst) {
-                (Mode::Normal | Mode::Force, Some(src), None) => Instruction::Push(Push::Single {
+                (Mode::Normal | Mode::Force, Some(src), None) => Instruction::Push(Push::Matching {
                     src,
                     dst: src,
                     allow_non_fast_forward: matches!(self.mode, Mode::Force),
@@ -52,22 +38,12 @@ impl RefSpecRef<'_> {
                 (Mode::Normal | Mode::Force, None, None) => Instruction::Push(Push::AllMatchingBranches {
                     allow_non_fast_forward: matches!(self.mode, Mode::Force),
                 }),
-                (Mode::Normal | Mode::Force, Some(src), Some(dst)) if has_pattern(src) => {
-                    Instruction::Push(Push::MultipleWithGlob {
-                        src,
-                        dst,
-                        allow_non_fast_forward: matches!(self.mode, Mode::Force),
-                    })
-                }
-                (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Push(Push::Single {
+                (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Push(Push::Matching {
                     src,
                     dst,
                     allow_non_fast_forward: matches!(self.mode, Mode::Force),
                 }),
-                (Mode::Negative, Some(src), None) if has_pattern(src) => {
-                    Instruction::Push(Push::ExcludeMultipleWithGlob { src })
-                }
-                (Mode::Negative, Some(src), None) => Instruction::Push(Push::ExcludeSingle { src }),
+                (Mode::Negative, Some(src), None) => Instruction::Push(Push::Exclude { src }),
                 (mode, src, dest) => {
                     unreachable!(
                         "BUG: push instructions with {:?} {:?} {:?} are not possible",

git-refspec/src/types.rs

@@ -26,7 +26,8 @@ pub enum Instruction<'a> {
     Fetch(Fetch<'a>),
 }
 
-/// Note that all sources can either be a ref-name, partial or full, or a rev-spec. Destinations can only be a partial or full ref name.
+/// Note that all sources can either be a ref-name, partial or full, or a rev-spec, unless specified otherwise, on the local side.
+/// Destinations can only be a partial or full ref names on the remote side.
 #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)]
 pub enum Push<'a> {
     /// Push all local branches to the matching destination on the remote, which has to exist to be updated.
@@ -40,67 +41,40 @@ pub enum Push<'a> {
         ref_or_pattern: &'a BStr,
     },
     /// Exclude a single ref.
-    ExcludeSingle {
-        /// A single full ref name to exclude.
-        src: &'a BStr,
-    },
-    /// Exclude multiple refs with single `*` glob.
-    ExcludeMultipleWithGlob {
-        /// A ref pattern with a single `*`.
+    Exclude {
+        /// A full or partial ref name to exclude, or multiple if a single `*` is used.
         src: &'a BStr,
     },
     /// Push a single ref or refspec to a known destination ref.
-    Single {
-        /// The source ref or refspec to push.
-        src: &'a BStr,
-        /// The ref to update with the object from `src`.
-        dst: &'a BStr,
-        /// If true, allow non-fast-forward updates of `dest`.
-        allow_non_fast_forward: bool,
-    },
-    /// Push a multiple refs to matching destination refs, with exactly a single glob on both sides.
-    MultipleWithGlob {
-        /// The source ref to match against all refs for pushing, as pattern with a single `*`.
+    Matching {
+        /// The source ref or refspec to push. If pattern, it contains a single `*`.
         src: &'a BStr,
-        /// The ref to update with object obtained from `src`, filling in the `*` with the portion that matched in `src`.
+        /// The ref to update with the object from `src`. If `src`  is a pattern, this is a pattern too.
         dst: &'a BStr,
         /// If true, allow non-fast-forward updates of `dest`.
         allow_non_fast_forward: bool,
     },
 }
 
-/// Note that any source can either be a ref name (full or partial) or a fully spelled out hex-sha for an object.
+/// Note that any source can either be a ref name (full or partial) or a fully spelled out hex-sha for an object, on the remote side.
 ///
-/// Destinations can only be a partial or full ref-name.
+/// Destinations can only be a partial or full ref-names on the local side.
 #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)]
 pub enum Fetch<'a> {
     Only {
         /// The ref name to fetch on the remote side, without updating the local side. This will write the result into `FETCH_HEAD`.
         src: &'a BStr,
     },
     /// Exclude a single ref.
-    ExcludeSingle {
-        /// A single full ref name to exclude.
+    Exclude {
+        /// A single partial or full ref name to exclude on the remote, or a pattern with a single `*`.
         src: &'a BStr,
     },
-    /// Exclude multiple refs with single `*` glob.
-    ExcludeMultipleWithGlob {
-        /// A ref pattern with a single `*`.
-        src: &'a BStr,
-    },
-    AndUpdateSingle {
-        /// The ref name to fetch on the remote side.
-        src: &'a BStr,
-        /// The local destination to update with what was fetched.
-        dst: &'a BStr,
-        /// If true, allow non-fast-forward updates of `dest`.
-        allow_non_fast_forward: bool,
-    },
-    /// Similar to `FetchAndUpdate`, but src and destination contain a single glob to fetch and update multiple refs.
-    AndUpdateMultipleWithGlob {
-        /// The ref glob to match against all refs on the remote side for fetching, as pattern with a single `*`.
+    AndUpdate {
+        /// The ref name to fetch on the remote side, or a pattern with a single `*` to match against.
         src: &'a BStr,
-        /// The local destination to update with what was fetched by replacing the single `*` with the matching portion from `src`.
+        /// The local destination to update with what was fetched, or a pattern whose single `*` will be replaced with the matching portion
+        /// of the `*` from `src`.
         dst: &'a BStr,
         /// If true, allow non-fast-forward updates of `dest`.
         allow_non_fast_forward: bool,

git-refspec/tests/parse/fetch.rs

@@ -0,0 +1,89 @@
+use crate::parse::{assert_parse, b};
+use git_refspec::{Fetch, Instruction, Mode};
+
+#[test]
+fn exclude() {
+    assert_parse("^a", Instruction::Fetch(Fetch::Exclude { src: b("a") }));
+    assert_parse("^a*", Instruction::Fetch(Fetch::Exclude { src: b("a*") }));
+}
+
+#[test]
+fn lhs_colon_empty_fetches_only() {
+    assert_parse("src:", Instruction::Fetch(Fetch::Only { src: b("src") }));
+    let spec = assert_parse("+src:", Instruction::Fetch(Fetch::Only { src: b("src") }));
+    assert_eq!(
+        spec.mode(),
+        Mode::Force,
+        "force is set, even though it has no effect in the actual instruction"
+    );
+}
+
+#[test]
+fn lhs_colon_rhs_updates_single_ref() {
+    assert_parse(
+        "a:b",
+        Instruction::Fetch(Fetch::AndUpdate {
+            src: b("a"),
+            dst: b("b"),
+            allow_non_fast_forward: false,
+        }),
+    );
+    assert_parse(
+        "+a:b",
+        Instruction::Fetch(Fetch::AndUpdate {
+            src: b("a"),
+            dst: b("b"),
+            allow_non_fast_forward: true,
+        }),
+    );
+
+    assert_parse(
+        "a/*:b/*",
+        Instruction::Fetch(Fetch::AndUpdate {
+            src: b("a/*"),
+            dst: b("b/*"),
+            allow_non_fast_forward: false,
+        }),
+    );
+    assert_parse(
+        "+a/*:b/*",
+        Instruction::Fetch(Fetch::AndUpdate {
+            src: b("a/*"),
+            dst: b("b/*"),
+            allow_non_fast_forward: true,
+        }),
+    );
+}
+
+#[test]
+fn empty_lhs_colon_rhs_fetches_head_to_destination() {
+    assert_parse(
+        ":a",
+        Instruction::Fetch(Fetch::AndUpdate {
+            src: b("HEAD"),
+            dst: b("a"),
+            allow_non_fast_forward: false,
+        }),
+    );
+
+    assert_parse(
+        "+:a",
+        Instruction::Fetch(Fetch::AndUpdate {
+            src: b("HEAD"),
+            dst: b("a"),
+            allow_non_fast_forward: true,
+        }),
+    );
+}
+
+#[test]
+fn colon_alone_is_for_fetching_head_into_fetchhead() {
+    assert_parse(":", Instruction::Fetch(Fetch::Only { src: b("HEAD") }));
+    let spec = assert_parse("+:", Instruction::Fetch(Fetch::Only { src: b("HEAD") }));
+    assert_eq!(spec.mode(), Mode::Force, "it's set even though it's not useful");
+}
+
+#[test]
+fn empty_refspec_is_enough_for_fetching_head_into_fetchhead() {
+    assert_parse("", Instruction::Fetch(Fetch::Only { src: b("HEAD") }));
+}