Re: [PATCH 1/3] Allow git-apply to ignore the hunk headers (AKA recountdiff)
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 24, 2008, 23:35 UTC
- Message-ID
- <7vfxr29zju.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <200806242108.16379.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
Show 12 quoted lines
> diff --git a/builtin-apply.c b/builtin-apply.c
> index c497889..34c220f 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -153,6 +153,7 @@ struct patch {
> unsigned int is_binary:1;
> unsigned int is_copy:1;
> unsigned int is_rename:1;
> + unsigned int recount:1;
> struct fragment *fragments;
> char *result;
> size_t resultsize;Why doesn't anybody find this quite wrong?
What is a "struct patch"? It describes a change to a single file (i.e. information contained from one "diff --git" til next "diff --git"), groups the hunks (called "fragments") together and holds the postimage after applying these hunks. Is this new "recount" field a per file attribute?
> + fragment->oldpos = 2; > + fragment->oldlines = fragment->newlines = 0;
Why is this discarding the position information?
It is none of this function's business. The ONLY THING this function should do is to discard fragment->oldlines and fragment->newlines, count the contents and update these two fields, and nothing else. Touching oldpos, leading and other things is an absolute no-no.
Show 7 quoted lines
> @@ -1013,6 +1058,9 @@ static int parse_fragment(char *line, unsigned long size, > offset = parse_fragment_header(line, len, fragment); > if (offset < 0) > return -1; > + if (offset > 0 && patch->recount && > + recount_diff(line + offset, size - offset, fragment)) > + return -1;
And recount should not cause parse_fragment() to fail out either. If you miscounted, the codepath that follows this part knows how to handle broken patch correctly anyway.
I think I've already mentioned the above two points when this was originally posted.
Somewhat disgusted...