Modify

Ticket #2982 (closed Feature Requests: fixed)

Opened 5 years ago

Last modified 4 years ago

Required Arguments

Reported by: David Doria <daviddoria@…> Owned by: s_ochsenknecht
Milestone: Boost 1.42.0 Component: program_options
Version: Boost Development Trunk Severity: Not Applicable
Keywords: Required Arguments Cc: s.ochsenknecht@…

Description

If I have a list of 10 parameters, and all are required,

I end up with a giant block of these:

if(!vm.count("input"))

{

std::cout << "Input was not set." << std::endl; exit(-1);

}

It would be nice to do something like: desc.add_options()

("input", po::value<std::string>(&InputFile?)->required(), "Set input file.")

That will perform that check automatically.

Attachments

patch_ticket2982.diff Download (5.5 KB) - added by s.ochsenknecht@… 4 years ago.
patch_ticket2982_2.diff Download (6.7 KB) - added by s.ochsenknecht@… 4 years ago.
patch
required.cpp Download (1.1 KB) - added by s.ochsenknecht@… 4 years ago.
testcase

Change History

Changed 4 years ago by s.ochsenknecht@…

comment:1 Changed 4 years ago by s.ochsenknecht@…

I find it useful, so I added a patch which implements the required() flag as described. It throws an exception from parsing if a required option isn't present.

This is my test case:

#include <boost/program_options.hpp>
#include <string>
#include <iostream>

using namespace boost::program_options;
using namespace std;

int main(int argc, char *argv[])
{
  options_description		opts;
  opts.add_options()
    ("cfgfile,c", value<std::string>()->required(), "the configfile")
    ("fritz,f", value<std::string>()->default_value("/other/file"), "the output file")
   ;

   variables_map vm;
   try {
       store(parse_command_line(argc, argv, opts), vm);
       notify(vm);    
   } 
   catch (required_option& e) {   // new exception type
       cout << "required option: " << e.what() << endl;
       return -1;
   }
   catch (...) {
       cout << "other exception: " << endl;
       return -1;   
   }
}

Does it make sense? Is it useful?

I'm not a Boost maintainer, so I won't change the state of this ticket :-).

Thanks

comment:2 Changed 4 years ago by Sascha Ochsenknecht <s.ochsenknecht@…>

  • Cc s.ochsenknecht@… added

comment:3 Changed 4 years ago by vladimir_prus

Sascha, I am not sure if the location of the check is right. I'd suspect that user wants the option to be always specified, but does not necessary care where it comes from. Maybe, a check inside 'notify' would be better?

comment:4 Changed 4 years ago by s.ochsenknecht@…

I was also not very sure if this is right place. Since I'm still not very familiar with the internals of this library. I just cam to that place by tracing with the debugger ...

I'm going to check your proposal with 'notify' within the next days. Keep you informed. Thanks for reviewing.

comment:5 Changed 4 years ago by s.ochsenknecht@…

I just checked if it is possible to place the check in notify(), but there is one problem. The notify function just gets the variables_map parameter and not the options_description's which have the information about required options.

I have following solutions in mind:

1) in store() we store any required options with an empty value (if not occur) in the variables_map. In notify we then check for empty() and throw exception if empty() and required(). Any side effects? My preferred solution.

2) we pass options_description's to notify() to be able to perform the cross check, but this breaks the interface (or we overload notify, but then its up to the user to choose the correct notify() ... :-(

3) we store a reference/pointer to options_description's in variables_map. :-(

...

Please comment. Thanks

Changed 4 years ago by s.ochsenknecht@…

patch

Changed 4 years ago by s.ochsenknecht@…

testcase

comment:6 Changed 4 years ago by s.ochsenknecht@…

I implemented option (1), see patch_ticket2982_2.diff and the attached testcase required.cpp.

store() now adds empty variable_value's to the variable_map. I also override the count() method of the underlying std::map in that way that the empty variable_values are not counted.

Hm, this can have some side effects when operator[] is used: const variable_value& operator[](const std::string& name) const

But I think the user has to check for !empty() anyhow ..!?

Please comment.

comment:7 follow-up: ↓ 8 Changed 4 years ago by vladimir_prus

Sascha,

thanks for the patch. I am out of time for today, but will review it as soon as possible.

Thanks, Volodya

comment:8 in reply to: ↑ 7 Changed 4 years ago by Sascha Ochsenknecht <s.ochsenknecht@…>

Replying to vladimir_prus:

Sascha,

thanks for the patch. I am out of time for today, but will review it as soon as possible.

Probably storing the required options together with the parsed options in the same map isn't a good idea. Maybe a new member variable (in variables_map) containing the required options would be better. I'll modify the patch again within the next days.

Thanks, Sascha

comment:9 Changed 4 years ago by s_ochsenknecht

  • Owner changed from vladimir_prus to s_ochsenknecht
  • Status changed from new to assigned
  • Milestone changed from Boost 1.39.0 to Boost 1.42.0

comment:10 Changed 4 years ago by s_ochsenknecht

  • Status changed from assigned to closed
  • Resolution set to fixed

(In [58263]) Enhancement to flag options as required, Fixes #2982

comment:11 Changed 4 years ago by anonymous

I modified the attached patch in that way that required options are stored as an additional member in variables_map. The check if all required options occur is done in notify().

Cheers, Sascha

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.