Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
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 mode words: git submodule init becomes git submodule --action=init git tag --delete TAG becomes git tag --action=delete TAGNAME That looks like a bad change in both cases --- it involves more typing without much upside to go along with it. But git submodule init gains synonym git submodule --init git tag --delete TAG stays as git tag --delete TAG looks fine to me. I agree 100% with the above that illustrates why --action=name is a bad idea. As I already said, adding action-option like --init, if doing so might help people, I am not opposed to it. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
(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), NULL, \
(h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (s) }
I agree that's a better proposal than mine.
By the way, I haven't convinced myself that it is a good idea in
general to encourage more use of command mode options, so I am a bit
reluctant to add this before knowing which direction in the longer
term we are going.
- Some large-ish Git subcommands, like git submodule, use the
mode word (e.g. git submodule status) to specify the operation
mode (youe could consider status a subsubcommand that
submodule subcommand takes). These commands typically began
their life from day one with the mode words.
- On the other hand, many Git subcommands, like git tag, have
the primary operation mode (e.g. create a new one is the
primary operation mode for git tag), and use command mode
options to specify other operation modes (e.g. --delete).
These commands started as single purpose commands (i.e. to
perform their primary operation) but have organically grown
over time and acquired command mode options to invoke their
secondary operations.
As an end user, you need to learn which style each command takes,
which is an unnecessary burden at the UI level. In the longer term,
we may want to consider picking a single style, and migrating
everybody to it. If I have to vote today, I would say we should
teach git submodule to also take command mode options (e.g. git
submodule --status will be understood the same way as git
submodule status), make them issue warnings when mode words are
used and encourage users to use command mode options instead, and
optionally remove the support of mode words at a large version bump
like 3.0.
One clear advantage mode words have over command mode options is
that there is no room for end user confusion. The first word after
git subcmd is the mode word, and you will not even dream of asking
what would 'git submodule add del foo' do? as it is nonsensical.
The command mode options, on the other hand, gives too much useless
flexibility to ask for nonsense, e.g. git tag --delete --verify,
git tag --no-delete --delete, etc., and extra code needs to detect
and reject combinations. But commands that took mode options cannot
be easily migrated to take mode words without hurting existing users
and scripts (e.g. git tag delete master can never be a request to
delete the tag 'master', as it is a request to create a tag whose
name is 'delete' that points at the same object as 'master' points
at).
Why not encourage the use of a standardized '--action' option instead?
This can work with lesser compatibility headaches for both the commands
taking mode options and the commands taking mode words:
git submodule init becomes git submodule --action=init
git tag --delete TAG becomes git tag --action=delete TAGNAME
Commands that have a primary operation mode can keep the use
of --action optional, while commands that have no such sensible
primary mode can make it mandatory (again, this gives more compatibility
with the existing syntax). And the old syntax
can be supported in parallel to the new one for a potentially
indefinite time; albeit, if I were to propose a timetable, I'd
got for:
- Git 2.0: the new syntax is introduced
- Git 2.x: any use the old syntax starts to trigger warnings
(which can be silenced with a config option)
- Git 3.0: any use the old syntax starts to trigger fatal
errors (which can be turned into mandatory warnings with
a config option)
- Git 4.0: any use the old syntax starts to trigger
mandatory fatal errors.
- Git 4.x: Remove any handling of the old syntax.
Just my 2 cents,
Stefano
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
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 git submodule --action=init git tag --delete TAG becomes git tag --action=delete TAGNAME That looks like a bad change in both cases --- it involves more typing without much upside to go along with it. But git submodule init gains synonym git submodule --init git tag --delete TAG stays as git tag --delete TAG looks fine to me. In the long run, we could require that for new commands the 'action' option must come immediately after the git command name if that makes things easier to learn. Thanks for some food for thought. My two cents, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
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 signature
Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
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 mine.
By the way, I haven't convinced myself that it is a good idea in
general to encourage more use of command mode options, so I am a bit
reluctant to add this before knowing which direction in the longer
term we are going.
- Some large-ish Git subcommands, like git submodule, use the
mode word (e.g. git submodule status) to specify the operation
mode (youe could consider status a subsubcommand that
submodule subcommand takes). These commands typically began
their life from day one with the mode words.
- On the other hand, many Git subcommands, like git tag, have
the primary operation mode (e.g. create a new one is the
primary operation mode for git tag), and use command mode
options to specify other operation modes (e.g. --delete).
These commands started as single purpose commands (i.e. to
perform their primary operation) but have organically grown
over time and acquired command mode options to invoke their
secondary operations.
As an end user, you need to learn which style each command takes,
which is an unnecessary burden at the UI level. In the longer term,
we may want to consider picking a single style, and migrating
everybody to it. If I have to vote today, I would say we should
teach git submodule to also take command mode options (e.g. git
submodule --status will be understood the same way as git
submodule status), make them issue warnings when mode words are
used and encourage users to use command mode options instead, and
optionally remove the support of mode words at a large version bump
like 3.0.
One clear advantage mode words have over command mode options is
that there is no room for end user confusion. The first word after
git subcmd is the mode word, and you will not even dream of asking
what would 'git submodule add del foo' do? as it is nonsensical.
The command mode options, on the other hand, gives too much useless
flexibility to ask for nonsense, e.g. git tag --delete --verify,
git tag --no-delete --delete, etc., and extra code needs to detect
and reject combinations. But commands that took mode options cannot
be easily migrated to take mode words without hurting existing users
and scripts (e.g. git tag delete master can never be a request to
delete the tag 'master', as it is a request to create a tag whose
name is 'delete' that points at the same object as 'master' points
at).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
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 + verify 1)
usage_with_options(git_tag_usage, options);
Now if we give one of the options twice, we'll get the usage information.
(i.e. 'git tag --verify --verify tagname' and
'git --delete --delete tagname' yield usage information and do not
do the intended command.)
This could have been fixed by rewriting the line to
if (!!list + !!delete + !!verify 1)
usage_with_options(git_tag_usage, options);
or as it happened in this patch by having the parameters not
counting up for each occurrence, but the OPT_BOOL just setting the
variables to either 0 if the option is not given or 1 if the option is
given multiple times.
However we could discuss if the negated options do make sense here, or if
we don't want to allow them here, as this seems valid (before and after
this patch):
git tag --no-verify --delete tagname
Signed-off-by: Stefan Beller stefanbel...@googlemail.com
---
builtin/tag.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index b3942e4..d155c9d 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -442,12 +442,12 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
struct option options[] = {
- OPT_BOOLEAN('l', list, list, N_(list tag names)),
+ OPT_BOOL('l', list, list, N_(list tag names)),
{ OPTION_INTEGER, 'n', NULL, lines, N_(n),
N_(print n lines of each tag message),
PARSE_OPT_OPTARG, NULL, 1 },
- OPT_BOOLEAN('d', delete, delete, N_(delete tags)),
- OPT_BOOLEAN('v', verify, verify, N_(verify tags)),
+ OPT_BOOL('d', delete, delete, N_(delete tags)),
+ OPT_BOOL('v', verify, verify, N_(verify tags)),
OPT_GROUP(N_(Tag creation options)),
OPT_BOOL('a', annotate, annotate,
@@ -455,7 +455,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_CALLBACK('m', message, msg, N_(message),
N_(tag message), parse_msg_arg),
OPT_FILENAME('F', file, msgfile, N_(read message from
file)),
- OPT_BOOLEAN('s', sign, opt.sign, N_(annotated and
GPG-signed tag)),
+ OPT_BOOL('s', sign, opt.sign, N_(annotated and GPG-signed
tag)),
OPT_STRING(0, cleanup, cleanup_arg, N_(mode),
N_(how to strip spaces and #comments from message)),
OPT_STRING('u', local-user, keyid, N_(key id),
--
1.8.4.rc0.1.g8f6a3e5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
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
bug fix!
In line 499 we have:
if (list + delete + verify 1)
usage_with_options(git_tag_usage, options);
Now if we give one of the options twice, we'll get the usage information.
(i.e. 'git tag --verify --verify tagname' and
'git --delete --delete tagname' yield usage information and do not
do the intended command.)
This could have been fixed by rewriting the line to
if (!!list + !!delete + !!verify 1)
usage_with_options(git_tag_usage, options);
or as it happened in this patch by having the parameters not
counting up for each occurrence, but the OPT_BOOL just setting the
variables to either 0 if the option is not given or 1 if the option is
given multiple times.
Makes twisted sort of sense ;-).
However we could discuss if the negated options do make sense
here, or if we don't want to allow them here, as this seems valid
(before and after this patch):
git tag --no-verify --delete tagname
It probably does not. As you hinted in your earlier patch, we may
want to introduce a only can set to true boolean used solely to
specify these things. They are disguised as options, but are in
fact command operation modes that are often mutually exclusive.
For these operation modes that are mutually exclusive, there are
multiple possible implementations:
* One OPT_BOOL_NONEG per option; the code ensures the mutual
exclusion with if (list + delete + verify 1);
* One OPT_BIT per option in a single variable; the code ensures the
mutual exclusion with count_bits, which may be a lot more
cumbersome;
* OPT_SET_INT that updates a single variable to enum; instead of
making it an error to give two conflicting modes, this would give
us the last-one-wins rule.
Unlike usual options, we generally do not want the last-one-wins
semantics for command operation modes, I think.
Perhaps we would want something like this?
-- 8 --
Subject: [PATCH] parse-options: add OPT_CMDMODE()
This can be used to define a set of mutually exclusive command
mode options, and automatically catch use of more than one from
that set as an error.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
parse-options.c | 58 -
parse-options.h | 3 +++
2 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index c2cbca2..62e9b1c 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -43,8 +43,42 @@ static void fix_filename(const char *prefix, const char
**file)
*file = xstrdup(prefix_filename(prefix, strlen(prefix), *file));
}
+static int opt_command_mode_error(const struct option *opt,
+ const struct option *all_opts,
+ int flags)
+{
+ const struct option *that;
+ struct strbuf message = STRBUF_INIT;
+ struct strbuf that_name = STRBUF_INIT;
+
+ /*
+* Find the other option that was used to set the variable
+* already, and report that this is not compatible with it.
+*/
+ for (that = all_opts; that-type != OPTION_END; that++) {
+ if (that == opt ||
+ that-type != OPTION_CMDMODE ||
+ that-value != opt-value ||
+ that-defval != *(int *)opt-value)
+ continue;
+
+ if (that-long_name)
+ strbuf_addf(that_name, --%s, that-long_name);
+ else
+ strbuf_addf(that_name, -%c, that-short_name);
+ strbuf_addf(message, : incompatible with %s, that_name.buf);
+ strbuf_release(that_name);
+ opterror(opt, message.buf, flags);
+ strbuf_release(message);
+ return -1;
+ }
+ return opterror(opt, : incompatible with something else, flags);
+}
+
static int get_value(struct parse_opt_ctx_t *p,
-const struct option *opt, int flags)
+const struct option *opt,
+const struct option *all_opts,
+int flags)
{
const char *s, *arg;
const int unset = flags OPT_UNSET;
@@ -83,6 +117,16 @@ static int get_value(struct parse_opt_ctx_t *p,
*(int *)opt-value = unset ? 0 : opt-defval;
return 0;
+ case OPTION_CMDMODE:
+ /*
+* Giving the same mode option twice, although is unnecessary,
+* is not a grave error, so let it pass.
+*/
+ if (*(int *)opt-value *(int *)opt-value != opt-defval)
+ return opt_command_mode_error(opt,
Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
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 existing behavior, this commit is actually a
bug fix!
In line 499 we have:
if (list + delete + verify 1)
usage_with_options(git_tag_usage, options);
Now if we give one of the options twice, we'll get the usage information.
(i.e. 'git tag --verify --verify tagname' and
'git --delete --delete tagname' yield usage information and do not
do the intended command.)
This could have been fixed by rewriting the line to
if (!!list + !!delete + !!verify 1)
usage_with_options(git_tag_usage, options);
or as it happened in this patch by having the parameters not
counting up for each occurrence, but the OPT_BOOL just setting the
variables to either 0 if the option is not given or 1 if the option is
given multiple times.
Makes twisted sort of sense ;-).
However we could discuss if the negated options do make sense
here, or if we don't want to allow them here, as this seems valid
(before and after this patch):
git tag --no-verify --delete tagname
It probably does not. As you hinted in your earlier patch, we may
want to introduce a only can set to true boolean used solely to
specify these things. They are disguised as options, but are in
fact command operation modes that are often mutually exclusive.
For these operation modes that are mutually exclusive, there are
multiple possible implementations:
* One OPT_BOOL_NONEG per option; the code ensures the mutual
exclusion with if (list + delete + verify 1);
* One OPT_BIT per option in a single variable; the code ensures the
mutual exclusion with count_bits, which may be a lot more
cumbersome;
* OPT_SET_INT that updates a single variable to enum; instead of
making it an error to give two conflicting modes, this would give
us the last-one-wins rule.
Unlike usual options, we generally do not want the last-one-wins
semantics for command operation modes, I think.
Perhaps we would want something like this?
-- 8 --
Subject: [PATCH] parse-options: add OPT_CMDMODE()
This can be used to define a set of mutually exclusive command
mode options, and automatically catch use of more than one from
that set as an error.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
parse-options.c | 58
-
parse-options.h | 3 +++
2 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index c2cbca2..62e9b1c 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -43,8 +43,42 @@ static void fix_filename(const char *prefix, const char
**file)
*file = xstrdup(prefix_filename(prefix, strlen(prefix), *file));
}
+static int opt_command_mode_error(const struct option *opt,
+ const struct option *all_opts,
+ int flags)
+{
+ const struct option *that;
+ struct strbuf message = STRBUF_INIT;
+ struct strbuf that_name = STRBUF_INIT;
+
+ /*
+ * Find the other option that was used to set the variable
+ * already, and report that this is not compatible with it.
+ */
+ for (that = all_opts; that-type != OPTION_END; that++) {
+ if (that == opt ||
+ that-type != OPTION_CMDMODE ||
+ that-value != opt-value ||
+ that-defval != *(int *)opt-value)
+ continue;
+
+ if (that-long_name)
+ strbuf_addf(that_name, --%s, that-long_name);
+ else
+ strbuf_addf(that_name, -%c, that-short_name);
+ strbuf_addf(message, : incompatible with %s, that_name.buf);
+ strbuf_release(that_name);
+ opterror(opt, message.buf, flags);
+ strbuf_release(message);
+ return -1;
+ }
+ return opterror(opt, : incompatible with something else, flags);
+}
+
static int get_value(struct parse_opt_ctx_t *p,
- const struct option *opt, int flags)
+ const struct option *opt,
+ const struct option *all_opts,
+ int flags)
{
const char *s, *arg;
const int unset = flags OPT_UNSET;
@@ -83,6 +117,16 @@ static int get_value(struct parse_opt_ctx_t *p,
*(int *)opt-value = unset ? 0 : opt-defval;
return 0;
+ case OPTION_CMDMODE:
+ /*
+ * Giving the same mode option twice, although is unnecessary,
+ * is not a grave error, so let it pass.
+ */
+ if (*(int *)opt-value *(int *)opt-value != opt-defval)
+
Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
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
+++ b/builtin/tag.c
@@ -436,18 +436,18 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
struct ref_lock *lock;
struct create_tag_options opt;
char *cleanup_arg = NULL;
- int annotate = 0, force = 0, lines = -1, list = 0,
- delete = 0, verify = 0;
+ int annotate = 0, force = 0, lines = -1;
+ int cmdmode = 0;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
struct option options[] = {
- OPT_BOOLEAN('l', list, list, N_(list tag names)),
+ OPT_CMDMODE('l', list, cmdmode, N_(list tag names), 'l'),
{ OPTION_INTEGER, 'n', NULL, lines, N_(n),
N_(print n lines of each tag message),
PARSE_OPT_OPTARG, NULL, 1 },
- OPT_BOOLEAN('d', delete, delete, N_(delete tags)),
- OPT_BOOLEAN('v', verify, verify, N_(verify tags)),
+ OPT_CMDMODE('d', delete, cmdmode, N_(delete tags), 'd'),
+ OPT_CMDMODE('v', verify, cmdmode, N_(verify tags), 'v'),
OPT_GROUP(N_(Tag creation options)),
OPT_BOOLEAN('a', annotate, annotate,
@@ -489,22 +489,19 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
}
if (opt.sign)
annotate = 1;
- if (argc == 0 !(delete || verify))
- list = 1;
+ if (argc == 0 !cmdmode)
+ cmdmode = 'l';
- if ((annotate || msg.given || msgfile || force)
- (list || delete || verify))
+ if ((annotate || msg.given || msgfile || force) (cmdmode != 0))
usage_with_options(git_tag_usage, options);
- if (list + delete + verify 1)
- usage_with_options(git_tag_usage, options);
finalize_colopts(colopts, -1);
- if (list lines != -1) {
+ if (cmdmode == 'l' lines != -1) {
if (explicitly_enable_column(colopts))
die(_(--column and -n are incompatible));
colopts = 0;
}
- if (list) {
+ if (cmdmode == 'l') {
int ret;
if (column_active(colopts)) {
struct column_options copts;
@@ -523,9 +520,9 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
die(_(--contains option is only allowed with -l.));
if (points_at.nr)
die(_(--points-at option is only allowed with -l.));
- if (delete)
+ if (cmdmode == 'd')
return for_each_tag_name(argv, delete_tag);
- if (verify)
+ if (cmdmode == 'v')
return for_each_tag_name(argv, verify_tag);
if (msg.given || msgfile) {
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, description, enum)
Also the variable would then only need to be an enum accepting variable,
and not an integer accepting all integer range, so we'd also catch
typos or wrong values in such a case:
+ if (argc == 0 !cmdmode)
+ cmdmode = 'l'; // maybe 'l' is a typo and not existing?
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
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 parsing infrastructure with more
lines of code. ;)
* You can only have one OPTION_CMDMODE in one argv vector right?
That is not what I intended, at least.
int one = 0, two = 0;
struct option options[] = {
OPT_CMDMODE('a', NULL, one, N_(set one to a), 'a'),
OPT_CMDMODE('b', NULL, one, N_(set one to b), 'b'),
OPT_CMDMODE('c', NULL, two, N_(set two to c), 'c'),
OPT_CMDMODE('d', NULL, two, N_(set two to d), 'd'),
OPT_END()
}
should give you two independent sets of modes, one and two.
The only reason I needed to add an extra parameter to get_value()
was so that I can tell the former two and the latter two belong to
different groups, and that is done by looking at the address of the
variable. In opt_command_mode_error(), opt-value == that-value
is used as a condition to see if the other option is possibly the
one that was used previously, which conflicted with us.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times
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, description, enum)
I actually was thinking about going totally in the opposite
direction.
People who grew up with the old Unix tradition getopt(3) are used to
code like this:
while ((opt = getopt(ac, av, abcde)) != -1) {
switch (opt) {
case 'a': perform_a(); break;
case 'b': perform_b(); break;
...
}
}
In other words, the enum is most convenient if it matches the
short option, so instead of having to repeat ourselves over and
over, like I did in that illustration patch for builtin/tag.c, e.g.
OPT_CMDMODE('d', delete, cmdmode, N_(delete tags), 'd'),
OPT_CMDMODE('v', verify, cmdmode, N_(verify tags), 'v'),
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) }
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
