[PATCH] api-parse-options.txt: document OPT_CMDMODE()

[PATCH] api-parse-options.txt: document OPT_CMDMODE()

[Pranit Bauva]

OPT_CMDMODE() was introduced in the release of 1.8.5 which makes the use
of subcommands in the form of arguments a lot cleaner and easier.
---
 Documentation/technical/api-parse-options.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 5f0757d..8130d26 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -231,6 +231,12 @@ There are some macros to easily define options:
 	pass the command-line option, which can be specified multiple times,
 	to another command.
 
+`OPT_CMDMODE(short, long, &int_var, description, enum_val)`::
+	Introduce an option for subcommands. It is useful when you want to use
+	the command with a particular sub command only and ignore other sub
+	commands it has. It will set `int_var` to enum_val if the argument is
+	invoked.
+
 
 The last element of the array must be `OPT_END()`.
 

--
https://github.com/git/git/pull/219

Re: [PATCH] api-parse-options.txt: document OPT_CMDMODE()

[Junio C Hamano]

Pranit Bauva <pranit.bauva@gmail.com> writes:

> OPT_CMDMODE() was introduced in the release of 1.8.5 which makes the use
> of subcommands in the form of arguments a lot cleaner and easier.
> ---

Sign-off?

>  Documentation/technical/api-parse-options.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
> index 5f0757d..8130d26 100644
> --- a/Documentation/technical/api-parse-options.txt
> +++ b/Documentation/technical/api-parse-options.txt
> @@ -231,6 +231,12 @@ There are some macros to easily define options:
>  	pass the command-line option, which can be specified multiple times,
>  	to another command.
>  
> +`OPT_CMDMODE(short, long, &int_var, description, enum_val)`::
> +	Introduce an option for subcommands. It is useful when you want to use
> +	the command with a particular sub command only and ignore other sub
> +	commands it has. It will set `int_var` to enum_val if the argument is
> +	invoked.
> +

Sorry, but I do not get what "when you want to... ignore other sub
command it has" wants to say.

CMDMODE is a mechanism to actively notice when multiple "operation
mode" options that specify mutually incompatible operation modes are
given and error out without the user of parse_options() to implement
that mutual exclusion herself.  That is, if you have 'add', 'remove'
and 'edit' operation modes, with OPT_BOOL(), you would have to say:

	options[] = {
                OPT_BOOL('a', "add", &add, ...),
                OPT_BOOL('r', "remove", &remove, ...),
                OPT_BOOL('e', "edit", &edit, ...),
                ...
	};
        parse_options(ac, av, prefix, options, ...);

	if (!!add + !!remove + !!edit > 1)
        	die("at most one add/remove/edit can be used at a time");

	if (add)
        	do_add();
	if (remove)
        	do_remove();
	if (edit)
        	do_edit();

but with CMDMODE, you can do:

	options[] = {
                OPT_BOOL('a', "add", &mode, ...),
                OPT_BOOL('r', "remove", &mode, ...),
                OPT_BOOL('e', "edit", &mode, ...),
                ...
	};
        parse_options(ac, av, prefix, options, ...);

        switch (mode) {
        case 'a': do_add(); break;
        case 'r': do_remove(); break;
        case 'e': do_edit(); break;
		...
	}

and parse_options notices that "mode" is shared across these three
options, and implements the mutual-exclusion itself.

Re: [PATCH] api-parse-options.txt: document OPT_CMDMODE()

[Pranit Bauva]

On Thu, Mar 24, 2016 at 9:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>> OPT_CMDMODE() was introduced in the release of 1.8.5 which makes the use
>> of subcommands in the form of arguments a lot cleaner and easier.
>> ---
>
> Sign-off?

Will include this.

>>  Documentation/technical/api-parse-options.txt | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
>> index 5f0757d..8130d26 100644
>> --- a/Documentation/technical/api-parse-options.txt
>> +++ b/Documentation/technical/api-parse-options.txt
>> @@ -231,6 +231,12 @@ There are some macros to easily define options:
>>       pass the command-line option, which can be specified multiple times,
>>       to another command.
>>
>> +`OPT_CMDMODE(short, long, &int_var, description, enum_val)`::
>> +     Introduce an option for subcommands. It is useful when you want to use
>> +     the command with a particular sub command only and ignore other sub
>> +     commands it has. It will set `int_var` to enum_val if the argument is
>> +     invoked.
>> +
>
> Sorry, but I do not get what "when you want to... ignore other sub
> command it has" wants to say.

What I meant by this statement is that (your example), let's say we
have "add", "remove" and "edit" sub commands. Now the user has to pick
between the three. He cannot choose more than 1 from these.

> CMDMODE is a mechanism to actively notice when multiple "operation
> mode" options that specify mutually incompatible operation modes are
> given and error out without the user of parse_options() to implement
> that mutual exclusion herself.  That is, if you have 'add', 'remove'
> and 'edit' operation modes, with OPT_BOOL(), you would have to say:
>
>         options[] = {
>                 OPT_BOOL('a', "add", &add, ...),
>                 OPT_BOOL('r', "remove", &remove, ...),
>                 OPT_BOOL('e', "edit", &edit, ...),
>                 ...
>         };
>         parse_options(ac, av, prefix, options, ...);
>
>         if (!!add + !!remove + !!edit > 1)
>                 die("at most one add/remove/edit can be used at a time");
>
>         if (add)
>                 do_add();
>         if (remove)
>                 do_remove();
>         if (edit)
>                 do_edit();
>
> but with CMDMODE, you can do:
>
>         options[] = {
>                 OPT_BOOL('a', "add", &mode, ...),
>                 OPT_BOOL('r', "remove", &mode, ...),
>                 OPT_BOOL('e', "edit", &mode, ...),
>                 ...
>         };
>         parse_options(ac, av, prefix, options, ...);
>
>         switch (mode) {
>         case 'a': do_add(); break;
>         case 'r': do_remove(); break;
>         case 'e': do_edit(); break;
>                 ...
>         }
>
> and parse_options notices that "mode" is shared across these three
> options, and implements the mutual-exclusion itself.

Thanks for taking time to explain all the details behind it. I can
include these bits in the documentation. :)

Re: [PATCH] api-parse-options.txt: document OPT_CMDMODE()

[Junio C Hamano]

Pranit Bauva <pranit.bauva@gmail.com> writes:

>>> +`OPT_CMDMODE(short, long, &int_var, description, enum_val)`::
>>> +     Introduce an option for subcommands. It is useful when you want to use
>>> +     the command with a particular sub command only and ignore other sub
>>> +     commands it has. It will set `int_var` to enum_val if the argument is
>>> +     invoked.
>>> +
>>
>> Sorry, but I do not get what "when you want to... ignore other sub
>> command it has" wants to say.
>
> What I meant by this statement is that (your example), let's say we
> have "add", "remove" and "edit" sub commands. Now the user has to pick
> between the three. He cannot choose more than 1 from these.

Then I find the word "ignore others" misleading.  Quite the
contrary, the user has to be aware of the others and not to give
them.

	Define an "operating mode" option, only one of which in the
	same group of "operating mode" options that share the same
	`int_var` can be given by the user.  `enum_val` is set to
	`int_var` when the option is used, but an error is reported
        if other "operating mode" option has already set its value
        to the same `int_var`.

or something?

Re: [PATCH] api-parse-options.txt: document OPT_CMDMODE()

[Pranit Bauva]

On Thu, Mar 24, 2016 at 10:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>>>> +`OPT_CMDMODE(short, long, &int_var, description, enum_val)`::
>>>> +     Introduce an option for subcommands. It is useful when you want to use
>>>> +     the command with a particular sub command only and ignore other sub
>>>> +     commands it has. It will set `int_var` to enum_val if the argument is
>>>> +     invoked.
>>>> +
>>>
>>> Sorry, but I do not get what "when you want to... ignore other sub
>>> command it has" wants to say.
>>
>> What I meant by this statement is that (your example), let's say we
>> have "add", "remove" and "edit" sub commands. Now the user has to pick
>> between the three. He cannot choose more than 1 from these.
>
> Then I find the word "ignore others" misleading.  Quite the
> contrary, the user has to be aware of the others and not to give
> them.
>
>         Define an "operating mode" option, only one of which in the
>         same group of "operating mode" options that share the same
>         `int_var` can be given by the user.  `enum_val` is set to
>         `int_var` when the option is used, but an error is reported
>         if other "operating mode" option has already set its value
>         to the same `int_var`.
>
> or something?

Seems a crystal clear explanation to me. Thanks. I was unaware that it
throws an error.

[PATCH v2] api-parse-options.txt: document OPT_CMDMODE()

[Pranit Bauva]

OPT_CMDMODE mechanism was introduced in the release of 1.8.5 to actively
notice when multiple "operation mode" options that specify mutually
incompatible operation modes are given.

Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 Documentation/technical/api-parse-options.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 5f0757d..695bd4b 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -231,6 +231,13 @@ There are some macros to easily define options:
 	pass the command-line option, which can be specified multiple times,
 	to another command.
 
+`OPT_CMDMODE(short, long, &int_var, description, enum_val)`::
+	Define an "operation mode" option, only one of which in the same
+	group of "operating mode" options that share the same `int_var`
+	can be given by the user. `enum_val` is set to `int_var` when the
+	option is used, but an error is reported if other "operating mode"
+	option has already set its value to the same `int_var`.
+
 
 The last element of the array must be `OPT_END()`.
 

--
https://github.com/git/git/pull/219

Re: [PATCH v2] api-parse-options.txt: document OPT_CMDMODE()

[Pranit Bauva]

I have used bits from your email. I forgot to add "Helped-by: Junio C
Hamano <gitster@pobox.com>" . Could you squash it in?

On Sat, Mar 26, 2016 at 12:28 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> OPT_CMDMODE mechanism was introduced in the release of 1.8.5 to actively
> notice when multiple "operation mode" options that specify mutually
> incompatible operation modes are given.
>
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> ---
>  Documentation/technical/api-parse-options.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
> index 5f0757d..695bd4b 100644
> --- a/Documentation/technical/api-parse-options.txt
> +++ b/Documentation/technical/api-parse-options.txt
> @@ -231,6 +231,13 @@ There are some macros to easily define options:
>         pass the command-line option, which can be specified multiple times,
>         to another command.
>
> +`OPT_CMDMODE(short, long, &int_var, description, enum_val)`::
> +       Define an "operation mode" option, only one of which in the same
> +       group of "operating mode" options that share the same `int_var`
> +       can be given by the user. `enum_val` is set to `int_var` when the
> +       option is used, but an error is reported if other "operating mode"
> +       option has already set its value to the same `int_var`.
> +
>
>  The last element of the array must be `OPT_END()`.
>
>
> --
> https://github.com/git/git/pull/219
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] api-parse-options.txt: document OPT_CMDMODE()

[Pranit Bauva]

OPT_CMDMODE mechanism was introduced in the release of 1.8.5 to actively
notice when multiple "operation mode" options that specify mutually
incompatible operation modes are given.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
 Documentation/technical/api-parse-options.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 5f0757d..695bd4b 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -231,6 +231,13 @@ There are some macros to easily define options:
 	pass the command-line option, which can be specified multiple times,
 	to another command.
 
+`OPT_CMDMODE(short, long, &int_var, description, enum_val)`::
+	Define an "operation mode" option, only one of which in the same
+	group of "operating mode" options that share the same `int_var`
+	can be given by the user. `enum_val` is set to `int_var` when the
+	option is used, but an error is reported if other "operating mode"
+	option has already set its value to the same `int_var`.
+
 
 The last element of the array must be `OPT_END()`.
 

--
https://github.com/git/git/pull/219

Re: [PATCH v2] api-parse-options.txt: document OPT_CMDMODE()

[Junio C Hamano]

Pranit Bauva <pranit.bauva@gmail.com> writes:

> I have used bits from your email. I forgot to add "Helped-by: Junio C
> Hamano <gitster@pobox.com>" . Could you squash it in?

It's OK to omit such a credit for a patch this small.  After
understanding the issues, you would have written essentially the
same thing yourself.

Thanks, queued v2 already.