All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: unlisted-recipients:; (no To-header on input)
Cc: peff@peff.net, davidel@xmailserver.org, jacob.keller@gmail.com,
	gitster@pobox.com, git@vger.kernel.org, Jens.Lehmann@web.de,
	Stefan Beller <sbeller@google.com>
Subject: [RFC PATCH, WAS: "weird diff output?"] Implement better chunk heuristics.
Date: Thu, 14 Apr 2016 17:07:30 -0700	[thread overview]
Message-ID: <20160415000730.26446-1-sbeller@google.com> (raw)
In-Reply-To: <CAGZ79ka8pgPNZKaVWnsa_S07esxkN9nJfhcMZvCfd5U6MtsrYQ@mail.gmail.com>

TODO(sbeller):
* describe the discussion on why this is better
* see if this can be tested?

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 xdiff/xdiffi.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 2358a2d..24eb9a0 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -400,9 +400,16 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,
 }
 
 
+static int starts_with_emptyline(const char *recs)
+{
+	return recs[0] == '\n'; /* CRLF not covered here */
+}
+
+
 int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 	long ix, ixo, ixs, ixref, grpsiz, nrec = xdf->nrec;
 	char *rchg = xdf->rchg, *rchgo = xdfo->rchg;
+	unsigned char has_emptyline;
 	xrecord_t **recs = xdf->recs;
 
 	/*
@@ -436,6 +443,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 
 		do {
 			grpsiz = ix - ixs;
+			has_emptyline = 0;
 
 			/*
 			 * If the line before the current change group, is equal to
@@ -447,6 +455,8 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 				rchg[--ixs] = 1;
 				rchg[--ix] = 0;
 
+				has_emptyline |=
+					starts_with_emptyline(recs[ix]->ptr);
 				/*
 				 * This change might have joined two change groups,
 				 * so we try to take this scenario in account by moving
@@ -475,6 +485,9 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 				rchg[ixs++] = 0;
 				rchg[ix++] = 1;
 
+				has_emptyline |=
+					starts_with_emptyline(recs[ix]->ptr);
+
 				/*
 				 * This change might have joined two change groups,
 				 * so we try to take this scenario in account by moving
@@ -498,6 +511,32 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 			rchg[--ix] = 0;
 			while (rchgo[--ixo]);
 		}
+
+		/*
+		 * If a group can be moved back and forth, see if there is an
+		 * empty line in the moving space. If there is an empty line,
+		 * make sure the last empty line is the end of the group.
+		 *
+		 * As we shifted the group forward as far as possible, we only
+		 * need to shift it back if at all.
+		 */
+		if (has_emptyline) {
+			while (ixs > 0 && recs[ixs - 1]->ha == recs[ix - 1]->ha &&
+			       xdl_recmatch(recs[ixs - 1]->ptr, recs[ixs - 1]->size, recs[ix - 1]->ptr, recs[ix - 1]->size, flags) &&
+			       !starts_with_emptyline(recs[ix - 1]->ptr)) {
+				rchg[--ixs] = 1;
+				rchg[--ix] = 0;
+
+				/*
+				 * This change did not join two change groups,
+				 * as we did that before already, so there is no
+				 * need to adapt the other-file, i.e.
+				 * running
+				 *     for (; rchg[ixs - 1]; ixs--);
+				 *     while (rchgo[--ixo]);
+				 */
+			}
+		}
 	}
 
 	return 0;
-- 
2.8.1.474.gffdc890.dirty

  reply	other threads:[~2016-04-15  0:08 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-29  0:26 weird diff output? Jacob Keller
2016-03-29 17:37 ` Stefan Beller
2016-03-29 17:54   ` Junio C Hamano
2016-03-29 18:16     ` Stefan Beller
2016-03-29 23:05       ` Jacob Keller
2016-03-30  0:04         ` Junio C Hamano
2016-03-30  4:55         ` Jeff King
2016-03-30  6:05           ` Stefan Beller
2016-03-30  6:05           ` Jacob Keller
2016-03-30 19:14             ` Jacob Keller
2016-03-30 19:31               ` Jacob Keller
2016-03-30 19:40                 ` Stefan Beller
2016-04-01 19:04                   ` Junio C Hamano
2016-03-31 13:47                 ` Jeff King
2016-04-06 17:47                   ` Jacob Keller
2016-04-12 19:34                     ` Stefan Beller
2016-04-14 13:56                       ` Davide Libenzi
2016-04-14 18:34                         ` Jeff King
2016-04-14 21:05                           ` Stefan Beller
2016-04-15  0:07                             ` Stefan Beller [this message]
2016-04-15  0:26                               ` [RFC PATCH, WAS: "weird diff output?"] Implement better chunk heuristics Jacob Keller
2016-04-15  0:43                                 ` Stefan Beller
2016-04-15  2:07                                   ` Jacob Keller
2016-04-15  2:09                               ` Junio C Hamano
2016-04-15  3:33                                 ` Stefan Beller
2016-04-15  0:21                             ` weird diff output? Jacob Keller
2016-04-15  2:18                             ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160415000730.26446-1-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=Jens.Lehmann@web.de \
    --cc=davidel@xmailserver.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.keller@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.