On Fri, May 01, 2020 at 03:30:26PM +0000, SZEDER Gábor via GitGitGadget wrote:
> From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= <szeder.dev@xxxxxxxxx>
>
> None of the tests in 't4211-line-log.sh' really check which parent
> object IDs are shown in the output, either implicitly as part of
> "Merge: ..." lines [1] or explicitly via the '%p' or '%P' format
> specifiers in a custom pretty format.
>
> Add two tests to 't4211-line-log.sh' to check which parent object IDs
> are shown, one without and one with explicitly requested parent
> rewriting, IOW without and with the '--parents' option.
>
> The test without '--parents' is marked as failing, because without
> that option parent rewriting should not be performed, and thus the
> parent object ID should be that of the immediate parent, just like in
> case of a pathspec-limited history traversal without parent rewriting.
> The current line-level log implementation, however, performs parent
> rewriting unconditionally and without a possibility to turn it off,
> and, consequently, it shows the object ID of the most recent ancestor
> that modified the given line range.
>
> In both of these new tests we only really care about the object IDs of
> the listed commits and their parents, but not the diffs of the line
> ranges; the diffs have already been thoroughly checked in the previous
> tests.
>
> [1] While one of the tests ('-M -L ':f:b.c' parallel-change') does
> list a merge commit, both of its parents happen to modify the
> given line range and are listed as well, so the implications of
> parent rewriting remained hidden and untested.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@xxxxxxxxx>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
These tests make sense, and demonstrate the problem nicely. I don't
think that I have a ton to add to this patch, so:
Reviewed-by: Taylor Blau <me@xxxxxxxxxxxx>
Thanks,
Taylor
