<div dir="ltr"><div>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:</div><div><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px"> // at most one option from a set</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px"> void G_option_exclusive(const char *name, ...);</span><br style="font-family:arial,sans-serif;font-size:13px">
<br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px"> // at least one option from a set</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px"> void G_option_required(const char *name, ...);</span><br style="font-family:arial,sans-serif;font-size:13px">
<br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px"> // if the first option is present, at least one of the other</span><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px"> // options must also be present</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px"> void G_option_requires(const char *name, ...);</span><br>
</div><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">Here, actually, name is not needed, but is there only for stdarg. And we use these functions:</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">G_option_exclusive("format", &opt1, &opt2, &flag1, NULL); // No opt1, opt2, and flag1 at all OR only one of them.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px">G_option_required("required", &opt1, &opt2, &flag1, NULL); // At least one of opt1, opt2, and flag1 must be given.</span></div><div><span style="font-family:arial,sans-serif;font-size:13px">G_option_requires("group1", &opt1, &opt2, NULL); // opt1 and opt2 must be used together.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">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:</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">opt = &st->first_option;</span><br></div><div><font face="arial, sans-serif">while(opt){</font></div>
<div><font face="arial, sans-serif"> struct Option *x; </font></div><div><span style="font-family:arial,sans-serif;font-size:13px"> va_start(ap, name);</span></div><div><span style="font-family:arial,sans-serif;font-size:13px"> while((x = va_arg(ap, struct Option *))){</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"> if (x == opt) {</span></div><div><span style="font-family:arial,sans-serif;font-size:13px"> ...</span></div><div><span style="font-family:arial,sans-serif;font-size:13px"> break;</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"> }</span></div><div><span style="font-family:arial,sans-serif;font-size:13px"> }</span></div><div><span style="font-family:arial,sans-serif;font-size:13px"> va_end(ap);</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"> opt = opt->next_opt;</span></div><div><span style="font-family:arial,sans-serif;font-size:13px">}</span></div><div class="gmail_extra"><br></div><div class="gmail_extra">
<div><span style="font-family:arial,sans-serif;font-size:13px">flag = &st->first_flag;</span><br></div><div><font face="arial, sans-serif">while(flag){</font></div><div><span style="font-family:arial,sans-serif;font-size:13px"> similar code here...</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px">}</span><br></div><br><div class="gmail_quote">On Mon, Jun 9, 2014 at 5:34 AM, Glynn Clements <span dir="ltr"><<a href="mailto:glynn@gclements.plus.com" target="_blank">glynn@gclements.plus.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><br>
Huidae Cho wrote:<br>
<br>
> My implementation of the "exclusive" member, which you reverted, can handle<br>
> all these cases, I think. But since you reverted it, I think you didn't<br>
> agree with my interface or implementation?<br>
<br>
</div>Not really.<br>
<br>
[I also wanted to be able to discuss this starting from a blank slate,<br>
without the temptation to take any specific approach as a "reference<br>
point". But that alone wouldn't have been sufficient for reversion.]<br>
<br>
Mostly, I find the interface to be cryptic. Information which should<br>
(IMHO) be in one place is spread out across the individual options.<br>
<br>
In order to understand the relationships, anyone reading the code<br>
needs to examine all of the options, and collate them into groups<br>
based upon matching group numbers. IOW, mentally replicate the<br>
(non-trivial) algorithms which form the bulk of r60713. The fact that<br>
this was deemed to require several examples (with comments) tends to<br>
suggest that it isn't particularly intuitive.<br>
<br>
Apart from that, declaring "rules" would make the transition more<br>
straightforward, as it mirrors the structure of the existing code<br>
(i.e. the "if (expression) error();" statements).<br>
<br>
By itself, that wouldn't be sufficient reason to prefer a rule-based<br>
approach if it is considered to be inferior on its own merits. But<br>
given the extent of the changes, it might be enough to tip the balance<br>
in the event that the choice between different approaches amounted to<br>
a toss-up.<br>
<div class=""><br>
> IMO, passing option/flag names to G_option_required() has its own<br>
> disadvantage because changing option/flag names takes more effort. If you<br>
> have valid reasoning behind adding functions rather than adding a member,<br>
> just like I did, I would prefer to pass *Option and *Flag pointers to those<br>
> functions. But I guess it can be a little tricky to pass two different<br>
> types of pointers.<br>
<br>
</div>Indeed. I originally considered passing the struct pointers before<br>
realising that approach wouldn't allow both flags and options to be<br>
used interchangeably. At least, not without changing the structures to<br>
carry some kind of type signature, although that wouldn't be<br>
particularly difficult, given that both structures have to be created<br>
via the appropriate functions.<br>
<br>
This issue can be mitigated somewhat by using opt->name in the call<br>
rather than a string literal[1]. We'd still need to use a literal for<br>
flags, though.<br>
<br>
[1] If we do that, modules which use "opt1", "opt2", etc for the<br>
variable names should be changed to use meaningful names. Actually,<br>
that should be done regardless.<br>
<span class=""><font color="#888888"><br>
--<br>
Glynn Clements <<a href="mailto:glynn@gclements.plus.com">glynn@gclements.plus.com</a>><br>
</font></span></blockquote></div><br></div></div>