Merge pull request #66 from eaasna/flag_defaults

[DOC, BUG] Clarify flag default value

doc/tutorial/argument_parser/index.md

@@ -20,7 +20,7 @@ This class will give you the following functionality:
 * Robust parsing of command line arguments.
 * Simple validation of arguments (e.g. within a range or one of a list of allowed values).
 * Automatically generated and nicely formatted help screens when your program is called with `--help`. You can also
-* export this help to HTML and man pages.
+export this help to HTML and man pages.
 * In the future, you are also able to automatically generate nodes for work flow engines such as KNIME or Galaxy.
 
 ## Command line argument terminology
@@ -79,12 +79,13 @@ screen.
 The argument parser checks the following restrictions and throws a sharg::design_error if they are not satisfied:
 
 * **Long identifiers**: must be unique, more than one character long, may only contain alphanumeric characters, as well
-* as `_`, `-`, or `@`, but never start with `-`.
+as `_`, `-`, or `@`, but never start with `-`.
 * **Short identifiers**: must be unique and consist of only a single letter that is alphanumeric characters, `_` or `@`.
-* either the short or long id may be empty but not both at the same time.
+Either the short or long id may be empty but not both at the same time.
 * Only the last positional option may be a list (see [lists](#section_list_positional_options)).
 * The flag identifiers `-h`, `--help`, `--advanced-help`, `--advanced-help`, `--export-help`, `--version`, `--copyright`
-* are predefined and cannot be specified manually or used otherwise.
+are predefined and cannot be specified manually or used otherwise.
+* **Flags**: value must be false by default. Passing the flag identifier on the command line marks it as true.
 * The sharg::argument_parser::parse function may only be called once (per parser).
 
 ## Input restrictions
@@ -204,6 +205,9 @@ Additionally to the variable that will store the value and the description, you
 identifier. The example above will recognize an option `-n` or `--my-number` given on the command line and expect it to
 be followed by a value separated only by `=` or space or by nothing at all.
 
+\note Unlike regular options which take id-value pairs, flags are passed as an identifier only. The variable associated 
+to a flag must be false by default and is switched to true when the flag is present on the command line.
+
 Finally, you can add a flag with the following call:
 
 \snippet doc/tutorial/argument_parser/small_snippets.cpp add_flag

doc/tutorial/argument_parser/small_snippets.cpp

@@ -36,7 +36,7 @@ parser.add_option(variable, 'n', "my-number", "This is a description.");
 {
 sharg::argument_parser parser{"Example-Parser", argc, argv};
 //![add_flag]
-bool variable{};
+bool variable{false};
 parser.add_flag(variable, 'f', "my_flag", "This is a description.");
 //![add_flag]
 }

doc/tutorial/argument_parser/solution3.cpp

@@ -60,7 +60,7 @@ struct cmd_arguments
     std::filesystem::path file_path{};
     uint32_t year{};
     std::string aggregate_by{"mean"};
-    bool header_is_set{};
+    bool header_is_set{false};
 };
 //![program]
 

doc/tutorial/argument_parser/solution4.cpp

@@ -62,7 +62,7 @@ struct cmd_arguments
     std::filesystem::path file_path{};
     std::vector<uint8_t> seasons{};
     std::string aggregate_by{"mean"};
-    bool header_is_set{};
+    bool header_is_set{false};
 };
 
 void initialise_argument_parser(sharg::argument_parser & parser, cmd_arguments & args)

doc/tutorial/argument_parser/solution5.cpp

@@ -59,7 +59,7 @@ struct cmd_arguments
     std::filesystem::path file_path{};
     std::vector<uint8_t> seasons{};
     std::string aggregate_by{"mean"};
-    bool header_is_set{};
+    bool header_is_set{false};
 };
 
 void initialise_argument_parser(sharg::argument_parser & parser, cmd_arguments & args)

doc/tutorial/argument_parser/solution6.cpp

@@ -59,7 +59,7 @@ struct cmd_arguments
     std::filesystem::path file_path{};
     std::vector<uint8_t> seasons{};
     std::string aggregate_by{"mean"};
-    bool header_is_set{};
+    bool header_is_set{false};
 };
 
 void initialise_argument_parser(sharg::argument_parser & parser, cmd_arguments & args)

include/sharg/argument_parser.hpp

@@ -251,18 +251,24 @@ class argument_parser
 
     /*!\brief Adds a flag to the sharg::argument_parser.
      *
-     * \param[in, out] value     The variable in which to store the given command line argument.
+     * \param[in, out] value     The variable which shows if the flag is turned off (default) or on.
      * \param[in]      short_id  The short identifier for the flag (e.g. 'i').
      * \param[in]      long_id   The long identifier for the flag (e.g. "integer").
      * \param[in]      desc      The description of the flag to be shown in the help page.
      * \param[in]      spec      Advanced flag specification, see sharg::option_spec.
+     *
+     * \throws sharg::design_error
+     *
      */
     void add_flag(bool & value,
                   char const short_id,
                   std::string const & long_id,
                   std::string const & desc,
                   option_spec const spec = option_spec::standard)
     {
+        if (value)
+            throw design_error("A flag's default value must be false.");
+
         verify_identifiers(short_id, long_id);
         // copy variables into the lambda because the calls are pushed to a stack
         // and the references would go out of scope.

include/sharg/detail/format_parse.hpp

@@ -718,7 +718,7 @@ class format_parse : public format_base
 
     /*!\brief Handles command line flags, whether they are set or not.
      *
-     * \param[out] value    The variable in which to store the given command line argument.
+     * \param[out] value    The variable which shows if the flag is turned off (default) or on.
      * \param[in]  short_id The short identifier for the flag (e.g. 'i').
      * \param[in]  long_id  The long identifier for the flag (e.g. "integer").
      *

include/sharg/exceptions.hpp

@@ -138,6 +138,7 @@ class validation_error : public argument_parser_error
  *
  * - Reuse of a short or long identifier (must be unique)
  * - Both identifiers must not be empty (one is ok)
+ * - Flag default value must be false
  */
 class design_error : public argument_parser_error
 {

test/unit/argument_parser_design_error_test.cpp

@@ -44,57 +44,64 @@ TEST(parse_test, design_error)
     EXPECT_THROW(parser3.add_option(option_value, '\0', "", "oh oh all is empty."),
                  sharg::design_error);
 
-    bool flag_value;
+    bool true_value{true};
 
-    // short flag
+    // default true
     sharg::argument_parser parser4{"test_parser", 1, argv};
-    parser4.add_flag(flag_value, 'i', "int1", "this is an int option.");
-    EXPECT_THROW(parser4.add_flag(flag_value, 'i', "int2", "oh oh another id."),
+    EXPECT_THROW(parser4.add_flag(true_value, 'i', "int", "oh oh default is true."),
                  sharg::design_error);
 
-    // long flag
+    bool flag_value{false};
+
+    // short flag
     sharg::argument_parser parser5{"test_parser", 1, argv};
-    parser5.add_flag(flag_value, 'i', "int", "this is an int option.");
-    EXPECT_THROW(parser5.add_flag(flag_value, 'a', "int", "oh oh another id."),
+    parser5.add_flag(flag_value, 'i', "int1", "this is an int option.");
+    EXPECT_THROW(parser5.add_flag(flag_value, 'i', "int2", "oh oh another id."),
                  sharg::design_error);
 
-    // empty identifier
+    // long flag
     sharg::argument_parser parser6{"test_parser", 1, argv};
-    EXPECT_THROW(parser6.add_flag(flag_value, '\0', "", "oh oh another id."),
+    parser6.add_flag(flag_value, 'i', "int", "this is an int option.");
+    EXPECT_THROW(parser6.add_flag(flag_value, 'a', "int", "oh oh another id."),
+                 sharg::design_error);
+
+    // empty identifier
+    sharg::argument_parser parser7{"test_parser", 1, argv};
+    EXPECT_THROW(parser7.add_flag(flag_value, '\0', "", "oh oh another id."),
                  sharg::design_error);
 
     // positional option not at the end
     const char * argv2[] = {"./argument_parser_test", "arg1", "arg2", "arg3"};
     std::vector<int> vec;
-    sharg::argument_parser parser7{"test_parser", 4, argv2};
-    parser7.add_positional_option(vec, "oh oh list not at the end.");
-    EXPECT_THROW(parser7.add_positional_option(option_value, "desc."), sharg::design_error);
+    sharg::argument_parser parser8{"test_parser", 4, argv2};
+    parser8.add_positional_option(vec, "oh oh list not at the end.");
+    EXPECT_THROW(parser8.add_positional_option(option_value, "desc."), sharg::design_error);
 
     // using h, help, advanced-help, and export-help
-    sharg::argument_parser parser8{"test_parser", 1, argv};
-    EXPECT_THROW(parser8.add_option(option_value, 'h', "", "-h is bad."),
+    sharg::argument_parser parser9{"test_parser", 1, argv};
+    EXPECT_THROW(parser9.add_option(option_value, 'h', "", "-h is bad."),
                  sharg::design_error);
-    EXPECT_THROW(parser8.add_option(option_value, '\0', "help", "help is bad."),
+    EXPECT_THROW(parser9.add_option(option_value, '\0', "help", "help is bad."),
                  sharg::design_error);
-    EXPECT_THROW(parser8.add_option(option_value, '\0', "advanced-help",
+    EXPECT_THROW(parser9.add_option(option_value, '\0', "advanced-help",
                  "advanced-help is bad"), sharg::design_error);
-    EXPECT_THROW(parser8.add_option(option_value, '\0', "export-help",
+    EXPECT_THROW(parser9.add_option(option_value, '\0', "export-help",
                  "export-help is bad"), sharg::design_error);
 
     // using one-letter long identifiers.
-    sharg::argument_parser parser9{"test_parser", 1, argv};
-    EXPECT_THROW(parser9.add_option(option_value, 'y', "z", "long identifier is one letter"),
+    sharg::argument_parser parser10{"test_parser", 1, argv};
+    EXPECT_THROW(parser10.add_option(option_value, 'y', "z", "long identifier is one letter"),
                  sharg::design_error);
-    EXPECT_THROW(parser9.add_flag(flag_value, 'y', "z", "long identifier is one letter"),
+    EXPECT_THROW(parser10.add_flag(flag_value, 'y', "z", "long identifier is one letter"),
                  sharg::design_error);
 
     // using non-printable characters
-    sharg::argument_parser parser10{"test_parser", 1, argv};
-    EXPECT_THROW(parser10.add_option(option_value, '\t', "no\n", "tab and newline don't work!"),
+    sharg::argument_parser parser11{"test_parser", 1, argv};
+    EXPECT_THROW(parser11.add_option(option_value, '\t', "no\n", "tab and newline don't work!"),
                  sharg::design_error);
-    EXPECT_THROW(parser10.add_flag(flag_value, 'i', "no\n", "tab and newline don't work!"),
+    EXPECT_THROW(parser11.add_flag(flag_value, 'i', "no\n", "tab and newline don't work!"),
                  sharg::design_error);
-    EXPECT_THROW(parser10.add_flag(flag_value, 'a', "-no", "can't start long_id with a hyphen"),
+    EXPECT_THROW(parser11.add_flag(flag_value, 'a', "-no", "can't start long_id with a hyphen"),
                  sharg::design_error);
 }
 
@@ -116,7 +123,7 @@ TEST(parse_test, parse_called_twice)
 
 TEST(parse_test, subcommand_argument_parser_error)
 {
-    bool flag_value{};
+    bool flag_value{false};
 
     // subcommand parsing was not enabled on construction but get_sub_parser() is called
     {

test/unit/detail/format_help_test.cpp

@@ -13,7 +13,7 @@
 std::string std_cout;
 std::string expected;
 int option_value{5};
-bool flag_value{};
+bool flag_value{false};
 std::vector<std::string> pos_opt_value{};
 const char * argv0[] = {"./help_add_test --version-check false"};
 const char * argv1[] = {"./help_add_test --version-check false", "-h"};
@@ -292,7 +292,7 @@ TEST(help_page_printing, advanced_options)
 {
     int32_t option_value{5};
     uint8_t another_option_value{2};
-    bool flag_value{};
+    bool flag_value{false};
 
     auto set_up = [&option_value, &flag_value, &another_option_value] (sharg::argument_parser & parser)
     {

test/unit/detail/format_html_test.cpp

@@ -70,7 +70,7 @@ TEST(html_format, full_information_information)
     std::string my_stdout;
     std::string expected;
     int option_value{5};
-    bool flag_value;
+    bool flag_value{false};
     int8_t non_list_pos_opt_value{1};
     std::vector<std::string> list_pos_opt_value{};
 

test/unit/detail/format_man_test.cpp

@@ -13,7 +13,7 @@
 struct format_man_test : public ::testing::Test
 {
     int option_value{5};
-    bool flag_value{};
+    bool flag_value{false};
     int8_t non_list_pos_opt_value{1};
     std::vector<std::string> list_pos_opt_value{};
     std::string my_stdout{};

test/unit/detail/version_check_test.hpp

@@ -114,7 +114,7 @@ struct version_check : public ::testing::Test
         parser.info.version = "2.3.4";
 
         // In case we don't want to specify --version-check but avoid that short help format will be set (no arguments)
-        bool dummy{};
+        bool dummy{false};
         parser.add_flag(dummy, 'f', "dummy-flag", "A dummy flag.");
 
         testing::internal::CaptureStdout();
@@ -313,7 +313,7 @@ TEST_F(version_check, environment_variable_set)
 
     sharg::argument_parser parser{app_name, 2, argv};
     parser.info.version = "2.3.4";
-    bool dummy{};
+    bool dummy{false};
     parser.add_flag(dummy, 'f', "dummy-flag", "A dummy flag.");
 
     testing::internal::CaptureStdout();