Re: [PATCH/RFC] range-diff: make output format more useful for "rebase --onto"

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 18, 2021, 07:29 UTC
Message-ID: <87r1dmcooj.fsf@evledraar.gmail.com>
In-Reply-To: <xmqqv92yg5y6.fsf@gitster.g>

On Fri, Sep 17 2021, Junio C Hamano wrote:

Show 72 quoted lines

> 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@pobox.com>
> ---
>
>  * 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.