Re: [PATCH v2] bisect: Honor log.date
- From
Peter Krefting <peter@softwolves.pp.se>
- Date
- Apr 1, 2024, 15:50 UTC
- Message-ID
- <c13c0751-0758-e068-282e-eb43496213b8@softwolves.pp.se>
- In-Reply-To
- <20240401023225.GA2639800@coredump.intra.peff.net>
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:
Show 5 quoted lines
> 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?
Show 7 quoted lines
> 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/