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