Re: [PATCH 2/2] describe: Exclude --all --match=PATTERN

[Greg Price <price@MIT.EDU>]

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

On Wed, Feb 27, 2013 at 12:20:07PM -0800, Junio C Hamano wrote:
> Without "--all" the command considers only the annotated tags to
> base the descripion on, and with "--all", a ref that is not
> annotated tags can be used as a base, but with a lower priority (if
> an annotated tag can describe a given commit, that tag is used).
> 
> So naïvely I would expect "--all" and "--match" to base the
> description on refs that match the pattern without limiting the
> choice of base to annotated tags, and refs that do not match the
> given pattern should not appear even as the last resort.  It appears
> to me that the current situation is (3).

Hmm.  It seems to me that "--all" says two things:

 (a) allow unannotated (rather than only annotated)

 (b) allow refs of any name (rather than only tags)

With "--match", particularly because the pattern always refers only to
tags, (b) is obliterated, and your proposed semantics are (a) plus a
sort of inverse of (b):

 (c) allow only refs matching the pattern

which is what "--match" means alone.  But if what we are going for is
(a) and (c), then we don't need "--all" for (a) -- we can get
precisely that with "--tags".  So these semantics make "--all --match=PAT"
equivalent to "--tags --match=PAT".

Given that, I think the user is better off if we reject "--all
--match" with an error message -- and perhaps the error message should
advise them to use "--tags" instead.  Otherwise we have "--all"
telling us (b) as well as (a), and "--match" countermanding (b) and
going precisely the other direction to (c).  If the user has written
that by hand, then they may be confused, and if the command line was
generated, perhaps called from a script, then I fear a bug in the
script is likely, what with the conflicting expectations expressed by
"--all" and "--match".

Patch below to suggest "--tags" in the error message.

Greg

[-- Attachment #2: 0001-describe-Better-error-message-on-all-match.patch --]
[-- Type: text/x-diff, Size: 1090 bytes --]

>From 66f985b2510c870e62e313732de4b6709894b074 Mon Sep 17 00:00:00 2001
From: Greg Price <price@mit.edu>
Date: Sun, 3 Mar 2013 12:19:57 -0800
Subject: [PATCH] describe: Better error message on --all --match

The reason they conflict is that --all means
 (a) allow unannotated, and
 (b) allow refs of any name (rather than only tags),
while --match contradicts (b) and goes further to
 (c) allow only tags matching pattern.

If what the user wants is (a) and (c), they can use --tags to get (a)
without (b).
---
 builtin/describe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 2ef3f10..6581c40 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -436,7 +436,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 		die(_("--long is incompatible with --abbrev=0"));
 
 	if (pattern && all)
-		die(_("--match is incompatible with --all"));
+		die(_("--all conflicts with --match; do you mean --tags?"));
 
 	if (contains) {
 		const char **args = xmalloc((7 + argc) * sizeof(char *));
-- 
1.7.11.3