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

Re: [PATCH] range-diff: add a --no-patch option to show a summary

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 6, 2018, 08:36 UTC
Message-ID
<87d0ri7gbs.fsf@evledraar.gmail.com>
In-Reply-To
<CAPig+cR85-7wMYCGGFoRT3jSQzQmda_84Ox1kF6roa5j-1XZ0Q@mail.gmail.com>
On Tue, Nov 06 2018, Eric Sunshine wrote:
Show 35 quoted lines
> On Mon, Nov 5, 2018 at 11:17 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>> > This change doesn't update git-format-patch with a --no-patch
>> > option. That can be added later similar to how format-patch first
>> > learned --range-diff, and then --creation-factor in
>> > 8631bf1cdd ("format-patch: add --creation-factor tweak for
>> > --range-diff", 2018-07-22). I don't see why anyone would want this for
>> > format-patch, it pretty much defeats the point of range-diff.
>>
>> Does it defeats the point of range-diff to omit the patch part in
>> the context of the cover letter?  How?
>>
>> I think the output with this option is a good addition to the cover
>> letter as an abbreviated form (as opposed to the full range-diff,
>> whose support was added earlier) that gives an overview.
>
> I had the same response when reading the commit message but didn't
> vocalize it. I could see people wanting to suppress the 'patch' part
> of the embedded range-diff in a cover letter (though probably not as
> commentary in a single-patch).
>
>> Calling this --[no-]patch might make it harder to integrate it to
>> format-patch later, though.  I suspect that people would expect
>> "format-patch --no-patch ..." to omit both the patch part of the
>> range-diff output *AND* the patch that should be applied to the
>> codebase (it of course would defeat the point of format-patch, so
>> today's format-patch would not pay attention to --no-patch, of
>> course).  We need to be careful not to break that when it happens.
>
> Same concern on my side, which is why I was thinking of other, less
> confusing, names, such as --summarize or such, though even that is too
> general against the full set of git-format-patch options. It could,
> perhaps be a separate option, say, "git format-patch
> --range-changes=<prev>" or something, which would embed the equivalent
> of "git range-diff --no-patch <prev>...<current>" in the cover letter.

Maybe this was discussed more when this range-diff format-patch integration was submitted, I wasn't following that closely:

Looking at this more carefully it seems like quite a design limitation that we're conflating the options for format-patch itself and for the range-diff invocation it makes.

Wouldn't it be better to make all these options e.g. --range-diff-creation-factor=*, --range-diff-no-patch, --range-diff-U1 etc. Now there's no way to say supply a different -U<ctx> for the range-diff & the patches themselves which seems like a semi-common use-case.

Doing that seems to be a matter of teaching setup_revisions() to accumulate unknown options, then parsing those into their own diffopts with the "range-diff-" prefix stripped and handing that data off to the range-diff machinery, not the parsed options for range-diff itself as happens now.

Previous: Junio C HamanoNext: Ævar Arnfjörð Bjarmason
Message 37 of 40 in “range-diff: add a --no-patch option to show a summary”
  1. range-diff: add a --no-patch option to show a summaryÆvar Arnfjörð Bjarmason, Nov 5, 2018
  2. Eric SunshineNov 5, 2018
  3. Ævar Arnfjörð BjarmasonNov 5, 2018
  4. Johannes SchindelinNov 6, 2018
  5. Ævar Arnfjörð BjarmasonNov 6, 2018
  6. Johannes SchindelinNov 7, 2018
  7. Johannes SchindelinNov 7, 2018
  8. Johannes SchindelinNov 7, 2018
  9. Johannes SchindelinNov 7, 2018
  10. 0/2 range-diff: doc + regression fixÆvar Arnfjörð Bjarmason, Nov 7, 2018
  11. 1/2 range-diff doc: add a section about output stabilityÆvar Arnfjörð Bjarmason, Nov 7, 2018
  12. Stephen & Linda SmithNov 7, 2018
  13. Junio C HamanoNov 7, 2018
  14. Martin ÅgrenNov 7, 2018
  15. 2/2 range-diff: fix regression in passing along diff optionsÆvar Arnfjörð Bjarmason, Nov 7, 2018
  16. Eric SunshineNov 8, 2018
  17. Ævar Arnfjörð BjarmasonNov 8, 2018
  18. Eric SunshineNov 9, 2018
  19. Ævar Arnfjörð BjarmasonNov 9, 2018
  20. 0/3 range-diff fixesÆvar Arnfjörð Bjarmason, Nov 9, 2018
  21. Johannes SchindelinNov 9, 2018
  22. 1/3 range-diff doc: add a section about output stabilityÆvar Arnfjörð Bjarmason, Nov 9, 2018
  23. 2/3 range-diff: fix regression in passing along diff optionsÆvar Arnfjörð Bjarmason, Nov 9, 2018
  24. 3/3 range-diff: make diff option behavior (e.g. --stat) consistentÆvar Arnfjörð Bjarmason, Nov 9, 2018
  25. Stephen & Linda SmithNov 9, 2018
  26. Eric SunshineNov 11, 2018
  27. Junio C HamanoNov 12, 2018
  28. Junio C HamanoNov 12, 2018
  29. 0/3 range-diff fixesÆvar Arnfjörð Bjarmason, Nov 13, 2018
  30. Johannes SchindelinNov 14, 2018
  31. 1/3 range-diff doc: add a section about output stabilityÆvar Arnfjörð Bjarmason, Nov 13, 2018
  32. 2/3 range-diff: fix regression in passing along diff optionsÆvar Arnfjörð Bjarmason, Nov 13, 2018
  33. 3/3 range-diff: make diff option behavior (e.g. --stat) consistentÆvar Arnfjörð Bjarmason, Nov 13, 2018
  34. Junio C HamanoNov 6, 2018
  35. Eric SunshineNov 6, 2018
  36. Junio C HamanoNov 6, 2018
  37. Ævar Arnfjörð BjarmasonNov 6, 2018
  38. range-diff: add a --no-patch option to show a summaryÆvar Arnfjörð Bjarmason, Nov 6, 2018
  39. Junio C HamanoNov 7, 2018
  40. Johannes SchindelinNov 7, 2018

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.