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

["Ping Yin" <pkufranky@gmail.com>]

On Sat, May 10, 2008 at 5:02 PM, Ping Yin <pkufranky@gmail.com> wrote:
> On Thu, May 8, 2008 at 6:34 PM, Teemu Likonen <tlikonen@iki.fi> wrote:
>> Junio C Hamano wrote (2008-05-07 12:13 -0700):
>>
>>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>>
>>> > 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.
>
>> 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