On Wed, Jun 19, 2019 at 01:56:56PM -0700, Emily Shaffer wrote: > Allow easier parsing by cat-file by giving rev-list an option to print > only the OID of a non-commit object without any additional information. > This is a short-term shim; later on, rev-list should be taught how to > print the types of objects it finds in a format similar to cat-file's. > [...] I missed some of the intermediate rounds, but fortunately Junio already said everything I was going to. :) This version looks good to me, though with one minor nit: > diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt > index 88609ff435..9392760b25 100644 > --- a/Documentation/git-rev-list.txt > +++ b/Documentation/git-rev-list.txt > @@ -48,6 +48,7 @@ SYNOPSIS > [ --date=<format>] > [ [ --objects | --objects-edge | --objects-edge-aggressive ] > [ --unpacked ] > + [ --object-names | --no-object-names ] > [ --filter=<filter-spec> [ --filter-print-omitted ] ] ] > [ --missing=<missing-action> ] > [ --pretty | --header ] Here you put --object-names along with the --objects. Which kind of makes sense, but everything else in that block is about choosing _which_ commits to show. In the short help, you put it near --pretty: > @@ -49,6 +49,7 @@ static const char rev_list_usage[] = > " --objects | --objects-edge\n" > " --unpacked\n" > " --header | --pretty\n" > +" --[no-]object-names\n" > " --abbrev=<n> | --no-abbrev\n" > " --abbrev-commit\n" > " --left-right\n" which I think makes more sense. I think maybe you were trying to imply that "--object-names" is not useful unless you're also using "--objects". Which is true, but I'm not sure it's obvious from that mass of brackets (and I think is sufficiently covered in the actual option descriptions you give later). > +test_expect_success '--no-object-names and --object-names are last-one-wins' ' > + git rev-list --objects --no-object-names --object-names --all >output && > + grep wanted_file output && > + git rev-list --objects --object-names --no-object-names --all >output && > + ! grep wanted_file output > +' We don't generally test this behavior for each option, since it would lead to a ton of uninteresting tests (and parse-options generally just handles it). But after our discussion about --no-abbrev, I can see how you might be more interested in the topic. :) So I'm OK with it either way. -Peff
