--- /dev/null
+Return-Path: <jani@nikula.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id E78AD431FBC\r
+ for <notmuch@notmuchmail.org>; Wed, 17 Oct 2012 00:48:22 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+ tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id 0n5Fn-4p70ac for <notmuch@notmuchmail.org>;\r
+ Wed, 17 Oct 2012 00:48:22 -0700 (PDT)\r
+Received: from mail-qc0-f181.google.com (mail-qc0-f181.google.com\r
+ [209.85.216.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+ (No client certificate requested)\r
+ by olra.theworths.org (Postfix) with ESMTPS id 44366431FB6\r
+ for <notmuch@notmuchmail.org>; Wed, 17 Oct 2012 00:48:22 -0700 (PDT)\r
+Received: by mail-qc0-f181.google.com with SMTP id x40so6567779qcp.26\r
+ for <notmuch@notmuchmail.org>; Wed, 17 Oct 2012 00:48:20 -0700 (PDT)\r
+X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
+ d=google.com; s=20120113;\r
+ h=from:to:subject:in-reply-to:references:user-agent:date:message-id\r
+ :mime-version:content-type:x-gm-message-state;\r
+ bh=rcMSW4IVgQikEb1FO4OYRTnQESF27tEKIpBqneVGwSs=;\r
+ b=DGSBMffJuxvkQ/ym4AhcKMwEDsBRn/Vyltfy4iPIR5UVOL0jrmw8o9wQS4ujBttgnG\r
+ tufFc/BWfgf4+rH27xWShf23T8P/kZDo0WwB06eRkZ+E2MPT0vmaPdl0OcsM8fvOC8Er\r
+ zNDm8wdwvsbizf/MFzdTkHOeSBsmJDLSXs3nwo3YqLZ9YVHjATXjYlkwJAcBEr4fk0as\r
+ oQVJjTKV9r7PEZhN7YHV0RLU0AfEIQnk1TuZQKWv411CjERSWj03Fr/BNNAY1TmMtXKM\r
+ 56M8+VxNFJcOOdAhmlzF/oM1m+t6cNRHIdJQb6gHwZr2hjqhe3URMWjlP8BdCT40F1Zp\r
+ E+yg==\r
+Received: by 10.49.104.194 with SMTP id gg2mr40693359qeb.6.1350460100482;\r
+ Wed, 17 Oct 2012 00:48:20 -0700 (PDT)\r
+Received: from localhost ([2001:4b98:dc0:43:216:3eff:fe1b:25f3])\r
+ by mx.google.com with ESMTPS id z9sm19122876qeg.9.2012.10.17.00.48.18\r
+ (version=SSLv3 cipher=OTHER); Wed, 17 Oct 2012 00:48:19 -0700 (PDT)\r
+From: Jani Nikula <jani@nikula.org>\r
+To: Ethan Glasser-Camp <ethan.glasser.camp@gmail.com>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v4 2/9] parse-time-string: add a date/time parser to\r
+ notmuch\r
+In-Reply-To: <87hapw9x99.fsf@betacantrips.com>\r
+References: <cover.1350164594.git.jani@nikula.org>\r
+ <0296be2a3899653549b3f95e1aa1a4a0632e92e7.1350164594.git.jani@nikula.org>\r
+ <87hapw9x99.fsf@betacantrips.com>\r
+User-Agent: Notmuch/0.14+39~ge21970d (http://notmuchmail.org) Emacs/23.2.1\r
+ (x86_64-pc-linux-gnu)\r
+Date: Wed, 17 Oct 2012 09:48:16 +0200\r
+Message-ID: <87zk3lzghr.fsf@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+X-Gm-Message-State:\r
+ ALoCoQlJ+XpqkOUcxHVoTc84rNg1HROel7p7i8cVbuHiF0cxJMs1+alvTXsKoYyiGc69v6WJcNWs\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Wed, 17 Oct 2012 07:48:23 -0000\r
+\r
+On Mon, 15 Oct 2012, Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> wrote:\r
+>> +/* Parse a previously postponed number if one exists. */\r
+>> +static int parse_postponed_number (struct state *state, int v, int n, char d);\r
+>> +static int\r
+>> +handle_postponed_number (struct state *state, enum field next_field)\r
+>> +{\r
+>> + int v = state->postponed_value;\r
+>> + int n = state->postponed_length;\r
+>> + char d = state->postponed_delim;\r
+>> + int r;\r
+>> +\r
+>> + if (!n)\r
+>> + return 0;\r
+>> +\r
+>> + state->postponed_value = 0;\r
+>> + state->postponed_length = 0;\r
+>> + state->postponed_delim = 0;\r
+>\r
+> This could be refactored to be a call to get_postponed_number. Also,\r
+> I'd prefer parse_postponed_number be up here, closer to its sole\r
+> caller (handle_postponed_number).\r
+\r
+I decided to nuke the intermediate handle_postponed_number altogether,\r
+and fix parse_postponed_number to call get_postponed_number. Thanks for\r
+pointing this out.\r
+\r
+>> +/*\r
+>> + * Postpone a number to be handled later. If one exists already,\r
+>> + * handle it first. n may be -1 to indicate a keyword that has no\r
+>> + * number length.\r
+>> + */\r
+>> +static int\r
+>> +set_postponed_number (struct state *state, int v, int n)\r
+>> +{\r
+>> + int r;\r
+>> + char d = state->delim;\r
+>> +\r
+>> + /* Parse a previously postponed number, if any. */\r
+>> + r = handle_postponed_number (state, TM_NONE);\r
+>> + if (r)\r
+>> + return r;\r
+>\r
+> I would love a comment explaining under what circumstances this could\r
+> occur and what the caller is expected to do.\r
+\r
+Any errors anywhere the caller is expected to pop up all the way to the\r
+main entry point. I did not verify, but, for example, I'd expect a\r
+sequence of "2012 2012 2012" to fail right here.\r
+\r
+>> +/*\r
+>> + * Accepted keywords.\r
+>> + */\r
+>> +static struct keyword keywords[] = {\r
+>> + /* Weekdays. */\r
+>> + { N_("sun|day"), TM_ABS_WDAY, 0, NULL },\r
+>> + { N_("mon|day"), TM_ABS_WDAY, 1, NULL },\r
+>\r
+> Maybe it's just my history with Python, but I'd prefer keywords, which\r
+> is a global and a constant, to be written in all caps (KEYWORDS).\r
+\r
+It's just your history with Python. ;) IMO it's more in line with\r
+notmuch coding style as it is. It could be made const though.\r
+\r
+>> +/*\r
+>> + * Compare strings s and keyword. Return number of matching chars on\r
+>> + * match, 0 for no match. Match must be at least n chars, or all of\r
+>> + * keyword if n < 0, otherwise it's not a match. Use match_case for\r
+>> + * case sensitive matching.\r
+>> + */\r
+>> +static size_t\r
+>> +stringcmp (const char *s, const char *keyword, ssize_t n, bool match_case)\r
+>> +{\r
+>\r
+> The name of this function makes it look uncomfortably like strcmp(3),\r
+> which has a very different calling semantics (specifically the -1, 0, 1\r
+> return value). I'd prefer a name like string_match_keyword.\r
+\r
+Agreed.\r
+\r
+\r
+BR,\r
+Jani.\r
projects / notmuch-archives.git / commitdiff