CommandLine parser: optional args and callback handlers
Hello
* Background
Currently aptitude implements it's own command line option parser and
dispatch mechanism. There are some issues with this code and given
that the process is basically identical to the APT tools' it seems the
best approach is to replace the whole deal with the CommandLine parser
class from apt-pkg/contrib.
I have been successful at implementing this [1] and very much like the
results :-)
There are two issues I came up against:
- one option has an optional argument; and
- some options are not simple 1:1 mappings to a configuration item.
So I'd like to work on supporting these in CommandLine and seek some feed back.
[1] http://anonscm.debian.org/gitweb/?p=aptitude/aptitude.git;a=shortlog;h=refs/heads/wip-cmdline
* 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".
All should be compatible with existing code.
Any value chosen for a) is potentially ambiguous with user supplied data.
b) and c) would separate the default value from the rest of the option
definition.
The most flexible is c) which can consider the value of previously
parsed options though this is not needed for default values which most
(all?) programs specify as a constant value.
d) and e) have the benefit of keeping option definition in a single place.
Attached are two patches demonstrating d) and e).
Side note: does anyone know of any actual uses for the 'prefix'
semantics of Args::ConfName? (ConfName = "APT::Item PREFIX" would
prepend "PREFIX" to every value assigned via that option to
"APT::Item").
* Callback handlers
These are for non-trival options, for example, aptitude has
"--without-recommends" which is equivalent to:
Apt::Install-Recommends "false";
Apt::AutoRemove::RecommendsImportant "true";
and "--log-resolver", equivalent to:
Aptitude::Logging::Levels:: "aptitude.resolver.search:trace";
Aptitude::Logging::Levels:: "aptitude.resolver.search.costs:info";
Currently I have a work-around for this:
1) option parsing sets a temp. config item
2) post-parsing checks for the temp. items and does the real config
This works except for edge cases, where a subsequent option sets
something which would be set in step 2:
$ aptitude versions -F "bad" \
-o Aptitude::CmdLine::Versions-Display-Format="%p" \
~nemacs23$
Package emacs23:
bad
where the expected output is:
Package emacs23:
23.3+1-4
However, due to bug #587671 (which would be fixed by [1]), most such
command-lines are currently broken anyway--so it doesn't really
matter.
I am thinking that an optional class-wide callback handler could be
introduced, which would be invoked on particular arguments after
'CommandLine::HandleOpt' has determined presence of any argument. The
handler should be passed as much state as possible (similar to "struct
argp_state"). This permits the handler to consume as many arguments
as it likes and even ask for the current option to be parsed again.
A possible signature:
bool HANDLER (int &I, int argc, const char *argv[],
const char *&Opt, Args *A, bool PreceedMatch,
const char *Argument, bool CertainArg,
int &IncI)
which includes all state used by 'HandleOpt'.
A very flexible option, however, some non-trivial amount of work to implement.
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...
These options could also be handled by 'chaining' several options together:
{'R', "without-recommends", "Apt::Install-Recommends", InvBoolean | Chain},
{'R', "without-recommends", "Apt::AutoRemove::RecommendsImportant", Boolean},
Then again... given that only three of aptitude's options are
involved--one a compatibility hack and the other only recently
becoming relevant--it may be simpler to just redefine those options
and forget the whole thing ;-)
Any ideas?
=== modified file 'apt-pkg/contrib/cmndline.cc'
--- apt-pkg/contrib/cmndline.cc 2011-09-21 16:42:08 +0000
+++ apt-pkg/contrib/cmndline.cc 2012-01-18 06:45:45 +0000
@@ -180,10 +180,17 @@
// Option is an argument set
if ((A->Flags & HasArg) == HasArg)
{
+ if ((A->Flags & ArgOptional) == ArgOptional &&
+ CertainArg == false)
+ Argument = A->DefaultVal;
if (Argument == 0)
return _error->Error(_("Option %s requires an argument."),argv[I]);
- Opt += strlen(Opt);
- I += IncI;
+ if ((A->Flags & ArgOptional) != ArgOptional ||
+ CertainArg == true)
+ {
+ Opt += strlen(Opt);
+ I += IncI;
+ }
// Parse a configuration file
if ((A->Flags & ConfigFile) == ConfigFile)
=== modified file 'apt-pkg/contrib/cmndline.h'
--- apt-pkg/contrib/cmndline.h 2010-03-12 18:41:30 +0000
+++ apt-pkg/contrib/cmndline.h 2012-01-18 06:32:38 +0000
@@ -37,6 +37,14 @@
ArbItem - Means the item is an arbitrary configuration string of
the form item=value, where item is passed directly
to the configuration class.
+ ArgOptional - Means the argument value is optional and, if not supplied,
+ will take a default value. Such optional values must be
+ supplied unambiguously:
+ -d=val (ok) -dval (not ok) -d val (not ok)
+ --long=val (ok) --long val (not ok)
+ In the ambiguous cases the option will take the value of
+ DefaultVal and 'val' will be treated as just another
+ option. Implies HasArg.
The default, if the flags are 0 is to use Boolean
##################################################################### */
@@ -71,7 +79,8 @@
Boolean = (1 << 2),
InvBoolean = (1 << 3),
ConfigFile = (1 << 4) | HasArg,
- ArbItem = (1 << 5) | HasArg
+ ArbItem = (1 << 5) | HasArg,
+ ArgOptional = (1 << 6) | HasArg
};
const char **FileList;
@@ -91,6 +100,7 @@
const char *LongOpt;
const char *ConfName;
unsigned long Flags;
+ const char *DefaultVal;
inline bool end() {return ShortOpt == 0 && LongOpt == 0;};
inline bool IsBoolean() {return Flags == 0 || (Flags & (Boolean|InvBoolean)) != 0;};
=== modified file 'apt-pkg/contrib/cmndline.cc'
--- apt-pkg/contrib/cmndline.cc 2011-09-21 16:42:08 +0000
+++ apt-pkg/contrib/cmndline.cc 2012-01-18 07:02:22 +0000
@@ -90,8 +90,8 @@
Args *A;
const char *OptEnd = strchrnul(Opt, '=');
for (A = ArgList; A->end() == false &&
- stringcasecmp(Opt,OptEnd,A->LongOpt) != 0; A++);
-
+ stringcasecmp(Opt,OptEnd,A->LongOpt,strchrnul(A->LongOpt, '=')) != 0; A++);
+
// Failed, look for a word after the first - (no-foo)
bool PreceedMatch = false;
if (A->end() == true)
@@ -180,10 +180,23 @@
// Option is an argument set
if ((A->Flags & HasArg) == HasArg)
{
+ if ((A->Flags & ArgOptional) == ArgOptional &&
+ CertainArg == false)
+ {
+ Argument = strchrnul(A->LongOpt, '=');
+ if (Argument != 0)
+ Argument++;
+ if (*Argument == '\0')
+ Argument = 0;
+ }
if (Argument == 0)
return _error->Error(_("Option %s requires an argument."),argv[I]);
- Opt += strlen(Opt);
- I += IncI;
+ if ((A->Flags & ArgOptional) != ArgOptional ||
+ CertainArg == true)
+ {
+ Opt += strlen(Opt);
+ I += IncI;
+ }
// Parse a configuration file
if ((A->Flags & ConfigFile) == ConfigFile)
=== modified file 'apt-pkg/contrib/cmndline.h'
--- apt-pkg/contrib/cmndline.h 2010-03-12 18:41:30 +0000
+++ apt-pkg/contrib/cmndline.h 2012-01-18 06:59:01 +0000
@@ -37,6 +37,14 @@
ArbItem - Means the item is an arbitrary configuration string of
the form item=value, where item is passed directly
to the configuration class.
+ ArgOptional - Means the argument value is optional and, if not supplied,
+ will take a default value. Such optional values must be
+ supplied unambiguously:
+ -d=val (ok) -dval (not ok) -d val (not ok)
+ --long=val (ok) --long val (not ok)
+ In the ambiguous cases the option will take the value
+ specified in LongOpt: "long=default"
+ Implies HasArg.
The default, if the flags are 0 is to use Boolean
##################################################################### */
@@ -71,7 +79,8 @@
Boolean = (1 << 2),
InvBoolean = (1 << 3),
ConfigFile = (1 << 4) | HasArg,
- ArbItem = (1 << 5) | HasArg
+ ArbItem = (1 << 5) | HasArg,
+ ArgOptional = (1 << 6) | HasArg
};
const char **FileList;
Reply to: