From cd98478ce96a5b43435420e0bc541d9989a4669d Mon Sep 17 00:00:00 2001 From: Konstantin Pospelov Date: Sun, 15 Apr 2018 17:33:02 +0300 Subject: [PATCH 1/2] join: minor improvements Move the code to get the current key into a separate function. Replace two 'combine' functions with one defined for Input. --- src/join/join.rs | 65 ++++++++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 38 deletions(-) diff --git a/src/join/join.rs b/src/join/join.rs index c76a99dbe..67d92d7be 100644 --- a/src/join/join.rs +++ b/src/join/join.rs @@ -147,6 +147,24 @@ impl Input { check_order, } } + + fn compare(&self, field1: Option<&str>, field2: Option<&str>) -> Ordering { + if let (Some(field1), Some(field2)) = (field1, field2) { + if self.ignore_case { + field1.to_lowercase().cmp(&field2.to_lowercase()) + } else { + field1.cmp(field2) + } + } else { + match field1 { + Some(_) => Ordering::Greater, + None => match field2 { + Some(_) => Ordering::Less, + None => Ordering::Equal, + }, + } + } + } } enum Spec { @@ -247,14 +265,6 @@ impl<'a> State<'a> { } } - /// Compare the key fields of the two current lines. - fn compare(&self, other: &State, ignore_case: bool) -> Ordering { - let key1 = self.seq[0].get_field(self.key); - let key2 = other.seq[0].get_field(other.key); - - compare(key1, key2, ignore_case) - } - /// Skip the current unpaired line. fn skip_line(&mut self, input: &Input, repr: &Repr) { if self.print_unpaired { @@ -268,11 +278,7 @@ impl<'a> State<'a> { /// the first line whose key differs. fn extend(&mut self, input: &Input) -> Option { while let Some(line) = self.next_line(input) { - let diff = compare( - self.seq[0].get_field(self.key), - line.get_field(self.key), - input.ignore_case, - ); + let diff = input.compare(self.get_current_key(), line.get_field(self.key)); if diff == Ordering::Equal { self.seq.push(line); @@ -299,7 +305,7 @@ impl<'a> State<'a> { /// Combine two line sequences. fn combine(&self, other: &State, repr: &Repr) { - let key = self.seq[0].get_field(self.key); + let key = self.get_current_key(); for line1 in &self.seq { for line2 in &other.seq { @@ -387,11 +393,7 @@ impl<'a> State<'a> { return Some(line); } - let diff = compare( - self.seq[self.seq.len() - 1].get_field(self.key), - line.get_field(self.key), - input.ignore_case, - ); + let diff = input.compare(self.get_current_key(), line.get_field(self.key)); if diff == Ordering::Greater { eprintln!("{}:{}: is not sorted", self.file_name, self.line_num); @@ -407,6 +409,11 @@ impl<'a> State<'a> { Some(line) } + /// Gets the key value of the lines stored in seq. + fn get_current_key(&self) -> Option<&str> { + self.seq[0].get_field(self.key) + } + fn print_line(&self, line: &Line, repr: &Repr) { if repr.uses_format() { repr.print_format(|spec| match spec { @@ -633,7 +640,7 @@ fn exec(file1: &str, file2: &str, settings: &Settings) -> i32 { } while state1.has_line() && state2.has_line() { - let diff = state1.compare(&state2, settings.ignore_case); + let diff = input.compare(state1.get_current_key(), state2.get_current_key()); match diff { Ordering::Less => { @@ -690,21 +697,3 @@ fn parse_field_number(value: &str) -> usize { fn parse_field_number_option(value: Option<&str>) -> Option { Some(parse_field_number(value?)) } - -fn compare(field1: Option<&str>, field2: Option<&str>, ignore_case: bool) -> Ordering { - if let (Some(field1), Some(field2)) = (field1, field2) { - return if ignore_case { - field1.to_lowercase().cmp(&field2.to_lowercase()) - } else { - field1.cmp(field2) - }; - } - - match field1 { - Some(_) => Ordering::Greater, - None => match field2 { - Some(_) => Ordering::Less, - None => Ordering::Equal, - }, - } -} From 4b8d4bfc055add6a42c88be1aa6a082de2cd7b42 Mon Sep 17 00:00:00 2001 From: Konstantin Pospelov Date: Sun, 15 Apr 2018 17:42:52 +0300 Subject: [PATCH 2/2] join: fix autoformat There was an issue with autoformat when the files had a different number of columns in the first line. This commit fixes the issue and extends the related test to cover this case. --- src/join/join.rs | 2 +- tests/fixtures/join/autoformat.expected | 10 +++++----- tests/fixtures/join/different_lengths.txt | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/join/join.rs b/src/join/join.rs index 67d92d7be..23761075f 100644 --- a/src/join/join.rs +++ b/src/join/join.rs @@ -327,7 +327,7 @@ impl<'a> State<'a> { } else { repr.print_field(key); repr.print_fields(&line1, self.key, self.max_fields); - repr.print_fields(&line2, other.key, self.max_fields); + repr.print_fields(&line2, other.key, other.max_fields); } println!(); diff --git a/tests/fixtures/join/autoformat.expected b/tests/fixtures/join/autoformat.expected index 576f91092..d1557e25b 100644 --- a/tests/fixtures/join/autoformat.expected +++ b/tests/fixtures/join/autoformat.expected @@ -1,5 +1,5 @@ -1 a a -2 b b -3 c d -4 d g -5 e i +1 a a b +2 b b c +3 c d e +4 d g h +5 e i diff --git a/tests/fixtures/join/different_lengths.txt b/tests/fixtures/join/different_lengths.txt index 3d4a53d78..0dfad2774 100644 --- a/tests/fixtures/join/different_lengths.txt +++ b/tests/fixtures/join/different_lengths.txt @@ -1,4 +1,4 @@ -1 a +1 a b 2 b c 3 d e f 4 g h