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@xxxxxxxxxxxxxxxxxxxxx which links to the earlier discussion about "git show": https://lore.kernel.org/git/CAP8UFD3QhTUj+j3vBGrm0sTQ2dSOLS-m2_PwFj6DZS4VZHKRTQ@xxxxxxxxxxxxxx/
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/
