[0/3] prettier bisect output

Message

Jeff King Feb. 22, 2019, 6:19 a.m. UTC

On Thu, Feb 21, 2019 at 01:39:46PM -0800, Junio C Hamano wrote:

> > git bisect good
> > 3a9388eef42efc87c78ce22158d55e69a278b4eb is the first bad commit
> >
> > git --version
> > git version 2.14.1
> 
> Are you sure (and if so how did you reach that conclusion) that the
> above difference comes from the last 'bad' vs 'good' you finished,
> and not comes from the difference between 77c044d8 vs 3a9388ee?
> 
> At the end of the bisection session, bisect.c::show_diff_tree() is
> called on that "culprit" commit.  Is it possible that 3a9388ee is a
> simple and trivial merge that does not have anything worth reporting
> for "git diff-tree"?

I've run across this many times, too. Since it's been bugging me for a
decade, I thought I'd finally try to address it. Here are some patches.

There was some discussion about a year ago about just using "git show"
for this output:

  https://public-inbox.org/git/[e-mail]/

Christian seemed generally OK with tweaking the output, but preferred
not to move all the way to running an external "git show". I'm not sure
I completely agree, but it was easy enough to get the results I wanted
just by fiddling the current code a bit. ;)

  [1/3]: bisect: use string arguments to feed internal diff-tree
  [2/3]: bisect: fix internal diff-tree config loading
  [3/3]: bisect: make diff-tree output prettier

 bisect.c                    | 19 +++++--------------
 t/t6030-bisect-porcelain.sh |  6 +++---
 2 files changed, 8 insertions(+), 17 deletions(-)

-Peff

Comments

Junio C Hamano Feb. 22, 2019, 5:39 p.m. UTC | #1

Jeff King <[e-mail]> writes:

> I've run across this many times, too. Since it's been bugging me for a
> decade, I thought I'd finally try to address it. Here are some patches.
>
> There was some discussion about a year ago about just using "git show"
> for this output:
>
>   https://public-inbox.org/git/[e-mail]/
>
> Christian seemed generally OK with tweaking the output, but preferred
> not to move all the way to running an external "git show". I'm not sure
> I completely agree, but it was easy enough to get the results I wanted
> just by fiddling the current code a bit. ;)
>
>   [1/3]: bisect: use string arguments to feed internal diff-tree
>   [2/3]: bisect: fix internal diff-tree config loading
>   [3/3]: bisect: make diff-tree output prettier
>
>  bisect.c                    | 19 +++++--------------
>  t/t6030-bisect-porcelain.sh |  6 +++---
>  2 files changed, 8 insertions(+), 17 deletions(-)

Looks good from a quick glance.

One unrelated thing that made me curious was that the output from

	git grep 'is the first '

had these two lines:

bisect.c:		printf("%s is the first %s commit\n", oid_to_hex(bisect_rev),
git-bisect.sh:		if sane_grep "is the first $TERM_BAD commit" "$GIT_DIR/BISECT_RUN" >/dev/null

which means that we cannot localize this message without thought,
unlike the usual "hey, this is end-user facing, so wrap it in _()
out of spinal reflex."

Christian Couder March 3, 2019, 6:33 p.m. UTC | #2

On Fri, Feb 22, 2019 at 7:19 AM Jeff King <[e-mail]> wrote:
>
> On Thu, Feb 21, 2019 at 01:39:46PM -0800, Junio C Hamano wrote:

> > At the end of the bisection session, bisect.c::show_diff_tree() is
> > called on that "culprit" commit.  Is it possible that 3a9388ee is a
> > simple and trivial merge that does not have anything worth reporting
> > for "git diff-tree"?
>
> I've run across this many times, too. Since it's been bugging me for a
> decade, I thought I'd finally try to address it. Here are some patches.
>
> There was some discussion about a year ago about just using "git show"
> for this output:
>
>   https://public-inbox.org/git/[e-mail]/
>
> Christian seemed generally OK with tweaking the output, but preferred
> not to move all the way to running an external "git show".

Yeah, I am still OK with tweaking the output, though I'd rather not go
back to using an external process.

> I'm not sure
> I completely agree, but it was easy enough to get the results I wanted
> just by fiddling the current code a bit. ;)
>
>   [1/3]: bisect: use string arguments to feed internal diff-tree
>   [2/3]: bisect: fix internal diff-tree config loading
>   [3/3]: bisect: make diff-tree output prettier

I am OK with the above patches.

Thanks,
Christian.


>  bisect.c                    | 19 +++++--------------
>  t/t6030-bisect-porcelain.sh |  6 +++---
>  2 files changed, 8 insertions(+), 17 deletions(-)
>
> -Peff

Jeff King March 5, 2019, 4:16 a.m. UTC | #3

On Sun, Mar 03, 2019 at 07:33:17PM +0100, Christian Couder wrote:

> > I've run across this many times, too. Since it's been bugging me for a
> > decade, I thought I'd finally try to address it. Here are some patches.
> >
> > There was some discussion about a year ago about just using "git show"
> > for this output:
> >
> >   https://public-inbox.org/git/[e-mail]/
> >
> > Christian seemed generally OK with tweaking the output, but preferred
> > not to move all the way to running an external "git show".
> 
> Yeah, I am still OK with tweaking the output, though I'd rather not go
> back to using an external process.

By the way, I've done a few bisects since writing this, and I've been
very happy with the result. So I endorse my own patch. ;)

-Peff