Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > diff --git a/builtin/tag.c b/builtin/tag.c > index ad29be6923..0bba3fd070 100644 > --- a/builtin/tag.c > +++ b/builtin/tag.c > @@ -454,10 +454,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix) > } > create_tag_object = (opt.sign || annotate || msg.given || msgfile); > > - if (argc == 0 && !cmdmode) > + if (argc == 0 && !cmdmode && !create_tag_object) > cmdmode = 'l'; So with this change, if we cannot infer that we are creating a tag object from other options, we leave cmdmode to its original 0. > - if ((create_tag_object || force) && (cmdmode != 0)) > + if ((create_tag_object || force) && (cmdmode || (!cmdmode && !argc))) > usage_with_options(git_tag_usage, options); And then immediately after that, we complain by detecting that we know we are creating a tag and a non-zero cmdmode is in effect (i.e. 'l', 'd' or 'v', none of which is about creating a tag). The way we used to detect that we are doing something other than tag creation was by seeing cmdmode is set to anything. Because of your earlier change, that no longer is true. You need to separately check (!cmdmode && !argc). By following the logic that way, I can see how this change at this step is a no-op, but I have to say that the code with this patch looks much more bizarre than the original. I am not sure why you want to do the first change at this step in the first place. Is it because you'd want to take over (!cmdmode && !argc) condtion to default to 'list'? With the change in 4/8 and 5/8, you are ensuring that cmdmode is set to 'l' for these new cases before the code hits the check to call usage_with_options(). And at that point, you can use the original "are we creating and !cmdmode says we are not? That's contradiction" logic without making it more bizarre with this patch, no?