Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > Sometimes, the easiest way to fix up a patch is to edit it directly, even > adding or deleting lines. Now, many people are not as divine as certain > benevolent dictators as to update the hunk headers correctly at the first > try. > > So teach the tool to do it for us. Two comments and a half. * Latest POSIX draft talks about unified context and allows an empty line to represent an empty common context line. GNU diff already emits such a diff. fixup_counts() should take this into account. * I'd sleep better at night if 'Probably "diff ..."' part were written in a bit more robust way. * (minor) There is an established term for this operation: recountdiff, so --recount might be a better name. fixup_counts() also is better called recount_diff() if we go this route. If you are too narrowly focused to only support "git add -e", the first issue does not matter, because we always emit "SP LF" for such a common context. The reason why I care about the first two points is because we may want to teach git-am about this new option as well in 1.6.0. And the robustness issue I worry about the second point also applies to a line that is "^-- $", especially if we were to make this available to git-am. Perhaps when the line begins with a '-', the logic could be extra careful to detect the case where the line looks like the e-mail signature separator and check one line beyond it to see if it does not look anything like part of a diff (in which case you stop, without considering the line you are currently looking at, "^-- $", a deletion of "^- $", as part of the preimage context). As to code structure, we might want to make the later parameters to apply_patch() an integer, of OR'ed flag values, or even a pointer to a structure that holds options. Other than that, the patch looks reasonably isolated and clean. -- 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
