From c1a385271265aa4e66bfcda8196d83a767ba614f Mon Sep 17 00:00:00 2001 From: Riccardo Attilio Galli Date: Thu, 15 Feb 2024 18:19:23 -0800 Subject: [PATCH] Remove UserBoundsList::new --- src/bounds.rs | 46 ++++++++++++++++++++++++---------------------- src/cut_lines.rs | 47 +++++++++++++++-------------------------------- src/options.rs | 4 ++-- 3 files changed, 41 insertions(+), 56 deletions(-) diff --git a/src/bounds.rs b/src/bounds.rs index 238dcbb..d0aec39 100644 --- a/src/bounds.rs +++ b/src/bounds.rs @@ -122,19 +122,20 @@ impl From> for UserBoundsList { let mut rightmost_bound: Option = None; let mut last_bound: Option<&mut UserBounds> = None; - // This is risky, we could end up using last_interesting_field - // internally. Didn't spend much time to figure out how to use - // is_sortable without major refactoring. - if ubl.is_sortable() { - ubl.list.iter_mut().for_each(|bof| { - if let BoundOrFiller::Bound(b) = bof { - if rightmost_bound.is_none() || b.r > rightmost_bound.unwrap() { - rightmost_bound = Some(b.r); - } - - last_bound = Some(b); + let is_sortable = ubl.is_sortable(); + + ubl.list.iter_mut().for_each(|bof| { + if let BoundOrFiller::Bound(b) = bof { + if rightmost_bound.is_none() || b.r > rightmost_bound.unwrap() { + rightmost_bound = Some(b.r); } - }); + + last_bound = Some(b); + } + }); + + if !is_sortable { + rightmost_bound = None; } last_bound @@ -149,15 +150,14 @@ impl From> for UserBoundsList { impl FromStr for UserBoundsList { type Err = anyhow::Error; fn from_str(s: &str) -> Result { + if s.trim().is_empty() { + bail!("UserBoundsList must contain at least one UserBounds"); + } Ok(parse_bounds_list(s)?.into()) } } impl UserBoundsList { - pub fn new(list: Vec) -> Self { - list.into() - } - /// Detect whether the list can be sorted. /// It can be sorted only if every bound /// has the same sign (all positive or all negative). @@ -423,14 +423,15 @@ impl UserBoundsTrait for UserBounds { /// # use tuc::bounds::{UserBounds, UserBoundsTrait}; /// # use std::ops::Range; /// # use tuc::bounds::Side; + /// # use std::str::FromStr; /// /// assert_eq!( - /// (UserBounds { l: Side::Some(1), r: Side::Some(2) }).try_into_range(5).unwrap(), + /// UserBounds::from_str("1:2").unwrap().try_into_range(5).unwrap(), /// Range { start: 0, end: 2} // 2, not 1, because it's exclusive /// ); /// /// assert_eq!( - /// (UserBounds { l: Side::Some(1), r: Side::Continue }).try_into_range(5).unwrap(), + /// UserBounds::from_str("1:").unwrap().try_into_range(5).unwrap(), /// Range { start: 0, end: 5} /// ); /// ``` @@ -769,9 +770,12 @@ mod tests { } #[test] - fn test_user_bounds_is_sortable() { - assert!(UserBoundsList::new(Vec::new()).is_sortable()); + fn test_user_bounds_cannot_be_empty() { + assert!(UserBoundsList::from_str("").is_err()); + } + #[test] + fn test_user_bounds_is_sortable() { assert!(UserBoundsList::from_str("1").unwrap().is_sortable()); assert!(UserBoundsList::from_str("1,2").unwrap().is_sortable()); @@ -787,8 +791,6 @@ mod tests { #[test] fn test_vec_of_bounds_is_sorted() { - assert!(UserBoundsList::from_str("").unwrap().is_sorted()); - assert!(UserBoundsList::from_str("1").unwrap().is_sorted()); assert!(UserBoundsList::from_str("1,2").unwrap().is_sorted()); diff --git a/src/cut_lines.rs b/src/cut_lines.rs index ffe9f0d..f781014 100644 --- a/src/cut_lines.rs +++ b/src/cut_lines.rs @@ -131,8 +131,10 @@ pub fn read_and_cut_lines( #[cfg(test)] mod tests { + use std::str::FromStr; + use crate::{ - bounds::{BoundsType, UserBounds, UserBoundsList}, + bounds::{BoundsType, UserBoundsList}, options::EOL, }; @@ -147,29 +149,10 @@ mod tests { } } - fn bof_f1() -> BoundOrFiller { - BoundOrFiller::Bound(UserBounds::new(Side::Some(1), Side::Some(1))) - } - - fn bof_f2() -> BoundOrFiller { - BoundOrFiller::Bound(UserBounds::new(Side::Some(2), Side::Some(2))) - } - fn bof_f3() -> BoundOrFiller { - BoundOrFiller::Bound(UserBounds::new(Side::Some(3), Side::Some(3))) - } - fn bof_r2_3() -> BoundOrFiller { - BoundOrFiller::Bound(UserBounds::new(Side::Some(2), Side::Some(3))) - } - fn bof_neg1() -> BoundOrFiller { - BoundOrFiller::Bound(UserBounds::new(Side::Some(-1), Side::Some(-1))) - } - fn bof_f1_to_end() -> BoundOrFiller { - BoundOrFiller::Bound(UserBounds::new(Side::Some(1), Side::Continue)) - } #[test] fn fwd_cut_one_field() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_f1()]); + opt.bounds = UserBoundsList::from_str("1").unwrap(); let mut input = b"a\nb".as_slice(); let mut output = Vec::with_capacity(100); @@ -180,7 +163,7 @@ mod tests { #[test] fn fwd_cut_multiple_fields() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_f1(), bof_f2()]); + opt.bounds = UserBoundsList::from_str("1:2").unwrap(); let mut input = b"a\nb".as_slice(); let mut output = Vec::with_capacity(100); @@ -191,7 +174,7 @@ mod tests { #[test] fn fwd_support_ranges() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_f1(), bof_r2_3()]); + opt.bounds = UserBoundsList::from_str("1,2:3").unwrap(); let mut input = b"a\nb\nc".as_slice(); let mut output = Vec::with_capacity(100); @@ -202,7 +185,7 @@ mod tests { #[test] fn fwd_supports_no_join() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_f1(), bof_f3()]); + opt.bounds = UserBoundsList::from_str("1,3").unwrap(); opt.join = false; let mut input = b"a\nb\nc".as_slice(); @@ -214,7 +197,7 @@ mod tests { #[test] fn fwd_supports_no_right_bound() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_f1_to_end()]); + opt.bounds = UserBoundsList::from_str("1:").unwrap(); let mut input = b"a\nb".as_slice(); let mut output = Vec::with_capacity(100); @@ -225,7 +208,7 @@ mod tests { #[test] fn fwd_handle_out_of_bounds() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_f3()]); + opt.bounds = UserBoundsList::from_str("3").unwrap(); opt.join = true; let mut input = b"a\nb".as_slice(); @@ -237,7 +220,7 @@ mod tests { #[test] fn fwd_ignore_last_empty() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_f3()]); + opt.bounds = UserBoundsList::from_str("3").unwrap(); let mut input1 = b"a\nb".as_slice(); let mut input2 = b"a\nb\n".as_slice(); @@ -255,7 +238,7 @@ mod tests { #[test] fn cut_lines_handle_negative_idx() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_neg1()]); + opt.bounds = UserBoundsList::from_str("-1").unwrap(); let mut input = b"a\nb".as_slice(); let mut output = Vec::with_capacity(100); @@ -266,7 +249,7 @@ mod tests { #[test] fn cut_lines_ignore_last_empty_when_using_positive_idx() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_f3()]); + opt.bounds = UserBoundsList::from_str("3").unwrap(); let mut input1 = b"a\nb".as_slice(); let mut input2 = b"a\nb\n".as_slice(); @@ -284,7 +267,7 @@ mod tests { #[test] fn cut_lines_ignore_last_empty_when_using_negative_idx() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_neg1()]); + opt.bounds = UserBoundsList::from_str("-1").unwrap(); let mut input1 = b"a\nb".as_slice(); let mut input2 = b"a\nb\n".as_slice(); @@ -302,7 +285,7 @@ mod tests { #[test] fn fwd_cut_zero_delimited() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_f1()]); + opt.bounds = UserBoundsList::from_str("1").unwrap(); opt.eol = EOL::Zero; opt.delimiter = String::from("\0"); @@ -315,7 +298,7 @@ mod tests { #[test] fn cut_lines_zero_delimited() { let mut opt = make_lines_opt(); - opt.bounds = UserBoundsList::new(vec![bof_f1()]); + opt.bounds = UserBoundsList::from_str("1").unwrap(); opt.eol = EOL::Zero; opt.delimiter = String::from("\0"); diff --git a/src/options.rs b/src/options.rs index 603b10c..c93eeb0 100644 --- a/src/options.rs +++ b/src/options.rs @@ -1,4 +1,4 @@ -use crate::bounds::{BoundOrFiller, BoundsType, UserBounds, UserBoundsList}; +use crate::bounds::{BoundsType, UserBoundsList}; use anyhow::Result; use std::str::FromStr; @@ -54,7 +54,7 @@ impl Default for Opt { Opt { delimiter: String::from("-"), eol: EOL::Newline, - bounds: UserBoundsList::new(vec![BoundOrFiller::Bound(UserBounds::default())]), + bounds: UserBoundsList::from_str("1:").unwrap(), bounds_type: BoundsType::Fields, only_delimited: false, greedy_delimiter: false,