Bug153 Editing
This can bite developers when they may call add on the same item twice. add() should check that no pre-existing option has a similar letter/string. If the intention of allowing this is to allow letter/string-only options, then we should modify the interface to properly support this. Suggest that we throw an exception if we try and add a duplicate. A good example of this giving strange behaviour is in GGI/src/pinyin_demo.cc, where an option with 'h' is added twice (--help and --height). Using -h only gives help, which is confusing since -h is shown in getopt.usage() as (also) being the option corresponding to --height.
This is important if we intend to continue using utility functions such as the one in resolve.hh which adds the resolving-functions to a getopt - we need to make sure that either a resolving- or other-option is not being 'lost' in this way.
could this be a simple : void GetOpt::add(char o, const std::string &opt, type t, const std::string &desc) const throw (NoSuchOption) { table_t::const_iterator i = find(opt); if (i != table.end()) throw NoSuchOption(); table.push_back(cell(o, opt, t, desc)); }
actually this would be thrown class DuplicateOption : std::exception { public: DuplicateOption() throw() {} virtual ~DuplicateOption() throw() {} virtual const char* what() const throw() { return "option already added";} };
yeah, throwing DuplicateOption looks fine to me.
this file should fix this
that one was screwed up try this one
Looks good by inspection, but who can test rohara's patch? neiljp, you filed the bug... If someone else can give it a thumbs up, I'll happily check it in.
I won't be able to test this at all until at least about the 3rd Jan onwards, when I should be back at my Big Fast Computer (tm). It looks as if it contains the right idea; however, two points: - do we want to derive from std::exception non- publicly? IIRC 'class foo : bar {};' means that foo inherits privately (the default) from bar...do we want a 'public' before the 'bar'? I've not done much work involving std::exception, but this strikes me as something strange in the patch. - I don't remember a piece in the (proposed) coding-standard concerning the use of throw specifications; its not so important here, but from what I read its normal to only (possibly!) include throw declarations when it is a no-throw function, and at no other time...I'm fairly certain this is discussed in gotw/exceptional-C++ (which I don't have to hand right now). Of course, this normally only reduces the efficiency, and getopt is not exactly likely to be in the time-critical path...but the point/question stands :)
this fixes the non-public derivation of std::exception and also clears up what option was already added when a DuplicateOption was thrown
Patch applied to CVS. Thanks rohara :)
not logged in
register
Home