Hi Ed,
Sorry for the delay, this is the updated version:
- in opts_parse(), you removed the asserts to check for duplicate
options in the input options table:
# make sure each key is unique
assert k not in rv
--8<--
- in opts_parse(), i'm not sure i agree with this comment:
# A dict doesn't work here because some options can be passed multiple
# times. The option processing has to figure out if that's valid, this
# here should be as generic as possible.
we already know if an option can be repeated or not based on it's
"default" value. only options with a default value of an integer or a
list can be repeated. so you can check for that here. (this code
seems to have been moved to client.py right after the call to
opts_parse().
--8<--
src/modules/client.py
- why isn't all the code for building "opt_dict" inside options.py? it
seems like it should be part of opts_assemble().
all the above pretty much relates to each other so I'll respond for all
this together.
As per our discussion before, the code doesn't belong in options.py
because this serves as some sort of interface so the types which go in
there should be set before. We were talking about moving it potentially
to misc.opts_parse().
The issue with this is that if I do all the work in misc.opts_parse I
can't use options.get_pkg_opts_defaults() for getting the default value
since misc should not know about options for being generic (and it would
create a circular import). So to create the opts_dict depending on the
option types I would have to loop through the opts_table, which is a
list of (option, default_value) tuples, for every single option to find
the default value. Even though we parse options only once and this isn't
a huge performance hit it seems kinda wrong to me.
I will do it if you feel strongly about moving it to misc but I would
rather not.
- audit_linked(), detach_linked(), list_linked(), pubcheck_linked(),
remote(), and sync_linked() no longer check for unnecessary arguments
(ie: "len(pargs) == 0")
I revived your old method for this by specifying the pargs_limit for
each function.
- previously, in the case of repeated options we displayed the option
and any optional argument that was repeated. now we just display the
optional argument instead and we don't display the option itself.
I created a new error type and it's now properly handled:
# pkg sync-linked -l blub:bla -l blub:bla
pkg sync-linked: Argument 'blub:bla' for option '-l' may not be repeated
The incremental is here:
https://cr.opensolaris.org/action/browse/pkg/erisch/15804991_1-2/webrev/
If you could glance over it one lats time I'd appreciate it.
I think the only issue left should be about moving the opt_dict
generation to some other module. Everything else should be fixed now.
Erik
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
