Christian Couder <christian.couder@xxxxxxxxx> writes: > On Wed, Jul 7, 2021 at 12:47 AM brian m. carlson > <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: >> >> In general, we encourage users to use plumbing commands, like git >> rev-list, over porcelain commands, like git log, when scripting. >> However, git rev-list has one glaring problem that prevents it from >> being used in certain cases: when --pretty is used, it always prints out >> a line containing "commit" and the object ID. > > You say always, but it looks like it doesn't do it when > --pretty=oneline is used. I've understood that he meant "when --pretty=format is used", though, as it is the only format whose intent was to allow generation of output stream that does not have to be preprocessed with "grep -v '^[0-9a-f]{40}'" etc. > It looks like the tests only check what happens in case > --pretty=format:'...' is used, but I wonder what the code does if a > builtin format is used. Good point. I also think the handling of --abbrev-commit may need to be rethought with this change. See here: diff --git a/builtin/rev-list.c b/builtin/rev-list.c index 7677b1af5a..f571cc9598 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -132,7 +132,7 @@ static void show_commit(struct commit *commit, void *data) if (revs->abbrev_commit && revs->abbrev) fputs(find_unique_abbrev(&commit->object.oid, revs->abbrev), stdout); - else + else if (revs->include_header) fputs(oid_to_hex(&commit->object.oid), stdout); if (revs->print_parents) { struct commit_list *parents = commit->parents; The original says that if --abbrev-commit is set and --abbrev is set to non-zero, we'd show unique abbreviation and if not, specifically, even when --abbrev-commit is set but --abbrev is set to 0, we didn't do the find_unique_abbrev() of full hexdigits but left the output to the else clause. This was because the original code KNOWS that the else clause unconditionally emits the full commit object name. That assumption, which made the original code's handling of "--abbrev-commit --abbrev=0" correct, no longer holds with the updated code. What happens is with --no-commit-header, if we give "--abbrev-commit --abbrev=4", we still see the unique abbreviation that is not shorter than 4 hexdigits (i.e. "--no-commit-header" is ignored), but if we say "--abbrev-commit --abbrev=0", we do not see any commit header. My hunch is that this "if / else", which determines if the commit header should give shortened or full length, should be skipped when the .include_header is false.