From: Jeff King <peff@peff.net>
Date: Wed, 07 May 2008 19:45:24 GMT
Subject: Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable
Message-ID: <20080507194524.GA31500@sigill.intra.peff.net>
In-Reply-To: <7viqxqc4gs.fsf@gitster.siamese.dyndns.org>

On Wed, May 07, 2008 at 12:13:39PM -0700, Junio C Hamano wrote:

>  /* this executes the word diff on the accumulated buffers */
> @@ -441,27 +473,18 @@ static void diff_words_show(struct diff_words_data *diff_words)
>  	xdemitconf_t xecfg;
>  	xdemitcb_t ecb;
>  	mmfile_t minus, plus;
> -	int i;
> +	unsigned long sz;

strbuf uses size_t; since we pass sz in as a pointer to strbuf_detach,
there can be a pointer type mismatch.

But more big-picture, comparing the output of the old color words and
this implementation, there is one thing I don't like: the new one
doesn't bring together runs of additions and deletions, which can make
parsing text much easier. For example:

  $ echo This is a complete sentence. >one
  $ echo Here is some totally different text. >two

  # with old implementation; /-.../ is red, /+.../ is green
  $ git diff --color-words one two
  ...
  /-This/ /+Here/ is /-a complete sentence./+some totally different text./

  # with this patch
  $ git diff --color-words one two
  ...
  /-This/+Here/ is /-a/+some/ /-complete/+totally/ /-sentence./+different text./

-Peff