Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-08-18 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes: Stefano Lattarini wrote: Why not encourage the use of a standardized '--action' option instead? Because it's an unpleasant UI. :) This can work with lesser compatibility headaches for both the commands taking mode options and the commands taking

Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-08-17 Thread Stefano Lattarini
(Going through old mail today, sorry for the late reply) On 08/01/2013 12:10 AM, Junio C Hamano wrote: Stefan Beller stefanbel...@googlemail.com writes: On 07/31/13 00:28, Junio C Hamano wrote: we could just do #define OPT_CMDMODE(s, l, v, h) \ { OPTION_CMDMODE, (s), (l), (v),

Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-08-17 Thread Jonathan Nieder
Stefano Lattarini wrote: Why not encourage the use of a standardized '--action' option instead? Because it's an unpleasant UI. :) This can work with lesser compatibility headaches for both the commands taking mode options and the commands taking mode words: git submodule init becomes

Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-07-31 Thread Stefan Beller
On 07/31/13 00:28, Junio C Hamano wrote: we could just do #define OPT_CMDMODE(s, l, v, h) \ { OPTION_CMDMODE, (s), (l), (v), NULL, \ (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (s) } I agree that's a better proposal than mine. signature.asc Description: OpenPGP digital

Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-07-31 Thread Junio C Hamano
Stefan Beller stefanbel...@googlemail.com writes: On 07/31/13 00:28, Junio C Hamano wrote: we could just do #define OPT_CMDMODE(s, l, v, h) \ { OPTION_CMDMODE, (s), (l), (v), NULL, \ (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (s) } I agree that's a better proposal than

[PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-07-30 Thread Stefan Beller
As of b04ba2bb (parse-options: deprecate OPT_BOOLEAN, 2011-09-27), the OPT_BOOLEAN was deprecated. While I am going to replace the OPT_BOOLEAN by the proposed OPT_BOOL or the OPT_COUNTUP to keep existing behavior, this commit is actually a bug fix! In line 499 we have: if (list + delete +

Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-07-30 Thread Junio C Hamano
Stefan Beller stefanbel...@googlemail.com writes: As of b04ba2bb (parse-options: deprecate OPT_BOOLEAN, 2011-09-27), the OPT_BOOLEAN was deprecated. While I am going to replace the OPT_BOOLEAN by the proposed OPT_BOOL or the OPT_COUNTUP to keep existing behavior, this commit is actually a

Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-07-30 Thread Stefan Beller
On 07/30/13 21:24, Junio C Hamano wrote: Stefan Beller stefanbel...@googlemail.com writes: As of b04ba2bb (parse-options: deprecate OPT_BOOLEAN, 2011-09-27), the OPT_BOOLEAN was deprecated. While I am going to replace the OPT_BOOLEAN by the proposed OPT_BOOL or the OPT_COUNTUP to keep

Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-07-30 Thread Stefan Beller
On 07/30/13 21:24, Junio C Hamano wrote: ... and then git tag may become like so. builtin/tag.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index af3af3f..d8ae5aa 100644 --- a/builtin/tag.c +++

Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-07-30 Thread Junio C Hamano
Stefan Beller stefanbel...@googlemail.com writes: Your approach seems more like what we really want, however I'd have some points: * Is it a good idea to have so many different OPT_MODE or OPTION_MODE defines? In my attempts I tried to reuse existing OPTION_s to not pollute the

Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-07-30 Thread Junio C Hamano
Stefan Beller stefanbel...@googlemail.com writes: Here is just another idea: if (cmdmode == 'v') This may be hard to read, (What is 'v'? I cannot remember all the alphabet ;)) So maybe we could have an enum instead of the last parameter? OPT_CMDMODE( short, long, variable,