From: David Kastrup <dak@gnu.org>
Date: Tue, 18 Sep 2007 08:55:25 GMT
Subject: Re: [PATCH 1/3] git-apply: fix whitespace stripping
Message-ID: <86r6kw4aki.fsf@lola.quinscape.zz>
In-Reply-To: <11899829424173-git-send-email-bfields@citi.umich.edu>

"J. Bruce Fields" <bfields@citi.umich.edu> writes:

> The algorithm isn't right here: it accumulates any set of 8 spaces into
> tabs even if they're separated by tabs, so
>
> 	<four spaces><tab><four spaces><tab>
>
> is converted to
>
> 	<tab><tab><tab>
>
> when it should be just
>
> 	<tab><tab>
>
> So teach git-apply that a tab hides any group of less than 8 previous
> spaces in a row.
>
> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> ---
>  builtin-apply.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/builtin-apply.c b/builtin-apply.c
> index 976ec77..70359c1 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -1642,15 +1642,22 @@ static int apply_line(char *output, const char *patch, int plen)
>  
>  	buf = output;
>  	if (need_fix_leading_space) {
> +		int consecutive_spaces = 0;
>  		/* between patch[1..last_tab_in_indent] strip the
>  		 * funny spaces, updating them to tab as needed.
>  		 */
>  		for (i = 1; i < last_tab_in_indent; i++, plen--) {
>  			char ch = patch[i];
> -			if (ch != ' ')
> +			if (ch != ' ') {
> +				consecutive_spaces = 0;
>  				*output++ = ch;
> -			else if ((i % 8) == 0)
> -				*output++ = '\t';
> +			} else {
> +				consecutive_spaces++;
> +				if (consecutive_spaces == 8) {
> +					*output++ = '\t';
> +					consecutive_spaces = 0;
> +				}
> +			}
>  		}
>  		fixed = 1;
>  		i = last_tab_in_indent;
> -- 
> 1.5.3.1.42.gfe5df

As far as I can see, this does not really work since it does not
maintain an idea of a current column.

If you have

abcd<four spaces><tab><four spaces><tab>

then indeed the resulting conversion needs to be <tab><tab><tab>
whereas with

abc<four spaces><tab><four spaces><tab>

the resulting conversion needs to be just <tab><tab>


-- 
David Kastrup