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

From: Ping Yin <pkufranky@gmail.com>
Date: May 10, 2008, 09:02 UTC
Message-ID: <46dff0320805100202j54b0922cy50a2c93c4eff1757@mail.gmail.com>
In-Reply-To: <20080508103436.GB3300@mithlond.arda.local>

On Thu, May 8, 2008 at 6:34 PM, Teemu Likonen <tlikonen@iki.fi> wrote:

Show 17 quoted lines

> 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.

Show 42 quoted lines

> 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);

-- 
Ping Yin