Stefan Sperling wrote on Wed, 18 Nov 2009 at 09:57 +0100:
> On Wed, Nov 18, 2009 at 12:41:37AM +0200, Daniel Shahaf wrote:
> > Stefan Sperling wrote on Tue, 17 Nov 2009 at 09:16 +0100:
> > > On Mon, Nov 16, 2009 at 11:31:12PM +0200, Daniel Shahaf wrote:
> > > > i.e., iterate all 1st candidates, then all 2nd candidates, etc. Why not
> > > > iterate by hunks first? (i.e., first all candidates of 1st hunk, then
> > > > all candidates of 2nd hunk, etc.)
> > >
>
> So you were asking why, in the following step, we loop over candidate
> offsets, considering each hunk in turn as we do so, rather than
> looping over hunks, considering each candidate offset as we do so.
>
> The answer is that we want to end up using the best possible offset for
> every hunk. Looking at hunks in isolation does not achieve this goal.
> When we pick a candidate offset for a hunk, we want to make sure the
> resulting line range occupied by the hunk does not overlap with a range
> we assigned to a different hunk earlier. If the range is already occupied,
> we know that it is occupied by a "better" hunk (because the hunk list
> is already sorted accordingly) and that ignoring the current candidate
> offset for the current hunk will give the best overall result.
>
In this case, how about losing the "GROUP BY hunk" from the sort? i.e.,
BIG_LIST = []
for each hunk:
for each candidate in hunk:
compute a score for candidate
BIG_LIST.append(candidate)
# based on line offset, endifs, whatever
sort BIG_LIST by score
for each candidate in BIG_LIST:
if candidate->hunk.state not in ['applied', 'rejected']:
# try to apply candidate->hunk at candidate->offset
I'm pretty sure it differs from your suggestion on at least one
pathological input sample.
> What you [first proposed] does not answer any interesting question.
> Iterating over hunks first and looking at each candidate offset for the
> current hunk in turn does not give us any new information because we
> already sorted the candidate offset list in the best way we can for
> the current hunk.
>
I see.
> Is it more clear now?
>
Yes, thanks a lot. (I've trimmed context, but it was an excellent
explanation.)
> Stefan
>
