Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes: > > + bool incremented = false; > if (next == '\0' && next_arg != NULL && ! try->opt_bool) { > next = ' '; > value = next_arg; > + incremented = true; > opt_index ++; > } Is incremented == true exactly when next == ' ' ? It might be nice to make that more explicit by setting one based on the other. You could also use (next == ' ') as your test condition, but I understand that might not be that obvious to read. > > diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh > index 192133c5..3f8a78a3 100755 > --- a/test/T410-argument-parsing.sh > +++ b/test/T410-argument-parsing.sh > @@ -65,4 +65,28 @@ flags 1 > EOF > test_expect_equal_file EXPECTED OUTPUT > > +test_begin_subtest "test keyword arguments without value" > +$TEST_DIRECTORY/arg-test --boolkeyword bananas > OUTPUT > +cat <<EOF > EXPECTED > +boolkeyword 1 > +positional arg 1 bananas > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + > +test_begin_subtest "test keyword arguments without value at the end" > +$TEST_DIRECTORY/arg-test bananas --boolkeyword > OUTPUT > +cat <<EOF > EXPECTED > +boolkeyword 1 > +positional arg 1 bananas > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + > +test_begin_subtest "test keyword arguments without value but with = (should be an error)" > +$TEST_DIRECTORY/arg-test bananas --boolkeyword= > OUTPUT 2>&1 > +cat <<EOF > EXPECTED > +Unknown keyword argument "" for option "boolkeyword". > +Unrecognized option: --boolkeyword= > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + The thing I'm most nervous about here is the interaction between this new code and the relatively recent code that permits ' ' as a seperator. Would you mind adding one or more tests for that case? For example, that I checked that ./notmuch show --format=json --decrypt true $id continues to work, and that's great, but it seems like something to check on the argument parsing level, i.e --keyword␣non-default-value (pardon my unicode) _______________________________________________ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch