git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 2/4] apply: Allow blank context lines to match beyond EOF

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 24, 2010, 20:56 UTC
Message-ID
<7vaauyfj3k.fsf@alter.siamese.dyndns.org>
In-Reply-To
<4B857CE4.4000201@gmail.com>
Björn Gustavsson <bgustavsson@gmail.com> writes:
> "git apply --whitespace=fix" will not always succeed when used
> on a series of patches in the following circumstances:
Very nicely done.

I wanted to explain to myself what "limit" and "preimage_limit" variables mean. preimage-limit has a very clear definition: this many lines from the beginning of preimage must match img, and the remainder of the preimage must be all blank (the remainder can exist only when we are trying to match at the end).

On the other hand "limit" does not have such a good definition, other than as a work around to bypass line-number check when we are trying to match at the end. It might be cleaner to read if we move the problematic "line numbers must match" logic and eliminate this variable, like the attached patch does on top of this one.

I couldn't figure out how this would interact with the ignore_ws_change codepath, though. That one shows a clear sign of being bolted on as an afterthought (once you fall into that "if()" statement you will not come back).

 builtin-apply.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 4374d33..ae4452c 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1854,26 +1854,24 @@ static int match_fragment(struct image *img,
 {
 	int i;
 	char *fixed_buf, *buf, *orig, *target;
-	int limit;
 	int preimage_limit;
 
 	if (preimage->nr + try_lno <= img->nr) {
 		/*
 		 * The hunk falls within the boundaries of img.
 		 */
-		limit = img->nr;
 		preimage_limit = preimage->nr;
+		if (match_end && (preimage->nr + try_lno != img->nr))
+			return 0;
 	} else if (ws_error_action == correct_ws_error &&
 		   (ws_rule & WS_BLANK_AT_EOF) && match_end) {
 		/*
 		 * This hunk that matches at the end extends beyond
 		 * the end of img, and we are removing blank lines
-		 * at the end of the file. Set up the limits so that
-		 * tests below will pass and the quick hash test
-		 * will only test the lines up to the last line
-		 * in img.
+		 * at the end of the file.  This many lines from the
+		 * beginning of the preimage must match with img, and
+		 * the remainder of the preimage must be blank.
 		 */
-		limit = try_lno + preimage->nr;
 		preimage_limit = img->nr - try_lno;
 	} else {
 		/*
@@ -1887,9 +1885,6 @@ static int match_fragment(struct image *img,
 	if (match_beginning && try_lno)
 		return 0;
 
-	if (match_end && preimage->nr + try_lno != limit)
-		return 0;
-
 	/* Quick hash check */
 	for (i = 0; i < preimage_limit; i++)
 		if (preimage->line[i].hash != img->line[try_lno + i].hash)
Previous: Björn GustavssonNext: Björn Gustavsson
Message 2 of 3 in “apply: Allow blank context lines to match beyond EOF”
  1. 2/4 apply: Allow blank context lines to match beyond EOFBjörn Gustavsson, Feb 24, 2010
  2. Junio C HamanoFeb 24, 2010
  3. Björn GustavssonFeb 24, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.