Re: [PATCH v2] revisions.c: put promisor option in specialized struct

2018-12-03 Thread Jeff King
On Mon, Dec 03, 2018 at 02:10:19PM -0800, Matthew DeVore wrote:

> Put the allow_exclude_promisor_objects flag in setup_revision_opt. When
> it was in rev_info, it was unclear when it was used, since rev_info is
> passed to functions that don't use the flag. This resulted in
> unnecessary setting of the flag in prune.c, so fix that as well.
> 
> Signed-off-by: Matthew DeVore 
> ---
>  builtin/pack-objects.c |  6 --
>  builtin/prune.c|  1 -
>  builtin/rev-list.c |  6 --
>  revision.c | 10 ++
>  revision.h |  4 ++--
>  5 files changed, 16 insertions(+), 11 deletions(-)

Thanks, this version looks good to me.

One style nit that I don't think is worth a re-roll, but that Junio
might want to tweak while applying:

> diff --git a/revision.c b/revision.c
> index 13e0519c02..f6b32e6a42 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1791,7 +1791,8 @@ static void add_message_grep(struct rev_info *revs, 
> const char *pattern)
>  }
>  
>  static int handle_revision_opt(struct rev_info *revs, int argc, const char 
> **argv,
> -int *unkc, const char **unkv)
> +int *unkc, const char **unkv,
> +const struct setup_revision_opt* opt)

We keep the "*" with the variable name, not the type.

-Peff


[PATCH v2] revisions.c: put promisor option in specialized struct

2018-12-03 Thread Matthew DeVore
Put the allow_exclude_promisor_objects flag in setup_revision_opt. When
it was in rev_info, it was unclear when it was used, since rev_info is
passed to functions that don't use the flag. This resulted in
unnecessary setting of the flag in prune.c, so fix that as well.

Signed-off-by: Matthew DeVore 
---
 builtin/pack-objects.c |  6 --
 builtin/prune.c|  1 -
 builtin/rev-list.c |  6 --
 revision.c | 10 ++
 revision.h |  4 ++--
 5 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 24bba8147f..889df2c755 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3084,14 +3084,16 @@ static void record_recent_commit(struct commit *commit, 
void *data)
 static void get_object_list(int ac, const char **av)
 {
struct rev_info revs;
+   struct setup_revision_opt s_r_opt = {
+   .allow_exclude_promisor_objects = 1,
+   };
char line[1000];
int flags = 0;
int save_warning;
 
repo_init_revisions(the_repository, , NULL);
save_commit_buffer = 0;
-   revs.allow_exclude_promisor_objects_opt = 1;
-   setup_revisions(ac, av, , NULL);
+   setup_revisions(ac, av, , _r_opt);
 
/* make sure shallows are read */
is_repository_shallow(the_repository);
diff --git a/builtin/prune.c b/builtin/prune.c
index e42653b99c..1ec9ddd751 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -120,7 +120,6 @@ int cmd_prune(int argc, const char **argv, const char 
*prefix)
save_commit_buffer = 0;
read_replace_refs = 0;
ref_paranoia = 1;
-   revs.allow_exclude_promisor_objects_opt = 1;
repo_init_revisions(the_repository, , prefix);
 
argc = parse_options(argc, argv, prefix, options, prune_usage, 0);
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 3a2c0c23b6..14ef659c12 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -362,6 +362,9 @@ int cmd_rev_list(int argc, const char **argv, const char 
*prefix)
 {
struct rev_info revs;
struct rev_list_info info;
+   struct setup_revision_opt s_r_opt = {
+   .allow_exclude_promisor_objects = 1,
+   };
int i;
int bisect_list = 0;
int bisect_show_vars = 0;
@@ -375,7 +378,6 @@ int cmd_rev_list(int argc, const char **argv, const char 
*prefix)
git_config(git_default_config, NULL);
repo_init_revisions(the_repository, , prefix);
revs.abbrev = DEFAULT_ABBREV;
-   revs.allow_exclude_promisor_objects_opt = 1;
revs.commit_format = CMIT_FMT_UNSPECIFIED;
revs.do_not_die_on_missing_tree = 1;
 
@@ -407,7 +409,7 @@ int cmd_rev_list(int argc, const char **argv, const char 
*prefix)
}
}
 
-   argc = setup_revisions(argc, argv, , NULL);
+   argc = setup_revisions(argc, argv, , _r_opt);
 
memset(, 0, sizeof(info));
info.revs = 
diff --git a/revision.c b/revision.c
index 13e0519c02..f6b32e6a42 100644
--- a/revision.c
+++ b/revision.c
@@ -1791,7 +1791,8 @@ static void add_message_grep(struct rev_info *revs, const 
char *pattern)
 }
 
 static int handle_revision_opt(struct rev_info *revs, int argc, const char 
**argv,
-  int *unkc, const char **unkv)
+  int *unkc, const char **unkv,
+  const struct setup_revision_opt* opt)
 {
const char *arg = argv[0];
const char *optarg;
@@ -2151,7 +2152,7 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs->limited = 1;
} else if (!strcmp(arg, "--ignore-missing")) {
revs->ignore_missing = 1;
-   } else if (revs->allow_exclude_promisor_objects_opt &&
+   } else if (opt && opt->allow_exclude_promisor_objects &&
   !strcmp(arg, "--exclude-promisor-objects")) {
if (fetch_if_missing)
BUG("exclude_promisor_objects can only be used when 
fetch_if_missing is 0");
@@ -2173,7 +2174,7 @@ void parse_revision_opt(struct rev_info *revs, struct 
parse_opt_ctx_t *ctx,
const char * const usagestr[])
 {
int n = handle_revision_opt(revs, ctx->argc, ctx->argv,
-   >cpidx, ctx->out);
+   >cpidx, ctx->out, NULL);
if (n <= 0) {
error("unknown option `%s'", ctx->argv[0]);
usage_with_options(usagestr, options);
@@ -2391,7 +2392,8 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
continue;
}
 
-   opts = handle_revision_opt(revs, argc - i, argv + i, 
, argv);
+   opts = handle_revision_opt(revs, argc - i, argv + i,
+  , argv, opt);