Skip to content

Commit

Permalink
Continue accumulating patterns into rule.
Browse files Browse the repository at this point in the history
Currently, the argument parser will terminate a rule after the first
pattern is added (accumulating future patterns into new rules). This
change causes patterns to accumulate into the same rule until another
`-e` option is provided. It also adds tests for that condition.
  • Loading branch information
joshkunz committed Dec 19, 2021
1 parent 19bcf72 commit 32c25d8
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/args.cc
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ Parser::Status Parser::Consume(std::string_view arg) {
prev_ = arg;
Parser::State next = std::get<Parser::State>(next_or_err);
// If we're transitioning out of a rule...
if (state_ == kRule && next != kRule) {
if (state_ == kRule && (next != kRule && next != kRuleValue)) {
FlushRule();
}
state_ = next;
Expand Down
3 changes: 3 additions & 0 deletions src/rule.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ class Rule {
// Empty returns true when this rule matches no patterns.
inline bool Empty() const { return patterns_.empty(); }

// Size returns the number of patterns in this rule.
inline size_t Size() const { return patterns_.size(); }

// Add the given pattern to this rule.
void AddPattern(enum mpd_tag_type, std::string value);

Expand Down
55 changes: 52 additions & 3 deletions t/args_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ TEST(ParseTest, Short) {

Options opts = std::move(std::get<Options>(result));

EXPECT_EQ(opts.ruleset.size(), 2U);
EXPECT_EQ(opts.ruleset.size(), 1U);
EXPECT_EQ(opts.queue_only, 5U);
EXPECT_THAT(opts.file_in, NotNull());
EXPECT_FALSE(opts.check_uris);
Expand Down Expand Up @@ -106,7 +106,7 @@ TEST(ParseTest, Long) {

Options opts = std::move(std::get<Options>(result));

EXPECT_EQ(opts.ruleset.size(), 2U);
EXPECT_EQ(opts.ruleset.size(), 1U);
EXPECT_EQ(opts.queue_only, 5U);
EXPECT_THAT(opts.file_in, NotNull());
EXPECT_FALSE(opts.check_uris);
Expand All @@ -133,7 +133,7 @@ TEST(ParseTest, MixedLongShort) {
}));
// clang-format on

EXPECT_EQ(opts.ruleset.size(), 2U);
EXPECT_EQ(opts.ruleset.size(), 1U);
EXPECT_EQ(opts.queue_only, 5U);
EXPECT_THAT(opts.file_in, NotNull());
EXPECT_FALSE(opts.check_uris);
Expand Down Expand Up @@ -163,6 +163,55 @@ TEST(ParseTest, Rule) {
<< "basic rule arg should not exclude non-matching song";
}

TEST(ParseTest, RuleWithMultipleTags) {
fake::TagParser tagger({
{"artist", MPD_TAG_ARTIST},
{"album", MPD_TAG_ALBUM},
});

Options opts = std::get<Options>(Options::Parse(
tagger, {"-e", "artist", "__artist__", "album", "__album__"}));

ASSERT_FALSE(opts.ruleset.empty());

EXPECT_EQ(opts.ruleset.size(), 1);

Rule &r = opts.ruleset[0];

EXPECT_EQ(r.Size(), 2) << "Rule should have two matching patterns";

fake::Song matching({
{MPD_TAG_ARTIST, "__artist__"},
{MPD_TAG_ALBUM, "__album__"},
});
fake::Song partial_artist({{MPD_TAG_ARTIST, "__artist__"}});
fake::Song partial_album({{MPD_TAG_ALBUM, "__album__"}});

EXPECT_FALSE(r.Accepts(matching));
EXPECT_TRUE(r.Accepts(partial_artist));
EXPECT_TRUE(r.Accepts(partial_album));
}

TEST(ParseTest, MultipleRules) {
fake::TagParser tagger({
{"artist", MPD_TAG_ARTIST},
});

// clang-format off
Options opts = std::get<Options>(
Options::Parse(tagger, {
"-e", "artist", "__artist__",
"-e", "artist", "__another_artist__"
}));
// clang-format on

ASSERT_EQ(opts.ruleset.size(), 2) << "Expected two rules";

constexpr char message[] = "Each ruleset should have exactly one pattern";
EXPECT_EQ(opts.ruleset[0].Size(), 1) << message;
EXPECT_EQ(opts.ruleset[1].Size(), 1) << message;
}

TEST(ParseTest, FileInStdin) {
Options opts;
fake::TagParser tagger;
Expand Down

0 comments on commit 32c25d8

Please sign in to comment.