[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: CommandLine parser: optional args and callback handlers



On 23 January 2012 23:29, David Kalnischkies
<kalnischkies+debian@gmail.com> wrote:
> On Sun, Jan 22, 2012 at 08:31, Daniel Hartwig <mandyke@gmail.com> wrote:
>> * Optional args
>>
>> Optional args can only be supplied with the "=arg" syntax (otherwise
>> it is ambiguous whether what follows is the argument or another
>> option).  This is already tracked within 'HandleOpt' by the variable
>> 'CertainArg', so the additional required is minimal.
>>
>> The default value to assign when no argument is present could be specified by:
>>  a) a well-known value, e.g. "1" (matches boolean options);
>>  b) a class-wide map (LongOpt -> Default);
>>  c) a callback handler (see next section);
>>  d) a new string member of struct Args; or
>>  e) extending Args::LongOpt to permit an optional syntax of "longopt=default".
>
> mhh. Currently the default values of an option are defined at the place they
> are used by e.g. Find("Config::Option", true) == false, so it feels a bit
> strange to move some default values for optional args now into the Args
> struct, while all others are determined at runtime. In that case we should
> allow to set default values for other options as well, or?

I was definitely looking at this from the CommandLine point-of-view,
which may have limited my thinking :-)

To elaborate though, there are two "default" values I am trying to distinguish:

$ apt-foo
Find("someopt", "disabled") => "disabled"

$ apt-foo --with-someopt
Find("someopt", "disabled") => "enabled"

$ apt-foo --with-someopt=foo
Find("someopt", "disabled") => "foo"

(but I think you gathered that anyway)

The first type of default is program-wide, and sometimes also done this way:

  Cnf.CndSet("Dir::State::lists","lists/");
  Cnf.CndSet("Dir::State::cdroms","cdroms.list");

The second type of default is specific to how the command-line option
being used, and that is why I thought to add it's definition to struct
CommandLine::Args [1].

>
> For me f) would be the addition of a Find("config::option", default, unset)
> method. The commandline parser could set the option to an empty value
> if he encounters an option with an optional arg which wasn't given on the
> commandline. An option can't be set to be 'empty' on the commandline,
> so this Find from above could work with the simple ruleset:
> If config::option.empty() == false
>   return value
> else if config::option.empty() == true
>   return default
> else // if exists(config::option) == false
>   return unset;
>
> That's the same behavior as before if we
> set unset = default by default ([no] pun intended).
>
> This gives empty a bit of a special meaning, but in the end it closely
> resembles what is given on the commandline…

So if I understand you correctly:

$ apt-foo
Find("someopt", "default", "unset") => "unset"

$ apt-foo --with-someopt
Find("someopt", "default", "unset") => "default"

$ apt-foo --with-someopt=foo
Find("someopt", "default", "unset") => "foo"


This would clearly cover my use case.

Note: Clear("someopt") would need to actually remove "someopt", not
just set it to "", but this is another minor change.  The program can
still use Set("someopt", "") for the old behaviour.


I think your method is better, since it not only covers what I was
trying to do but also enhances Configuration generally by making
"unset" distinct from "empty".

If noone beats me to it, I will whip up a patch for this shortly.


>
> ...
> More than two years of APT development and i still learn new things…
> A quick bzr blame told me that it was added in revision 212 (1999!)
> but doesn't indicate any user for it and I at least couldn't find a user now.
>

"Hey!  Wouldn't it be cool if ..."


>
>> * Callback handlers
>>
...
>
>> It may be just as much effort to convert CommandLine to a wrapper
>> around argp or similar, which already has support for callback
>> handlers and some other features that would be of use...
>
> argp is interesting, indeed, but i think for APT we should avoid it
> as it is glibc specific and APT lowlevel enough to be ported to a lot of
> plattforms with different (less fearureful) libc's so we should keep it
> relatively easy if possible.

Makes sense.


>
> How about a new Callback member for the Args struct which is called in
> case the option is parsed and the callback isn't NULL similar to
> DispatchArg. If we talk about a boolean for example it would just have the
> parameters handle_config_option("config::option", void* -> bool)

'Args *A' instead of "config::option"?  It provides some more info
which may be useful to the handler for the same number of parameters.

I gather that you also mean to pass any value of 'Argument' by setting
it in "config::option" before calling handle_config_option?


What do you mean by "void* -> bool" parameter?  This looks like a
function type (bool F(void*) ?) but I don't see why you would pass one
to the handler ...?


> That's very few state, but should be easy to implement and enough for chains
> and alike and in the end i can't imagine too many uses for a handler who
> consumes multiple arguments and would therefore require a global handler,
> but even if we would need it we could still add that, too.
> A global handler just feels like overkill for chaining two (or more)
> options together…
>

Agreed.  Less state is good and does cover most uses.

And on second thought, it is insane to pass "int &I", "char *&Opt"
etc. to the handler and expect it to properly parse and consume
arguments by itself...  what! was I thinking?


Reply to: