Re: [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN
[notmuch-archives.git] / c3 / 2050071ba5db157a3716db15b9246a0678a62f
1 Return-Path: <m.walters@qmul.ac.uk>\r
2 X-Original-To: notmuch@notmuchmail.org\r
3 Delivered-To: notmuch@notmuchmail.org\r
4 Received: from localhost (localhost [127.0.0.1])\r
5         by olra.theworths.org (Postfix) with ESMTP id 5117C431FB6\r
6         for <notmuch@notmuchmail.org>; Thu,  8 Mar 2012 21:12:00 -0800 (PST)\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
8 X-Spam-Flag: NO\r
9 X-Spam-Score: -1.098\r
10 X-Spam-Level: \r
11 X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
12         tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
13         NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
14 Received: from olra.theworths.org ([127.0.0.1])\r
15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
16         with ESMTP id wBVw3qVaO9qL for <notmuch@notmuchmail.org>;\r
17         Thu,  8 Mar 2012 21:11:59 -0800 (PST)\r
18 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
20         (No client certificate requested)\r
21         by olra.theworths.org (Postfix) with ESMTPS id 738C7431FAE\r
22         for <notmuch@notmuchmail.org>; Thu,  8 Mar 2012 21:11:59 -0800 (PST)\r
23 Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
24         by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
25         (envelope-from <m.walters@qmul.ac.uk>)\r
26         id 1S5s7O-0003sj-FA; Fri, 09 Mar 2012 05:11:54 +0000\r
27 Received: from 94-192-233-223.zone6.bethere.co.uk ([94.192.233.223]\r
28         helo=localhost)\r
29         by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69)\r
30         (envelope-from <m.walters@qmul.ac.uk>)\r
31         id 1S5s7O-00035u-5D; Fri, 09 Mar 2012 05:11:54 +0000\r
32 From: Mark Walters <markwalters1009@gmail.com>\r
33 To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org\r
34 Subject: Re: [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN\r
35 In-Reply-To: <87lina8uqk.fsf@nikula.org>\r
36 References: <1331244944-7960-1-git-send-email-markwalters1009@gmail.com>\r
37         <87lina8uqk.fsf@nikula.org>\r
38 User-Agent: Notmuch/0.11.1+309~g045f9e7 (http://notmuchmail.org) Emacs/23.3.1\r
39         (x86_64-pc-linux-gnu)\r
40 Date: Fri, 09 Mar 2012 05:11:54 +0000\r
41 Message-ID: <87399iicit.fsf@qmul.ac.uk>\r
42 MIME-Version: 1.0\r
43 Content-Type: text/plain; charset=us-ascii\r
44 X-Sender-Host-Address: 94.192.233.223\r
45 X-QM-SPAM-Info: Sender has good ham record.  :)\r
46 X-QM-Body-MD5: b5e51a92b3f65372ef49311528464ee2 (of first 20000 bytes)\r
47 X-SpamAssassin-Score: -1.8\r
48 X-SpamAssassin-SpamBar: -\r
49 X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
50         determine if it is\r
51         spam. We require at least 5.0 points to mark a message as spam.\r
52         This message scored -1.8 points.\r
53         Summary of the scoring: \r
54         * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/,\r
55         *      medium trust\r
56         *      [138.37.6.40 listed in list.dnswl.org]\r
57         * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
58         provider *      (markwalters1009[at]gmail.com)\r
59         * -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay\r
60         *      domain\r
61         *  0.5 AWL AWL: From: address is in the auto white-list\r
62 X-QM-Scan-Virus: ClamAV says the message is clean\r
63 X-BeenThere: notmuch@notmuchmail.org\r
64 X-Mailman-Version: 2.1.13\r
65 Precedence: list\r
66 List-Id: "Use and development of the notmuch mail system."\r
67         <notmuch.notmuchmail.org>\r
68 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
69         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
70 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
71 List-Post: <mailto:notmuch@notmuchmail.org>\r
72 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
73 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
74         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
75 X-List-Received-Date: Fri, 09 Mar 2012 05:12:00 -0000\r
76 \r
77 \r
78 Hi\r
79 \r
80 On Fri, 09 Mar 2012 02:48:35 +0200, Jani Nikula <jani@nikula.org> wrote:\r
81 > On Thu,  8 Mar 2012 22:15:42 +0000, Mark Walters <markwalters1009@gmail.com> wrote:\r
82 > > The first patch adds a new command line parsing option\r
83 > > NOTMUCH_OPT_INT_OR_BOOLEAN for command line parsing which accepts\r
84 > > --verbose=3 and --verbose with the latter setting verbose to 1. It\r
85 > > also allows --verbose=0 so (with a little caller support) the user can\r
86 > > turn off boolean options.\r
87 > > \r
88 > > It also means that extra options can be added to the command line\r
89 > > programs in a backwards compatible manner (e.g. if --verbose already\r
90 > > exists we could add --verbose=2).\r
91 > > \r
92 > > The second patch uses this to make the --entire-thread option to\r
93 > > notmuch-show a NOTMUCH_OPT_INT_OR_BOOLEAN. In particular this allows\r
94 > > the caller to disable --entire-thread (with --entire-thread=0) when\r
95 > > format=json.\r
96\r
97 > I'm afraid I find both of the patches a bit hacky. The first because\r
98 > it's more about optional arguments to options than int-or-bool. The\r
99 > second because it's more about detecting whether the user has provided\r
100 > an option than int-or-bool.\r
101 \r
102 Yes to both of these (although I don't think of it has hacky). The\r
103 second of these is the important consideration.\r
104 \r
105 > For booleans, I think the notmuch style would be to allow --foo=true and\r
106 > --foo=false in addition to just --foo (which implies true) so you could\r
107 > also disable booleans. I think this would be fairly simple to implement,\r
108 > and would require no changes to option parser users.\r
109 >\r
110 > With --entire-thread the bigger problem is detecting whether the option\r
111 > was specified by the user or not. It would be great if the option parser\r
112 > could provide this information, but it might take a while to get\r
113 > there... In the mean time, I think I would rather see a well commented\r
114 > local hack here initializing the notmuch_bool_t params.entire_thread to\r
115 > -1, and changing it to TRUE or FALSE if not already done so by\r
116 > parse_arguments().\r
117 \r
118 I think these need to be considered together. There are three important\r
119 possibilities for a boolean option foo: 1) foo not mentioned 2)\r
120 --foo=false and 3) --foo=true (--foo as an alias) but the parser only\r
121 has a boolean to store this in. We could overload the boolean as you\r
122 suggest (it is really an int so should be safe) but that does seem\r
123 hacky. \r
124 \r
125 That was why I decided to make the parser set an int rather than a\r
126 boolean. Since there are not very many OPT_BOOLEANs currently in the\r
127 code I think my decision to allow general ints to be returned is a\r
128 needless extension, but I do think we wish to allow the 3 states of\r
129 UNSET, FALSE and TRUE. Otherwise we limit any future boolean options to\r
130 always have the same default regardless of other settings.\r
131 \r
132 If people are happy with setting a notmuch_bool_t to -1 (for unset) then\r
133 that is definitely the simplest option. Or if people think that default\r
134 boolean options should not depend on other options then we can just add\r
135 this hack for --entire-thread.\r
136 \r
137 Best wishes\r
138 \r
139 Mark\r
140 \r
141 \r