[v3] rev-list: teach --no-object-names to enable piping
diff mbox series

Message ID 20190614234855.257318-1-emilyshaffer@google.com
State New
Headers show
Series
  • [v3] rev-list: teach --no-object-names to enable piping
Related show

Commit Message

Emily Shaffer June 14, 2019, 11:48 p.m. UTC
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.

Before this commit, the output from rev-list needed to be massaged
before being piped to cat-file, like so:

  git rev-list --objects HEAD | cut -f 1 -d ' ' \
    | git cat-file --batch-check

This was especially unexpected when dealing with root trees, as an
invisible whitespace exists at the end of the OID:

  git rev-list --objects --filter=tree:1 --max-count=1 HEAD \
    | xargs -I% echo "AA%AA"

Now, it can be piped directly, as in the added test case:

  git rev-list --objects --no-object-names HEAD | git cat-file --batch-check

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Change-Id: I489bdf0a8215532e540175188883ff7541d70e1b
---
Based on Peff and Junio's comments, made following changes since v2:

 - Removed interaction with commit objects
 - Renamed option to "no-object-names"
 - Removed warnings when new option is combined with commit-formatting
   options (and reflected in usage)
 - Simplified logic in show_object()

Thanks for the thoughts, all.

 - Emily

 builtin/rev-list.c       | 14 +++++++++++++-
 t/t6000-rev-list-misc.sh | 18 ++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

Comments

Junio C Hamano June 17, 2019, 10:32 p.m. UTC | #1
Emily Shaffer <emilyshaffer@google.com> writes:

> 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.
>
> Before this commit, the output from rev-list needed to be massaged
> before being piped to cat-file, like so:
>
>   git rev-list --objects HEAD | cut -f 1 -d ' ' \
>     | git cat-file --batch-check

Write this with '|' at the end of the first line; that way the shell
knows you haven't finished your sentence without any backslash.

> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 660172b014..7e2598fd22 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -49,6 +49,7 @@ static const char rev_list_usage[] =
>  "    --objects | --objects-edge\n"
>  "    --unpacked\n"
>  "    --header | --pretty\n"
> +"    --no-object-names\n"

Ideally, this should be "--[no-]object-names", i.e. --object-names
which may be the default when using --objects could be tweaked with
--no-object-names and then again turned on with a --object-names
later on the command line, following the usual "last one wins".

> @@ -75,6 +76,9 @@ enum missing_action {
>  };
>  static enum missing_action arg_missing_action;
>  
> +/* display only the oid of each object encountered */
> +static int arg_no_object_names;

And this would become

    static int show_object_names = 1;

that can be turned off via --no-object-names (and --object-names
would flip it on).  It would reduce the double negation brain
twister, hopefully.
Emily Shaffer June 18, 2019, 10:08 p.m. UTC | #2
On Mon, Jun 17, 2019 at 03:32:34PM -0700, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > 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.
> >
> > Before this commit, the output from rev-list needed to be massaged
> > before being piped to cat-file, like so:
> >
> >   git rev-list --objects HEAD | cut -f 1 -d ' ' \
> >     | git cat-file --batch-check
> 
> Write this with '|' at the end of the first line; that way the shell
> knows you haven't finished your sentence without any backslash.

Oh cool. Will do.

> 
> > diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> > index 660172b014..7e2598fd22 100644
> > --- a/builtin/rev-list.c
> > +++ b/builtin/rev-list.c
> > @@ -49,6 +49,7 @@ static const char rev_list_usage[] =
> >  "    --objects | --objects-edge\n"
> >  "    --unpacked\n"
> >  "    --header | --pretty\n"
> > +"    --no-object-names\n"
> 
> Ideally, this should be "--[no-]object-names", i.e. --object-names
> which may be the default when using --objects could be tweaked with
> --no-object-names and then again turned on with a --object-names
> later on the command line, following the usual "last one wins".

Sure, good point. Will fix.

> 
> > @@ -75,6 +76,9 @@ enum missing_action {
> >  };
> >  static enum missing_action arg_missing_action;
> >  
> > +/* display only the oid of each object encountered */
> > +static int arg_no_object_names;
> 
> And this would become
> 
>     static int show_object_names = 1;
> 
> that can be turned off via --no-object-names (and --object-names
> would flip it on).  It would reduce the double negation brain
> twister, hopefully.
> 

I'll send a new patch today. Thanks.

 - Emily

Patch
diff mbox series

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 660172b014..7e2598fd22 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -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"
@@ -75,6 +76,9 @@  enum missing_action {
 };
 static enum missing_action arg_missing_action;
 
+/* display only the oid of each object encountered */
+static int arg_no_object_names;
+
 #define DEFAULT_OIDSET_SIZE     (16*1024)
 
 static void finish_commit(struct commit *commit);
@@ -255,7 +259,10 @@  static void show_object(struct object *obj, const char *name, void *cb_data)
 	display_progress(progress, ++progress_counter);
 	if (info->flags & REV_LIST_QUIET)
 		return;
-	show_object_with_name(stdout, obj, name);
+	if (arg_no_object_names)
+		printf("%s\n", oid_to_hex(&obj->oid));
+	else
+		show_object_with_name(stdout, obj, name);
 }
 
 static void show_edge(struct commit *commit)
@@ -484,6 +491,11 @@  int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		if (skip_prefix(arg, "--missing=", &arg))
 			continue; /* already handled above */
 
+		if (!strcmp(arg, ("--no-object-names"))) {
+			arg_no_object_names = 1;
+			continue;
+		}
+
 		usage(rev_list_usage);
 
 	}
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 0507999729..5d87171b99 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -48,6 +48,24 @@  test_expect_success 'rev-list --objects with pathspecs and copied files' '
 	! grep one output
 '
 
+test_expect_success 'rev-list --objects --no-object-names has no space/names' '
+	git rev-list --objects --no-object-names HEAD >output &&
+	! grep wanted_file output &&
+	! grep unwanted_file output &&
+	! grep " " output
+'
+
+test_expect_success 'rev-list --objects --no-object-names works with cat-file' '
+	git rev-list --objects --no-object-names --all >list-output &&
+	git cat-file --batch-check <list-output >cat-output &&
+	! grep missing cat-output
+'
+
+test_expect_success 'rev-list --oid-only is incompatible with --pretty' '
+	test_must_fail git rev-list --objects --oid-only --pretty HEAD &&
+	test_must_fail git rev-list --objects --oid-only --header HEAD
+'
+
 test_expect_success 'rev-list A..B and rev-list ^A B are the same' '
 	git commit --allow-empty -m another &&
 	git tag -a -m "annotated" v1.0 &&