From 483766ba31c6095020d4e033761d0597e7293c4c Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Sat, 3 Feb 2024 15:21:04 +1300 Subject: [PATCH 1/2] applier: correct logic for printing location of failed hunk We were not correctly considering the case that the old file was empty. To fix this, use the same logic as what is used in the locator to determine the expected file location when printing this information. --- include/patch/locator.h | 2 ++ src/applier.cpp | 2 +- src/locator.cpp | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/patch/locator.h b/include/patch/locator.h index b559342..3a6f51f 100644 --- a/include/patch/locator.h +++ b/include/patch/locator.h @@ -29,6 +29,8 @@ struct Location { LineNumber offset { -1 }; }; +LineNumber expected_line_number(const Hunk& hunk); + Location locate_hunk(const std::vector& content, const Hunk& hunk, bool ignore_whitespace = false, LineNumber offset = 0, LineNumber max_fuzz = 2); bool matches_ignoring_whitespace(const std::string& as, const std::string& bs); diff --git a/src/applier.cpp b/src/applier.cpp index 8e5750d..264631b 100644 --- a/src/applier.cpp +++ b/src/applier.cpp @@ -167,7 +167,7 @@ static void print_hunk_statistics(std::ostream& out, size_t hunk_num, bool skipp } out << ".\n"; } else { - out << hunk.old_file_range.start_line + offset_old_lines_to_new << ".\n"; + out << expected_line_number(hunk) + offset_old_lines_to_new << ".\n"; } } diff --git a/src/locator.cpp b/src/locator.cpp index c3f7222..b6987c0 100644 --- a/src/locator.cpp +++ b/src/locator.cpp @@ -80,7 +80,7 @@ bool matches(const Line& line1, const Line& line2, bool ignore_whitespace) return matches_ignoring_whitespace(line1.content, line2.content); } -static LineNumber expected_line_number(const Hunk& hunk) +LineNumber expected_line_number(const Hunk& hunk) { auto line = hunk.old_file_range.start_line; if (hunk.old_file_range.number_of_lines == 0) From 6353a27043faf7e1d1b94880be614208b82f8bdd Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Sat, 3 Feb 2024 15:26:33 +1300 Subject: [PATCH 2/2] locator: correctly reject patches adding a file for non-empty files This corrects the actual logic needed to fix the: `applier_add_file_but_file_already_exits_with_conflicts` test. We just need to also add an informational message about when this case is detected. --- src/locator.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/locator.cpp b/src/locator.cpp index b6987c0..826520a 100644 --- a/src/locator.cpp +++ b/src/locator.cpp @@ -93,6 +93,21 @@ Location locate_hunk(const std::vector& content, const Hunk& hunk, bool ig // Make a first best guess at where the from-file range is telling us where the hunk should be. LineNumber offset_guess = expected_line_number(hunk) - 1 + offset; + // If there's no lines surrounding this hunk - it will always succeed, + // so there is no point in checking any further. Note that this check is + // also what makes matching against an empty 'from file' work (with no lines), + // as in that case there is no content for us to even match against in the + // first place! + // + // Furthermore, we also should reject patches being added when the hunk is + // claiming the file is completely empty - but there are actually lines in + // that file. + if (hunk.old_file_range.number_of_lines == 0) { + if (hunk.old_file_range.start_line == 0 && !content.empty()) + return {}; + return { offset_guess, 0, 0 }; + } + LineNumber patch_prefix_content = 0; for (const auto& line : hunk.lines) { if (line.operation != ' ') @@ -109,14 +124,6 @@ Location locate_hunk(const std::vector& content, const Hunk& hunk, bool ig LineNumber context = std::max(patch_prefix_content, patch_suffix_content); - // If there's no lines surrounding this hunk - it will always succeed, - // so there is no point in checking any further. Note that this check is - // also what makes matching against an empty 'from file' work (with no lines), - // as in that case there is no content for us to even match against in the - // first place! - if (hunk.old_file_range.number_of_lines == 0) - return { offset_guess, 0, 0 }; - for (LineNumber fuzz = 0; fuzz <= max_fuzz; ++fuzz) { auto suffix_fuzz = std::max(fuzz + patch_suffix_content - context, 0);