Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable

On Sat, May 10, 2008 at 5:02 PM, Ping Yin [off-list ref] wrote:

On Thu, May 8, 2008 at 6:34 PM, Teemu Likonen [off-list ref] wrote:

quoted

Junio C Hamano wrote (2008-05-07 12:13 -0700):

quoted

Johannes Schindelin [off-list ref] writes:

quoted

I am rather interested in the semantics, i.e. if you can punch holes
into this 3-class approach.

This is not the 3-class thing, but was done as a lunchtime hack.  It
removes more lines than it adds, with real comments ;-).

I tested your lunchtime hack from the "pu" branch. I'm perfectly happy
with the colored output itself but I noticed some different line feed
behaviour that you might want to know. Look at the example below. The
first is normal line diff. The second is the same text with the old
--color-words behaviour and the last is with the lunchtime hack version.
There are only three words added to the text; additions are written as
{+word} in the --color-words output.

You not only added the three words, but also wrap line at different position.

quoted

Normal line diff
----------------

-OpenOffice.org has user setting for defining the minimum length for
+OpenOffice.org has a user setting for defining the minimum length for
 words to be hyphenated. By default the word length is counted from the
-whole word - even for compound words. For example the word
-'elokuvalippu' is 12 characters long. The word will be hyphenated like
-'elo-ku-va-lip-pu' in all cases when the minimum word length is set to
-12 or less. If the minimum length is set to 13 or more the word is not
-hyphenated at all.
+whole word - even for compound words. For example the compound word
+'elokuvalippu' is considered 12 characters long. The word will be
+hyphenated like 'elo-ku-va-lip-pu' in all cases when the minimum word
+length is set to 12 or less. If the minimum length is set to 13 or more
+the word is not hyphenated at all.

With the old --color-words
--------------------------

OpenOffice.org has {+a }user setting for defining the minimum length for
words to be hyphenated. By default the word length is counted from the
whole word - even for compound words. For example the {+compound }word
'elokuvalippu' is {+considered }12 characters long. The word will be
hyphenated like 'elo-ku-va-lip-pu' in all cases when the minimum word
length is set to 12 or less. If the minimum length is set to 13 or more
the word is not hyphenated at all.

With the lunchtime hack --color-words
-------------------------------------

OpenOffice.org has {+a }user setting for defining the minimum length for
words to be hyphenated. By default the word length is counted from the
whole word - even for compound words. For example the {+compound }word
'elokuvalippu' is {+considered }12 characters long. The word will be
hyphenated like
 'elo-ku-va-lip-pu' in all cases when the minimum word
length is set to
 12 or less. If the minimum length is set to 13 or more
the word is not
 hyphenated at all.

With junio's following code, the number of LF can be more than any of
the original two input. Because it will output a LF whenever we
encounter a real LF in the original input. So  some LFs are outputed
as {-LF}  or {+LF} . We can't differentiate them because we can't see
the colored output for added or removed LF. The same case is that we
can't differentiate added and removed spaces.

That's why i proposed colored background for added/removed space
characters in former reply of this thread.

+       if (line[1] == ' ') {
+               /* A token */
+               line += 2;
+               len -= 3; /* drop the trailing LF */
+       } else {
+               /* A real LF */
+               line++;
+               len--;
      }
+       emit_line(diff_words->file, set, reset, line, len);

With following patch, the diff output becomes (i don't know which one is better)

OpenOffice.org has {+a }user setting for defining the minimum length for
words to be hyphenated. By default the word length is counted from the
whole word - even for compound words. For example the {compound +}word
'elokuvalippu' is {+considered }12 characters long. The word will be
hyphenated like
 'elo-ku-va-lip-pu' in all cases when the minimum word length is set to
 12 or less. If the minimum length is set to 13 or more the word is not
 hyphenated at all.

diff --git a/diff.c b/diff.c
index 51048c6..06fbace 100644
--- a/diff.c
+++ b/diff.c

@@ -446,6 +446,7 @@ struct diff_words_data {
 	struct xdiff_emit_state xm;
 	struct strbuf minus;
 	struct strbuf plus;
+	int suppressed_newline;
 	FILE *file;
 };

@@ -480,12 +481,16 @@ static void fn_out_diff_words_aux(
 		/* A token */
 		line += 2;
 		len -= 3; /* drop the trailing LF */
+		emit_line(diff_words->file, set, reset, line, len);
 	} else {
 		/* A real LF */
-		line++;
-		len--;
+		if (diff_words->suppressed_newline || line[0] == ' ') {
+			diff_words->suppressed_newline = 0;
+			emit_line(diff_words->file, set, reset, "\n", 1);
+		}
+		else
+			diff_words->suppressed_newline = 1;
 	}
-	emit_line(diff_words->file, set, reset, line, len);
 }

 /* this executes the word diff on the accumulated buffers */

@@ -510,8 +515,14 @@ static void diff_words_show(
 	ecb.outf = xdiff_outf;
 	ecb.priv = diff_words;
 	diff_words->xm.consume = fn_out_diff_words_aux;
+	diff_words->suppressed_newline = 0;
 	xdi_diff(&minus, &plus, &xpp, &xecfg, &ecb);

+	if (diff_words->suppressed_newline) {
+		putc('\n', diff_words->file);
+		diff_words->suppressed_newline = 0;
+	}
+
 	free(minus.ptr);
 	free(plus.ptr);
 }

-- 
Ping Yin