[RFC,v2,11/13] walken: add filtered object walk
diff mbox series

Message ID 20190626235032.177551-12-emilyshaffer@google.com
State New
Headers show
Series
  • example implementation of revwalk tutorial
Related show

Commit Message

Emily Shaffer June 26, 2019, 11:50 p.m. UTC
Demonstrate how filter specs can be used when performing a revision walk
of all object types. In this case, tree depth is used. Contributors who
are following the revision walking tutorial will be encouraged to run
the revision walk with and without the filter in order to compare the
number of objects seen in each case.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Change-Id: I6d22ba153c1afbc780c261c47f1fa03ea478b5ed
---
 builtin/walken.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

Comments

Eric Sunshine June 27, 2019, 5:42 a.m. UTC | #1
On Wed, Jun 26, 2019 at 7:51 PM Emily Shaffer <emilyshaffer@google.com> wrote:
> Demonstrate how filter specs can be used when performing a revision walk
> of all object types. In this case, tree depth is used. Contributors who
> are following the revision walking tutorial will be encouraged to run
> the revision walk with and without the filter in order to compare the
> number of objects seen in each case.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
> diff --git a/builtin/walken.c b/builtin/walken.c
> @@ -143,6 +144,10 @@ static void walken_show_object(struct object *obj, const char *str, void *buf)
>  static void walken_object_walk(struct rev_info *rev)
>  {
> +       struct list_objects_filter_options filter_options = {};
> +
> +       printf("walken_object_walk beginning...\n");

Is this debugging code which you accidentally left in? Or is it meant
to use trace_printf()? Or something else? If it is a genuine message,
should it be localizable?

> @@ -157,7 +162,24 @@ static void walken_object_walk(struct rev_info *rev)
>         blob_count = 0;
>         tree_count = 0;
>
> -       traverse_commit_list(rev, walken_show_commit, walken_show_object, NULL);
> +       if (1) {
> +               /* Unfiltered: */

The subject talks about adding a _filtered_ object walk (which is in
the 'else' arm), so should this be "if (0)" instead?

> +               trace_printf(_("Unfiltered object walk.\n"));
> +               traverse_commit_list(rev, walken_show_commit,
> +                               walken_show_object, NULL);
> +       } else {
> +               trace_printf(_("Filtered object walk with filterspec "
> +                               "'tree:1'.\n"));
> +               /*
> +                * We can parse a tree depth of 1 to demonstrate the kind of
> +                * filtering that could occur during various operations (see
> +                * `git help rev-list` and read the entry on `--filter`).
> +                */
> +               parse_list_objects_filter(&filter_options, "tree:1");
> +
> +               traverse_commit_list_filtered(&filter_options, rev,
> +                       walken_show_commit, walken_show_object, NULL, NULL);
> +       }
Emily Shaffer June 27, 2019, 10:33 p.m. UTC | #2
On Thu, Jun 27, 2019 at 01:42:45AM -0400, Eric Sunshine wrote:
> On Wed, Jun 26, 2019 at 7:51 PM Emily Shaffer <emilyshaffer@google.com> wrote:
> > Demonstrate how filter specs can be used when performing a revision walk
> > of all object types. In this case, tree depth is used. Contributors who
> > are following the revision walking tutorial will be encouraged to run
> > the revision walk with and without the filter in order to compare the
> > number of objects seen in each case.
> >
> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> > ---
> > diff --git a/builtin/walken.c b/builtin/walken.c
> > @@ -143,6 +144,10 @@ static void walken_show_object(struct object *obj, const char *str, void *buf)
> >  static void walken_object_walk(struct rev_info *rev)
> >  {
> > +       struct list_objects_filter_options filter_options = {};
> > +
> > +       printf("walken_object_walk beginning...\n");
> 
> Is this debugging code which you accidentally left in? Or is it meant
> to use trace_printf()? Or something else? If it is a genuine message,
> should it be localizable?

The former. Removed.

> 
> > @@ -157,7 +162,24 @@ static void walken_object_walk(struct rev_info *rev)
> >         blob_count = 0;
> >         tree_count = 0;
> >
> > -       traverse_commit_list(rev, walken_show_commit, walken_show_object, NULL);
> > +       if (1) {
> > +               /* Unfiltered: */
> 
> The subject talks about adding a _filtered_ object walk (which is in
> the 'else' arm), so should this be "if (0)" instead?

Done.

Patch
diff mbox series

diff --git a/builtin/walken.c b/builtin/walken.c
index 42b23ba1ec..a744d042d8 100644
--- a/builtin/walken.c
+++ b/builtin/walken.c
@@ -12,6 +12,7 @@ 
 #include "pretty.h"
 #include "line-log.h"
 #include "list-objects.h"
+#include "list-objects-filter-options.h"
 #include "grep.h"
 
 /*
@@ -143,6 +144,10 @@  static void walken_show_object(struct object *obj, const char *str, void *buf)
  */
 static void walken_object_walk(struct rev_info *rev)
 {
+	struct list_objects_filter_options filter_options = {};
+
+	printf("walken_object_walk beginning...\n");
+
 	rev->tree_objects = 1;
 	rev->blob_objects = 1;
 	rev->tag_objects = 1;
@@ -157,7 +162,24 @@  static void walken_object_walk(struct rev_info *rev)
 	blob_count = 0;
 	tree_count = 0;
 
-	traverse_commit_list(rev, walken_show_commit, walken_show_object, NULL);
+	if (1) {
+		/* Unfiltered: */
+		trace_printf(_("Unfiltered object walk.\n"));
+		traverse_commit_list(rev, walken_show_commit,
+				walken_show_object, NULL);
+	} else {
+		trace_printf(_("Filtered object walk with filterspec "
+				"'tree:1'.\n"));
+		/*
+		 * We can parse a tree depth of 1 to demonstrate the kind of
+		 * filtering that could occur during various operations (see
+		 * `git help rev-list` and read the entry on `--filter`).
+		 */
+		parse_list_objects_filter(&filter_options, "tree:1");
+
+		traverse_commit_list_filtered(&filter_options, rev,
+			walken_show_commit, walken_show_object, NULL, NULL);
+	}
 
 	/*
 	 * This print statement is designed to be script-parseable. Script