Return-Path: <jani@nikula.org>
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 <notmuch@notmuchmail.org>; 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 <notmuch@notmuchmail.org>;
	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 <notmuch@notmuchmail.org>; Mon, 10 Jun 2013 09:49:49 -0700 (PDT)
Received: by mail-la0-f44.google.com with SMTP id er20so5955107lab.3
	for <notmuch@notmuchmail.org>; 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 <multiple recipients>
	(version=TLSv1.2 cipher=RC4-SHA bits=128/128);
	Mon, 10 Jun 2013 09:49:47 -0700 (PDT)
From: Jani Nikula <jani@nikula.org>
To: Mark Walters <markwalters1009@gmail.com>, 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: <cover.1370775663.git.jani@nikula.org>
	<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."
	<notmuch.notmuchmail.org>
List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
List-Archive: <http://notmuchmail.org/pipermail/notmuch>
List-Post: <mailto:notmuch@notmuchmail.org>
List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=subscribe>
X-List-Received-Date: Mon, 10 Jun 2013 16:49:58 -0000

On Sun, 09 Jun 2013, Mark Walters <markwalters1009@gmail.com> 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 <jani@nikula.org> 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