From: Jani Nikula Date: Mon, 10 Jun 2013 16:49:42 +0000 (+0300) Subject: Re: [PATCH v2 1/7] cli: add --duplicate=N option to notmuch search X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=9fea0484914a2d49a179b02adbdcb520e1631825;p=notmuch-archives.git Re: [PATCH v2 1/7] cli: add --duplicate=N option to notmuch search --- diff --git a/02/a1e9834fcd03be33e93ac01d0b21a29be763f1 b/02/a1e9834fcd03be33e93ac01d0b21a29be763f1 new file mode 100644 index 000000000..f07ec7281 --- /dev/null +++ b/02/a1e9834fcd03be33e93ac01d0b21a29be763f1 @@ -0,0 +1,193 @@ +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 1AA98431FD0 + for ; Mon, 10 Jun 2013 09:49:58 -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 sQKeSvw9-uro for ; + Mon, 10 Jun 2013 09:49:50 -0700 (PDT) +Received: from mail-la0-f44.google.com (mail-la0-f44.google.com + [209.85.215.44]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id AD37B431FB6 + for ; Mon, 10 Jun 2013 09:49:49 -0700 (PDT) +Received: by mail-la0-f44.google.com with SMTP id er20so5955107lab.3 + for ; Mon, 10 Jun 2013 09:49:48 -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=Al9+cOvOAtoI9T1o2JzSTk7hTeu0+naIAmYz9Igf4tg=; + b=KyO0D5DTmgFBoVk4E/x3k9xwpPycvt46GR4jTnkC4w/02CzQr+jMlVz3SPKqg5JcEL + U2gb4sZ1i46YkZmH4De7jgExznQVVtJdzzgGS3mEdc7FH/JbeLq0YWmV5GnrvRs/i9TR + TR+JaHgeSm2G47igqh1oDrie9VZOLp3DbYXZBzQ3kUbj3OtXjx/oLA4JhGBxcvyHu0lK + 4ZHhrgTJG6A78WmQRZOWFvHSAj27+X8QBNPRBRoiuVhqi6cIJWbIuw5fcF3kKBuzbE8j + VKMN8pLJ3Qg/pbA/mxxX0Jhsgw+HQnnkBzzK8+WmjCbh9bQi1lTnT0r7SQUBy8LoFOsh + JS1g== +X-Received: by 10.152.120.35 with SMTP id kz3mr5432918lab.55.1370882988163; + Mon, 10 Jun 2013 09:49:48 -0700 (PDT) +Received: from localhost (dsl-hkibrasgw2-58c376-211.dhcp.inet.fi. + [88.195.118.211]) + by mx.google.com with ESMTPSA id x5sm5977938lbx.8.2013.06.10.09.49.46 + for + (version=TLSv1.2 cipher=RC4-SHA bits=128/128); + Mon, 10 Jun 2013 09:49:47 -0700 (PDT) +From: Jani Nikula +To: Mark Walters , notmuch@notmuchmail.org +Subject: Re: [PATCH v2 1/7] cli: add --duplicate=N option to notmuch search +In-Reply-To: <871u8b11bh.fsf@qmul.ac.uk> +References: + <61ed86f221d65b4dba438cbc2b4c5b77a484a534.1370775663.git.jani@nikula.org> + <871u8b11bh.fsf@qmul.ac.uk> +User-Agent: Notmuch/0.15.2+179~g8952790 (http://notmuchmail.org) Emacs/24.3.1 + (x86_64-pc-linux-gnu) +Date: Mon, 10 Jun 2013 19:49:42 +0300 +Message-ID: <87ppvt6hcp.fsf@nikula.org> +MIME-Version: 1.0 +Content-Type: text/plain +X-Gm-Message-State: + ALoCoQmZUbdJ1BbvCmTk3yBlVq1+CXTstxdkeNwhMOz0lBs0cKov6Z0fCEoZVq1MXEw1zivZY+jA +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: Mon, 10 Jun 2013 16:49:58 -0000 + +On Sun, 09 Jun 2013, Mark Walters wrote: +> Overall I like this series and am happy to give it a +1 as is but have a +> few comments which might be worth considering. +> +> Is the order of filenames clear? eg is it the order that notmuch new met +> them? In particular is duplicate=1 the oldest and duplicate=N the +> newest? If so that might be worth mentioning in the manpage. + +AFAICT it's the order in which notmuch new encountered them. Which may +change if the user rebuilds the database. Which is why I intentionally +avoided making any promises about what the numbers mean. + +> +> On Sun, 09 Jun 2013, Jani Nikula wrote: +>> Effective with --output=files, output the Nth filename associated with +>> each message matching the query (N is 0-based). If N is equal to or +>> greater than the number of files associated with the message, don't +>> print anything. +>> --- +>> notmuch-search.c | 18 ++++++++++++------ +>> 1 file changed, 12 insertions(+), 6 deletions(-) +>> +>> diff --git a/notmuch-search.c b/notmuch-search.c +>> index 4323201..196934b 100644 +>> --- a/notmuch-search.c +>> +++ b/notmuch-search.c +>> @@ -177,7 +177,8 @@ do_search_messages (sprinter_t *format, +>> notmuch_query_t *query, +>> output_t output, +>> int offset, +>> - int limit) +>> + int limit, +>> + int dupe) +>> { +>> notmuch_message_t *message; +>> notmuch_messages_t *messages; +>> @@ -206,14 +207,17 @@ do_search_messages (sprinter_t *format, +>> message = notmuch_messages_get (messages); +>> +>> if (output == OUTPUT_FILES) { +>> + int j; +>> filenames = notmuch_message_get_filenames (message); +>> +>> - for (; +>> + for (j = 1; +>> notmuch_filenames_valid (filenames); +>> - notmuch_filenames_move_to_next (filenames)) +>> + notmuch_filenames_move_to_next (filenames), j++) +>> { +>> - format->string (format, notmuch_filenames_get (filenames)); +>> - format->separator (format); +>> + if (dupe < 0 || dupe == j) { +>> + format->string (format, notmuch_filenames_get (filenames)); +>> + format->separator (format); +> +> Is it deliberate that dupe == 0 is not covered? If my newest oldest +> thing above is correct then maybe dupe == 0 could be the all option +N +> the Nth oldest and -N the Nth newest. This may be not-trivial enough +> it's not worth doing. + +See my answer above. We can do this later if we decide it's worth the +trouble. + +I don't check for 0 because it doesn't match anything. Similarly for +values < 0. + +> +>> + } +>> } +>> +>> notmuch_filenames_destroy( filenames ); +>> @@ -303,6 +307,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[]) +>> int offset = 0; +>> int limit = -1; /* unlimited */ +>> int exclude = EXCLUDE_TRUE; +>> + int dupe = -1; +>> unsigned int i; +>> +>> enum { +>> @@ -339,6 +344,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[]) +>> { 0, 0 } } }, +>> { NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 }, +>> { NOTMUCH_OPT_INT, &limit, "limit", 'L', 0 }, +>> + { NOTMUCH_OPT_INT, &dupe, "duplicate", 'D', 0 }, +>> { 0, 0, 0, 0, 0 } +>> }; +>> +>> @@ -424,7 +430,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[]) +>> break; +>> case OUTPUT_MESSAGES: +>> case OUTPUT_FILES: +>> - ret = do_search_messages (format, query, output, offset, limit); +>> + ret = do_search_messages (format, query, output, offset, limit, dupe); +> +> Should there be an error message if duplicate=x is chosen with +> output!=files? + +I avoided adding checks upon checks, complicating the code, because +there's no harm in allowing it. Matter of taste I suppose. + +Thanks for your comments. + +BR, +Jani. + + +> +> Best wishes +> +> Mark +> +> +>> break; +>> case OUTPUT_TAGS: +>> ret = do_search_tags (notmuch, format, query); +>> -- +>> 1.7.10.4 +>> +>> _______________________________________________ +>> notmuch mailing list +>> notmuch@notmuchmail.org +>> http://notmuchmail.org/mailman/listinfo/notmuch