Johannes Sixt [off-list ref] writes:
Junio C Hamano schrieb:
quoted
René Scharfe [off-list ref] writes:
quoted
I think it makes sense to make 1, or even 3, the default for this
option for all commands that create patches intended for human
consumption. The patch keeps the default at 0, though.
I think defaulting to 1 would make sense, or alternatively, just
hardcoding that behaviour without any new option. That would give you
more information with the same number of patch lines, iow, upside without
any downside.
Are you sure about the "without any downside" part? The extra context line
inhibits that the patch applies cleanly to a version of the file that has
that very line modified (including a different number of lines).
Yeah. René wanted this for _human consumption_, not mechanical patch
application, so "hardcoding" literally there in the very low level of the
diff callchain is not quite right (it would affect format-patch which is
primarily for mechanical application).
I guess you could make the hardcoded value 1 for everybody else and 0 for
format-patch.
On Tue, Oct 21, 2008 at 12:12:17AM -0700, Junio C Hamano wrote:
Yeah. René wanted this for _human consumption_, not mechanical patch
application, so "hardcoding" literally there in the very low level of the
diff callchain is not quite right (it would affect format-patch which is
primarily for mechanical application).
I guess you could make the hardcoded value 1 for everybody else and 0 for
format-patch.
I see your reasoning, but at the same time, a large portion of patches I
read are from format-patch (and René even said that he was trying to
save the user from the "apply then diff just to look at the patch"
annoyance). And I have personally, as a patch submitter, created some
format-patch output sent to the git list with -U5 to combine hunks and
make it more readable for reviewers.
Not to mention that I sometimes apply or post the output of "git diff".
To me that it implies that either:
- the increased chance of conflict is not a problem in practice, and we
should have the option on by default everywhere
- it is a problem, in which case we should ask the user to turn on the
feature manually instead of second-guessing how they will use the
resulting patch (which they might not even know, since they are
making assumptions about how other people might use the patch, and
they must decide for their situation between shipping something that
is more readable but slightly more conflict prone, or as easy to
apply as possible)
-Peff
Jeff King schrieb:
On Tue, Oct 21, 2008 at 12:12:17AM -0700, Junio C Hamano wrote:
quoted
Yeah. René wanted this for _human consumption_, not mechanical patch
application, so "hardcoding" literally there in the very low level of the
diff callchain is not quite right (it would affect format-patch which is
primarily for mechanical application).
I guess you could make the hardcoded value 1 for everybody else and 0 for
format-patch.
I see your reasoning, but at the same time, a large portion of patches I
read are from format-patch (and René even said that he was trying to
save the user from the "apply then diff just to look at the patch"
annoyance). And I have personally, as a patch submitter, created some
format-patch output sent to the git list with -U5 to combine hunks and
make it more readable for reviewers.
Not to mention that I sometimes apply or post the output of "git diff".
Well, yes, perhaps I was trying to get ahead of myself. I sure would
like to see everyone create patches with fused hunks (because they are
easier to read), but step 1 is to have the option to create such patches
at all. We should then try it out for some time or verify its
usefulness statistically and only then turn it on by default. Or
perhaps throw it away, depending on the results.
And I consider the output of format-patch and git-diff to be intended
primarily for human consumption.
To me that it implies that either:
- the increased chance of conflict is not a problem in practice, and we
should have the option on by default everywhere
- it is a problem, in which case we should ask the user to turn on the
feature manually instead of second-guessing how they will use the
resulting patch (which they might not even know, since they are
making assumptions about how other people might use the patch, and
they must decide for their situation between shipping something that
is more readable but slightly more conflict prone, or as easy to
apply as possible)
To decide which one it is, I'd like to see numbers: how many times would
a patch with fused hunks have led to a problem for e.g. the kernel repo?
What is the optimal default value (0, 1, even more)? Before even
thinking about how to get these stats, I'd better head for bed for
today, though..
René