Re: [PATCH v6 2/3] revision: add peek functions for lookahead
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 20, 2026, 15:15 UTC
- Message-ID
- <xmqqtsqxfdl4.fsf@gitster.g>
- In-Reply-To
- <xmqqzf0pfefp.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
Show 17 quoted lines
> I _think_ this is OK, as "--graph" sets .rewrite_parents bit (as
> well as .topo_order bit) on, which makes want_ancestry() to return
> true. Which in turn means even if -L is in effect, we will not call
> line_log_process_ranges_arbitrary_commit() that is the only source
> of side effect in this function. Somebody needs to sanity check
> this, but we may want to leave an in-code comment to warn future
> developers not to call get_commit_action() on random commits outside
> of the normal history traversal under what condition (namely, -L
> without rewrite_parents).
>
> Even better, perhaps add
>
> if (revs->line_level_traverse && !want_ancestry(revs))
> BUG("do not call this");
>
> at the beginning of revision_has_commits_after() function, and
> describe why in the header file comment for this function below?Having said all that, in the longer term we might be better off if we fix the line-log code so that get_commit_action() becomes a pure function again.
It might be a very simple change to move the "if we are doing -L and !want_ancestry(), call the line_log_process_ranges_arbitrary_commit()" to simplify_commit() before it calls get_commit_action(), but I haven't thought things through.
[jc: Michael Cc'ed as there are a few topics on line-log recently from him; SZEDER Cc'ed as his 3cb9d2b6 (line-log: more responsive, incremental 'git log -L', 2020-05-11) introduced this side effect there.]
In any case, this is a remote tangent that does not affect how we want to proceed with this series ;-).