git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH, RFC] diff: add option to show context between close chunks

From
René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date
Oct 21, 2008, 20:48 UTC
Message-ID
<48FE4034.2090707@lsrfire.ath.cx>
In-Reply-To
<20081021112040.GB17363@coredump.intra.peff.net>
Jeff King schrieb:
Show 18 quoted lines
> 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".

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.

Show 12 quoted lines
> 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é
Previous: Jeff KingNext: Daniel Barkalow
Message 10 of 11 in “diff: add option to show context between close chunks”
  1. diff: add option to show context between close chunksRené Scharfe, Oct 19, 2008
  2. Johannes SixtOct 20, 2008
  3. René ScharfeOct 20, 2008
  4. Johannes SixtOct 21, 2008
  5. René ScharfeOct 21, 2008
  6. Junio C HamanoOct 20, 2008
  7. Johannes SixtOct 21, 2008
  8. Junio C HamanoOct 21, 2008
  9. Jeff KingOct 21, 2008
  10. René ScharfeOct 21, 2008
  11. Daniel BarkalowOct 21, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.