[FIX] is_option_set: match both long and short ids

[eseiler]

To-do

• Decide whether to merge
• Cleanup
• Try to handle short/long IDs better

Problem

sharg::parser parser{"test_parser", 5, argv, sharg::update_notifications::off};
parser.add_option(option_value, sharg::config{.short_id = 'b', .long_id = "bar"});
parser.add_option(option_value, sharg::config{.short_id = 'l', .long_id = "loo"});
parser.add_option(option_value, sharg::config{.short_id = 'f', .long_id = "foobar"});

When calling ./parser_test -b blah:

parser.is_option_set('-b'); // True
parser.is_option_set("--bar"); // False

This is quite unintuitive and has already caused me trouble.
A workaround is of course

parser.is_option_set('-b') || parser.is_option_set("--bar");

But this relies on the user to get the short/long ID pairing correct.

Solution
Store short/long ID as a pair. When calling is_option_set, we can search for both short and long ID.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.75%. Comparing base (d3b3a7b) to head (6ea2963).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #226      +/-   ##
==========================================
+ Coverage   94.62%   94.75%   +0.12%     
==========================================
  Files          18       19       +1     
  Lines        1730     1772      +42     
  Branches       46       47       +1     
==========================================
+ Hits         1637     1679      +42     
  Misses         93       93              

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[smehringer]

So I'm in favour of this change, even if its an API break (right?)

[eseiler]

So I'm in favour of this change, even if its an API break (right?)

Shouldn't break API as all the ID handling is internal. It does change the API in the sense that the behavior is fixed :D

[smehringer]

So I'm in favour of this change, even if its an API break (right?)

Shouldn't break API as all the ID handling is internal. It does change the API in the sense that the behavior is fixed :D

Yeah I guess :) I try to think about cases where people deliberately distinguish between short and long id.. but I can't find an actual use case.

[seqan-actions]

Documentation preview available at https://docs.seqan.de/preview/seqan/sharg-parser/226

[SGSSGene]

If this is not given, the following 2 lines might be invalid accesses.

Suggested change

	EXPECT_EQ(positional_options.size(), 2u);
	ASSERT_EQ(positional_options.size(), 2u);

[eseiler]

Went for EXPECT_EQ(positional_options, (std::vector<std::string>{"--loo", "--bar"}));