From 4f297eaa519593eac3a2c176aaeefe01887f7090 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Sun, 19 May 2024 11:59:43 +0200 Subject: [PATCH 1/2] Initial test case --- testdata/conflict-with-context.diff | 84 ++++++++++++++++++++++ testdata/conflict-with-context.riff-output | 84 ++++++++++++++++++++++ 2 files changed, 168 insertions(+) create mode 100644 testdata/conflict-with-context.diff create mode 100644 testdata/conflict-with-context.riff-output diff --git a/testdata/conflict-with-context.diff b/testdata/conflict-with-context.diff new file mode 100644 index 0000000..a83ec3c --- /dev/null +++ b/testdata/conflict-with-context.diff @@ -0,0 +1,84 @@ +diff --cc m/search.go +index 5b67346,e5b580f..0000000 +--- m/search.go ++++ m/search.go +@@@ -10,45 -6,9 +10,79 @@@ import + "github.com/walles/moar/m/linenumbers" + ) + +++<<<<<<< HEAD + +func (p *Pager) scrollToSearchHits() { + + if p.searchPattern == nil { + + // This is not a search + + return + + } + + + + lineNumber := p.scrollPosition.lineNumber(p) + + if lineNumber == nil { + + // No lines to search + + return + + } + + + + firstHitPosition := p.findFirstHit(*lineNumber, nil, false) + + if firstHitPosition == nil && (*lineNumber != linenumbers.LineNumber{}) { + + // Try again from the top + + firstHitPosition = p.findFirstHit(linenumbers.LineNumber{}, lineNumber, false) + + } + + if firstHitPosition == nil { + + // No match, give up + + return + + } + + + + if firstHitPosition.isVisible(p) { + + // Already on-screen, never mind + + return + + } + + + + p.scrollPosition = *firstHitPosition + +} + + + +// NOTE: When we search, we do that by looping over the *input lines*, not the + +// screen lines. That's why startPosition is a LineNumber rather than a + +// scrollPosition. + +// + +// The `beforePosition` parameter is exclusive, meaning that line will not be + +// searched. + +// + +// For the actual searching, this method will call _findFirstHit() in parallel + +// on multiple cores, to help large file search performance. +++||||||| parent of b835e9a (Fix the warnings) +++func (p *Pager) scrollToSearchHits() { +++ if p.searchPattern == nil { +++ // This is not a search +++ return +++ } +++ +++ firstHitPosition := p.findFirstHit(*p.scrollPosition.lineNumber(p), nil, false) +++ if firstHitPosition == nil { +++ // Try again from the top +++ firstHitPosition = p.findFirstHit(linenumbers.LineNumber{}, p.scrollPosition.lineNumber(p), false) +++ } +++ if firstHitPosition == nil { +++ // No match, give up +++ return +++ } +++ +++ if firstHitPosition.isVisible(p) { +++ // Already on-screen, never mind +++ return +++ } +++ +++ p.scrollPosition = *firstHitPosition +++} +++ ++ // NOTE: When we search, we do that by looping over the *input lines*, not ++ // the screen lines. That's why we're using a line number rather than a ++ // scrollPosition for searching. +++======= +++// NOTE: When we search, we do that by looping over the *input lines*, not +++// the screen lines. That's why we're using a line number rather than a +++// scrollPosition for searching. +++>>>>>>> b835e9a (Fix the warnings) + // + // FIXME: We should take startPosition.deltaScreenLines into account as well! + func (p *Pager) findFirstHit(startPosition linenumbers.LineNumber, beforePosition *linenumbers.LineNumber, backwards bool) *scrollPosition { diff --git a/testdata/conflict-with-context.riff-output b/testdata/conflict-with-context.riff-output new file mode 100644 index 0000000..1bcaf98 --- /dev/null +++ b/testdata/conflict-with-context.riff-output @@ -0,0 +1,84 @@ +diff --cc m/search.go +index 5b67346,e5b580f..0000000 +--- m/search.go ++++ m/search.go +@@@ -10,45 -6,9 +10,79 @@@ import  + "github.com/walles/moar/m/linenumbers" + ) + +++<<<<<<< HEAD + +func (p *Pager) scrollToSearchHits() { + + if p.searchPattern == nil { + + // This is not a search + + return + + } + + + + lineNumber := p.scrollPosition.lineNumber(p) + + if lineNumber == nil { + + // No lines to search + + return + + } + + + + firstHitPosition := p.findFirstHit(*lineNumber, nil, false) + + if firstHitPosition == nil && (*lineNumber != linenumbers.LineNumber{}) { + + // Try again from the top + + firstHitPosition = p.findFirstHit(linenumbers.LineNumber{}, lineNumber, false) + + } + + if firstHitPosition == nil { + + // No match, give up + + return + + } + + + + if firstHitPosition.isVisible(p) { + + // Already on-screen, never mind + + return + + } + + + + p.scrollPosition = *firstHitPosition + +} + + + +// NOTE: When we search, we do that by looping over the *input lines*, not the + +// screen lines. That's why startPosition is a LineNumber rather than a + +// scrollPosition. + +// + +// The `beforePosition` parameter is exclusive, meaning that line will not be + +// searched. + +// + +// For the actual searching, this method will call _findFirstHit() in parallel + +// on multiple cores, to help large file search performance. +++||||||| parent of b835e9a (Fix the warnings) +++func (p *Pager) scrollToSearchHits() { +++ if p.searchPattern == nil { +++ // This is not a search +++ return +++ } +++ +++ firstHitPosition := p.findFirstHit(*p.scrollPosition.lineNumber(p), nil, false) +++ if firstHitPosition == nil { +++ // Try again from the top +++ firstHitPosition = p.findFirstHit(linenumbers.LineNumber{}, p.scrollPosition.lineNumber(p), false) +++ } +++ if firstHitPosition == nil { +++ // No match, give up +++ return +++ } +++ +++ if firstHitPosition.isVisible(p) { +++ // Already on-screen, never mind +++ return +++ } +++ +++ p.scrollPosition = *firstHitPosition +++} +++ ++ // NOTE: When we search, we do that by looping over the *input lines*, not ++ // the screen lines. That's why we're using a line number rather than a ++ // scrollPosition for searching. +++======= +++// NOTE: When we search, we do that by looping over the *input lines*, not +++// the screen lines. That's why we're using a line number rather than a +++// scrollPosition for searching. +++>>>>>>> b835e9a (Fix the warnings) + // + // FIXME: We should take startPosition.deltaScreenLines into account as well! + func (p *Pager) findFirstHit(startPosition linenumbers.LineNumber, beforePosition *linenumbers.LineNumber, backwards bool) *scrollPosition { From ac5a713301cbe4f9646802a599f8c94824ca531d Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Sun, 19 May 2024 12:29:08 +0200 Subject: [PATCH 2/2] Be more lenient when consuming conflict diffs --- src/conflicts_highlighter.rs | 47 +++++++++------- testdata/conflict-with-context.riff-output | 62 +++++++++++----------- 2 files changed, 59 insertions(+), 50 deletions(-) diff --git a/src/conflicts_highlighter.rs b/src/conflicts_highlighter.rs index c6d6a4c..95e883e 100644 --- a/src/conflicts_highlighter.rs +++ b/src/conflicts_highlighter.rs @@ -86,33 +86,42 @@ impl LinesHighlighter for ConflictsHighlighter { }); } - let (maybe_prefix, destination) = if !self.c2_header.is_empty() { - ("+ ", &mut self.c2) + // + // All header lines handled, this is a content line + // + + let destination = if !self.c2_header.is_empty() { + &mut self.c2 } else if !self.base_header.is_empty() { - ("++", &mut self.base) + &mut self.base } else { - (" +", &mut self.c1) + &mut self.c1 }; - let prefix = if self.c1_header.starts_with("++") { - maybe_prefix + let prefixes = if self.c1_header.starts_with("++") { + // Possible content line prefixes when doing "git diff" + vec!["+ ", "++", " +"] } else { - "" + vec![""] }; - if let Some(line) = line.strip_prefix(prefix) { - destination.push_str(line); - destination.push('\n'); - return Ok(Response { - line_accepted: LineAcceptance::AcceptedWantMore, - highlighted: vec![], - }); - } else { - return Ok(Response { - line_accepted: LineAcceptance::RejectedDone, - highlighted: vec![self.render_plain()], - }); + for prefix in prefixes { + if let Some(line) = line.strip_prefix(prefix) { + // Handle the context line + destination.push_str(line); + destination.push('\n'); + return Ok(Response { + line_accepted: LineAcceptance::AcceptedWantMore, + highlighted: vec![], + }); + } } + + // Not a context line, just give up and do a simple render + return Ok(Response { + line_accepted: LineAcceptance::RejectedDone, + highlighted: vec![self.render_plain()], + }); } fn consume_eof(&mut self, _thread_pool: &ThreadPool) -> Result, String> { diff --git a/testdata/conflict-with-context.riff-output b/testdata/conflict-with-context.riff-output index 1bcaf98..dcca5d2 100644 --- a/testdata/conflict-with-context.riff-output +++ b/testdata/conflict-with-context.riff-output @@ -5,7 +5,7 @@ @@@ -10,45 -6,9 +10,79 @@@ import  "github.com/walles/moar/m/linenumbers" ) - + ++<<<<<<< HEAD  +func (p *Pager) scrollToSearchHits() {  + if p.searchPattern == nil { @@ -13,14 +13,14 @@  + return  + }  + - + lineNumber := p.scrollPosition.lineNumber(p) + + lineNumber := p.scrollPosition.lineNumber(p)  + if lineNumber == nil {  + // No lines to search  + return  + }  + - + firstHitPosition := p.findFirstHit(*lineNumber, nil, false) - + if firstHitPosition == nil && (*lineNumber != linenumbers.LineNumber{}) { + + firstHitPosition := p.findFirstHit(*lineNumber, nil, false) + + if firstHitPosition == nil && (*lineNumber != linenumbers.LineNumber{}) {  + // Try again from the top  + firstHitPosition = p.findFirstHit(linenumbers.LineNumber{}, lineNumber, false)  + } @@ -37,9 +37,9 @@  + p.scrollPosition = *firstHitPosition  +}  + - +// NOTE: When we search, we do that by looping over the *input lines*, not the - +// screen lines. That's why startPosition is a LineNumber rather than a - +// scrollPosition. + +// NOTE: When we search, we do that by looping over the *input lines*, not the + +// screen lines. That's why startPosition is a LineNumber rather than a + +// scrollPosition.  +//  +// The `beforePosition` parameter is exclusive, meaning that line will not be  +// searched. @@ -47,37 +47,37 @@  +// For the actual searching, this method will call _findFirstHit() in parallel  +// on multiple cores, to help large file search performance. ++||||||| parent of b835e9a (Fix the warnings) -++func (p *Pager) scrollToSearchHits() { -++ if p.searchPattern == nil { -++ // This is not a search -++ return -++ } +++func (p *Pager) scrollToSearchHits() { +++ if p.searchPattern == nil { +++ // This is not a search +++ return +++ } ++ -++ firstHitPosition := p.findFirstHit(*p.scrollPosition.lineNumber(p), nil, false) -++ if firstHitPosition == nil { -++ // Try again from the top -++ firstHitPosition = p.findFirstHit(linenumbers.LineNumber{}, p.scrollPosition.lineNumber(p), false) -++ } -++ if firstHitPosition == nil { -++ // No match, give up -++ return -++ } +++ firstHitPosition := p.findFirstHit(*p.scrollPosition.lineNumber(p), nil, false) +++ if firstHitPosition == nil { +++ // Try again from the top +++ firstHitPosition = p.findFirstHit(linenumbers.LineNumber{}, p.scrollPosition.lineNumber(p), false) +++ } +++ if firstHitPosition == nil { +++ // No match, give up +++ return +++ } ++ -++ if firstHitPosition.isVisible(p) { -++ // Already on-screen, never mind -++ return -++ } +++ if firstHitPosition.isVisible(p) { +++ // Already on-screen, never mind +++ return +++ } ++ -++ p.scrollPosition = *firstHitPosition -++} +++ p.scrollPosition = *firstHitPosition +++} ++ +++// NOTE: When we search, we do that by looping over the *input lines*, not +++// the screen lines. That's why we're using a line number rather than a +++// scrollPosition for searching. +++======= + // NOTE: When we search, we do that by looping over the *input lines*, not + // the screen lines. That's why we're using a line number rather than a + // scrollPosition for searching. -++======= -++// NOTE: When we search, we do that by looping over the *input lines*, not -++// the screen lines. That's why we're using a line number rather than a -++// scrollPosition for searching. ++>>>>>>> b835e9a (Fix the warnings) // // FIXME: We should take startPosition.deltaScreenLines into account as well!