On Wed, Jan 25, 2017 at 1:27 PM, Jeff King <peff@xxxxxxxx> wrote: > On Wed, Jan 25, 2017 at 03:57:18PM -0500, Jeff King wrote: > >> IOW, the ref-selector options build up until a group option is given, >> which acts on the built-up options (over that group) and then resets the >> built-up options. Doing "--unrelated" as above is orthogonal (though I >> think in practice nobody would do that, because it's hard to read). > > So here's what I would have expected your series to look more like (with > probably one patch adding clear_ref_selection_options, and the other > adding the decorate stuff): > I agree that this is how I would have expected it to work as well. Thanks, Jake > diff --git a/revision.c b/revision.c > index b37dbec37..2f67707c7 100644 > --- a/revision.c > +++ b/revision.c > @@ -1156,6 +1156,11 @@ static int handle_one_ref(const char *path, const struct object_id *oid, > > if (ref_excluded(cb->all_revs->ref_excludes, path)) > return 0; > + if (cb->all_revs->decorate_reflog) { > + /* TODO actually do it for real */ > + warning("would decorate %s", path); > + return 0; /* do not add it as a tip */ > + } > > object = get_reference(cb->all_revs, path, oid->hash, cb->all_flags); > add_rev_cmdline(cb->all_revs, object, path, REV_CMD_REF, cb->all_flags); > @@ -1188,6 +1193,12 @@ void add_ref_exclusion(struct string_list **ref_excludes_p, const char *exclude) > string_list_append(*ref_excludes_p, exclude); > } > > +static void clear_ref_selection_options(struct rev_info *revs) > +{ > + clear_ref_exclusion(&revs->ref_excludes); > + revs->decorate_reflog = 0; > +} > + > static void handle_refs(const char *submodule, struct rev_info *revs, unsigned flags, > int (*for_each)(const char *, each_ref_fn, void *)) > { > @@ -2080,10 +2091,10 @@ static int handle_revision_pseudo_opt(const char *submodule, > if (!strcmp(arg, "--all")) { > handle_refs(submodule, revs, *flags, for_each_ref_submodule); > handle_refs(submodule, revs, *flags, head_ref_submodule); > - clear_ref_exclusion(&revs->ref_excludes); > + clear_ref_selection_options(revs); > } else if (!strcmp(arg, "--branches")) { > handle_refs(submodule, revs, *flags, for_each_branch_ref_submodule); > - clear_ref_exclusion(&revs->ref_excludes); > + clear_ref_selection_options(revs); > } else if (!strcmp(arg, "--bisect")) { > read_bisect_terms(&term_bad, &term_good); > handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref); > @@ -2091,15 +2102,15 @@ static int handle_revision_pseudo_opt(const char *submodule, > revs->bisect = 1; > } else if (!strcmp(arg, "--tags")) { > handle_refs(submodule, revs, *flags, for_each_tag_ref_submodule); > - clear_ref_exclusion(&revs->ref_excludes); > + clear_ref_selection_options(revs); > } else if (!strcmp(arg, "--remotes")) { > handle_refs(submodule, revs, *flags, for_each_remote_ref_submodule); > - clear_ref_exclusion(&revs->ref_excludes); > + clear_ref_selection_options(revs); > } else if ((argcount = parse_long_opt("glob", argv, &optarg))) { > struct all_refs_cb cb; > init_all_refs_cb(&cb, revs, *flags); > for_each_glob_ref(handle_one_ref, optarg, &cb); > - clear_ref_exclusion(&revs->ref_excludes); > + clear_ref_selection_options(revs); > return argcount; > } else if ((argcount = parse_long_opt("exclude", argv, &optarg))) { > add_ref_exclusion(&revs->ref_excludes, optarg); > @@ -2108,17 +2119,19 @@ static int handle_revision_pseudo_opt(const char *submodule, > struct all_refs_cb cb; > init_all_refs_cb(&cb, revs, *flags); > for_each_glob_ref_in(handle_one_ref, arg + 11, "refs/heads/", &cb); > - clear_ref_exclusion(&revs->ref_excludes); > + clear_ref_selection_options(revs); > } else if (starts_with(arg, "--tags=")) { > struct all_refs_cb cb; > init_all_refs_cb(&cb, revs, *flags); > for_each_glob_ref_in(handle_one_ref, arg + 7, "refs/tags/", &cb); > - clear_ref_exclusion(&revs->ref_excludes); > + clear_ref_selection_options(revs); > } else if (starts_with(arg, "--remotes=")) { > struct all_refs_cb cb; > init_all_refs_cb(&cb, revs, *flags); > for_each_glob_ref_in(handle_one_ref, arg + 10, "refs/remotes/", &cb); > - clear_ref_exclusion(&revs->ref_excludes); > + clear_ref_selection_options(revs); > + } else if (!strcmp(arg, "--decorate-reflog")) { > + revs->decorate_reflog = 1; > } else if (!strcmp(arg, "--reflog")) { > add_reflogs_to_pending(revs, *flags); > } else if (!strcmp(arg, "--indexed-objects")) { > diff --git a/revision.h b/revision.h > index 9fac1a607..c74879829 100644 > --- a/revision.h > +++ b/revision.h > @@ -66,6 +66,8 @@ struct rev_info { > /* excluding from --branches, --refs, etc. expansion */ > struct string_list *ref_excludes; > > + int decorate_reflog; > + > /* Basic information */ > const char *prefix; > const char *def;