From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id C8E656DE035A for ; Sat, 23 Dec 2017 06:29:34 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[AWL=0.011, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hwqhqw2O6q5v for ; Sat, 23 Dec 2017 06:29:33 -0800 (PST) Received: from fethera.tethera.net (fethera.tethera.net [198.245.60.197]) by arlo.cworth.org (Postfix) with ESMTPS id 47CB16DE0183 for ; Sat, 23 Dec 2017 06:29:33 -0800 (PST) Received: from remotemail by fethera.tethera.net with local (Exim 4.89) (envelope-from ) id 1eSknr-00037E-NV; Sat, 23 Dec 2017 09:29:31 -0500 Received: (nullmailer pid 27575 invoked by uid 1000); Sat, 23 Dec 2017 14:29:30 -0000 From: David Bremner To: Daniel Kahn Gillmor , Notmuch Mail Subject: Re: [PATCH v4 1/3] cli: some keyword options can be supplied with no argument In-Reply-To: <20171219164055.20778-2-dkg@fifthhorseman.net> References: <20171219164055.20778-1-dkg@fifthhorseman.net> <20171219164055.20778-2-dkg@fifthhorseman.net> Date: Sat, 23 Dec 2017 10:29:30 -0400 Message-ID: <87a7y9a56d.fsf@tethera.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 23 Dec 2017 14:29:34 -0000 Daniel Kahn Gillmor writes: >=20=20 > + bool incremented =3D false; > if (next =3D=3D '\0' && next_arg !=3D NULL && ! try->opt_bool) { > next =3D ' '; > value =3D next_arg; > + incremented =3D true; > opt_index ++; > } Is incremented =3D=3D true exactly when next =3D=3D ' ' ? It might be nice = to make that more explicit by setting one based on the other. You could also use (next =3D=3D ' ') as your test condition, but I understand that might not be that obvious to read. >=20=20 > 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 >=20=20 > +test_begin_subtest "test keyword arguments without value" > +$TEST_DIRECTORY/arg-test --boolkeyword bananas > OUTPUT > +cat < 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 < EXPECTED > +boolkeyword 1 > +positional arg 1 bananas > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + > +test_begin_subtest "test keyword arguments without value but with =3D (s= hould be an error)" > +$TEST_DIRECTORY/arg-test bananas --boolkeyword=3D > OUTPUT 2>&1 > +cat < EXPECTED > +Unknown keyword argument "" for option "boolkeyword". > +Unrecognized option: --boolkeyword=3D > +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=3Djson --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=E2=90=A3non-default-value (pardon my unicode)