{"thread":{"id":"51264","subject":"[PATCH] revision: remove stray whitespace when name empty","startedAt":"2019-06-07T22:59:26Z","lastAt":"2019-06-19T21:38:08Z","messageCount":22,"participants":["Emily Shaffer","Eric Sunshine","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"376854","messageId":"20190607225900.89299-1-emilyshaffer@google.com","threadId":"51264","inReplyTo":null,"subject":"[PATCH] revision: remove stray whitespace when name empty","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-07T22:59:00Z","receivedAt":"2019-06-07T22:59:26Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Teach show_object_with_name() to avoid writing a space before a name\nwhich is empty. Also teach tests for rev-list --objects --filter to not\nrequire a space between the object ID and name.\n\nshow_object_with_name() inserts a space between an object's OID and name\nregardless of whether the name is empty or not. This causes 'git\ncat-file (--batch | --batch-check)' to fail to discover the type of root\ndirectories:\n\n  git rev-list --objects --filter=tree:1 --max-count=1 HEAD \\\n    | git cat-file --batch-check\n  git rev-parse HEAD: | xargs -I% git cat-file -t %\n  git rev-list --objects --filter=tree:1 --max-count=1 HEAD \\\n    | xargs -I% echo \"AA%AA\"\n  git rev-list --objects --filter=tree:1 --max-count=1 HEAD \\\n    | cut -f 1 -d ' ' | git cat-file --batch-check\n\nThe extra space provided by 'show_object_with_name()' sticks to the\noutput of 'git rev-list' even if it's piped into a file.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\nI don't see any reason _not_ to remove this stray whitespace at the end,\nsince it seems like it just gets in the way of easy scripting. I also\nthink this case will only present itself for root trees.\n\nNot sure if making the whitespace optional is the right solution for the\ntest, although I couldn't come up with a cleaner approach. Maybe\nsomething like this would be better, even though handling it in the\nregex is shorter?\n\n  if [[ -z \"$name\" ]] then\n    grep \"^$hash\" actual\n  else\n    grep \"^$hash $name\" actual\n  fi\n\n - Emily\n\n revision.c                          | 3 ++-\n t/t6112-rev-list-filters-objects.sh | 2 +-\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex d4aaf0ef25..260258b371 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -40,7 +40,8 @@ void show_object_with_name(FILE *out, struct object *obj, const char *name)\n {\n \tconst char *p;\n \n-\tfprintf(out, \"%s \", oid_to_hex(&obj->oid));\n+\tfprintf(out, \"%s%s\", oid_to_hex(&obj->oid),\n+\t\tstrcmp(name, \"\") == 0 ? \"\" : \" \");\n \tfor (p = name; *p && *p != '\\n'; p++)\n \t\tfputc(*p, out);\n \tfputc('\\n', out);\ndiff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh\nindex acd7f5ab80..e05faa7a67 100755\n--- a/t/t6112-rev-list-filters-objects.sh\n+++ b/t/t6112-rev-list-filters-objects.sh\n@@ -288,7 +288,7 @@ expect_has () {\n \tname=$2 &&\n \n \thash=$(git -C r3 rev-parse $commit:$name) &&\n-\tgrep \"^$hash $name$\" actual\n+\tgrep \"^$hash \\?$name$\" actual\n }\n \n test_expect_success 'verify tree:1 includes root trees' '\n-- \n2.22.0.rc2.383.gf4fbbf30c2-goog\n\n"},{"id":"376855","messageId":"CAPig+cS7ocDU3fGRPnQZqN+rT1Ujd2b=v5b1p5V6G++XmNPmsQ@mail.gmail.com","threadId":"51264","inReplyTo":"20190607225900.89299-1-emilyshaffer@google.com","subject":"Re: [PATCH] revision: remove stray whitespace when name empty","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-06-08T00:42:45Z","receivedAt":"2019-06-08T00:43:01Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jun 7, 2019 at 6:59 PM Emily Shaffer <emilyshaffer@google.com> wrote:\n> Teach show_object_with_name() to avoid writing a space before a name\n> which is empty. Also teach tests for rev-list --objects --filter to not\n> require a space between the object ID and name.\n> [...]\n> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> ---\n> Not sure if making the whitespace optional is the right solution for the\n> test, although I couldn't come up with a cleaner approach. Maybe\n> something like this would be better, even though handling it in the\n> regex is shorter?\n>\n>   if [[ -z \"$name\" ]] then\n\nIn Git, we avoid Bash-isms. Just use 'test'. And, style is to place\n'then' on its own line.\n\n    if test -z \"$name\"\n    then\n\n> diff --git a/revision.c b/revision.c\n> @@ -40,7 +40,8 @@ void show_object_with_name(FILE *out, struct object *obj, const char *name)\n>  {\n> -       fprintf(out, \"%s \", oid_to_hex(&obj->oid));\n> +       fprintf(out, \"%s%s\", oid_to_hex(&obj->oid),\n> +               strcmp(name, \"\") == 0 ? \"\" : \" \");\n>         for (p = name; *p && *p != '\\n'; p++)\n>                 fputc(*p, out);\n>         fputc('\\n', out);\n\nIt's subjective, but this starts to be a bit too noisy and unreadable.\nAn alternative:\n\n    fputs(oid_to_hex(...), out);\n    if (*name)\n        putc(' ', out);\n\n> diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh\n> @@ -288,7 +288,7 @@ expect_has () {\n>         hash=$(git -C r3 rev-parse $commit:$name) &&\n> -       grep \"^$hash $name$\" actual\n> +       grep \"^$hash \\?$name$\" actual\n\nThis would be the first use of \\? with 'grep' in the test suite. I\nwould worry about portability. Your suggestion earlier to tailor\n'grep' invocation based upon whether $name is empty would be safer.\n"},{"id":"376882","messageId":"20190609130004.GB23555@sigill.intra.peff.net","threadId":"51264","inReplyTo":"20190607225900.89299-1-emilyshaffer@google.com","subject":"Re: [PATCH] revision: remove stray whitespace when name empty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-09T13:00:04Z","receivedAt":"2019-06-09T13:00:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 07, 2019 at 03:59:00PM -0700, Emily Shaffer wrote:\n\n> Teach show_object_with_name() to avoid writing a space before a name\n> which is empty. Also teach tests for rev-list --objects --filter to not\n> require a space between the object ID and name.\n> [...]\n> ---\n> I don't see any reason _not_ to remove this stray whitespace at the end,\n> since it seems like it just gets in the way of easy scripting. I also\n> think this case will only present itself for root trees.\n\nI'm a bit worried that this might break existing scripts. As ugly as\ntrailing whitespace is, it does tell you something here: that the object\nis a root tree and not a commit.\n\nSo in the past I have done things like:\n\n  git rev-list --objects --all | grep ' '\n\nto get only the non-commits. I'm undecided on whether we're straying\ninto https://xkcd.com/1172/ territory here. I'd be more in favor if this\nwere making things significantly easier, but...\n\n> show_object_with_name() inserts a space between an object's OID and name\n> regardless of whether the name is empty or not. This causes 'git\n> cat-file (--batch | --batch-check)' to fail to discover the type of root\n> directories:\n> \n>   git rev-list --objects --filter=tree:1 --max-count=1 HEAD \\\n>     | git cat-file --batch-check\n>   git rev-parse HEAD: | xargs -I% git cat-file -t %\n>   git rev-list --objects --filter=tree:1 --max-count=1 HEAD \\\n>     | xargs -I% echo \"AA%AA\"\n>   git rev-list --objects --filter=tree:1 --max-count=1 HEAD \\\n>     | cut -f 1 -d ' ' | git cat-file --batch-check\n\nYour patch only helps with this at all because you're using the \"tree:1\"\nfilter. It would not help:\n\n  git rev-list --objects HEAD | git cat-file --batch-check\n\nbecause there you'll have actual names which cat-file will choke on. So\nit seems like this is helping only a very limited use case.\n\ncat-file actually does know how to split on whitespace. Unfortunately it\ndoes not do so by default, because that breaks some cases. See\n97be04077f (cat-file: only split on whitespace when %(rest) is used,\n2013-08-02).\n\nSo you _can_ do:\n\n  git rev-list --objects HEAD |\n  git cat-file --batch-check='%(objectname) %(objecttype) %(rest)'\n\nBut:\n\n  1. That puts the %(rest) bits in your output, which you may not want.\n\n  2. You have to actually specify the full format, so you might have to\n     repeat batch-check's default format items.\n\nI think it would be reasonable for cat-file to have an option to split\non whitespace (and if not given explicitly by the user, default to the\npresence of %(rest) as we do now).\n\nAlternatively, it would be reasonable to me to have an option for\n\"rev-list --objects\" to have an option to suppress the filename (and\nspace) entirely.\n\nI think in the longer run along those lines that \"--objects\" should\nallow cat-file style pretty-print formats, which would eliminate the\nneed to pipe to cat-file in the first place. That makes this parsing\nproblem go away entirely, and it's way more efficient to boot (rev-list\nalready knows the types!).\n\n-Peff\n"},{"id":"376944","messageId":"xmqqsgsho145.fsf@gitster-ct.c.googlers.com","threadId":"51264","inReplyTo":"20190609130004.GB23555@sigill.intra.peff.net","subject":"Re: [PATCH] revision: remove stray whitespace when name empty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-10T16:29:14Z","receivedAt":"2019-06-10T16:29:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Your patch only helps with this at all because you're using the \"tree:1\"\n> ...\n> because there you'll have actual names which cat-file will choke on. So\n> it seems like this is helping only a very limited use case.\n> ...\n> Alternatively, it would be reasonable to me to have an option for\n> \"rev-list --objects\" to have an option to suppress the filename (and\n> space) entirely.\n\nYup, I think that is a more reasonable short-term change compared to\nthe patch being discussed, and ...\n\n> I think in the longer run along those lines that \"--objects\" should\n> allow cat-file style pretty-print formats, which would eliminate the\n> need to pipe to cat-file in the first place. That makes this parsing\n> problem go away entirely, and it's way more efficient to boot (rev-list\n> already knows the types!).\n\n... of course this makes tons of sense.\n\nThanks.\n"},{"id":"377106","messageId":"20190612192302.GA233791@google.com","threadId":"51264","inReplyTo":"CAPig+cS7ocDU3fGRPnQZqN+rT1Ujd2b=v5b1p5V6G++XmNPmsQ@mail.gmail.com","subject":"Re: [PATCH] revision: remove stray whitespace when name empty","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-12T19:23:02Z","receivedAt":"2019-06-12T19:23:10Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Jun 07, 2019 at 08:42:45PM -0400, Eric Sunshine wrote:\n> On Fri, Jun 7, 2019 at 6:59 PM Emily Shaffer <emilyshaffer@google.com> wrote:\n> > Teach show_object_with_name() to avoid writing a space before a name\n> > which is empty. Also teach tests for rev-list --objects --filter to not\n> > require a space between the object ID and name.\n> > [...]\n> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> > ---\n> > Not sure if making the whitespace optional is the right solution for the\n> > test, although I couldn't come up with a cleaner approach. Maybe\n> > something like this would be better, even though handling it in the\n> > regex is shorter?\n> >\n> >   if [[ -z \"$name\" ]] then\n> \n> In Git, we avoid Bash-isms. Just use 'test'. And, style is to place\n> 'then' on its own line.\n> \n>     if test -z \"$name\"\n>     then\n> \n> > diff --git a/revision.c b/revision.c\n> > @@ -40,7 +40,8 @@ void show_object_with_name(FILE *out, struct object *obj, const char *name)\n> >  {\n> > -       fprintf(out, \"%s \", oid_to_hex(&obj->oid));\n> > +       fprintf(out, \"%s%s\", oid_to_hex(&obj->oid),\n> > +               strcmp(name, \"\") == 0 ? \"\" : \" \");\n> >         for (p = name; *p && *p != '\\n'; p++)\n> >                 fputc(*p, out);\n> >         fputc('\\n', out);\n> \n> It's subjective, but this starts to be a bit too noisy and unreadable.\n> An alternative:\n> \n>     fputs(oid_to_hex(...), out);\n>     if (*name)\n>         putc(' ', out);\n> \n> > diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh\n> > @@ -288,7 +288,7 @@ expect_has () {\n> >         hash=$(git -C r3 rev-parse $commit:$name) &&\n> > -       grep \"^$hash $name$\" actual\n> > +       grep \"^$hash \\?$name$\" actual\n> \n> This would be the first use of \\? with 'grep' in the test suite. I\n> would worry about portability. Your suggestion earlier to tailor\n> 'grep' invocation based upon whether $name is empty would be safer.\n\n\nThanks for the review, Eric. I'm going to try again from scratch with\nPeff and Junio's suggestion about adding an option to remove object\nnames, and I'll try to keep your general style concerns in mind when I\ndo so.\n\n - Emily\n"},{"id":"377108","messageId":"20190612193745.GB233791@google.com","threadId":"51264","inReplyTo":"xmqqsgsho145.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] revision: remove stray whitespace when name empty","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-12T19:37:45Z","receivedAt":"2019-06-12T19:37:54Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Jun 10, 2019 at 09:29:14AM -0700, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > Your patch only helps with this at all because you're using the \"tree:1\"\n> > ...\n> > because there you'll have actual names which cat-file will choke on. So\n> > it seems like this is helping only a very limited use case.\n> > ...\n> > Alternatively, it would be reasonable to me to have an option for\n> > \"rev-list --objects\" to have an option to suppress the filename (and\n> > space) entirely.\n> \n> Yup, I think that is a more reasonable short-term change compared to\n> the patch being discussed, and ...\n\nI like this too. Starting work on it today.\n\n> \n> > I think in the longer run along those lines that \"--objects\" should\n> > allow cat-file style pretty-print formats, which would eliminate the\n> > need to pipe to cat-file in the first place. That makes this parsing\n> > problem go away entirely, and it's way more efficient to boot (rev-list\n> > already knows the types!).\n> \n> ... of course this makes tons of sense.\n\nProbably not going to start work on this now, but I will make a note to\nmyself to have a look if I have some spare time soon, and keep an eye\nout for reviews in that area.\n\n> \n> Thanks.\n\nThanks all for the feedback, I'll have a reroll soon.\n\n - Emily\n"},{"id":"377120","messageId":"20190613152028.GA7459@sigill.intra.peff.net","threadId":"51264","inReplyTo":"20190612193745.GB233791@google.com","subject":"Re: [PATCH] revision: remove stray whitespace when name empty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-13T15:20:28Z","receivedAt":"2019-06-13T15:20:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 12, 2019 at 12:37:45PM -0700, Emily Shaffer wrote:\n\n> > > Alternatively, it would be reasonable to me to have an option for\n> > > \"rev-list --objects\" to have an option to suppress the filename (and\n> > > space) entirely.\n> > \n> > Yup, I think that is a more reasonable short-term change compared to\n> > the patch being discussed, and ...\n> \n> I like this too. Starting work on it today.\n\nGreat!\n\n> > > I think in the longer run along those lines that \"--objects\" should\n> > > allow cat-file style pretty-print formats, which would eliminate the\n> > > need to pipe to cat-file in the first place. That makes this parsing\n> > > problem go away entirely, and it's way more efficient to boot (rev-list\n> > > already knows the types!).\n> > \n> > ... of course this makes tons of sense.\n> \n> Probably not going to start work on this now, but I will make a note to\n> myself to have a look if I have some spare time soon, and keep an eye\n> out for reviews in that area.\n\nThat's fine. I suspect it will be quite involved, because the format\nplaceholders you'd want for \"objects\" are going to be more like\n\"cat-file\" ones than like the existing pretty.c ones. So I think this\nreally implies unifying those format systems, which is a long-term\nproject that Olga has been working on.\n\n-Peff\n"},{"id":"377190","messageId":"20190613215102.44627-1-emilyshaffer@google.com","threadId":"51264","inReplyTo":"20190607225900.89299-1-emilyshaffer@google.com","subject":"[PATCH v2] rev-list: teach --oid-only to enable piping","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-13T21:51:03Z","receivedAt":"2019-06-13T21:51:24Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Allow easier parsing by cat-file by giving rev-list an option to print\nonly the OID of an object without any additional information. This is a\nshort-term shim; later on, rev-list should be taught how to print the\ntypes of objects it finds in a format similar to cat-file's.\n\nBefore this commit, the output from rev-list needed to be massaged\nbefore being piped to cat-file, like so:\n\n  git rev-list --objects HEAD | cut -f 1 -d ' ' \\\n    | git cat-file --batch-check\n\nThis was especially unexpected when dealing with root trees, as an\ninvisible whitespace exists at the end of the OID:\n\n  git rev-list --objects --filter=tree:1 --max-count=1 HEAD \\\n    | xargs -I% echo \"AA%AA\"\n\nNow, it can be piped directly, as in the added test case:\n\n  git rev-list --objects --oid-only HEAD | git cat-file --batch-check\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\nChange-Id: I489bdf0a8215532e540175188883ff7541d70e1b\n---\nIt didn't appear that using an existing --pretty string would do the\ntrick, as the formatting options for --pretty apply specifically to\ncommit objects (you can specify the commit hash and the tree hash, but\nI didn't see a way to more generally pretty-print all types of objects).\nrev-list doesn't appear to use parse-options-h, so I added a new option\nas simply as I could see without breaking existing style; it seemed\nwrong to add a flag to the `struct rev_list_info` as the definition of\nstruct and flag values alike are contained in bisect.h. There are a\ncouple other options to rev-list which are stored as globals.\n\nAt the moment, this doesn't work with --abbrev, and although I think\nit'd be a good fit, right now --abbrev doesn't seem to work without\n--abbrev-commit, and both those options end up being eaten by\nsetup_revisions() rather than by rev-list itself.\n\n - Emily\n\n builtin/rev-list.c       | 21 ++++++++++++++++++++-\n t/t6000-rev-list-misc.sh | 18 ++++++++++++++++++\n 2 files changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 9f31837d30..ff07ea9564 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -48,7 +48,7 @@ static const char rev_list_usage[] =\n \"    --children\\n\"\n \"    --objects | --objects-edge\\n\"\n \"    --unpacked\\n\"\n-\"    --header | --pretty\\n\"\n+\"    --header | --pretty | --oid-only\\n\"\n \"    --abbrev=<n> | --no-abbrev\\n\"\n \"    --abbrev-commit\\n\"\n \"    --left-right\\n\"\n@@ -75,6 +75,8 @@ enum missing_action {\n };\n static enum missing_action arg_missing_action;\n \n+static int arg_oid_only; /* display only the oid of each object encountered */\n+\n #define DEFAULT_OIDSET_SIZE     (16*1024)\n \n static void finish_commit(struct commit *commit, void *data);\n@@ -90,6 +92,11 @@ static void show_commit(struct commit *commit, void *data)\n \t\treturn;\n \t}\n \n+\tif (arg_oid_only) {\n+\t\tprintf(\"%s\\n\", oid_to_hex(&commit->object.oid));\n+\t\treturn;\n+\t}\n+\n \tgraph_show_commit(revs->graph);\n \n \tif (revs->count) {\n@@ -255,6 +262,10 @@ static void show_object(struct object *obj, const char *name, void *cb_data)\n \tdisplay_progress(progress, ++progress_counter);\n \tif (info->flags & REV_LIST_QUIET)\n \t\treturn;\n+\tif (arg_oid_only) {\n+\t\tprintf(\"%s\\n\", oid_to_hex(&obj->oid));\n+\t\treturn;\n+\t}\n \tshow_object_with_name(stdout, obj, name);\n }\n \n@@ -484,6 +495,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\tif (skip_prefix(arg, \"--missing=\", &arg))\n \t\t\tcontinue; /* already handled above */\n \n+\t\tif (!strcmp(arg, (\"--oid-only\"))) {\n+\t\t\targ_oid_only = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tusage(rev_list_usage);\n \n \t}\n@@ -499,6 +515,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t/* Only --header was specified */\n \t\trevs.commit_format = CMIT_FMT_RAW;\n \n+\tif (arg_oid_only && revs.commit_format != CMIT_FMT_UNSPECIFIED)\n+\t\tdie(_(\"cannot combine --oid-only and --pretty or --header\"));\n+\n \tif ((!revs.commits && reflog_walk_empty(revs.reflog_info) &&\n \t     (!(revs.tag_objects || revs.tree_objects || revs.blob_objects) &&\n \t      !revs.pending.nr) &&\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 0507999729..996ba1799b 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -48,6 +48,24 @@ test_expect_success 'rev-list --objects with pathspecs and copied files' '\n \t! grep one output\n '\n \n+test_expect_success 'rev-list --objects --oid-only has no whitespace/names' '\n+\tgit rev-list --objects --oid-only HEAD >output &&\n+\t! grep wanted_file output &&\n+\t! grep unwanted_file output &&\n+\t! grep \" \" output\n+'\n+\n+test_expect_success 'rev-list --objects --oid-only is usable by cat-file' '\n+\tgit rev-list --objects --oid-only --all >list-output &&\n+\tgit cat-file --batch-check <list-output >cat-output &&\n+\t! grep missing cat-output\n+'\n+\n+test_expect_success 'rev-list --oid-only is incompatible with --pretty' '\n+\ttest_must_fail git rev-list --objects --oid-only --pretty HEAD &&\n+\ttest_must_fail git rev-list --objects --oid-only --header HEAD\n+'\n+\n test_expect_success 'rev-list A..B and rev-list ^A B are the same' '\n \tgit commit --allow-empty -m another &&\n \tgit tag -a -m \"annotated\" v1.0 &&\n-- \n2.22.0.rc2.383.gf4fbbf30c2-goog\n\n"},{"id":"377242","messageId":"20190614160728.GA30083@sigill.intra.peff.net","threadId":"51264","inReplyTo":"20190613215102.44627-1-emilyshaffer@google.com","subject":"Re: [PATCH v2] rev-list: teach --oid-only to enable piping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-14T16:07:28Z","receivedAt":"2019-06-14T16:07:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 13, 2019 at 02:51:03PM -0700, Emily Shaffer wrote:\n\n> It didn't appear that using an existing --pretty string would do the\n> trick, as the formatting options for --pretty apply specifically to\n> commit objects (you can specify the commit hash and the tree hash, but\n> I didn't see a way to more generally pretty-print all types of objects).\n\nRight, it would be a big change to start custom-formatting the\nnon-commits. I think this is a good step in the right direction.\n\n> rev-list doesn't appear to use parse-options-h, so I added a new option\n> as simply as I could see without breaking existing style; it seemed\n> wrong to add a flag to the `struct rev_list_info` as the definition of\n> struct and flag values alike are contained in bisect.h. There are a\n> couple other options to rev-list which are stored as globals.\n\nI think that's reasonable. This is definitely a rev-list option (not a\nrevision.c one). The interaction between bisect.h and rev-list is a bit\nfunny, but it's probably simpler not to try solving it here.\n\n> +\tif (arg_oid_only && revs.commit_format != CMIT_FMT_UNSPECIFIED)\n> +\t\tdie(_(\"cannot combine --oid-only and --pretty or --header\"));\n\nAs you've implemented it here, I definitely agree that this conflicts\nwith --pretty. But I think it also interacts badly with other options,\nlike \"--graph\".\n\nTBH, I am not sure that \"rev-list --graph --objects\" is all that useful,\nbecause the non-commit objects are not prefixed with the graph lines.\nAnd without \"--objects\", this new option is not useful because the\ndefault is already to show only the oid.\n\nBut I wonder if things would be simpler if we did not touch the commit\ncode path at all. I.e., if this were simply \"--no-object-names\", and it\ntouched only show_object(). And then you do not have to worry about\nfunny interactions with commit formatting at all. It would be valid to\ndo:\n\n  git rev-list --pretty=raw --objects --no-object-names HEAD\n\nif you really wanted to (though I admit I do not have an immediate use\nfor it).\n\n> @@ -255,6 +262,10 @@ static void show_object(struct object *obj, const char *name, void *cb_data)\n>  \tdisplay_progress(progress, ++progress_counter);\n>  \tif (info->flags & REV_LIST_QUIET)\n>  \t\treturn;\n> +\tif (arg_oid_only) {\n> +\t\tprintf(\"%s\\n\", oid_to_hex(&obj->oid));\n> +\t\treturn;\n> +\t}\n>  \tshow_object_with_name(stdout, obj, name);\n>  }\n>  \n\nA minor style point, but I think this might be easier to follow without\nthe early return, since we are really choosing to do A or B. Writing:\n\n  if (arg_oid_only)\n\tprintf(...);\n  else\n\tshow_object_with_name(...);\n\nshows that more clearly, I think.\n\n> [...]\n\nOtherwise, this looks good, including the tests. One thing I did notice\nin the tests:\n\n> +test_expect_success 'rev-list --objects --oid-only is usable by cat-file' '\n> +\tgit rev-list --objects --oid-only --all >list-output &&\n> +\tgit cat-file --batch-check <list-output >cat-output &&\n> +\t! grep missing cat-output\n> +'\n\nUsually we prefer to look for the expected output, rather than making\nsure we did not find the unexpected. But I'm not sure if that might be\nburdensome in this case (i.e., if there's a bunch of cruft coming out of\n\"rev-list\" that would be annoying to match, and might even change as\npeople add more tests). So I'm OK with it either way.\n\n-Peff\n"},{"id":"377254","messageId":"xmqqlfy3ud60.fsf@gitster-ct.c.googlers.com","threadId":"51264","inReplyTo":"20190614160728.GA30083@sigill.intra.peff.net","subject":"Re: [PATCH v2] rev-list: teach --oid-only to enable piping","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-14T20:25:59Z","receivedAt":"2019-06-14T20:26:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> But I wonder if things would be simpler if we did not touch the commit\n> code path at all. I.e., if this were simply \"--no-object-names\", and it\n> touched only show_object().\n\nYeah, that sounds more tempting.  And the refined code structure you\nsuggested ...\n\n>> @@ -255,6 +262,10 @@ static void show_object(struct object *obj, const char *name, void *cb_data)\n>>  \tdisplay_progress(progress, ++progress_counter);\n>>  \tif (info->flags & REV_LIST_QUIET)\n>>  \t\treturn;\n>> +\tif (arg_oid_only) {\n>> +\t\tprintf(\"%s\\n\", oid_to_hex(&obj->oid));\n>> +\t\treturn;\n>> +\t}\n>>  \tshow_object_with_name(stdout, obj, name);\n>>  }\n>>  \n>\n> A minor style point, but I think this might be easier to follow without\n> the early return, since we are really choosing to do A or B. Writing:\n>\n>   if (arg_oid_only)\n> \tprintf(...);\n>   else\n> \tshow_object_with_name(...);\n>\n> shows that more clearly, I think.\n\n... is a good way to clearly show that intention, I would think.\n\n"},{"id":"377273","messageId":"20190614231848.GE233791@google.com","threadId":"51264","inReplyTo":"xmqqlfy3ud60.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2] rev-list: teach --oid-only to enable piping","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-14T23:18:48Z","receivedAt":"2019-06-14T23:18:55Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Jun 14, 2019 at 01:25:59PM -0700, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > But I wonder if things would be simpler if we did not touch the commit\n> > code path at all. I.e., if this were simply \"--no-object-names\", and it\n> > touched only show_object().\n> \n> Yeah, that sounds more tempting.  And the refined code structure you\n> suggested ...\n> \n> >> @@ -255,6 +262,10 @@ static void show_object(struct object *obj, const char *name, void *cb_data)\n> >>  \tdisplay_progress(progress, ++progress_counter);\n> >>  \tif (info->flags & REV_LIST_QUIET)\n> >>  \t\treturn;\n> >> +\tif (arg_oid_only) {\n> >> +\t\tprintf(\"%s\\n\", oid_to_hex(&obj->oid));\n> >> +\t\treturn;\n> >> +\t}\n> >>  \tshow_object_with_name(stdout, obj, name);\n> >>  }\n> >>  \n> >\n> > A minor style point, but I think this might be easier to follow without\n> > the early return, since we are really choosing to do A or B. Writing:\n> >\n> >   if (arg_oid_only)\n> > \tprintf(...);\n> >   else\n> > \tshow_object_with_name(...);\n> >\n> > shows that more clearly, I think.\n> \n> ... is a good way to clearly show that intention, I would think.\n\nSounds good. Thanks, both; I'll reroll that quickly today.\n"},{"id":"377275","messageId":"20190614232946.GF233791@google.com","threadId":"51264","inReplyTo":"20190614160728.GA30083@sigill.intra.peff.net","subject":"Re: [PATCH v2] rev-list: teach --oid-only to enable piping","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-14T23:29:46Z","receivedAt":"2019-06-14T23:29:54Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Jun 14, 2019 at 12:07:28PM -0400, Jeff King wrote:\n> On Thu, Jun 13, 2019 at 02:51:03PM -0700, Emily Shaffer wrote:\n\n> > +test_expect_success 'rev-list --objects --oid-only is usable by cat-file' '\n> > +\tgit rev-list --objects --oid-only --all >list-output &&\n> > +\tgit cat-file --batch-check <list-output >cat-output &&\n> > +\t! grep missing cat-output\n> > +'\n> \n> Usually we prefer to look for the expected output, rather than making\n> sure we did not find the unexpected. But I'm not sure if that might be\n> burdensome in this case (i.e., if there's a bunch of cruft coming out of\n> \"rev-list\" that would be annoying to match, and might even change as\n> people add more tests). So I'm OK with it either way.\n\nMy (newbie) opinion is that in this case, we specifically want to know\nthat cat-file didn't choke on objects which we know exist (since they\ncame from rev-list). I have the feeling that checking for the exact\nobjects returned instead (or a sample of them) would be more brittle and\nwould also make the wording of the test less direct.\n\nSo if there's no complaint either way, I'd prefer to leave it the way it\nis.\n\nBy the way, rev-list-misc.sh has a number of other existing \"! grep ...\"\nlines.\n\n - Emily\n"},{"id":"377276","messageId":"20190614234855.257318-1-emilyshaffer@google.com","threadId":"51264","inReplyTo":"20190613215102.44627-1-emilyshaffer@google.com","subject":"[PATCH v3] rev-list: teach --no-object-names to enable piping","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-14T23:48:55Z","receivedAt":"2019-06-14T23:49:07Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Allow easier parsing by cat-file by giving rev-list an option to print\nonly the OID of a non-commit object without any additional information.\nThis is a short-term shim; later on, rev-list should be taught how to\nprint the types of objects it finds in a format similar to cat-file's.\n\nBefore this commit, the output from rev-list needed to be massaged\nbefore being piped to cat-file, like so:\n\n  git rev-list --objects HEAD | cut -f 1 -d ' ' \\\n    | git cat-file --batch-check\n\nThis was especially unexpected when dealing with root trees, as an\ninvisible whitespace exists at the end of the OID:\n\n  git rev-list --objects --filter=tree:1 --max-count=1 HEAD \\\n    | xargs -I% echo \"AA%AA\"\n\nNow, it can be piped directly, as in the added test case:\n\n  git rev-list --objects --no-object-names HEAD | git cat-file --batch-check\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\nChange-Id: I489bdf0a8215532e540175188883ff7541d70e1b\n---\nBased on Peff and Junio's comments, made following changes since v2:\n\n - Removed interaction with commit objects\n - Renamed option to \"no-object-names\"\n - Removed warnings when new option is combined with commit-formatting\n   options (and reflected in usage)\n - Simplified logic in show_object()\n\nThanks for the thoughts, all.\n\n - Emily\n\n builtin/rev-list.c       | 14 +++++++++++++-\n t/t6000-rev-list-misc.sh | 18 ++++++++++++++++++\n 2 files changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 660172b014..7e2598fd22 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -49,6 +49,7 @@ static const char rev_list_usage[] =\n \"    --objects | --objects-edge\\n\"\n \"    --unpacked\\n\"\n \"    --header | --pretty\\n\"\n+\"    --no-object-names\\n\"\n \"    --abbrev=<n> | --no-abbrev\\n\"\n \"    --abbrev-commit\\n\"\n \"    --left-right\\n\"\n@@ -75,6 +76,9 @@ enum missing_action {\n };\n static enum missing_action arg_missing_action;\n \n+/* display only the oid of each object encountered */\n+static int arg_no_object_names;\n+\n #define DEFAULT_OIDSET_SIZE     (16*1024)\n \n static void finish_commit(struct commit *commit);\n@@ -255,7 +259,10 @@ static void show_object(struct object *obj, const char *name, void *cb_data)\n \tdisplay_progress(progress, ++progress_counter);\n \tif (info->flags & REV_LIST_QUIET)\n \t\treturn;\n-\tshow_object_with_name(stdout, obj, name);\n+\tif (arg_no_object_names)\n+\t\tprintf(\"%s\\n\", oid_to_hex(&obj->oid));\n+\telse\n+\t\tshow_object_with_name(stdout, obj, name);\n }\n \n static void show_edge(struct commit *commit)\n@@ -484,6 +491,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\tif (skip_prefix(arg, \"--missing=\", &arg))\n \t\t\tcontinue; /* already handled above */\n \n+\t\tif (!strcmp(arg, (\"--no-object-names\"))) {\n+\t\t\targ_no_object_names = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tusage(rev_list_usage);\n \n \t}\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 0507999729..5d87171b99 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -48,6 +48,24 @@ test_expect_success 'rev-list --objects with pathspecs and copied files' '\n \t! grep one output\n '\n \n+test_expect_success 'rev-list --objects --no-object-names has no space/names' '\n+\tgit rev-list --objects --no-object-names HEAD >output &&\n+\t! grep wanted_file output &&\n+\t! grep unwanted_file output &&\n+\t! grep \" \" output\n+'\n+\n+test_expect_success 'rev-list --objects --no-object-names works with cat-file' '\n+\tgit rev-list --objects --no-object-names --all >list-output &&\n+\tgit cat-file --batch-check <list-output >cat-output &&\n+\t! grep missing cat-output\n+'\n+\n+test_expect_success 'rev-list --oid-only is incompatible with --pretty' '\n+\ttest_must_fail git rev-list --objects --oid-only --pretty HEAD &&\n+\ttest_must_fail git rev-list --objects --oid-only --header HEAD\n+'\n+\n test_expect_success 'rev-list A..B and rev-list ^A B are the same' '\n \tgit commit --allow-empty -m another &&\n \tgit tag -a -m \"annotated\" v1.0 &&\n-- \n2.22.0.410.gd8fdbe21b5-goog\n\n"},{"id":"377374","messageId":"xmqqk1djsv0d.fsf@gitster-ct.c.googlers.com","threadId":"51264","inReplyTo":"20190614234855.257318-1-emilyshaffer@google.com","subject":"Re: [PATCH v3] rev-list: teach --no-object-names to enable piping","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-17T22:32:34Z","receivedAt":"2019-06-17T22:32:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n> Allow easier parsing by cat-file by giving rev-list an option to print\n> only the OID of a non-commit object without any additional information.\n> This is a short-term shim; later on, rev-list should be taught how to\n> print the types of objects it finds in a format similar to cat-file's.\n>\n> Before this commit, the output from rev-list needed to be massaged\n> before being piped to cat-file, like so:\n>\n>   git rev-list --objects HEAD | cut -f 1 -d ' ' \\\n>     | git cat-file --batch-check\n\nWrite this with '|' at the end of the first line; that way the shell\nknows you haven't finished your sentence without any backslash.\n\n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index 660172b014..7e2598fd22 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -49,6 +49,7 @@ static const char rev_list_usage[] =\n>  \"    --objects | --objects-edge\\n\"\n>  \"    --unpacked\\n\"\n>  \"    --header | --pretty\\n\"\n> +\"    --no-object-names\\n\"\n\nIdeally, this should be \"--[no-]object-names\", i.e. --object-names\nwhich may be the default when using --objects could be tweaked with\n--no-object-names and then again turned on with a --object-names\nlater on the command line, following the usual \"last one wins\".\n\n> @@ -75,6 +76,9 @@ enum missing_action {\n>  };\n>  static enum missing_action arg_missing_action;\n>  \n> +/* display only the oid of each object encountered */\n> +static int arg_no_object_names;\n\nAnd this would become\n\n    static int show_object_names = 1;\n\nthat can be turned off via --no-object-names (and --object-names\nwould flip it on).  It would reduce the double negation brain\ntwister, hopefully.\n\n"},{"id":"377460","messageId":"20190618220837.GE100487@google.com","threadId":"51264","inReplyTo":"xmqqk1djsv0d.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] rev-list: teach --no-object-names to enable piping","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-18T22:08:37Z","receivedAt":"2019-06-18T22:08:46Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Jun 17, 2019 at 03:32:34PM -0700, Junio C Hamano wrote:\n> Emily Shaffer <emilyshaffer@google.com> writes:\n> \n> > Allow easier parsing by cat-file by giving rev-list an option to print\n> > only the OID of a non-commit object without any additional information.\n> > This is a short-term shim; later on, rev-list should be taught how to\n> > print the types of objects it finds in a format similar to cat-file's.\n> >\n> > Before this commit, the output from rev-list needed to be massaged\n> > before being piped to cat-file, like so:\n> >\n> >   git rev-list --objects HEAD | cut -f 1 -d ' ' \\\n> >     | git cat-file --batch-check\n> \n> Write this with '|' at the end of the first line; that way the shell\n> knows you haven't finished your sentence without any backslash.\n\nOh cool. Will do.\n\n> \n> > diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> > index 660172b014..7e2598fd22 100644\n> > --- a/builtin/rev-list.c\n> > +++ b/builtin/rev-list.c\n> > @@ -49,6 +49,7 @@ static const char rev_list_usage[] =\n> >  \"    --objects | --objects-edge\\n\"\n> >  \"    --unpacked\\n\"\n> >  \"    --header | --pretty\\n\"\n> > +\"    --no-object-names\\n\"\n> \n> Ideally, this should be \"--[no-]object-names\", i.e. --object-names\n> which may be the default when using --objects could be tweaked with\n> --no-object-names and then again turned on with a --object-names\n> later on the command line, following the usual \"last one wins\".\n\nSure, good point. Will fix.\n\n> \n> > @@ -75,6 +76,9 @@ enum missing_action {\n> >  };\n> >  static enum missing_action arg_missing_action;\n> >  \n> > +/* display only the oid of each object encountered */\n> > +static int arg_no_object_names;\n> \n> And this would become\n> \n>     static int show_object_names = 1;\n> \n> that can be turned off via --no-object-names (and --object-names\n> would flip it on).  It would reduce the double negation brain\n> twister, hopefully.\n> \n\nI'll send a new patch today. Thanks.\n\n - Emily\n"},{"id":"377462","messageId":"20190618222917.261701-1-emilyshaffer@google.com","threadId":"51264","inReplyTo":"20190614234855.257318-1-emilyshaffer@google.com","subject":"[PATCH v4] rev-list: teach --no-object-names to enable piping","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-18T22:29:17Z","receivedAt":"2019-06-18T22:29:26Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Allow easier parsing by cat-file by giving rev-list an option to print\nonly the OID of a non-commit object without any additional information.\nThis is a short-term shim; later on, rev-list should be taught how to\nprint the types of objects it finds in a format similar to cat-file's.\n\nBefore this commit, the output from rev-list needed to be massaged\nbefore being piped to cat-file, like so:\n\n  git rev-list --objects HEAD | cut -f 1 -d ' ' |\n    git cat-file --batch-check\n\nThis was especially unexpected when dealing with root trees, as an\ninvisible whitespace exists at the end of the OID:\n\n  git rev-list --objects --filter=tree:1 --max-count=1 HEAD |\n    xargs -I% echo \"AA%AA\"\n\nNow, it can be piped directly, as in the added test case:\n\n  git rev-list --objects --no-object-names HEAD | git cat-file --batch-check\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\nChange-Id: I489bdf0a8215532e540175188883ff7541d70e1b\n---\nSince v3, added a corresponding \"--object-names\" arg to pair with\n\"--no-object-names\", and \"last-one-wins\" logic. Also added a test to\nvalidate this new arg and the logic.\n\nI did not take Junio's suggestion of naming the arg \"show_object_names\"\nas \"arg_show_object_names\" better matches the existing style of\nbuiltin/revlist.c.\n\nIn adding the test, I noticed that I had left in a test about --oid-only\nthat doesn't apply after the changes from v2->v3; that test is removed.\n\n - Emily\n\n builtin/rev-list.c       | 19 ++++++++++++++++++-\n t/t6000-rev-list-misc.sh | 20 ++++++++++++++++++++\n 2 files changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 660172b014..301ccb970b 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -49,6 +49,7 @@ static const char rev_list_usage[] =\n \"    --objects | --objects-edge\\n\"\n \"    --unpacked\\n\"\n \"    --header | --pretty\\n\"\n+\"    --[no-]object-names\\n\"\n \"    --abbrev=<n> | --no-abbrev\\n\"\n \"    --abbrev-commit\\n\"\n \"    --left-right\\n\"\n@@ -75,6 +76,9 @@ enum missing_action {\n };\n static enum missing_action arg_missing_action;\n \n+/* display only the oid of each object encountered */\n+static int arg_show_object_names = 1;\n+\n #define DEFAULT_OIDSET_SIZE     (16*1024)\n \n static void finish_commit(struct commit *commit);\n@@ -255,7 +259,10 @@ static void show_object(struct object *obj, const char *name, void *cb_data)\n \tdisplay_progress(progress, ++progress_counter);\n \tif (info->flags & REV_LIST_QUIET)\n \t\treturn;\n-\tshow_object_with_name(stdout, obj, name);\n+\tif (arg_show_object_names)\n+\t\tshow_object_with_name(stdout, obj, name);\n+\telse\n+\t\tprintf(\"%s\\n\", oid_to_hex(&obj->oid));\n }\n \n static void show_edge(struct commit *commit)\n@@ -484,6 +491,16 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\tif (skip_prefix(arg, \"--missing=\", &arg))\n \t\t\tcontinue; /* already handled above */\n \n+\t\tif (!strcmp(arg, (\"--no-object-names\"))) {\n+\t\t\targ_show_object_names = 0;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (!strcmp(arg, (\"--object-names\"))) {\n+\t\t\targ_show_object_names = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tusage(rev_list_usage);\n \n \t}\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 0507999729..52a9e38d66 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -48,6 +48,26 @@ test_expect_success 'rev-list --objects with pathspecs and copied files' '\n \t! grep one output\n '\n \n+test_expect_success 'rev-list --objects --no-object-names has no space/names' '\n+\tgit rev-list --objects --no-object-names HEAD >output &&\n+\t! grep wanted_file output &&\n+\t! grep unwanted_file output &&\n+\t! grep \" \" output\n+'\n+\n+test_expect_success 'rev-list --objects --no-object-names works with cat-file' '\n+\tgit rev-list --objects --no-object-names --all >list-output &&\n+\tgit cat-file --batch-check <list-output >cat-output &&\n+\t! grep missing cat-output\n+'\n+\n+test_expect_success '--no-object-names and --object-names are last-one-wins' '\n+\tgit rev-list --objects --no-object-names --object-names --all >output &&\n+\tgrep wanted_file output &&\n+\tgit rev-list --objects --object-names --no-object-names --all >output &&\n+\t! grep wanted_file output\n+'\n+\n test_expect_success 'rev-list A..B and rev-list ^A B are the same' '\n \tgit commit --allow-empty -m another &&\n \tgit tag -a -m \"annotated\" v1.0 &&\n-- \n2.22.0.410.gd8fdbe21b5-goog\n\n"},{"id":"377520","messageId":"xmqq1rzpsm5t.fsf@gitster-ct.c.googlers.com","threadId":"51264","inReplyTo":"20190618222917.261701-1-emilyshaffer@google.com","subject":"Re: [PATCH v4] rev-list: teach --no-object-names to enable piping","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-19T14:08:14Z","receivedAt":"2019-06-19T14:08:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n> Since v3, added a corresponding \"--object-names\" arg to pair with\n> \"--no-object-names\", and \"last-one-wins\" logic. Also added a test to\n> validate this new arg and the logic.\n\nThanks for a quick turnaround (unfortunately, I was OOO yesterday\nand I am half-sick today, so please expect slow responses---sorry\nabout that).\n\n> In adding the test, I noticed that I had left in a test about --oid-only\n> that doesn't apply after the changes from v2->v3; that test is removed.\n\nI noticed in range-diff, too.  So now --object-names can be used\nwith --pretty (not that \"rev-list --pretty --objects\" makes much\nsense in the first place, so no point in testing that it works).\n"},{"id":"377553","messageId":"20190619193134.GF100487@google.com","threadId":"51264","inReplyTo":"xmqq1rzpsm5t.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4] rev-list: teach --no-object-names to enable piping","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-19T19:31:34Z","receivedAt":"2019-06-19T19:31:42Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Wed, Jun 19, 2019 at 07:08:14AM -0700, Junio C Hamano wrote:\n> Emily Shaffer <emilyshaffer@google.com> writes:\n> \n> > Since v3, added a corresponding \"--object-names\" arg to pair with\n> > \"--no-object-names\", and \"last-one-wins\" logic. Also added a test to\n> > validate this new arg and the logic.\n> \n> Thanks for a quick turnaround (unfortunately, I was OOO yesterday\n> and I am half-sick today, so please expect slow responses---sorry\n> about that).\n> \n> > In adding the test, I noticed that I had left in a test about --oid-only\n> > that doesn't apply after the changes from v2->v3; that test is removed.\n> \n> I noticed in range-diff, too.  So now --object-names can be used\n> with --pretty (not that \"rev-list --pretty --objects\" makes much\n> sense in the first place, so no point in testing that it works).\n\nYeah, it works. It looks weird, but it works pretty much as you'd\nexpect:\n\nemilyshaffer@podkayne:~/git-second [stray-whitespace]$ tg2 rev-list\n--object-names --objects --pretty=short --max-count=1 HEAD | head -n 20\ncommit 701c66d5f2fafe163892fa0968ce8bca041dbc92\nAuthor: Emily Shaffer <emilyshaffer@google.com>\n\n    rev-list: teach --no-object-names to enable piping\n\n    d4b1d372d16aaff35b221afce017f90542fd9293 \n    41d4cd23fd97f599053a19555a173894da71e560 .clang-format\n    42cdc4bbfb05934bb9c3ed2fe0e0d45212c32d7a .editorconfig\n\nemilyshaffer@podkayne:~/git-second [stray-whitespace]$ tg2 rev-list\n--no-object-names --objects --pretty=short --max-count=1 HEAD | head -n\n20\ncommit 701c66d5f2fafe163892fa0968ce8bca041dbc92\nAuthor: Emily Shaffer <emilyshaffer@google.com>\n\n    rev-list: teach --no-object-names to enable piping\n\n    d4b1d372d16aaff35b221afce017f90542fd9293\n    41d4cd23fd97f599053a19555a173894da71e560\n    42cdc4bbfb05934bb9c3ed2fe0e0d45212c32d7a\n\nAh, but when I was grabbing these samples, I noticed that I didn't\nupdate the manpage for rev-list. So please wait while I reroll again...\nsorry!\n\n - Emily\n"},{"id":"377558","messageId":"20190619205656.117272-1-emilyshaffer@google.com","threadId":"51264","inReplyTo":"20190618222917.261701-1-emilyshaffer@google.com","subject":"[PATCH v5] rev-list: teach --no-object-names to enable piping","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-06-19T20:56:56Z","receivedAt":"2019-06-19T20:57:12Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Allow easier parsing by cat-file by giving rev-list an option to print\nonly the OID of a non-commit object without any additional information.\nThis is a short-term shim; later on, rev-list should be taught how to\nprint the types of objects it finds in a format similar to cat-file's.\n\nBefore this commit, the output from rev-list needed to be massaged\nbefore being piped to cat-file, like so:\n\n  git rev-list --objects HEAD | cut -f 1 -d ' ' |\n    git cat-file --batch-check\n\nThis was especially unexpected when dealing with root trees, as an\ninvisible whitespace exists at the end of the OID:\n\n  git rev-list --objects --filter=tree:1 --max-count=1 HEAD |\n    xargs -I% echo \"AA%AA\"\n\nNow, it can be piped directly, as in the added test case:\n\n  git rev-list --objects --no-object-names HEAD | git cat-file --batch-check\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\nChange-Id: I489bdf0a8215532e540175188883ff7541d70e1b\n---\nSince v4, added the new options to `git help rev-list`.\n\n Documentation/git-rev-list.txt     |  1 +\n Documentation/rev-list-options.txt | 10 ++++++++++\n builtin/rev-list.c                 | 19 ++++++++++++++++++-\n t/t6000-rev-list-misc.sh           | 20 ++++++++++++++++++++\n 4 files changed, 49 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\nindex 88609ff435..9392760b25 100644\n--- a/Documentation/git-rev-list.txt\n+++ b/Documentation/git-rev-list.txt\n@@ -48,6 +48,7 @@ SYNOPSIS\n \t     [ --date=<format>]\n \t     [ [ --objects | --objects-edge | --objects-edge-aggressive ]\n \t       [ --unpacked ]\n+\t       [ --object-names | --no-object-names ]\n \t       [ --filter=<filter-spec> [ --filter-print-omitted ] ] ]\n \t     [ --missing=<missing-action> ]\n \t     [ --pretty | --header ]\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 71a1fcc093..286fc163f1 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -708,6 +708,16 @@ ifdef::git-rev-list[]\n \tOnly useful with `--objects`; print the object IDs that are not\n \tin packs.\n \n+--object-names::\n+\tOnly useful with `--objects`; print the names of the object IDs\n+\tthat are found. This is the default behavior.\n+\n+--no-object-names::\n+\tOnly useful with `--objects`; does not print the names of the object\n+\tIDs that are found. This inverts `--object-names`. This flag allows\n+\tthe output to be more easily parsed by commands such as\n+\tlinkgit:git-cat-file[1].\n+\n --filter=<filter-spec>::\n \tOnly useful with one of the `--objects*`; omits objects (usually\n \tblobs) from the list of printed objects.  The '<filter-spec>'\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 660172b014..301ccb970b 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -49,6 +49,7 @@ static const char rev_list_usage[] =\n \"    --objects | --objects-edge\\n\"\n \"    --unpacked\\n\"\n \"    --header | --pretty\\n\"\n+\"    --[no-]object-names\\n\"\n \"    --abbrev=<n> | --no-abbrev\\n\"\n \"    --abbrev-commit\\n\"\n \"    --left-right\\n\"\n@@ -75,6 +76,9 @@ enum missing_action {\n };\n static enum missing_action arg_missing_action;\n \n+/* display only the oid of each object encountered */\n+static int arg_show_object_names = 1;\n+\n #define DEFAULT_OIDSET_SIZE     (16*1024)\n \n static void finish_commit(struct commit *commit);\n@@ -255,7 +259,10 @@ static void show_object(struct object *obj, const char *name, void *cb_data)\n \tdisplay_progress(progress, ++progress_counter);\n \tif (info->flags & REV_LIST_QUIET)\n \t\treturn;\n-\tshow_object_with_name(stdout, obj, name);\n+\tif (arg_show_object_names)\n+\t\tshow_object_with_name(stdout, obj, name);\n+\telse\n+\t\tprintf(\"%s\\n\", oid_to_hex(&obj->oid));\n }\n \n static void show_edge(struct commit *commit)\n@@ -484,6 +491,16 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\tif (skip_prefix(arg, \"--missing=\", &arg))\n \t\t\tcontinue; /* already handled above */\n \n+\t\tif (!strcmp(arg, (\"--no-object-names\"))) {\n+\t\t\targ_show_object_names = 0;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (!strcmp(arg, (\"--object-names\"))) {\n+\t\t\targ_show_object_names = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tusage(rev_list_usage);\n \n \t}\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 0507999729..52a9e38d66 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -48,6 +48,26 @@ test_expect_success 'rev-list --objects with pathspecs and copied files' '\n \t! grep one output\n '\n \n+test_expect_success 'rev-list --objects --no-object-names has no space/names' '\n+\tgit rev-list --objects --no-object-names HEAD >output &&\n+\t! grep wanted_file output &&\n+\t! grep unwanted_file output &&\n+\t! grep \" \" output\n+'\n+\n+test_expect_success 'rev-list --objects --no-object-names works with cat-file' '\n+\tgit rev-list --objects --no-object-names --all >list-output &&\n+\tgit cat-file --batch-check <list-output >cat-output &&\n+\t! grep missing cat-output\n+'\n+\n+test_expect_success '--no-object-names and --object-names are last-one-wins' '\n+\tgit rev-list --objects --no-object-names --object-names --all >output &&\n+\tgrep wanted_file output &&\n+\tgit rev-list --objects --object-names --no-object-names --all >output &&\n+\t! grep wanted_file output\n+'\n+\n test_expect_success 'rev-list A..B and rev-list ^A B are the same' '\n \tgit commit --allow-empty -m another &&\n \tgit tag -a -m \"annotated\" v1.0 &&\n-- \n2.22.0.410.gd8fdbe21b5-goog\n\n"},{"id":"377584","messageId":"20190619212411.GB6571@sigill.intra.peff.net","threadId":"51264","inReplyTo":"20190614232946.GF233791@google.com","subject":"Re: [PATCH v2] rev-list: teach --oid-only to enable piping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-19T21:24:12Z","receivedAt":"2019-06-19T21:24:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 14, 2019 at 04:29:46PM -0700, Emily Shaffer wrote:\n\n> On Fri, Jun 14, 2019 at 12:07:28PM -0400, Jeff King wrote:\n> > On Thu, Jun 13, 2019 at 02:51:03PM -0700, Emily Shaffer wrote:\n> \n> > > +test_expect_success 'rev-list --objects --oid-only is usable by cat-file' '\n> > > +\tgit rev-list --objects --oid-only --all >list-output &&\n> > > +\tgit cat-file --batch-check <list-output >cat-output &&\n> > > +\t! grep missing cat-output\n> > > +'\n> > \n> > Usually we prefer to look for the expected output, rather than making\n> > sure we did not find the unexpected. But I'm not sure if that might be\n> > burdensome in this case (i.e., if there's a bunch of cruft coming out of\n> > \"rev-list\" that would be annoying to match, and might even change as\n> > people add more tests). So I'm OK with it either way.\n> \n> My (newbie) opinion is that in this case, we specifically want to know\n> that cat-file didn't choke on objects which we know exist (since they\n> came from rev-list). I have the feeling that checking for the exact\n> objects returned instead (or a sample of them) would be more brittle and\n> would also make the wording of the test less direct.\n> \n> So if there's no complaint either way, I'd prefer to leave it the way it\n> is.\n\nYeah, that's fine with me if it seems more clear to use grep here (and I\nwas on the fence).\n\n> By the way, rev-list-misc.sh has a number of other existing \"! grep ...\"\n> lines.\n\nIt never fails that when I complain about a style issue, the surrounding\ncode is full of the same thing. ;) I'd have to look at each one to\ndetermine if they're sensible or not, and it's probably not worth\nanybody's time to do that cleanup at this point in time.\n\n-Peff\n"},{"id":"377585","messageId":"20190619213033.GC6571@sigill.intra.peff.net","threadId":"51264","inReplyTo":"20190619193134.GF100487@google.com","subject":"Re: [PATCH v4] rev-list: teach --no-object-names to enable piping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-19T21:30:33Z","receivedAt":"2019-06-19T21:30:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 19, 2019 at 12:31:34PM -0700, Emily Shaffer wrote:\n\n> > I noticed in range-diff, too.  So now --object-names can be used\n> > with --pretty (not that \"rev-list --pretty --objects\" makes much\n> > sense in the first place, so no point in testing that it works).\n> \n> Yeah, it works. It looks weird, but it works pretty much as you'd\n> expect:\n\nI think that something like:\n\n  git rev-list --in-commit-order --oneline --objects HEAD\n\ncould make sense to get a rough idea of which commits are mentioning\nwhich objects (though in most cases I'd probably use \"log --find-object\"\nthese days, since it's more precise; rev-list is looking in a funny\nreverse order).\n\n-Peff\n"},{"id":"377587","messageId":"20190619213804.GD6571@sigill.intra.peff.net","threadId":"51264","inReplyTo":"20190619205656.117272-1-emilyshaffer@google.com","subject":"Re: [PATCH v5] rev-list: teach --no-object-names to enable piping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-19T21:38:05Z","receivedAt":"2019-06-19T21:38:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 19, 2019 at 01:56:56PM -0700, Emily Shaffer wrote:\n\n> Allow easier parsing by cat-file by giving rev-list an option to print\n> only the OID of a non-commit object without any additional information.\n> This is a short-term shim; later on, rev-list should be taught how to\n> print the types of objects it finds in a format similar to cat-file's.\n> [...]\n\nI missed some of the intermediate rounds, but fortunately Junio already\nsaid everything I was going to. :) This version looks good to me, though\nwith one minor nit:\n\n> diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\n> index 88609ff435..9392760b25 100644\n> --- a/Documentation/git-rev-list.txt\n> +++ b/Documentation/git-rev-list.txt\n> @@ -48,6 +48,7 @@ SYNOPSIS\n>  \t     [ --date=<format>]\n>  \t     [ [ --objects | --objects-edge | --objects-edge-aggressive ]\n>  \t       [ --unpacked ]\n> +\t       [ --object-names | --no-object-names ]\n>  \t       [ --filter=<filter-spec> [ --filter-print-omitted ] ] ]\n>  \t     [ --missing=<missing-action> ]\n>  \t     [ --pretty | --header ]\n\nHere you put --object-names along with the --objects. Which kind of\nmakes sense, but everything else in that block is about choosing _which_\ncommits to show. In the short help, you put it near --pretty:\n\n> @@ -49,6 +49,7 @@ static const char rev_list_usage[] =\n>  \"    --objects | --objects-edge\\n\"\n>  \"    --unpacked\\n\"\n>  \"    --header | --pretty\\n\"\n> +\"    --[no-]object-names\\n\"\n>  \"    --abbrev=<n> | --no-abbrev\\n\"\n>  \"    --abbrev-commit\\n\"\n>  \"    --left-right\\n\"\n\nwhich I think makes more sense. I think maybe you were trying to imply\nthat \"--object-names\" is not useful unless you're also using\n\"--objects\". Which is true, but I'm not sure it's obvious from that mass\nof brackets (and I think is sufficiently covered in the actual option\ndescriptions you give later).\n\n> +test_expect_success '--no-object-names and --object-names are last-one-wins' '\n> +\tgit rev-list --objects --no-object-names --object-names --all >output &&\n> +\tgrep wanted_file output &&\n> +\tgit rev-list --objects --object-names --no-object-names --all >output &&\n> +\t! grep wanted_file output\n> +'\n\nWe don't generally test this behavior for each option, since it would\nlead to a ton of uninteresting tests (and parse-options generally just\nhandles it).  But after our discussion about --no-abbrev, I can see how\nyou might be more interested in the topic. :) So I'm OK with it either\nway.\n\n-Peff\n"}]}