Re: [PATCH 05/13] Drive the debug editor

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 7, 2010, 19:53 UTC
Message-ID: <20100707195321.GB2911@burratino>
In-Reply-To: <20100707190813.GA16065@debian>

Ramkumar Ramachandra wrote:

> Jonathan Nieder writes:
>> Ramkumar Ramachandra wrote:

>>> -	de->apply_textdelta = apply_textdelta;
>>> +	/* de->apply_textdelta = apply_textdelta; */

[...]

> Without this, the program segfaults because the necessary setup for
> applying a text delta hasn't been set up. Perhaps I should explain
> this in my commit message?

Is the default apply_textdelta not a no-op? What work does it have to do, and can the skeleton editor be convinced to do the same in patch 4?

Show 10 quoted lines

>>> +{
>>> +	/* Extract editor and editor_baton from the replay_baton and
>>> +	   set them so that the editor callbacks can use them */
>>
>> This comment just paraphrases the code.  What in particular requires
>> explanation here?
>
> This concept took me some time to wrap my head around: I had to stuff
> the replay_baton with the editor/ editor_baton so that I could set
> them for use in the callback functions.

Ah, okay.  Then I suppose it belongs in the commit message.

Alternatively: why does the tree editor have to persist between calls
replaying the various revisions?  That information could help the
reader understand what is going on.