On Sat, Sep 18 2021, Ævar Arnfjörð Bjarmason wrote:
> On Fri, Sep 17 2021, Junio C Hamano wrote:
>
>> In a range-diff output, we often see an early part of an updated
>> series having no changes since the previous iteration. After
>> applying an updated patch submission to the same base as the
>> previous round,
>>
>> $ git range-diff master..topic@{1} master..topic
>>
>> to view the differences since the previous edition, we might see
>> something like this:
>>
>> 1: 9a05f02b1d = 1: a05f02b1d9 t/helper/test-bitmap.c: add ...
>> 2: 78de300e1f = 2: 8de300e1f7 pack-bitmap.c: propagate nam ...
>> 3: 7caca3c9f0 = 3: caca3c9f07 midx.c: respect 'pack.writeB ...
>> 4: 72082224f1 = 4: 2082224f17 p5326: create missing 'perf- ...
>> 5: 097b89c815 = 5: 97b89c8150 p5326: don't set core.multiP ...
>> 6: a1dd4c97b9 < -: ---------- p5326: generate pack bitmaps ...
>> -: ---------- > 6: bf4a60874a p5326: generate pack bitmaps ...
>> 7: 2b909ebad3 ! 7: 54156af0d6 t5326: test propagating hash ...
>> @@ t/t5326-multi-pack-bitmaps.sh: test_expect_success 'pack.preferBitmapTips' '
>> + (
>> + cd repo &&
>> ...
>>
>> Now, after noticing that up to step #5 there is no change since the
>> previous round, cleaning up the application result with
>>
>> $ git rebase --onto 097b89c815 97b89c8150
>>
>> will help making it easier to see that earliser part did not change
>> before committing this in the longer term history.
>>
>> The output format of the range-diff unfortunately makes it a bit
>> cumbersome than necessary to come up with the rebase command line.
>> Because "= 5:" gets in the way, copying the two object names from
>> there and pasting them as the command line arguments to "git rebase
>> --onto" becomes a chore.
>>
>> Tweak the output so that the change numbers and comparison sign come
>> first on the line, followed by two object names and then the title
>> of commit, to make it easier to copy the two object names together.
>>
>> The updated output format looks like this instead:
>>
>> 1 = 1 a05f02b1d9 a05f02b1d9 : t/helper/test-bitmap.c: add ...
>> 2 = 2 8de300e1f7 8de300e1f7 : pack-bitmap.c: propagate nam ...
>> 3 = 3 caca3c9f07 caca3c9f07 : midx.c: respect 'pack.writeB ...
>> 4 = 4 2082224f17 2082224f17 : p5326: create missing 'perf- ...
>> 5 = 5 97b89c8150 97b89c8150 : p5326: don't set core.multiP ...
>> 6 < - a1dd4c97b9 ---------- : p5326: generate pack bitmaps ...
>> - > 6 ---------- bf4a60874a : p5326: generate pack bitmaps ...
>> 7 ! 7 2b909ebad3 54156af0d6 : t5326: test propagating hash ...
>> @@ t/t5326-multi-pack-bitmaps.sh: test_expect_success 'pack.preferBitmapTips' '
>> + (
>> + cd repo &&
>>
>> Incidentally, it becomes easier to see the correspondence and spot
>> the reordering of the commits with this change, for the same reason
>> why it becomes easier to see the two commit object names---they sit
>> close to each other with their peers.
>>
>> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
>> ---
>>
>> * Obviously an RFC. The output format would be fairly subjective,
>> but I do not care deeply enough to make it configurable. If
>> enough people appreciate the convenience of seeing the two object
>> names and two change numbers next to each other like I do, and
>> nobody processes the current output with tools, then it may be OK
>> to take this patch as the final version, but otherwise, this is
>> only meant as an inspiration piece for somebody else to have a
>> mechanism to allow the output configurable in some way (which I
>> am not intereseted in doing myself).
>
> I think we carved out a sufficient exception in df569c3f31f (range-diff
> doc: add a section about output stability, 2018-11-09) to just change
> the output.
>
> I do happen to have a one-liner as part of my build process that relies
> on the current output, but I've only got myself to blame. Aside from the
> change being proposed here I think we can just change it in general if
> we come up with better output.
>
> As for the proposed output, I'm a bit negative on it, so first, if we're
> trying to note that two commits are the same wouldn't it be much better
> to just omit the second SHA-1? I.e. consider this variation of your
> proposed output;
>
> 1 = 1 a05f02b1d9 a05102bfd9 : t/helper/test-bitmap.c: add ...
>
> You might eyeball that for a while before discovering that I switched
> the "f" and "1" around, i.e. the SHA-1s look /almost/ the same. Isn't
> this better? (or we could use "++++++++++" to not overlead any meaning
> "----------" has):
>
> 1 = 1 a05f02b1d9 ---------- : t/helper/test-bitmap.c: add ...
> 2 = 2 8de300e1f7 ---------- : pack-bitmap.c: propagate nam ...
> 3 = 3 caca3c9f07 ---------- : midx.c: respect 'pack.writeB ...
> 4 = 4 2082224f17 ---------- : p5326: create missing 'perf- ...
> 5 = 5 97b89c8150 ---------- : p5326: don't set core.multiP ...
> 6 < - a1dd4c97b9 ---------- : p5326: generate pack bitmaps ...
> - > 6 ---------- bf4a60874a : p5326: generate pack bitmaps ...
> 7 ! 7 2b909ebad3 54156af0d6 : t5326: test propagating hash ...
>
> (Or whatever other syntax would follow from the shorthand of the "1 =
> 1". Having it repeated in the human-readable output just for the one
> use-case of passing it to rebase --onto doesn't seem worth it.
>
> But then if we get support for say --ignore-matching-lines (which I've
> wanted to for a while to omit the Signed-off-by lines you add) we'll
> note the same SHA1 if you used that option, but a different one in the
> same slot if you omit it (i.e. we'd start showing that part of the
> diff).
>
> Then combine that with --left-only or --right-only and we'd ignore the
> diff depending on what "side" it was, and then either duplicate the
> SHA-1 or not.
As a follow-up, this proposal really mixes two things, the appearance of
the text format & its semantics. I.e. it's changing (1)
1: 9a05f02b1d = 1: a05f02b1d9 t/helper/test-bitmap.c: add ...
To (2)
1 = 1 a05f02b1d9 a05f02b1d9 : t/helper/test-bitmap.c: add ...
Not just (3)
1 = 1 9a05f02b1d a05f02b1d9 : t/helper/test-bitmap.c: add ...
I happen to like (3) a lot better to format the existing information, it
emits the "1 = 1" so you can see at a glance what maps to what, but I'm
not sure about (2) as a semantic change..
