On Thu, Mar 24, 2011 at 04:03:16PM -0700, Junio C Hamano wrote:
> > [1/4]: pager: save the original stderr when redirecting to pager
> > [2/4]: progress: use pager's original_stderr if available
> > [3/4]: show: turn on rename detection progress reporting
> > [4/4]: diff: turn on rename detection progress reporting
>
> Thanks, but why does it affect t0101 and many others...?
Because I'm an idiot who, despite manually testing the option-parsing
part of the code a million times, didn't actually run the full suite.
> > while ((commit = get_revision(rev)) != NULL) {
> > - if (!log_tree_commit(rev, commit) &&
> > + int showed = log_tree_commit(rev, commit);
> > + if (showed &&
> > rev->max_count >= 0)
Ugh. Of course, it should be:
diff --git a/builtin/log.c b/builtin/log.c
index 4d52e99..b19e10d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -275,7 +275,7 @@ static int cmd_log_walk(struct rev_info *rev)
*/
while ((commit = get_revision(rev)) != NULL) {
int showed = log_tree_commit(rev, commit);
- if (showed &&
+ if (!showed &&
rev->max_count >= 0)
/*
* We decremented max_count in get_revision,
on top.
> After looking at the implementation of log_tree_commit(), shouldn't this
> part be more like this?
>
> int shown = log_tree_commit(rev, commit);
> if (!shown && rev->max_count >=0)
> rev->max_count++;
> if (shown)
> rev->diffopt.show_rename_progress = 0;
Yes. You shouldn't even need to look at log_tree_commit; you can see in
the hunk of my patch that I accidentally inverted the unrelated
max_count conditional while factoring out the call.
I'll go find a brown paper bag now.
-Peff
PS I assume you can squash in the above, or take your version (I don't
care about the verb tense we use, but re-indenting the max_count
conditional as you did is good style).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html