Hi,
2014-06-04 19:17 GMT+02:00 <svn_grass@osgeo.org>:
1. Consolite option/flag mutually exslusive messages.
it would be nice to solve it in the parser level...
Martin
--
Martin Landa * http://geo.fsv.cvut.cz/gwiki/Landa
I was thinking about the same thing. Maybe add char *exclusive_group to Option and Flag and G_parser() takes care of exclusiveness?
···
On Wed, Jun 4, 2014 at 2:03 PM, Martin Landa <landa.martin@gmail.com> wrote:
Hi,
2014-06-04 19:17 GMT+02:00 <svn_grass@osgeo.org>:
- Consolite option/flag mutually exslusive messages.
it would be nice to solve it in the parser level…
Martin
–
Martin Landa * http://geo.fsv.cvut.cz/gwiki/Landa
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev
Huidae Cho wrote:
I was thinking about the same thing. Maybe add char *exclusive_group to
Option and Flag and G_parser() takes care of exclusiveness?
There are two relatively straightforward cases for option
interdependencies:
1. At most one option from a set may be provided (mutual exclusivity).
2. At least one option from a set must be provided (more general form
of the ->required field).
Both of these requirements may apply, i.e. exactly one option from a
set must be provided.
But there may still be cases where that isn't sufficient, and you'd
need generalised boolean expressions. E.g. Where you need either A or
both of B and C. Or if you use A then B is required, otherwise it's
invalid.
It would be a fairly simple matter to add a function to evaluate a
boolean expression supplied by the module. But that wouldn't be much
help either to the GUI (which ideally would use the interdependency
information to e.g. grey out invalid options) or for generating error
messages (we want something more informative than "that combination of
options isn't valid").
So, in the first instance I'd suggest adding:
// at most one option from a set
void G_option_exclusive(const char *opt, ...);
// at least one option from a set
void G_option_required(const char *opt, ...);
// if the first option is present, at least one of the other
// options must also be present
void G_option_requires(const char *opt, ...);
All functions take a NULL-terminated list of option/flag names
(leading "-" indicates a flag).
The argument-checking code would be migrated to the new functions. Any
cases which still cannot be expressed would be reported to the list
for further consideration.
Once we've either covered all of the cases or concluding that any
remaining cases cannot reasonably be incorporated into any general
framework, we can work on exporting this information via
--interface-description etc.
--
Glynn Clements <glynn@gclements.plus.com>
My implementation of the “exclusive” member, which you reverted, can handle all these cases, I think. But since you reverted it, I think you didn’t agree with my interface or implementation?
IMO, passing option/flag names to G_option_required() has its own disadvantage because changing option/flag names takes more effort. If you have valid reasoning behind adding functions rather than adding a member, just like I did, I would prefer to pass *Option and *Flag pointers to those functions. But I guess it can be a little tricky to pass two different types of pointers.
On Jun 6, 2014 2:42 AM, “Glynn Clements” <glynn@gclements.plus.com> wrote:
Huidae Cho wrote:
I was thinking about the same thing. Maybe add char *exclusive_group to
Option and Flag and G_parser() takes care of exclusiveness?There are two relatively straightforward cases for option
interdependencies:
At most one option from a set may be provided (mutual exclusivity).
At least one option from a set must be provided (more general form
of the ->required field).Both of these requirements may apply, i.e. exactly one option from a
set must be provided.But there may still be cases where that isn’t sufficient, and you’d
need generalised boolean expressions. E.g. Where you need either A or
both of B and C. Or if you use A then B is required, otherwise it’s
invalid.It would be a fairly simple matter to add a function to evaluate a
boolean expression supplied by the module. But that wouldn’t be much
help either to the GUI (which ideally would use the interdependency
information to e.g. grey out invalid options) or for generating error
messages (we want something more informative than “that combination of
options isn’t valid”).So, in the first instance I’d suggest adding:
// at most one option from a set
void G_option_exclusive(const char *opt, …);// at least one option from a set
void G_option_required(const char *opt, …);// if the first option is present, at least one of the other
// options must also be present
void G_option_requires(const char *opt, …);All functions take a NULL-terminated list of option/flag names
(leading “-” indicates a flag).The argument-checking code would be migrated to the new functions. Any
cases which still cannot be expressed would be reported to the list
for further consideration.Once we’ve either covered all of the cases or concluding that any
remaining cases cannot reasonably be incorporated into any general
framework, we can work on exporting this information via
–interface-description etc.–
Glynn Clements <glynn@gclements.plus.com>
Huidae Cho wrote:
My implementation of the "exclusive" member, which you reverted, can handle
all these cases, I think. But since you reverted it, I think you didn't
agree with my interface or implementation?
Not really.
[I also wanted to be able to discuss this starting from a blank slate,
without the temptation to take any specific approach as a "reference
point". But that alone wouldn't have been sufficient for reversion.]
Mostly, I find the interface to be cryptic. Information which should
(IMHO) be in one place is spread out across the individual options.
In order to understand the relationships, anyone reading the code
needs to examine all of the options, and collate them into groups
based upon matching group numbers. IOW, mentally replicate the
(non-trivial) algorithms which form the bulk of r60713. The fact that
this was deemed to require several examples (with comments) tends to
suggest that it isn't particularly intuitive.
Apart from that, declaring "rules" would make the transition more
straightforward, as it mirrors the structure of the existing code
(i.e. the "if (expression) error();" statements).
By itself, that wouldn't be sufficient reason to prefer a rule-based
approach if it is considered to be inferior on its own merits. But
given the extent of the changes, it might be enough to tip the balance
in the event that the choice between different approaches amounted to
a toss-up.
IMO, passing option/flag names to G_option_required() has its own
disadvantage because changing option/flag names takes more effort. If you
have valid reasoning behind adding functions rather than adding a member,
just like I did, I would prefer to pass *Option and *Flag pointers to those
functions. But I guess it can be a little tricky to pass two different
types of pointers.
Indeed. I originally considered passing the struct pointers before
realising that approach wouldn't allow both flags and options to be
used interchangeably. At least, not without changing the structures to
carry some kind of type signature, although that wouldn't be
particularly difficult, given that both structures have to be created
via the appropriate functions.
This issue can be mitigated somewhat by using opt->name in the call
rather than a string literal[1]. We'd still need to use a literal for
flags, though.
[1] If we do that, modules which use "opt1", "opt2", etc for the
variable names should be changed to use meaningful names. Actually,
that should be done regardless.
--
Glynn Clements <glynn@gclements.plus.com>
I somehow agree with you that my interface can be cryptic and requires some interpretation because information is spread out. I think we can define something like:
// at most one option from a set
void G_option_exclusive(const char *name, …);
// at least one option from a set
void G_option_required(const char *name, …);
// if the first option is present, at least one of the other
// options must also be present
void G_option_requires(const char *name, …);
Here, actually, name is not needed, but is there only for stdarg. And we use these functions:
G_option_exclusive(“format”, &opt1, &opt2, &flag1, NULL); // No opt1, opt2, and flag1 at all OR only one of them.
G_option_required(“required”, &opt1, &opt2, &flag1, NULL); // At least one of opt1, opt2, and flag1 must be given.
G_option_requires(“group1”, &opt1, &opt2, NULL); // opt1 and opt2 must be used together.
In each function, we can use the state variable in parser.c (st->first_option and st->first_flag) to find defined options and flags:
opt = &st->first_option;
while(opt){
struct Option *x;
va_start(ap, name);
while((x = va_arg(ap, struct Option *))){
if (x == opt) {
…
break;
}
}
va_end(ap);
opt = opt->next_opt;
}
···
flag = &st->first_flag;
while(flag){
similar code here…
}
On Mon, Jun 9, 2014 at 5:34 AM, Glynn Clements <glynn@gclements.plus.com> wrote:
Huidae Cho wrote:
My implementation of the “exclusive” member, which you reverted, can handle
all these cases, I think. But since you reverted it, I think you didn’t
agree with my interface or implementation?Not really.
[I also wanted to be able to discuss this starting from a blank slate,
without the temptation to take any specific approach as a “reference
point”. But that alone wouldn’t have been sufficient for reversion.]Mostly, I find the interface to be cryptic. Information which should
(IMHO) be in one place is spread out across the individual options.In order to understand the relationships, anyone reading the code
needs to examine all of the options, and collate them into groups
based upon matching group numbers. IOW, mentally replicate the
(non-trivial) algorithms which form the bulk of r60713. The fact that
this was deemed to require several examples (with comments) tends to
suggest that it isn’t particularly intuitive.Apart from that, declaring “rules” would make the transition more
straightforward, as it mirrors the structure of the existing code
(i.e. the “if (expression) error();” statements).By itself, that wouldn’t be sufficient reason to prefer a rule-based
approach if it is considered to be inferior on its own merits. But
given the extent of the changes, it might be enough to tip the balance
in the event that the choice between different approaches amounted to
a toss-up.IMO, passing option/flag names to G_option_required() has its own
disadvantage because changing option/flag names takes more effort. If you
have valid reasoning behind adding functions rather than adding a member,
just like I did, I would prefer to pass *Option and *Flag pointers to those
functions. But I guess it can be a little tricky to pass two different
types of pointers.Indeed. I originally considered passing the struct pointers before
realising that approach wouldn’t allow both flags and options to be
used interchangeably. At least, not without changing the structures to
carry some kind of type signature, although that wouldn’t be
particularly difficult, given that both structures have to be created
via the appropriate functions.This issue can be mitigated somewhat by using opt->name in the call
rather than a string literal[1]. We’d still need to use a literal for
flags, though.[1] If we do that, modules which use “opt1”, “opt2”, etc for the
variable names should be changed to use meaningful names. Actually,
that should be done regardless.–
Glynn Clements <glynn@gclements.plus.com>
Maybe, we can use variadic macros in C99 to remove the first name argument.
#define G_option_exclusive(…) G__option_exclusive(NULL, VA_ARGS)
···
On Mon, Jun 9, 2014 at 10:54 AM, Huidae Cho <grass4u@gmail.com> wrote:
I somehow agree with you that my interface can be cryptic and requires some interpretation because information is spread out. I think we can define something like:
// at most one option from a set
void G_option_exclusive(const char *name, …);
// at least one option from a set
void G_option_required(const char *name, …);
// if the first option is present, at least one of the other
// options must also be presentvoid G_option_requires(const char *name, …);
Here, actually, name is not needed, but is there only for stdarg. And we use these functions:
G_option_exclusive(“format”, &opt1, &opt2, &flag1, NULL); // No opt1, opt2, and flag1 at all OR only one of them.
G_option_required(“required”, &opt1, &opt2, &flag1, NULL); // At least one of opt1, opt2, and flag1 must be given.
G_option_requires(“group1”, &opt1, &opt2, NULL); // opt1 and opt2 must be used together.In each function, we can use the state variable in parser.c (st->first_option and st->first_flag) to find defined options and flags:
opt = &st->first_option;
while(opt){
struct Option *x;
va_start(ap, name);
while((x = va_arg(ap, struct Option *))){
if (x == opt) {
…
break;
}
}
va_end(ap);
opt = opt->next_opt;
}
flag = &st->first_flag;
while(flag){
similar code here…
}
On Mon, Jun 9, 2014 at 5:34 AM, Glynn Clements <glynn@gclements.plus.com> wrote:
Huidae Cho wrote:
My implementation of the “exclusive” member, which you reverted, can handle
all these cases, I think. But since you reverted it, I think you didn’t
agree with my interface or implementation?Not really.
[I also wanted to be able to discuss this starting from a blank slate,
without the temptation to take any specific approach as a “reference
point”. But that alone wouldn’t have been sufficient for reversion.]Mostly, I find the interface to be cryptic. Information which should
(IMHO) be in one place is spread out across the individual options.In order to understand the relationships, anyone reading the code
needs to examine all of the options, and collate them into groups
based upon matching group numbers. IOW, mentally replicate the
(non-trivial) algorithms which form the bulk of r60713. The fact that
this was deemed to require several examples (with comments) tends to
suggest that it isn’t particularly intuitive.Apart from that, declaring “rules” would make the transition more
straightforward, as it mirrors the structure of the existing code
(i.e. the “if (expression) error();” statements).By itself, that wouldn’t be sufficient reason to prefer a rule-based
approach if it is considered to be inferior on its own merits. But
given the extent of the changes, it might be enough to tip the balance
in the event that the choice between different approaches amounted to
a toss-up.IMO, passing option/flag names to G_option_required() has its own
disadvantage because changing option/flag names takes more effort. If you
have valid reasoning behind adding functions rather than adding a member,
just like I did, I would prefer to pass *Option and *Flag pointers to those
functions. But I guess it can be a little tricky to pass two different
types of pointers.Indeed. I originally considered passing the struct pointers before
realising that approach wouldn’t allow both flags and options to be
used interchangeably. At least, not without changing the structures to
carry some kind of type signature, although that wouldn’t be
particularly difficult, given that both structures have to be created
via the appropriate functions.This issue can be mitigated somewhat by using opt->name in the call
rather than a string literal[1]. We’d still need to use a literal for
flags, though.[1] If we do that, modules which use “opt1”, “opt2”, etc for the
variable names should be changed to use meaningful names. Actually,
that should be done regardless.–
Glynn Clements <glynn@gclements.plus.com>
Interesting, actually this works better I think.
#define G_option_exclusive(…) G__option_exclusive(NULL, VA_ARGS, NULL)
G_option_exclusive(&opt1, &opt2, &flag1); // no need for the first name and last NULL arguments.
···
On Mon, Jun 9, 2014 at 11:13 AM, Huidae Cho <grass4u@gmail.com> wrote:
Maybe, we can use variadic macros in C99 to remove the first name argument.
#define G_option_exclusive(…) G__option_exclusive(NULL, VA_ARGS)
On Mon, Jun 9, 2014 at 10:54 AM, Huidae Cho <grass4u@gmail.com> wrote:
I somehow agree with you that my interface can be cryptic and requires some interpretation because information is spread out. I think we can define something like:
// at most one option from a set
void G_option_exclusive(const char *name, …);
// at least one option from a set
void G_option_required(const char *name, …);
// if the first option is present, at least one of the other
// options must also be presentvoid G_option_requires(const char *name, …);
Here, actually, name is not needed, but is there only for stdarg. And we use these functions:
G_option_exclusive(“format”, &opt1, &opt2, &flag1, NULL); // No opt1, opt2, and flag1 at all OR only one of them.
G_option_required(“required”, &opt1, &opt2, &flag1, NULL); // At least one of opt1, opt2, and flag1 must be given.
G_option_requires(“group1”, &opt1, &opt2, NULL); // opt1 and opt2 must be used together.In each function, we can use the state variable in parser.c (st->first_option and st->first_flag) to find defined options and flags:
opt = &st->first_option;
while(opt){
struct Option *x;
va_start(ap, name);
while((x = va_arg(ap, struct Option *))){
if (x == opt) {
…
break;
}
}
va_end(ap);
opt = opt->next_opt;
}
flag = &st->first_flag;
while(flag){
similar code here…
}
On Mon, Jun 9, 2014 at 5:34 AM, Glynn Clements <glynn@gclements.plus.com> wrote:
Huidae Cho wrote:
My implementation of the “exclusive” member, which you reverted, can handle
all these cases, I think. But since you reverted it, I think you didn’t
agree with my interface or implementation?Not really.
[I also wanted to be able to discuss this starting from a blank slate,
without the temptation to take any specific approach as a “reference
point”. But that alone wouldn’t have been sufficient for reversion.]Mostly, I find the interface to be cryptic. Information which should
(IMHO) be in one place is spread out across the individual options.In order to understand the relationships, anyone reading the code
needs to examine all of the options, and collate them into groups
based upon matching group numbers. IOW, mentally replicate the
(non-trivial) algorithms which form the bulk of r60713. The fact that
this was deemed to require several examples (with comments) tends to
suggest that it isn’t particularly intuitive.Apart from that, declaring “rules” would make the transition more
straightforward, as it mirrors the structure of the existing code
(i.e. the “if (expression) error();” statements).By itself, that wouldn’t be sufficient reason to prefer a rule-based
approach if it is considered to be inferior on its own merits. But
given the extent of the changes, it might be enough to tip the balance
in the event that the choice between different approaches amounted to
a toss-up.IMO, passing option/flag names to G_option_required() has its own
disadvantage because changing option/flag names takes more effort. If you
have valid reasoning behind adding functions rather than adding a member,
just like I did, I would prefer to pass *Option and *Flag pointers to those
functions. But I guess it can be a little tricky to pass two different
types of pointers.Indeed. I originally considered passing the struct pointers before
realising that approach wouldn’t allow both flags and options to be
used interchangeably. At least, not without changing the structures to
carry some kind of type signature, although that wouldn’t be
particularly difficult, given that both structures have to be created
via the appropriate functions.This issue can be mitigated somewhat by using opt->name in the call
rather than a string literal[1]. We’d still need to use a literal for
flags, though.[1] If we do that, modules which use “opt1”, “opt2”, etc for the
variable names should be changed to use meaningful names. Actually,
that should be done regardless.–
Glynn Clements <glynn@gclements.plus.com>
On Mon, Jun 9, 2014 at 11:16 AM, Huidae Cho <grass4u@gmail.com> wrote:
Interesting, actually this works better I think.
#define G_option_exclusive(...) G__option_exclusive(NULL, __VA_ARGS__,
NULL)G_option_exclusive(&opt1, &opt2, &flag1); // no need for the first name
and last NULL arguments.On Mon, Jun 9, 2014 at 11:13 AM, Huidae Cho <grass4u@gmail.com> wrote:
Maybe, we can use variadic macros in C99 to remove the first name
argument.#define G_option_exclusive(...) G__option_exclusive(NULL, __VA_ARGS__)
Note that C99 is not what GRASS currently requires. Details:
[GRASS-dev] C and C++ standards used by GRASS
http://lists.osgeo.org/pipermail/grass-dev/2014-March/068017.html
C code is supposed to conform to C89 and POSIX.
(C89 with POSIX extensions)
On Mon, Jun 9, 2014 at 10:54 AM, Huidae Cho <grass4u@gmail.com> wrote:
I somehow agree with you that my interface can be cryptic and requires
some interpretation because information is spread out. I think we can
define something like:// at most one option from a set
void G_option_exclusive(const char *name, ...);// at least one option from a set
void G_option_required(const char *name, ...);// if the first option is present, at least one of the other
// options must also be present
void G_option_requires(const char *name, ...);Here, actually, name is not needed, but is there only for stdarg. And we
use these functions:G_option_exclusive("format", &opt1, &opt2, &flag1, NULL); // No opt1,
opt2, and flag1 at all OR only one of them.
G_option_required("required", &opt1, &opt2, &flag1, NULL); // At least
one of opt1, opt2, and flag1 must be given.
G_option_requires("group1", &opt1, &opt2, NULL); // opt1 and opt2 must
be used together.In each function, we can use the state variable in parser.c
(st->first_option and st->first_flag) to find defined options and flags:opt = &st->first_option;
while(opt){
struct Option *x;
va_start(ap, name);
while((x = va_arg(ap, struct Option *))){
if (x == opt) {
...
break;
}
}
va_end(ap);
opt = opt->next_opt;
}flag = &st->first_flag;
while(flag){
similar code here...
}On Mon, Jun 9, 2014 at 5:34 AM, Glynn Clements <glynn@gclements.plus.com
> wrote:Huidae Cho wrote:
> My implementation of the "exclusive" member, which you reverted, can
handle
> all these cases, I think. But since you reverted it, I think you
didn't
> agree with my interface or implementation?Not really.
[I also wanted to be able to discuss this starting from a blank slate,
without the temptation to take any specific approach as a "reference
point". But that alone wouldn't have been sufficient for reversion.]Mostly, I find the interface to be cryptic. Information which should
(IMHO) be in one place is spread out across the individual options.In order to understand the relationships, anyone reading the code
needs to examine all of the options, and collate them into groups
based upon matching group numbers. IOW, mentally replicate the
(non-trivial) algorithms which form the bulk of r60713. The fact that
this was deemed to require several examples (with comments) tends to
suggest that it isn't particularly intuitive.Apart from that, declaring "rules" would make the transition more
straightforward, as it mirrors the structure of the existing code
(i.e. the "if (expression) error();" statements).By itself, that wouldn't be sufficient reason to prefer a rule-based
approach if it is considered to be inferior on its own merits. But
given the extent of the changes, it might be enough to tip the balance
in the event that the choice between different approaches amounted to
a toss-up.> IMO, passing option/flag names to G_option_required() has its own
> disadvantage because changing option/flag names takes more effort. If
you
> have valid reasoning behind adding functions rather than adding a
member,
> just like I did, I would prefer to pass *Option and *Flag pointers to
those
> functions. But I guess it can be a little tricky to pass two different
> types of pointers.Indeed. I originally considered passing the struct pointers before
realising that approach wouldn't allow both flags and options to be
used interchangeably. At least, not without changing the structures to
carry some kind of type signature, although that wouldn't be
particularly difficult, given that both structures have to be created
via the appropriate functions.This issue can be mitigated somewhat by using opt->name in the call
rather than a string literal[1]. We'd still need to use a literal for
flags, though.[1] If we do that, modules which use "opt1", "opt2", etc for the
variable names should be changed to use meaningful names. Actually,
that should be done regardless.--
Glynn Clements <glynn@gclements.plus.com>_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev
Huidae Cho wrote:
Maybe, we can use variadic macros in C99 to remove the first name argument.
GRASS doesn't require C99. In general, we try to keep the hard
dependencies to a bare minimium, particularly in core functionality
such as libgis.
In any case, the <stdarg.h> requirement for an explicit first argument
isn't a significant problem. The functions would never be called with
less than two arguments.
In the worst case, it complicates the implementation slightly, as we
can't simply iterate over all of the arguments using va_arg(), but
have to treat the explicit argument separately. That can be handled
like:
void G_option_exclusive(void *first, ...)
{
void *opt = first;
va_start(ap, first);
for (;
{
process(opt); // search for opt in options/flags
opt = va_arg(ap, void*);
if (!opt) // NULL terminator
break;
}
va_end(ap);
}
--
Glynn Clements <glynn@gclements.plus.com>
Right, G_option_exclusive(void *first, …) should work.
···
On Wed, Jun 11, 2014 at 6:43 AM, Glynn Clements <glynn@gclements.plus.com> wrote:
Huidae Cho wrote:
Maybe, we can use variadic macros in C99 to remove the first name argument.
GRASS doesn’t require C99. In general, we try to keep the hard
dependencies to a bare minimium, particularly in core functionality
such as libgis.In any case, the <stdarg.h> requirement for an explicit first argument
isn’t a significant problem. The functions would never be called with
less than two arguments.In the worst case, it complicates the implementation slightly, as we
can’t simply iterate over all of the arguments using va_arg(), but
have to treat the explicit argument separately. That can be handled
like:void G_option_exclusive(void *first, …)
{
void opt = first;
va_start(ap, first);
for (;
process(opt); // search for opt in options/flags
opt = va_arg(ap, void);
if (!opt) // NULL terminator
break;
}
va_end(ap);
}–
Glynn Clements <glynn@gclements.plus.com>
I think we need two more functions:
// at most one option from a set
void G_option_exclusive(void *first, …);
// at least one option from a set
void G_option_required(void *first, …);
// if the first option is present, at least one of the other
// options must also be present
void G_option_requires(void *first, …);
// if the first option is present, all the other options must also be present.
// same as multiple G_option_requires(first, opt)…
void G_option_requires_all(void *first, …);
// G_option_requires_all() doesn’t make the second/third… option require all the other options.
// if any option is present, all the other options must also be present.
// all or nothing from a set
void G_option_inclusive(void *first, …);
···
On Wed, Jun 11, 2014 at 10:12 AM, Huidae Cho <grass4u@gmail.com> wrote:
Right, G_option_exclusive(void *first, …) should work.
On Wed, Jun 11, 2014 at 6:43 AM, Glynn Clements <glynn@gclements.plus.com> wrote:
Huidae Cho wrote:
Maybe, we can use variadic macros in C99 to remove the first name argument.
GRASS doesn’t require C99. In general, we try to keep the hard
dependencies to a bare minimium, particularly in core functionality
such as libgis.In any case, the <stdarg.h> requirement for an explicit first argument
isn’t a significant problem. The functions would never be called with
less than two arguments.In the worst case, it complicates the implementation slightly, as we
can’t simply iterate over all of the arguments using va_arg(), but
have to treat the explicit argument separately. That can be handled
like:void G_option_exclusive(void *first, …)
{
void opt = first;
va_start(ap, first);
for (;
process(opt); // search for opt in options/flags
opt = va_arg(ap, void);
if (!opt) // NULL terminator
break;
}
va_end(ap);
}–
Glynn Clements <glynn@gclements.plus.com>
Huidae Cho wrote:
I think we need two more functions:
// if the first option is present, all the other options must also be present.
// same as multiple G_option_requires(first, opt)...
void G_option_requires_all(void *first, ...);
As you note, this can be achieved with multiple G_option_requires()
calls.
// if any option is present, all the other options must also be present.
// all or nothing from a set
void G_option_inclusive(void *first, ...);
This could be achieved with multiple G_option_requires_all() calls. Or
even multiple G_option_requires() calls, but the number of such calls
would grow quadratically.
Apart from minimising the number of calls, there's probably some value
in terms of "documentation" to having specific rules for these cases.
Ultimately, there's no limit to the number of functions which *might*
be required. We should aim to capture all of the common cases first,
then figure out how to deal with anything which remains.
I'll make a start on this.
--
Glynn Clements <glynn@gclements.plus.com>
Glynn Clements wrote:
I'll make a start on this.
A first draft of the code has been added in r60871. Not tested yet.
--
Glynn Clements <glynn@gclements.plus.com>
Looks good to me. I’ll try it with g.mlist/g.mremove later.
···
On Thu, Jun 19, 2014 at 10:58 AM, Glynn Clements <glynn@gclements.plus.com> wrote:
Glynn Clements wrote:
I’ll make a start on this.
A first draft of the code has been added in r60871. Not tested yet.
–
Glynn Clements <glynn@gclements.plus.com>
I assume G__check_option_rules() is supposed to be called by G_parser(). Then, instead of calling G_fatal_error, it should append errors to st->errors. If so… for g.mlist we can define two different versions of rules:
This version prints more correct errors because only present options/flags will be displayed in errors, but too much typing.
G_option_exclusive(flag.regex, flag.extended, NULL);
G_option_exclusive(flag.pretty, flag.full, NULL);
G_option_exclusive(flag.pretty, opt.output, NULL);
G_option_exclusive(flag.pretty, flag.mapset, NULL);
G_option_exclusive(flag.pretty, flag.type, NULL);
G_option_exclusive(flag.full, opt.output, NULL);
G_option_exclusive(flag.full, flag.mapset, NULL);
G_option_exclusive(flag.full, flag.type, NULL);
This version is shorter, but -p -f will print three errors including options/flags not present.
G_option_exclusive(flag.regex, flag.extended, NULL);
G_option_exclusive(flag.pretty, flag.full, opt.output, NULL);
G_option_exclusive(flag.pretty, flag.full, flag.mapset, NULL);
G_option_exclusive(flag.pretty, flag.full, flag.type, NULL);
Can we implement something like G_option_exclusive(pretty, full, G_option_or(output, mapset, type))?
pretty, full, and any of output, mapset, and type are mutually exclusive, but output, mapset, and type are not exclusive.
···
On Thu, Jun 19, 2014 at 1:42 PM, Huidae Cho <grass4u@gmail.com> wrote:
Looks good to me. I’ll try it with g.mlist/g.mremove later.
On Thu, Jun 19, 2014 at 10:58 AM, Glynn Clements <glynn@gclements.plus.com> wrote:
Glynn Clements wrote:
I’ll make a start on this.
A first draft of the code has been added in r60871. Not tested yet.
–
Glynn Clements <glynn@gclements.plus.com>
Huidae Cho wrote:
I assume G__check_option_rules() is supposed to be called by G_parser().
Then, instead of calling G_fatal_error, it should append errors to
st->errors.
Okay; I presume that the intent is so that it will report all errors,
not just the first.
If so... for g.mlist we can define two different versions of
rules:This version prints more correct errors because only present options/flags
will be displayed in errors, but too much typing.
G_option_exclusive(flag.regex, flag.extended, NULL);
G_option_exclusive(flag.pretty, flag.full, NULL);
G_option_exclusive(flag.pretty, opt.output, NULL);
G_option_exclusive(flag.pretty, flag.mapset, NULL);
G_option_exclusive(flag.pretty, flag.type, NULL);
G_option_exclusive(flag.full, opt.output, NULL);
G_option_exclusive(flag.full, flag.mapset, NULL);
G_option_exclusive(flag.full, flag.type, NULL);This version is shorter, but -p -f will print three errors including
options/flags not present.
G_option_exclusive(flag.regex, flag.extended, NULL);
G_option_exclusive(flag.pretty, flag.full, opt.output, NULL);
G_option_exclusive(flag.pretty, flag.full, flag.mapset, NULL);
G_option_exclusive(flag.pretty, flag.full, flag.type, NULL);Can we implement something like
G_option_exclusive(pretty, full, G_option_or(output, mapset, type))
?
I'd rather stick to "flat" rules rather than heading in the direction
of generalised boolean expressions.
Not because it's hard to implement, but because it makes it harder to
utilise the information.
The next logical step is to include the rules in the
--interface-description output so that the GUI can convey the
relationships to the user. E.g. mutually-exclusive options might use
radio buttons, or greying-out excluded options. Options which are
required by another option could be marked as "required" when a value
is given for the first option. And so on.
Realistically, that requires that the rules belong to a finite set of
patterns.
pretty, full, and any of output, mapset, and type are mutually exclusive,
but output, mapset, and type are not exclusive.
How about another rule type:
G_option_excludes(flag.pretty, opt.output, flag.mapset, flag.type, NULL);
G_option_excludes(flag.full, opt.output, flag.mapset, flag.type, NULL);
where the first option excludes all remaining options.
This is essentially the negation of G_option_requires().
--
Glynn Clements <glynn@gclements.plus.com>
G_option_excludes() works for me.
···
On Fri, Jun 20, 2014 at 7:58 PM, Glynn Clements <glynn@gclements.plus.com> wrote:
Huidae Cho wrote:
I assume G__check_option_rules() is supposed to be called by G_parser().
Then, instead of calling G_fatal_error, it should append errors to
st->errors.Okay; I presume that the intent is so that it will report all errors,
not just the first.If so… for g.mlist we can define two different versions of
rules:This version prints more correct errors because only present options/flags
will be displayed in errors, but too much typing.
G_option_exclusive(flag.regex, flag.extended, NULL);
G_option_exclusive(flag.pretty, flag.full, NULL);
G_option_exclusive(flag.pretty, opt.output, NULL);
G_option_exclusive(flag.pretty, flag.mapset, NULL);
G_option_exclusive(flag.pretty, flag.type, NULL);
G_option_exclusive(flag.full, opt.output, NULL);
G_option_exclusive(flag.full, flag.mapset, NULL);
G_option_exclusive(flag.full, flag.type, NULL);This version is shorter, but -p -f will print three errors including
options/flags not present.
G_option_exclusive(flag.regex, flag.extended, NULL);
G_option_exclusive(flag.pretty, flag.full, opt.output, NULL);
G_option_exclusive(flag.pretty, flag.full, flag.mapset, NULL);
G_option_exclusive(flag.pretty, flag.full, flag.type, NULL);Can we implement something like
G_option_exclusive(pretty, full, G_option_or(output, mapset, type))
?I’d rather stick to “flat” rules rather than heading in the direction
of generalised boolean expressions.Not because it’s hard to implement, but because it makes it harder to
utilise the information.The next logical step is to include the rules in the
–interface-description output so that the GUI can convey the
relationships to the user. E.g. mutually-exclusive options might use
radio buttons, or greying-out excluded options. Options which are
required by another option could be marked as “required” when a value
is given for the first option. And so on.Realistically, that requires that the rules belong to a finite set of
patterns.pretty, full, and any of output, mapset, and type are mutually exclusive,
but output, mapset, and type are not exclusive.How about another rule type:
G_option_excludes(flag.pretty, opt.output, flag.mapset, flag.type, NULL);
G_option_excludes(flag.full, opt.output, flag.mapset, flag.type, NULL);where the first option excludes all remaining options.
This is essentially the negation of G_option_requires().
–
Glynn Clements <glynn@gclements.plus.com>
We also need to add option_rules or something to g.parser so that python scripts can have access to these functions. Something like:
#%option_rules
#% exclusive: -a, -b
#% requires_all: opt1, opt2, -a
#%end
···
On Fri, Jun 20, 2014 at 9:59 PM, Huidae Cho <grass4u@gmail.com> wrote:
G_option_excludes() works for me.
On Fri, Jun 20, 2014 at 7:58 PM, Glynn Clements <glynn@gclements.plus.com> wrote:
Huidae Cho wrote:
I assume G__check_option_rules() is supposed to be called by G_parser().
Then, instead of calling G_fatal_error, it should append errors to
st->errors.Okay; I presume that the intent is so that it will report all errors,
not just the first.If so… for g.mlist we can define two different versions of
rules:This version prints more correct errors because only present options/flags
will be displayed in errors, but too much typing.
G_option_exclusive(flag.regex, flag.extended, NULL);
G_option_exclusive(flag.pretty, flag.full, NULL);
G_option_exclusive(flag.pretty, opt.output, NULL);
G_option_exclusive(flag.pretty, flag.mapset, NULL);
G_option_exclusive(flag.pretty, flag.type, NULL);
G_option_exclusive(flag.full, opt.output, NULL);
G_option_exclusive(flag.full, flag.mapset, NULL);
G_option_exclusive(flag.full, flag.type, NULL);This version is shorter, but -p -f will print three errors including
options/flags not present.
G_option_exclusive(flag.regex, flag.extended, NULL);
G_option_exclusive(flag.pretty, flag.full, opt.output, NULL);
G_option_exclusive(flag.pretty, flag.full, flag.mapset, NULL);
G_option_exclusive(flag.pretty, flag.full, flag.type, NULL);Can we implement something like
G_option_exclusive(pretty, full, G_option_or(output, mapset, type))
?I’d rather stick to “flat” rules rather than heading in the direction
of generalised boolean expressions.Not because it’s hard to implement, but because it makes it harder to
utilise the information.The next logical step is to include the rules in the
–interface-description output so that the GUI can convey the
relationships to the user. E.g. mutually-exclusive options might use
radio buttons, or greying-out excluded options. Options which are
required by another option could be marked as “required” when a value
is given for the first option. And so on.Realistically, that requires that the rules belong to a finite set of
patterns.pretty, full, and any of output, mapset, and type are mutually exclusive,
but output, mapset, and type are not exclusive.How about another rule type:
G_option_excludes(flag.pretty, opt.output, flag.mapset, flag.type, NULL);
G_option_excludes(flag.full, opt.output, flag.mapset, flag.type, NULL);where the first option excludes all remaining options.
This is essentially the negation of G_option_requires().
–
Glynn Clements <glynn@gclements.plus.com>
On Wed, Jun 25, 2014 at 2:09 AM, Huidae Cho <grass4u@gmail.com> wrote:
We also need to add option_rules or something to g.parser so that python
scripts can have access to these functions. Something like:#%option_rules
#% exclusive: -a, -b
#% requires_all: opt1, opt2, -a
#%endOn Fri, Jun 20, 2014 at 9:59 PM, Huidae Cho <grass4u@gmail.com> wrote:
G_option_excludes() works for me.
On Fri, Jun 20, 2014 at 7:58 PM, Glynn Clements <glynn@gclements.plus.com
> wrote:Huidae Cho wrote:
> I assume G__check_option_rules() is supposed to be called by
G_parser().
> Then, instead of calling G_fatal_error, it should append errors to
> st->errors.Okay; I presume that the intent is so that it will report all errors,
not just the first.> If so... for g.mlist we can define two different versions of
> rules:
>
> This version prints more correct errors because only present
options/flags
> will be displayed in errors, but too much typing.
> G_option_exclusive(flag.regex, flag.extended, NULL);
> G_option_exclusive(flag.pretty, flag.full, NULL);
> G_option_exclusive(flag.pretty, opt.output, NULL);
> G_option_exclusive(flag.pretty, flag.mapset, NULL);
> G_option_exclusive(flag.pretty, flag.type, NULL);
> G_option_exclusive(flag.full, opt.output, NULL);
> G_option_exclusive(flag.full, flag.mapset, NULL);
> G_option_exclusive(flag.full, flag.type, NULL);
>
> This version is shorter, but -p -f will print three errors including
> options/flags not present.
> G_option_exclusive(flag.regex, flag.extended, NULL);
> G_option_exclusive(flag.pretty, flag.full, opt.output, NULL);
> G_option_exclusive(flag.pretty, flag.full, flag.mapset, NULL);
> G_option_exclusive(flag.pretty, flag.full, flag.type, NULL);
>
> Can we implement something like
> G_option_exclusive(pretty, full, G_option_or(output, mapset, type))
> ?I'd rather stick to "flat" rules rather than heading in the direction
of generalised boolean expressions.Not because it's hard to implement, but because it makes it harder to
utilise the information.The next logical step is to include the rules in the
--interface-description output so that the GUI can convey the
relationships to the user. E.g. mutually-exclusive options might use
radio buttons, or greying-out excluded options. Options which are
required by another option could be marked as "required" when a value
is given for the first option. And so on.Realistically, that requires that the rules belong to a finite set of
patterns.> pretty, full, and any of output, mapset, and type are mutually
exclusive,
> but output, mapset, and type are not exclusive.How about another rule type:
> G_option_excludes(flag.pretty, opt.output, flag.mapset, flag.type,
NULL);
> G_option_excludes(flag.full, opt.output, flag.mapset, flag.type,
NULL);where the first option excludes all remaining options.
This is essentially the negation of G_option_requires().
--
Glynn Clements <glynn@gclements.plus.com>_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev
Hi,
is this new API now stable enough to be actually used in C modules? Should
we start to replace the manual checks in modules with these functions? BTW,
documentation is still missing.
I attached a patch which should fix this ticket [1]: when user doesn't type
any command arguments and the module has no explicitly required options
(like g.region), it checks if there is any RULE_REQUIRED and if yes, it
opens GUI dialog.
Also I would consider a little bit better error message, the name of the
option is not very visible in the sentence. Perhaps using quotes or <>
around the option would help. Also, "Option myoption1 requires at least one
of myoption2" sounds weird, perhaps there should be special handling of
cases when there is just one option required. These are just details.
Thanks,
Anna
[1]http://trac.osgeo.org/grass/ticket/1778
(attachments)
parser.diff (1.27 KB)