[PATCH v2 1/7] cli: add --duplicate=N option to notmuch search
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", 'O', 0 },
>> { NOTMUCH_OPT_INT, , "limit", 'L', 0 },
>> +{ NOTMUCH_OPT_INT, , "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 at notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 1/7] cli: add --duplicate=N option to notmuch search
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.
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.
> + }
> }
>
> 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", 'O', 0 },
> { NOTMUCH_OPT_INT, , "limit", 'L', 0 },
> + { NOTMUCH_OPT_INT, , "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?
Best wishes
Mark
> break;
> case OUTPUT_TAGS:
> ret = do_search_tags (notmuch, format, query);
> --
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 1/7] cli: add --duplicate=N option to notmuch search
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);
+ }
}
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", 'O', 0 },
{ NOTMUCH_OPT_INT, , "limit", 'L', 0 },
+ { NOTMUCH_OPT_INT, , "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);
break;
case OUTPUT_TAGS:
ret = do_search_tags (notmuch, format, query);
--
1.7.10.4
