Re: git diff woes

From: Andreas Ericsson <ae@op5.se>
Date: Nov 12, 2007, 11:19 UTC
Message-ID: <473836AC.6090802@op5.se>
In-Reply-To: <Pine.LNX.4.64.0711121047590.4362@racer.site>

Johannes Schindelin wrote:

Show 27 quoted lines

> Hi,
> 
> On Mon, 12 Nov 2007, Andreas Ericsson wrote:
> 
>> Johannes Schindelin wrote:
>>
>>>  And sure you can trust the hunk header.  Like most of the things, the 
>>> relate to the _original_ version, since the diff is meant to be 
>>> applied as a forward patch.
>>>
>>> So for all practical matters, the diff shows the correct thing: "in 
>>> this hunk, which (still) belongs to that function, change this and 
>>> this."
>>>
>>> Of course, that is only the case if you accept that the diff should be 
>>> applied _in total_, not piecewise.  IOW if you are a fan of GNU patch 
>>> which happily clobbers your file until it fails with the last hunk, 
>>> you will not be happy.
>>>
>> You're right. GNU patch will apply one hunk and then happily churn on 
>> even if it fails. git-apply will apply all hunks or none, so all hunks 
>> can assume that all previous hunks were successfully applied. So what 
>> was your point again?
> 
> My point was that this diff is not to be read as if the previous hunks had 
> been applied.  Just look at the context: it is also the original file.
>

The context is ambiguous, as it must be present in both the new and the old file for it to actually *be* context. Otherwise it would be part of the +- diff text.

> It seems I am singularly unable to explain plain concepts as this: a diff 
> assumes that the file is yet unchanged.
>

Sure, but the useraid with writing the apparent function declaration in the hunk header *will* be confusing if the function declaration changes in the same patch as other things in the function.

> So I'll stop.
>

Give me something valuable instead, such as your opinion on whether it would be better to not print the function declaration at all if it will be changed by applying the same patch, or if one should pick one of the declarations from old or new and, if so, which one to pick.

I simply refuse to believe that you wouldn't immediately think the hunk below holds an obvious bug. I thought so because of the helpful function context git diff prints (which is a helper for human reviewers, and not something git-apply or GNU patch needs to work), and now I want to do something about it so others won't have to suffer the same confusion.

@@ -583,75 +346,100 @@ double jitter_request(const char *host, int *status){
       if(verbose) printf("%d candiate peers available\n", num_candidates);
       if(verbose && syncsource_found) printf("synchronization source found\n")
       if(! syncsource_found){
-               *status = STATE_UNKNOWN;
+               status = STATE_WARNING;
               if(verbose) printf("warning: no synchronization source found\n")
       }

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231