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

Re: [PATCH 2/3] xdiff: -W: include immediately preceding non-empty lines in context

From
René Scharfe <l.s.r@web.de>
Date
Jan 15, 2017, 16:57 UTC
Message-ID
<57bba5fa-015d-5629-73c2-5052ffb39d2a@web.de>
In-Reply-To
<xmqqy3ydcaia.fsf@gitster.mtv.corp.google.com>
Am 15.01.2017 um 03:39 schrieb Junio C Hamano:
Show 27 quoted lines
> René Scharfe <l.s.r@web.de> writes:
> 
>>> I am also more focused on keeping the codebase maintainable in good
>>> health by making sure that we made an effort to find a solution that
>>> is general-enough before solving a single specific problem you have
>>> today.  We may end up deciding that a blank-line heuristics gives us
>>> good enough tradeoff, but I do not want us to make a decision before
>>> thinking.
>>
>> How about extending the context upward only up to and excluding a line
>> that is either empty *or* a function line?  That would limit the extra
>> context to a single function in the worst case.
>>
>> Reducing context at the bottom with the aim to remove comments for the
>> next section is more tricky as it could remove part of the function
>> that we'd like to show if we get the boundary wrong.  How bad would it
>> be to keep the southern border unchanged?
> 
> I personally do not think there is any robust heuristic other than
> Vegard's "a blank line may be a signal enough that lines before that
> are not part of the beginning of the function", and I think your
> "hence we look for a blank line but if there is a line that matches
> the function header, stop there as we know we came too far back"
> will be a good-enough safety measure.
> 
> I also agree with you that we probably do not want to futz with the
> southern border.
A replacement patch for 2/3 with these changes would look like this:
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 8c88dbde38..9ed54cd318 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -174,11 +174,11 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 		s2 = XDL_MAX(xch->i2 - xecfg->ctxlen, 0);
 
 		if (xecfg->flags & XDL_EMIT_FUNCCONTEXT) {
+			char dummy[1];
 			long fs1, i1 = xch->i1;
 
 			/* Appended chunk? */
 			if (i1 >= xe->xdf1.nrec) {
-				char dummy[1];
 				long i2 = xch->i2;
 
 				/*
@@ -200,6 +200,10 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 			}
 
 			fs1 = get_func_line(xe, xecfg, NULL, i1, -1);
+			while (fs1 > 0 && !is_empty_rec(&xe->xdf1, fs1 - 1) &&
+			       match_func_rec(&xe->xdf1, xecfg, fs1 - 1,
+					      dummy, sizeof(dummy)) < 0)
+				fs1--;
 			if (fs1 < 0)
 				fs1 = 0;
 			if (fs1 < s1) {
Previous: Junio C HamanoNext: Vegard Nossum
Message 13 of 15 in “xdiff: -W: relax end-of-file function detection”
  1. 1/3 xdiff: -W: relax end-of-file function detectionVegard Nossum, Jan 13, 2017
  2. 2/3 xdiff: -W: include immediately preceding non-empty lines in contextVegard Nossum, Jan 13, 2017
  3. René ScharfeJan 13, 2017
  4. Stefan BellerJan 13, 2017
  5. Junio C HamanoJan 13, 2017
  6. Vegard NossumJan 13, 2017
  7. Junio C HamanoJan 13, 2017
  8. René ScharfeJan 14, 2017
  9. Junio C HamanoJan 15, 2017
  10. Vegard NossumJan 15, 2017
  11. René ScharfeJan 15, 2017
  12. Junio C HamanoJan 15, 2017
  13. René ScharfeJan 15, 2017
  14. 3/3 t/t4051-diff-function-context: improve tests for new diff -W behaviourVegard Nossum, Jan 13, 2017
  15. René ScharfeJan 13, 2017

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.