[FIX] flags setting value to false

[eseiler]

I find it rather unintuitive that something that is not on the command line affects the result.

On the command line:

• If I don't give an option, nothing happens.
• If I don't give a positional option, I get reminded that those are required :)
• If I don't add a flag, it still sets the value to false.

Also, the result shouldn't really depend on the implementation:

    parser = get_parser("-a");
    parser.add_flag(flag_value, sharg::config{.short_id = 'a'});
    parser.add_flag(flag_value, sharg::config{.short_id = 'z'});
    EXPECT_NO_THROW(parser.parse());
    EXPECT_EQ(flag_value, false);

    parser = get_parser("-a");
    parser.add_flag(flag_value, sharg::config{.short_id = 'z'});
    parser.add_flag(flag_value, sharg::config{.short_id = 'a'});
    EXPECT_NO_THROW(parser.parse());
    EXPECT_EQ(flag_value, true);

So, if you add the z-option before the a-option, the result also inverts...

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
sharg-parser	✅ Ready (Inspect)	Visit Preview	Feb 21, 2024 0:52am

[eseiler]

flag_value would have been false before this PR.

The flags are evaluated in the order of the add_flag calls.

-a would set the value to true, but because -z was not found in the arguments, it would be set to false.

[smehringer]

So if you use the same Boolean for multiple options (I think we do not recommend this) then the value should be seen as "or"-ed between all flag calls?

I think I disagree. The original code is what I expected. For me, using the same Boolean for multiple options /flags is a design fail. And option -z was not set so the value should be false?

[eseiler]

You shouldn't do it, but if you do aliases (different flags for same thing), it would be weird if both flags needed to be given on the command line.

I find it rather unintuitive that something that is not on the command line affects the result.

On the command line:

• If I don't give an option, nothing happens.
• If I don't give a positional option, I get reminded that those are required :)
• If I don't add a flag, it still sets the value to false.

Also, the result shouldn't really depend on the implementation:

    parser = get_parser("-a");
    parser.add_flag(flag_value, sharg::config{.short_id = 'a'});
    parser.add_flag(flag_value, sharg::config{.short_id = 'z'});
    EXPECT_NO_THROW(parser.parse());
    EXPECT_EQ(flag_value, false);

    parser = get_parser("-a");
    parser.add_flag(flag_value, sharg::config{.short_id = 'z'});
    parser.add_flag(flag_value, sharg::config{.short_id = 'a'});
    EXPECT_NO_THROW(parser.parse());
    EXPECT_EQ(flag_value, true);

So, if you add the z-option before the a-option, the result also inverts...

[smehringer]

I see that it might be useful if you deprecate a flag and replace it by another flag, you would them to be "or"-ed

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (31d3207) 95.23% compared to head (5c9e2f9) 95.23%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #243   +/-   ##
=======================================
  Coverage   95.23%   95.23%           
=======================================
  Files          18       18           
  Lines        1700     1700           
=======================================
  Hits         1619     1619           
  Misses         81       81           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[SGSSGene]

Nice catch!