On Mon, Apr 18, 2016 at 02:12:30PM -0700, Stefan Beller wrote: > + > + /* > + * If a group can be moved back and forth, see if there is an > + * blank line in the moving space. If there is a blank line, > + * make sure the last blank line is the end of the group. s/an/a/ on the first line > + * As we shifted the group forward as far as possible, we only > + * need to shift it back if at all. Maybe because I'm reading it as a diff that only contains this hunk and not the whole rest of the function, but the "we" here confused me. You mean the earlier, existing loop in xdl_change_compact, right? Maybe something like: As we already shifted the group forward as far as possible in the earlier loop... would help. > + if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) { > + while (ixs > 0 && > + !is_blank_line(recs, ix - 1, flags) && > + recs_match(recs, ixs - 1, ix - 1, flags)) { > + rchg[--ixs] = 1; > + rchg[--ix] = 0; > + } > + } This turned out to be delightfully simple (especially compared to the perl monstrosity). I tried comparing the output to the perl one, but it's not quite the same. In that one we had to work with the existing hunks and context lines, so any hunk that got shifted ended up with extra context on one side, and too little on the other. But here, we can actually bump the context lines to give the correct amount on both sides, which is good. I guess this will invalidate old patch-ids, but there's not much to be done about that. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html