Re: [RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
Eric Freese writes: > However, this still prints an empty line for each ref that does not match the > condition. This can be cleaned up by piping through `grep .`, but what would > you think of adding a new optional flag to git-for-each-ref to prevent it from > printing empty expanded format strings? Offhand I do not think of a reason why anybody wants to give a format that results in a total blank, so it might not even need to be an optional behaviour, but certainly a new option that triggers such a behaviour would be a safe thing to add.
Re: [RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
On Sun, Sep 08, 2019 at 10:01:33PM -0600, Eric Freese wrote: > On Sun, Sep 9, 2019 at 4:34 PM Junio C Hamano wrote: > > > I guess with "%(if)...%(then)...%(else)...%(end)" you might be able > > to do either one of --include/--exclude without supporting the > > other, e.g. "--include='%(if)%(symref)%(then)%(else)not a > > symref%(end)" would be usable as "I do not want to see symrefs" in a > > system that supports only "--include" without "--exclude". > > This has made me realize that I can get the behavior I need by using `%(if)`. > > Exclude symrefs: > "%(if)%(symref)%(then)%(else)%(refname)%(end)" > > Only symrefs: > "%(if)%(symref)%(then)%(refname)%(end)" > > However, this still prints an empty line for each ref that does not match the > condition. This can be cleaned up by piping through `grep .`, but what would > you think of adding a new optional flag to git-for-each-ref to prevent it from > printing empty expanded format strings? Yeah, I think that is a much nicer direction, in that it covers many more cases. I'm tempted to suggest it should even be the default, since the blank lines are mostly useless (by definition they carry no information except the single bit of "there was a ref that didn't have any output for your format"). My only reservation is that for-each-ref is plumbing, and it's _possible_ somebody is counting up the blank lines as part of some workflow (say, counting up tags and non-tags). It seems kind of unlikely, though. Much more likely is having an output format that has some non-blank elements and some blank, but the proposal wouldn't change the behavior there at all. -Peff
Re: [RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
On Sun, Sep 9, 2019 at 4:34 PM Junio C Hamano wrote: > I guess with "%(if)...%(then)...%(else)...%(end)" you might be able > to do either one of --include/--exclude without supporting the > other, e.g. "--include='%(if)%(symref)%(then)%(else)not a > symref%(end)" would be usable as "I do not want to see symrefs" in a > system that supports only "--include" without "--exclude". This has made me realize that I can get the behavior I need by using `%(if)`. Exclude symrefs: "%(if)%(symref)%(then)%(else)%(refname)%(end)" Only symrefs: "%(if)%(symref)%(then)%(refname)%(end)" However, this still prints an empty line for each ref that does not match the condition. This can be cleaned up by piping through `grep .`, but what would you think of adding a new optional flag to git-for-each-ref to prevent it from printing empty expanded format strings?
Re: [RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
Junio C Hamano writes: > Jeff King writes: > >> So in my mind there's an endgame we'd like to eventually reach where >> the option added by your patch isn't needed anymore. But we're a long >> way from that. And it's not entirely clear where we'd draw the line >> anyway. > > All true and very good "thinking out loud". > >> So in the meantime, this seems like a useful thing, and it >> wouldn't be a burden to carry it even if we eventually added >> "--omit=%(symref)" or something. We can introduce two new options, e.g. --(include|exclude)= where * Without either, there is no filtering based on the placeholder expansion; * With only --include, only the refs for which expands to non-empty are included. * With only --exclude, the refs for which expands to non-empty are excluded (and everything else included). * With both --include and --exclude, only the refs for which the for --include expands to non-empty are eligible to be included, but among them, the ones for which the for --exclude expands to non-empty are discarded. Then "--exclude=%(symref)" would be Eric's --no-symref, "--include=%(symref)" would be the opposite (i.e. "show only symbolic refs"), etc. I guess with "%(if)...%(then)...%(else)...%(end)" you might be able to do either one of --include/--exclude without supporting the other, e.g. "--include='%(if)%(symref)%(then)%(else)not a symref%(end)" would be usable as "I do not want to see symrefs" in a system that supports only "--include" without "--exclude".
Re: [RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
Jeff King writes: > So in my mind there's an endgame we'd like to eventually reach where > the option added by your patch isn't needed anymore. But we're a long > way from that. And it's not entirely clear where we'd draw the line > anyway. All true and very good "thinking out loud". > So in the meantime, this seems like a useful thing, and it > wouldn't be a burden to carry it even if we eventually added > "--omit=%(symref)" or something. I would draw the line above this particular change, though. >> +--no-symbolic:: >> +Only list refs that are not symbolic. >> + > > I wonder if "symbolic" might be too vague here. Would "--no-symref" be a > better name? Definitely. Another disturbing thing is the design mistake that made this a bool. If it is useful to filter out symrefs, it would equally be useful to only show symrefs. --[no-]symbolic-refs does not capture the tristate-ness, and that is why I do not think this is good enough in the meantime, without causing us trouble carrying forward. THanks.
Re: [RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
On Sat, Sep 07, 2019 at 03:36:46PM -0600, Eric Freese wrote: > Using the new flag will omit symbolic refs from the output. > > Without this flag, it is possible to get this behavior by using the > `%(symref)` formatting field name and piping output through grep to > include only those refs that do not output a value for `%(symref)`, but > having this flag is more elegant and intention revealing. This seems like a reasonable addition to me. As you note, it can be done by post-processing the result, but it can get a little clumsy. Just letting my mind wander for a moment, a more general solution would be providing some mechanism by which you could ask for-each-ref to omit results based on their expansions. E.g., you might want to ask for only refs for which %(taggerdate) is non-empty (i.e., just the annotated tags). But that opens a can of worms. How do we negate it? You want to omit non-empty %(symref) here, but my tag example would only show non-empty %(taggerdate). Howe do we combine options ("non-symrefs that point to commits")? How do we express more complex logic, like string matching or numeric comparison? It's a slippery slope that ends in embedding a Turing complete language. :) And you can already do these complex things in the language of your choice by post-processing the output. The one advantage to doing it inside for-each-ref is that we may save some work by filtering early. This probably matters more for other tools like cat-file (where I might want to say "print all the blobs", but I can't do so without a multi-process pipeline that accesses each object twice), but we'd eventually like to unify the formatting languages of all tools. So in my mind there's an endgame we'd like to eventually reach where the option added by your patch isn't needed anymore. But we're a long way from that. And it's not entirely clear where we'd draw the line anyway. So in the meantime, this seems like a useful thing, and it wouldn't be a burden to carry it even if we eventually added "--omit=%(symref)" or something. > diff --git a/Documentation/git-for-each-ref.txt > b/Documentation/git-for-each-ref.txt > index 6dcd39f6f6..be19111510 100644 > --- a/Documentation/git-for-each-ref.txt > +++ b/Documentation/git-for-each-ref.txt > @@ -95,6 +95,9 @@ OPTIONS > --ignore-case:: > Sorting and filtering refs are case insensitive. > > +--no-symbolic:: > + Only list refs that are not symbolic. > + I wonder if "symbolic" might be too vague here. Would "--no-symref" be a better name? I responded separately to Taylor's questions about negation, but one thing I didn't bring up there: another option to avoid it is to specify the action positively. E.g., "--ignore-symrefs" or similar. I could go either way on that. > diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c > index 465153e853..b71ab2f135 100644 > --- a/builtin/for-each-ref.c > +++ b/builtin/for-each-ref.c > @@ -18,7 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const > char *prefix) > { > int i; > struct ref_sorting *sorting = NULL, **sorting_tail = &sorting; > - int maxcount = 0, icase = 0; > + int maxcount = 0, icase = 0, nosym = 0; Likewise, maybe worth writing out "symref" here (and in the struct option)? But... > @@ -46,6 +46,7 @@ int cmd_for_each_ref(int argc, const char **argv, const > char *prefix) > OPT_CONTAINS(&filter.with_commit, N_("print only refs which > contain the commit")), > OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which > don't contain the commit")), > OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering > are case insensitive")), > + OPT_BOOL(0, "no-symbolic", &nosym, N_("exclude symbolic refs")), I think you could just write directly to &filter.no_symbolic here, dropping nosym entirely. But I guess ignore-case directly above set a bad example. ;) > diff --git a/ref-filter.c b/ref-filter.c > index f27cfc8c3e..01beb279dc 100644 > --- a/ref-filter.c > +++ b/ref-filter.c > @@ -2093,6 +2093,10 @@ static int ref_filter_handler(const char *refname, > const struct object_id *oid, > return 0; > } > > + if (filter->no_symbolic && flag & REF_ISSYMREF) { > + return 0; > + } > + Ooh, here we avoid the double negation of "if (!filter->no_symbolic)" with an early return. :) (Nothing wrong with that, just an amusing outcome given the discussion elsewhere in the thread). > [...] The rest of the patch looked OK, aside from other review comments. The whole thing looks cleanly done. I don't have a strong opinion on adding the feature to branch/tag. We've only ever promised that HEAD and refs/remotes/.../HEAD work as symrefs, though of course they do work anywhere in the namespace, and I imagine people have taken advantage of that. So I don't know how useful the option would be in other contexts. -Peff
Re: [RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
On Sat, Sep 07, 2019 at 07:28:21PM -0400, Taylor Blau wrote: > > diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c > > index 465153e853..b71ab2f135 100644 > > --- a/builtin/for-each-ref.c > > +++ b/builtin/for-each-ref.c > > @@ -18,7 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const > > char *prefix) > > { > > int i; > > struct ref_sorting *sorting = NULL, **sorting_tail = &sorting; > > - int maxcount = 0, icase = 0; > > + int maxcount = 0, icase = 0, nosym = 0; > > I'm a little timid around a single-bit value prefixed with 'no'. Maybe > it would be clearer as: > > int sym = 1; > > ...instead of the negated form. Of course, the rest of the readers of > this variable would have to be updated, too, but involving fewer > negations seems like it would only improve the clarity. In general, it is nice to avoid negations in the variable names, so you don't end up with double negations like "if (!no_symrefs)". However, it can be tricky because of zero-initialization. Ultimately this flag ends up in "struct ref_filter", and the default initialization of that struct would set it to false, which is what we want. So either we end up flipping the variable when we assign to the struct, or we have to start providing a more detailed initializer for the struct (and switch all callsites to start using it). > Applying your patch shows that I can write the following: > > $ git for-each-ref --no-no-symbolic > > Which is likely unintended. There are two ways that you can go about > this: > > - write this as 'OPT_BOOL(0, "symbolic", ...)', to make sure that the > option you _actually_ want is the one generated by the complement, > not the complement's complement. > > - or, pass 'PARSE_OPT_NONEG' to tell the parse-options API not to > generate the complement in the first place. > > I'd lean towards the former, at the peril of having a meaningless > default option (i.e., passing '--symbolic' is wasteful, since > '--symbolic' is implied by its default value). But, there are certainly > counter-examples, which you can find with > > $ git grep 'OPT_BOOL(.*\"no-' > > So, I'd be curious to hear about the thoughts of others. This has been discussed off and on over the years, which is why you'll find both types of solution in the code base. :) Less important than making sure "--no-no-symbolic" does not work is making sure that "--symbolic" _does_ work. You're right that it's usually pointless, but it can be used to countermand an earlier --no-symbolic. And in fact this _does_ work due to 0f1930c587 (parse-options: allow positivation of options starting, with no-, 2012-02-25). Another advantage of using the "no-" form is that the "-h" usage message will show it rather than its positive counterpart. The "no-no" form is a weird artifact of the parsing. It's probably not _hurting_ anybody, since you don't see it unless you try to use it. There was patch a while ago to disallow these: https://public-inbox.org/git/20170419090820.20279-1-jacob.e.kel...@intel.com/ but ultimately we never did anything. If we do want to disable these, I think I'd rather do it centrally like that, rather than having to specify NONEG in the individual options. > > +test_expect_success 'filtering with --no-symbolic' ' > > + git symbolic-ref refs/symbolic refs/heads/master && > > + git for-each-ref --format="%(refname)" --no-symbolic >actual && > > + test_must_fail grep refs/symbolic actual > > This style is uncommon, and instead it is preferred to write: > > ! grep refs/symbolic actual > > Since 'test_must_fail' also catches segfaults, whereas '!' does not. > Since we'd like this test to fail if/when grep segfaults, use of the > later is preferred here. Your suggestion is correct, but I'm not sure I follow the reasoning. Using "!" would cause us _not_ to notice a segfault, whereas test_must_fail would. For non-git tools we prefer to use the simpler "!", because should be able to safely assume they do not segfault or die by signal. -Peff
Re: [RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
On Sat, Sep 07, 2019 at 07:28:21PM -0400, Taylor Blau wrote: > ... Oh, great. I was pretty sure that vger accidentally ate my last mail, but I guess not. Sorry for the re-send. Thanks, Taylor
Re: [RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
Hi Eric, On Sat, Sep 07, 2019 at 03:36:46PM -0600, Eric Freese wrote: > Using the new flag will omit symbolic refs from the output. > > Without this flag, it is possible to get this behavior by using the > `%(symref)` formatting field name and piping output through grep to > include only those refs that do not output a value for `%(symref)`, but > having this flag is more elegant and intention revealing. Please include a 'Signed-off-by' trailer from yourself in this commit. You can write one yourself, but instead use 'git commit -s' when committing. I'm a little timid around a single-bit value prefixed with 'no'. Maybe it would be clearer as: int sym = 1; ...instead of the negated form. Of course, the rest of the readers of this variable would have to be updated, too, but involving fewer negations seems like it would only improve the clarity. > struct ref_array array; > struct ref_filter filter; > struct ref_format format = REF_FORMAT_INIT; > @@ -46,6 +46,7 @@ int cmd_for_each_ref(int argc, const char **argv, const > char *prefix) > OPT_CONTAINS(&filter.with_commit, N_("print only refs which > contain the commit")), > OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which > don't contain the commit")), > OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering > are case insensitive")), > + OPT_BOOL(0, "no-symbolic", &nosym, N_("exclude symbolic refs")), I'm a little bit weary of this "no-" prefixing, but this time for a different reason. The parse-options API has a built-in "negative" option to pair with each 'OPT_BOOL'. So, for example, in the line above yours, it is actually the case that you can run 'git for-each-ref --ignore-case' just as much as you can run 'git for-each-ref --no-ignore-case'. Applying your patch shows that I can write the following: $ git for-each-ref --no-no-symbolic Which is likely unintended. There are two ways that you can go about this: - write this as 'OPT_BOOL(0, "symbolic", ...)', to make sure that the option you _actually_ want is the one generated by the complement, not the complement's complement. - or, pass 'PARSE_OPT_NONEG' to tell the parse-options API not to generate the complement in the first place. I'd lean towards the former, at the peril of having a meaningless default option (i.e., passing '--symbolic' is wasteful, since '--symbolic' is implied by its default value). But, there are certainly counter-examples, which you can find with $ git grep 'OPT_BOOL(.*\"no-' So, I'd be curious to hear about the thoughts of others. > OPT_END(), > }; > > @@ -72,6 +73,7 @@ int cmd_for_each_ref(int argc, const char **argv, const > char *prefix) > sorting = ref_default_sorting(); > sorting->ignore_case = icase; > filter.ignore_case = icase; > + filter.no_symbolic = nosym; > > filter.name_patterns = argv; > filter.match_as_path = 1; > diff --git a/ref-filter.c b/ref-filter.c > index f27cfc8c3e..01beb279dc 100644 > --- a/ref-filter.c > +++ b/ref-filter.c > @@ -2093,6 +2093,10 @@ static int ref_filter_handler(const char *refname, > const struct object_id *oid, > return 0; > } > > + if (filter->no_symbolic && flag & REF_ISSYMREF) { > + return 0; > + } > + In Documentation/CodingGuidelines, we avoid braces around single-line if-statements. This style is uncommon, and instead it is preferred to write: ! grep refs/symbolic actual Since 'test_must_fail' also catches segfaults, whereas '!' does not. Since we'd like this test to fail if/when grep segfaults, use of the later is preferred here. > +' > + > test_done > -- > 2.23.0 Thanks, Taylor
Re: [RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
Hi Eric, On Sat, Sep 07, 2019 at 03:36:46PM -0600, Eric Freese wrote: > Using the new flag will omit symbolic refs from the output. > > Without this flag, it is possible to get this behavior by using the > `%(symref)` formatting field name and piping output through grep to > include only those refs that do not output a value for `%(symref)`, but > having this flag is more elegant and intention revealing. Please include a 'Signed-off-by' trailer from yourself in this commit. You can write one yourself, but instead use 'git commit -s' when committing. > --- > Documentation/git-for-each-ref.txt | 3 +++ > builtin/for-each-ref.c | 4 +++- > ref-filter.c | 4 > ref-filter.h | 3 ++- > t/t6302-for-each-ref-filter.sh | 6 ++ > 5 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/Documentation/git-for-each-ref.txt > b/Documentation/git-for-each-ref.txt > index 6dcd39f6f6..be19111510 100644 > --- a/Documentation/git-for-each-ref.txt > +++ b/Documentation/git-for-each-ref.txt > @@ -95,6 +95,9 @@ OPTIONS > --ignore-case:: > Sorting and filtering refs are case insensitive. > > +--no-symbolic:: > + Only list refs that are not symbolic. > + > FIELD NAMES > --- > > diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c > index 465153e853..b71ab2f135 100644 > --- a/builtin/for-each-ref.c > +++ b/builtin/for-each-ref.c > @@ -18,7 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const > char *prefix) > { > int i; > struct ref_sorting *sorting = NULL, **sorting_tail = &sorting; > - int maxcount = 0, icase = 0; > + int maxcount = 0, icase = 0, nosym = 0; I'm a little timid around a single-bit value prefixed with 'no'. Maybe it would be clearer as: int sym = 1; ...instead of the negated form. Of course, the rest of the readers of this variable would have to be updated, too, but involving fewer negations seems like it would only improve the clarity. > struct ref_array array; > struct ref_filter filter; > struct ref_format format = REF_FORMAT_INIT; > @@ -46,6 +46,7 @@ int cmd_for_each_ref(int argc, const char **argv, const > char *prefix) > OPT_CONTAINS(&filter.with_commit, N_("print only refs which > contain the commit")), > OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which > don't contain the commit")), > OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering > are case insensitive")), > + OPT_BOOL(0, "no-symbolic", &nosym, N_("exclude symbolic refs")), I'm a little bit weary of this "no-" prefixing, but this time for a different reason. The parse-options API has a built-in "negative" option to pair with each 'OPT_BOOL'. So, for example, in the line above yours, it is actually the case that you can run 'git for-each-ref --ignore-case' just as much as you can run 'git for-each-ref --no-ignore-case'. Applying your patch shows that I can write the following: $ git for-each-ref --no-no-symbolic Which is likely unintended. There are two ways that you can go about this: - write this as 'OPT_BOOL(0, "symbolic", ...)', to make sure that the option you _actually_ want is the one generated by the complement, not the complement's complement. - or, pass 'PARSE_OPT_NONEG' to tell the parse-options API not to generate the complement in the first place. I'd lean towards the former, at the peril of having a meaningless default option (i.e., passing '--symbolic' is wasteful, since '--symbolic' is implied by its default value). But, there are certainly counter-examples, which you can find with $ git grep 'OPT_BOOL(.*\"no-' So, I'd be curious to hear about the thoughts of others. > OPT_END(), > }; > > @@ -72,6 +73,7 @@ int cmd_for_each_ref(int argc, const char **argv, const > char *prefix) > sorting = ref_default_sorting(); > sorting->ignore_case = icase; > filter.ignore_case = icase; > + filter.no_symbolic = nosym; > > filter.name_patterns = argv; > filter.match_as_path = 1; > diff --git a/ref-filter.c b/ref-filter.c > index f27cfc8c3e..01beb279dc 100644 > --- a/ref-filter.c > +++ b/ref-filter.c > @@ -2093,6 +2093,10 @@ static int ref_filter_handler(const char *refname, > const struct object_id *oid, > return 0; > } > > + if (filter->no_symbolic && flag & REF_ISSYMREF) { > + return 0; > + } > + In Documentation/CodingGuidelines, we avoid braces around single-line if-statements. > /* Obtain the current ref kind from filter_ref_kind() and ignore > unwanted refs. */ > kind = filter_ref_kind(filter, refname); > if (!(kind & filter->kind)) > diff --git a/ref-filter.h b/ref-filter.h > index f1dcff4c6e..23e0d01a33 100644 > --- a/ref-filter.h > +++ b/ref-filter.h > @@ -65,7 +65,8 @@ struct ref_filter { > unsigned int with_commit_tag_algo : 1, >
[RFC PATCH 1/1] for-each-ref: add '--no-symbolic' option
Using the new flag will omit symbolic refs from the output. Without this flag, it is possible to get this behavior by using the `%(symref)` formatting field name and piping output through grep to include only those refs that do not output a value for `%(symref)`, but having this flag is more elegant and intention revealing. --- Documentation/git-for-each-ref.txt | 3 +++ builtin/for-each-ref.c | 4 +++- ref-filter.c | 4 ref-filter.h | 3 ++- t/t6302-for-each-ref-filter.sh | 6 ++ 5 files changed, 18 insertions(+), 2 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 6dcd39f6f6..be19111510 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -95,6 +95,9 @@ OPTIONS --ignore-case:: Sorting and filtering refs are case insensitive. +--no-symbolic:: + Only list refs that are not symbolic. + FIELD NAMES --- diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 465153e853..b71ab2f135 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -18,7 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) { int i; struct ref_sorting *sorting = NULL, **sorting_tail = &sorting; - int maxcount = 0, icase = 0; + int maxcount = 0, icase = 0, nosym = 0; struct ref_array array; struct ref_filter filter; struct ref_format format = REF_FORMAT_INIT; @@ -46,6 +46,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")), OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which don't contain the commit")), OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")), + OPT_BOOL(0, "no-symbolic", &nosym, N_("exclude symbolic refs")), OPT_END(), }; @@ -72,6 +73,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) sorting = ref_default_sorting(); sorting->ignore_case = icase; filter.ignore_case = icase; + filter.no_symbolic = nosym; filter.name_patterns = argv; filter.match_as_path = 1; diff --git a/ref-filter.c b/ref-filter.c index f27cfc8c3e..01beb279dc 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2093,6 +2093,10 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid, return 0; } + if (filter->no_symbolic && flag & REF_ISSYMREF) { + return 0; + } + /* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */ kind = filter_ref_kind(filter, refname); if (!(kind & filter->kind)) diff --git a/ref-filter.h b/ref-filter.h index f1dcff4c6e..23e0d01a33 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -65,7 +65,8 @@ struct ref_filter { unsigned int with_commit_tag_algo : 1, match_as_path : 1, ignore_case : 1, - detached : 1; + detached : 1, + no_symbolic : 1; unsigned int kind, lines; int abbrev, diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh index 35408d53fd..ab9c00fff4 100755 --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -454,4 +454,10 @@ test_expect_success 'validate worktree atom' ' test_cmp expect actual ' +test_expect_success 'filtering with --no-symbolic' ' + git symbolic-ref refs/symbolic refs/heads/master && + git for-each-ref --format="%(refname)" --no-symbolic >actual && + test_must_fail grep refs/symbolic actual +' + test_done -- 2.23.0