Re: [PATCH v2] bisect: Honor log.date

[Peter Krefting <peter@softwolves.pp.se>]

Junio C Hamano:

> Yup, that is semi-understandable, but especially given that it is 
> one of the options used by the original "diff-tree"'s invocation, 
> and that we are trying to replace it with "show" from the same 
> family of commands, it is a bit of disappointment.

Indeed. I will make the necessary adjustments.

> FYI, attached is a comparison between the diff-tree output and 
> output from show with my choice of options for "show" picked from 
> the top of my head.

I am trying to run some comparisons, but I'm not entirely certain what 
the parameters are that were passed to "ls-tree", as it doesn't 
actually run it through a command line. I tried the v1.0.0^0 and are 
seeing discrepancies in the line count. I need to check if it is my 
configuration that causes it, or something else:

   $ git diff-tree --pretty --stat --summary --cc v1.0.0^0 | grep clone-pack.c
    clone-pack.c                                     | 153 ++----------------
   $ git show --stat --summary --no-abbrev-commit v1.0.0^0 | grep clone-pack.c
    clone-pack.c                                     | 151 ++----------------

(these are the options I've currently landed on)

> I do not think I personally like the --stat output applied to a 
> merge (--stat and --summary do not work N-way like --cc does for 
> patch text), but I think these options are the closest parallel to 
> what we have been giving to "diff-tree".

I don't really have a preference here. I usually only look at when 
something changed (which is why I initially targetted the date format; 
in Sweden the YYYY-MM-DD date format is the most prevalent) and the 
commit message (for bug tracker and code-review references and so on), 
less so the actual diff details (those I can look into later).

> $ git show -s --stat --summary --first-parent v1.0.0^0

Hmm, the git show manual page doesn't document supporting 
"--first-parent".

Jeff King:

> I guess that commit is what brought me into the cc. I have not been 
> following this topic too closely, but generally I'm in favor of 
> using "git show". I even suggested it back then, but I think 
> Christian preferred not using an external process if we could avoid 
> it.

I saw the code that tried to avoid calling one. I don't know the 
internals well enough here to figure out if we can do without, even 
when using git show?



That made me realize, if "git show" runs things through a pager, 
wouldn't it then lose the "%s is the first %s commit\n" message 
printed by bisect_next_all() before calling the function to show the 
contents?

Is that fixable?

> The thread from 2019 is here:
>
>  http://lore.kernel.org/git/20190222061949.GA9875@sigill.intra.peff.net
>
> which links to the earlier discussion about "git show":
>
>  https://lore.kernel.org/git/CAP8UFD3QhTUj+j3vBGrm0sTQ2dSOLS-m2_PwFj6DZS4VZHKRTQ@mail.gmail.com/

These two seems to also get it to honor settings (one to not colorize 
output, for instance). So this would be a step further.

> I do think keeping --summary is important; it's the only place we show
> mode changes, for example.

Yes, will fix that. I hadn't realized I lost that, since it wasn't 
something I have been using myself.

>  - "--no-patch" is doing nothing (passing --stat is enough to suppress
>    the default behavior of showing the patch).

Indeed. And it also negates "--summary", so I have dropped that.

>  - "--pretty=medium" is redundant at best (it's the default),

Dropped.

>  - I'm not sure what the intent is in adding --no-abbrev-commit. It is
>    already the default not to abbreviate it in the "commit <oid>" line,
>    and if the user has set log.abbrevcommit, shouldn't we respect that?

I think I added it because the diff-tree command did something 
similar. I can drop that as well ("bisect" displays the full commit 
hash anyway). I guess it mostly is for merges where we show the parent 
hashes?

-- 
\\// Peter - http://www.softwolves.pp.se/