Re: [PATCH 0/3] Teach Git about the patience diff algorithm

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 9, 2009, 06:54 UTC
Message-ID: <7v7i552clz.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20090108195511.GA8734@chistera.yi.org>

Adeodato Simó <dato@net.com.org.es> writes:

Show 16 quoted lines

> * Linus Torvalds [Fri, 02 Jan 2009 08:42:04 -0800]:
>
>> Yes, this one is a real patience diff change, but it's also the same one 
>> that I've seen in the google fanboi findings. What google did _not_ show 
>> was any real-life examples, or anybody doing any critical analysis.
>
> This comes a bit late and maybe it's redundant, but somebody just sent
> to a Debian mailing list a patch that was hard to read, and patience
> improved it. (I realize it's quite similar in spirit to the "toy
> patience example" that google returns, but this at list is a *real*
> example where patience helped me read a patch.)
>
> I'm also attaching bzr diff output, because it's still more readable
> IMHO. (I realize that's independent of patience, as you explained, but
> I'm making a point that it'd be nice to have this addressed by somebody
> knowledgeable.)

I found thd difference between the Bzr diff and Dscho diff somewhat interesting. It shows that sometimes it makes the results easier to read to consider blank lines (and lines with only punctuation marks) that match to be non-matching when they appear inside a block of other consecutive non-matching lines, even though the result may become larger.

The part Bzr gives this result:

Show 24 quoted lines

> +/****************************************************************************
>   Write data to a fd.
>  ****************************************************************************/
>  
>  ssize_t write_data(int fd, const char *buffer, size_t N)
>  {
> -	size_t total=0;
>  	ssize_t ret;
> -	char addr[INET6_ADDRSTRLEN];
> ... all "removed"
> -	while (total < N) {
> -		total += ret;
> -	}
> -	return (ssize_t)total;
> +	struct iovec iov;
> +
> +	iov.iov_base = CONST_DISCARD(char *, buffer);
> ... all "added"
> +
> +
> +	return -1;
>  }
>  
>  /****************************************************************************

is shown by the Dscho git-diff like this:

Show 38 quoted lines

>   Write data to a fd.
>  ****************************************************************************/
>  
>  ssize_t write_data(int fd, const char *buffer, size_t N)
>  {
> -	size_t total=0;
>  	ssize_t ret;
> -	char addr[INET6_ADDRSTRLEN];
> +	struct iovec iov;
>  
> -	while (total < N) {
> -		ret = sys_write(fd,buffer + total,N - total);
> +	iov.iov_base = CONST_DISCARD(char *, buffer);
> +	iov.iov_len = N;
>  
> -		if (ret == -1) {
> -			if (fd == get_client_fd()) {
> ... all "removed"
> -
> -		if (ret == 0) {
> -			return total;
> -		}
> +	ret = write_data_iov(fd, &iov, 1);
> +	if (ret >= 0) {
> +		return ret;
> +	}
>  
> -		total += ret;
> +	if (fd == get_client_fd()) {
> +		char addr[INET6_ADDRSTRLEN];
> ... all "added"
> +		DEBUG(0,("write_data: write failure. Error = %s\n",
> +			 strerror(errno) ));
>  	}
> -	return (ssize_t)total;
> +
> +	return -1;
>  }

If we find the "common" context lines that have only blank and punctuation letters in Dscho output, turn each of them into "-" and "+", and rearrange them so that all "-" are together followed by "+", it will match Bzr output.