From e8c072e99e845ed1b4a0cc0a0ec7146c53561dcd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 20:41:04 +0800 Subject: [PATCH] refactor (#450) --- git-refspec/src/spec.rs | 34 +----- git-refspec/src/types.rs | 56 +++------- git-refspec/tests/parse/fetch.rs | 89 +++++++++++++++ git-refspec/tests/parse/mod.rs | 185 +------------------------------ git-refspec/tests/parse/push.rs | 71 ++++++++++++ 5 files changed, 182 insertions(+), 253 deletions(-) create mode 100644 git-refspec/tests/parse/fetch.rs create mode 100644 git-refspec/tests/parse/push.rs diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index 25e1ce5c74..f74b669776 100644 --- a/git-refspec/src/spec.rs +++ b/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", diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index 348dfc5427..969663c034 100644 --- a/git-refspec/src/types.rs +++ b/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,38 +41,24 @@ 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 { @@ -79,28 +66,15 @@ pub enum Fetch<'a> { 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, diff --git a/git-refspec/tests/parse/fetch.rs b/git-refspec/tests/parse/fetch.rs new file mode 100644 index 0000000000..0a9ec05ef6 --- /dev/null +++ b/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") })); +} diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 6b2cdac8db..174019a9cc 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -108,189 +108,8 @@ mod invalid { } } -mod fetch { - use crate::parse::{assert_parse, b}; - use git_refspec::{Fetch, Instruction, Mode}; - - #[test] - fn exclude_single() { - assert_parse("^a", Instruction::Fetch(Fetch::ExcludeSingle { src: b("a") })); - } - - #[test] - fn exclude_multiple() { - assert_parse( - "^a*", - Instruction::Fetch(Fetch::ExcludeMultipleWithGlob { 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::AndUpdateSingle { - src: b("a"), - dst: b("b"), - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+a:b", - Instruction::Fetch(Fetch::AndUpdateSingle { - src: b("a"), - dst: b("b"), - allow_non_fast_forward: true, - }), - ); - } - - #[test] - fn lhs_colon_rhs_with_glob_updates_multiple_refs() { - assert_parse( - "a/*:b/*", - Instruction::Fetch(Fetch::AndUpdateMultipleWithGlob { - src: b("a/*"), - dst: b("b/*"), - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+a/*:b/*", - Instruction::Fetch(Fetch::AndUpdateMultipleWithGlob { - 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::AndUpdateSingle { - src: b("HEAD"), - dst: b("a"), - allow_non_fast_forward: false, - }), - ); - - assert_parse( - "+:a", - Instruction::Fetch(Fetch::AndUpdateSingle { - 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") })); - } -} - -mod push { - use crate::parse::{assert_parse, b}; - use git_refspec::{Instruction, Mode, Push}; - - #[test] - fn exclude_single() { - assert_parse("^a", Instruction::Push(Push::ExcludeSingle { src: b("a") })); - } - - #[test] - fn exclude_multiple() { - assert_parse("^a*", Instruction::Push(Push::ExcludeMultipleWithGlob { src: b("a*") })); - } - - #[test] - fn lhs_colon_rhs_pushes_single_ref() { - assert_parse( - "a:b", - Instruction::Push(Push::Single { - src: b("a"), - dst: b("b"), - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+a:b", - Instruction::Push(Push::Single { - src: b("a"), - dst: b("b"), - allow_non_fast_forward: true, - }), - ); - } - - #[test] - fn lhs_colon_rhs_with_glob_pushes_multiple_refs() { - assert_parse( - "a/*:b/*", - Instruction::Push(Push::MultipleWithGlob { - src: b("a/*"), - dst: b("b/*"), - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+a/*:b/*", - Instruction::Push(Push::MultipleWithGlob { - src: b("a/*"), - dst: b("b/*"), - allow_non_fast_forward: true, - }), - ); - } - - #[test] - fn colon_alone_is_for_pushing_matching_refs() { - assert_parse( - ":", - Instruction::Push(Push::AllMatchingBranches { - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+:", - Instruction::Push(Push::AllMatchingBranches { - allow_non_fast_forward: true, - }), - ); - } - - #[test] - fn delete() { - assert_parse(":a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); - let spec = assert_parse("+:a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); - assert_eq!( - spec.mode(), - Mode::Force, - "force is set, even though it has no effect in the actual instruction" - ); - } -} +mod fetch; +mod push; mod util { use git_refspec::{Instruction, Operation, RefSpecRef}; diff --git a/git-refspec/tests/parse/push.rs b/git-refspec/tests/parse/push.rs new file mode 100644 index 0000000000..bcda808f97 --- /dev/null +++ b/git-refspec/tests/parse/push.rs @@ -0,0 +1,71 @@ +use crate::parse::{assert_parse, b}; +use git_refspec::{Instruction, Mode, Push}; + +#[test] +fn exclude() { + assert_parse("^a", Instruction::Push(Push::Exclude { src: b("a") })); + assert_parse("^a*", Instruction::Push(Push::Exclude { src: b("a*") })); +} + +#[test] +fn lhs_colon_rhs_pushes_single_ref() { + assert_parse( + "a:b", + Instruction::Push(Push::Matching { + src: b("a"), + dst: b("b"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+a:b", + Instruction::Push(Push::Matching { + src: b("a"), + dst: b("b"), + allow_non_fast_forward: true, + }), + ); + assert_parse( + "a/*:b/*", + Instruction::Push(Push::Matching { + src: b("a/*"), + dst: b("b/*"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+a/*:b/*", + Instruction::Push(Push::Matching { + src: b("a/*"), + dst: b("b/*"), + allow_non_fast_forward: true, + }), + ); +} + +#[test] +fn colon_alone_is_for_pushing_matching_refs() { + assert_parse( + ":", + Instruction::Push(Push::AllMatchingBranches { + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+:", + Instruction::Push(Push::AllMatchingBranches { + allow_non_fast_forward: true, + }), + ); +} + +#[test] +fn delete() { + assert_parse(":a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); + let spec = assert_parse("+:a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); + assert_eq!( + spec.mode(), + Mode::Force, + "force is set, even though it has no effect in the actual instruction" + ); +}