Re: [RFC/PATCH] Re: BUG? git log -Sfoo --max-count=N
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:44
Possibly related (same subject, not in this thread)
- 2016-06-15 · [RFC/PATCH] Re: BUG? git log -Sfoo --max-count=N · Matthieu Moy <hidden>
Matthieu Moy [off-list ref] writes:
In case the commit isn't actually shown, cancel the decrement of max_count.
I briefly wondered if this change is enough to deal with the way boundary commits are handled in revision.c::get_revision_internal(). Once the count drops to zero, the function switches to "boundary commits output mode", and incrementing the variable after that happens wouldn't have any effect, but that happens only upon the next call to get_revision() that has zero in max_count upon entry to the function, so incrementing here would be compatible with the precondition of that function. I agree that this codepath is subtle, but it doesn't feel a particularly good change to move the call to log_tree_commit() to get_revision() as a general callback, either. The function does way too many things other than "determine if this commit is shown and return 0/1".
quoted hunk
--- builtin/log.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)diff --git a/builtin/log.c b/builtin/log.c index f5ed690..b83900b 100644 --- a/builtin/log.c +++ b/builtin/log.c@@ -263,7 +263,12 @@ static int cmd_log_walk(struct rev_info *rev) * retain that state information if replacing rev->diffopt in this loop */ while ((commit = get_revision(rev)) != NULL) { - log_tree_commit(rev, commit); + if (!log_tree_commit(rev, commit)) + /* + * We decremented max_count in get_revision, + * but we didn't actually show the commit. + */ + rev->max_count++; if (!rev->reflog_info) { /* we allow cycles in reflog ancestry */ free(commit->buffer);-- 1.7.4.1.176.g6b069.dirty