unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* output file argument to notmuch dump.
@ 2011-10-07  0:20 David Bremner
  2011-10-07  0:37 ` Dmitry Kurochkin
                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: David Bremner @ 2011-10-07  0:20 UTC (permalink / raw)
  To: notmuch

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]


I'd like to add a search term argument to notmuch dump (see
id:"87wrcijn1w.fsf@zancas.localnet" and followup for context). The
"notmuch" way would be to have

    notmuch dump <search-term>

do the right thing, and that is easy enough _except_ that 

   notmuch dump foo

writes the output to file foo.

I see several options.

1) just delete the output file option from notmuch-dump, and use shell
   redirection. So far I don't see a non-contrived example when writing
   an output file directly is useful, but maybe that is just a failure
   of imagination.

2) delete the output file option and a global "--write" option that 
   calls freopen(... stdout). As a bonus(?) this would work for any 
   notmuch command.

3) leave the output option alone and implement a --query=foo option for 
   notmuch dump. This would be the most backward compatible of course,
   but also confusing for users.

Thoughts?

d



[-- Attachment #2: Type: application/pgp-signature, Size: 315 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-07  0:20 output file argument to notmuch dump David Bremner
@ 2011-10-07  0:37 ` Dmitry Kurochkin
  2011-10-07  0:53   ` David Bremner
  2011-10-07  1:14 ` Jameson Graef Rollins
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 23+ messages in thread
From: Dmitry Kurochkin @ 2011-10-07  0:37 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

On Thu, 06 Oct 2011 21:20:40 -0300, David Bremner <bremner@unb.ca> wrote:
> 
> I'd like to add a search term argument to notmuch dump (see
> id:"87wrcijn1w.fsf@zancas.localnet" and followup for context). The
> "notmuch" way would be to have
> 
>     notmuch dump <search-term>
> 
> do the right thing, and that is easy enough _except_ that 
> 
>    notmuch dump foo
> 
> writes the output to file foo.
> 
> I see several options.
> 
> 1) just delete the output file option from notmuch-dump, and use shell
>    redirection. So far I don't see a non-contrived example when writing
>    an output file directly is useful, but maybe that is just a failure
>    of imagination.
> 
> 2) delete the output file option and a global "--write" option that 
>    calls freopen(... stdout). As a bonus(?) this would work for any 
>    notmuch command.
> 
> 3) leave the output option alone and implement a --query=foo option for 
>    notmuch dump. This would be the most backward compatible of course,
>    but also confusing for users.
> 
> Thoughts?
> 

IMHO 1[+2] is the way.  It breaks the dump command interface, but would
make it consistent with other commands.  Implementing the second option
(for all commands) is nice but independent and optional.  Also, perhaps
--output, --output-file or smth would a better name than --write.

Regards,
  Dmitry

> d
> 
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-07  0:37 ` Dmitry Kurochkin
@ 2011-10-07  0:53   ` David Bremner
  2011-10-07  1:43     ` Tom Prince
  0 siblings, 1 reply; 23+ messages in thread
From: David Bremner @ 2011-10-07  0:53 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 603 bytes --]

On Fri, 07 Oct 2011 04:37:56 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Thu, 06 Oct 2011 21:20:40 -0300, David Bremner <bremner@unb.ca> wrote:

> IMHO 1[+2] is the way.  It breaks the dump command interface, but would
> make it consistent with other commands.  Implementing the second option
> (for all commands) is nice but independent and optional.  Also, perhaps
> --output, --output-file or smth would a better name than --write.

I'm not fussy about the name, except that --output is taken, and I
thought there might be some benefit of making options have unique prefixes.

d

[-- Attachment #2: Type: application/pgp-signature, Size: 315 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-07  0:20 output file argument to notmuch dump David Bremner
  2011-10-07  0:37 ` Dmitry Kurochkin
@ 2011-10-07  1:14 ` Jameson Graef Rollins
  2011-10-07 10:23 ` Jesse Rosenthal
  2011-10-09 16:01 ` David Bremner
  3 siblings, 0 replies; 23+ messages in thread
From: Jameson Graef Rollins @ 2011-10-07  1:14 UTC (permalink / raw)
  To: David Bremner, notmuch

[-- Attachment #1: Type: text/plain, Size: 843 bytes --]

On Thu, 06 Oct 2011 21:20:40 -0300, David Bremner <bremner@unb.ca> wrote:
> I see several options.
> 
> 1) just delete the output file option from notmuch-dump, and use shell
>    redirection. So far I don't see a non-contrived example when writing
>    an output file directly is useful, but maybe that is just a failure
>    of imagination.
> 
> 2) delete the output file option and a global "--write" option that 
>    calls freopen(... stdout). As a bonus(?) this would work for any 
>    notmuch command.

I strongly support either of these two options (they're basically the
same as far as I'm concerned).  The consistency that this would provide
with rest of the notmuch commands alone kind of makes this a no-brainer
in my opinion.  And I certainly don't see this particular CLI breakage
as an issue either.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-07  0:53   ` David Bremner
@ 2011-10-07  1:43     ` Tom Prince
  0 siblings, 0 replies; 23+ messages in thread
From: Tom Prince @ 2011-10-07  1:43 UTC (permalink / raw)
  To: David Bremner, Dmitry Kurochkin; +Cc: notmuch

On Thu, 06 Oct 2011 21:53:57 -0300, David Bremner <bremner@unb.ca> wrote:
> On Fri, 07 Oct 2011 04:37:56 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > On Thu, 06 Oct 2011 21:20:40 -0300, David Bremner <bremner@unb.ca> wrote:
> 
> > IMHO 1[+2] is the way.  It breaks the dump command interface, but would
> > make it consistent with other commands.  Implementing the second option
> > (for all commands) is nice but independent and optional.  Also, perhaps
> > --output, --output-file or smth would a better name than --write.
> 
> I'm not fussy about the name, except that --output is taken, and I
> thought there might be some benefit of making options have unique
> prefixes.

Is -o/--output taken before the subcommand? i.e.
notmuch -o <dumpfile> dump <query> <terms>
Or would that be to confusing?

(-o is fairly standard for stdout redirection, so it would be nice to
use that for notmuch, as well.)

  Tom

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-07  0:20 output file argument to notmuch dump David Bremner
  2011-10-07  0:37 ` Dmitry Kurochkin
  2011-10-07  1:14 ` Jameson Graef Rollins
@ 2011-10-07 10:23 ` Jesse Rosenthal
  2011-10-07 11:15   ` Tomi Ollila
  2011-10-09 16:01 ` David Bremner
  3 siblings, 1 reply; 23+ messages in thread
From: Jesse Rosenthal @ 2011-10-07 10:23 UTC (permalink / raw)
  To: David Bremner, notmuch

On Thu, 06 Oct 2011 21:20:40 -0300, David Bremner <bremner@unb.ca> wrote:
> 1) just delete the output file option from notmuch-dump, and use shell
>    redirection. So far I don't see a non-contrived example when writing
>    an output file directly is useful, but maybe that is just a failure
>    of imagination.
> 
> 2) delete the output file option and a global "--write" option that 
>    calls freopen(... stdout). As a bonus(?) this would work for any 
>    notmuch command.

Either of these seem like the right approach to me. I might favor (1)
just a bit so I don't have to choose between different ways of doing the
same thing, but that's a small point. As Jamie said, they're pretty
much the same.

--Jesse

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-07 10:23 ` Jesse Rosenthal
@ 2011-10-07 11:15   ` Tomi Ollila
  2011-10-07 17:22     ` Jameson Graef Rollins
  0 siblings, 1 reply; 23+ messages in thread
From: Tomi Ollila @ 2011-10-07 11:15 UTC (permalink / raw)
  To: Jesse Rosenthal; +Cc: David Bremner, notmuch

On Fri 07 Oct 2011 13:23, Jesse Rosenthal <jrosenthal@jhu.edu> writes:

> On Thu, 06 Oct 2011 21:20:40 -0300, David Bremner <bremner@unb.ca> wrote:
>> 1) just delete the output file option from notmuch-dump, and use shell
>>    redirection. So far I don't see a non-contrived example when writing
>>    an output file directly is useful, but maybe that is just a failure
>>    of imagination.
>> 
>> 2) delete the output file option and a global "--write" option that 
>>    calls freopen(... stdout). As a bonus(?) this would work for any 
>>    notmuch command.
>
> Either of these seem like the right approach to me. I might favor (1)
> just a bit so I don't have to choose between different ways of doing the
> same thing, but that's a small point. As Jamie said, they're pretty
> much the same.

The option 2, i.e. optional --write (or -o|--output) should be available;
someone may run notmuch without using environment that provides
redirections (or it is just plain simpler to give the command line option
instead of doing redirections).

> --Jesse

Tomi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-07 11:15   ` Tomi Ollila
@ 2011-10-07 17:22     ` Jameson Graef Rollins
  2011-10-10  7:32       ` Tomi Ollila
  0 siblings, 1 reply; 23+ messages in thread
From: Jameson Graef Rollins @ 2011-10-07 17:22 UTC (permalink / raw)
  To: Tomi Ollila, Jesse Rosenthal; +Cc: David Bremner, notmuch

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On Fri, 07 Oct 2011 14:15:39 +0300, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> The option 2, i.e. optional --write (or -o|--output) should be available;
> someone may run notmuch without using environment that provides
> redirections (or it is just plain simpler to give the command line option
> instead of doing redirections).

If that were the case then none of the other notmuch commands would work
in the environment either.  If that's a concern, then notmuch should
have a generic --redirect option that works for all commands, as Tom
suggested out earlier.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-07  0:20 output file argument to notmuch dump David Bremner
                   ` (2 preceding siblings ...)
  2011-10-07 10:23 ` Jesse Rosenthal
@ 2011-10-09 16:01 ` David Bremner
  2011-10-10 13:49   ` david
                     ` (2 more replies)
  3 siblings, 3 replies; 23+ messages in thread
From: David Bremner @ 2011-10-09 16:01 UTC (permalink / raw)
  To: notmuch

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

On Thu, 06 Oct 2011 21:20:40 -0300, David Bremner <bremner@unb.ca> wrote:
> 
> I'd like to add a search term argument to notmuch dump (see
> id:"87wrcijn1w.fsf@zancas.localnet" and followup for context). The
> "notmuch" way would be to have
> 
>     notmuch dump <search-term>
> 
> do the right thing

Another option occured to me that is consistent at least with notmuch
tag and notmuch show would be to support the following transitional
syntaxes

        notmuch dump file
        notmuch dump file [--] search terms
        notmuch dump -- search terms

the first two could then be deprecated, and eventually the syntax

    notmuch dump search terms 

could be enabled.

the question of whether to support 

    notmuch dump --file foo.txt

or something like 

   notmuch --stdout=foo.txt dump

could be dealt with later.

David

[-- Attachment #2: Type: application/pgp-signature, Size: 315 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-07 17:22     ` Jameson Graef Rollins
@ 2011-10-10  7:32       ` Tomi Ollila
  0 siblings, 0 replies; 23+ messages in thread
From: Tomi Ollila @ 2011-10-10  7:32 UTC (permalink / raw)
  To: Jameson Graef Rollins; +Cc: Tomi Ollila, David Bremner, notmuch

On Fri 07 Oct 2011 20:22, Jameson Graef Rollins <jrollins@finestructure.net> writes:

> On Fri, 07 Oct 2011 14:15:39 +0300, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>> The option 2, i.e. optional --write (or -o|--output) should be available;
>> someone may run notmuch without using environment that provides
>> redirections (or it is just plain simpler to give the command line option
>> instead of doing redirections).
>
> If that were the case then none of the other notmuch commands would work
> in the environment either.  If that's a concern, then notmuch should
> have a generic --redirect option that works for all commands, as Tom
> suggested out earlier.

Good point. I Agree. So, no spesific --write|-o|--output for dump is my
current vote.

>
> jamie.

Tomi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* (no subject)
  2011-10-09 16:01 ` David Bremner
@ 2011-10-10 13:49   ` david
  2011-10-10 13:49     ` [PATCH 1/6] test: update dump-restore to use redirection instead of filename args david
                       ` (6 more replies)
  2011-10-10 13:59   ` output file argument to notmuch dump Tomi Ollila
  2011-10-10 16:41   ` Jameson Graef Rollins
  2 siblings, 7 replies; 23+ messages in thread
From: david @ 2011-10-10 13:49 UTC (permalink / raw)
  To: notmuch

OK, here is my proposal to add search terms to notmuch dump.

Most of the work is in argument processing. It would be nice if we
could factor some of that out.

02be821 notmuch-dump: deprecate use of output file argument.          
2b7781d test: all dump-restore tests should be working now
7a203d6 notmuch-dump: treat any remaining arguments after the filename as search t
be762d9 notmuch-dump: update handling of file name argument
d6715d7 test: add tests for command line arguments to notmuch-dump
08e76cc test: update dump-restore to use redirection instead of filename args

 notmuch-dump.c    |   37 ++++++++++++++++++++++++++-----------
 test/dump-restore |   39 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 60 insertions(+), 16 deletions(-)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/6] test: update dump-restore to use redirection instead of filename args
  2011-10-10 13:49   ` david
@ 2011-10-10 13:49     ` david
  2011-10-10 13:49     ` [PATCH 2/6] test: add tests for command line arguments to notmuch-dump david
                       ` (5 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: david @ 2011-10-10 13:49 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

The idea here is that we want to deprecate the use of arguments to
dump and restore to specify paths, since in particular we want to use
the non-option arguments to dump to form a query.
---
 test/dump-restore |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index a4de370..96c4f19 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -6,17 +6,17 @@ add_email_corpus
 
 test_expect_success "Dumping all tags" "generate_message &&
 notmuch new &&
-notmuch dump dump.expected"
+notmuch dump > dump.expected"
 
 test_begin_subtest "Clearing all tags"
 sed -e "s/(\([^(]*\))$/()/" < dump.expected > clear.expected
-notmuch restore clear.expected
-notmuch dump clear.actual
+notmuch restore < clear.expected
+notmuch dump > clear.actual
 test_expect_equal "$(< clear.actual)" "$(< clear.expected)"
 
 test_begin_subtest "Restoring original tags"
-notmuch restore dump.expected
-notmuch dump dump.actual
+notmuch restore < dump.expected
+notmuch dump > dump.actual
 test_expect_equal "$(< dump.actual)" "$(< dump.expected)"
 
 test_expect_success "Restore with nothing to do" "notmuch restore dump.expected"
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/6] test: add tests for command line arguments to notmuch-dump
  2011-10-10 13:49   ` david
  2011-10-10 13:49     ` [PATCH 1/6] test: update dump-restore to use redirection instead of filename args david
@ 2011-10-10 13:49     ` david
  2011-10-10 13:49     ` [PATCH 3/6] notmuch-dump: update handling of file name argument david
                       ` (4 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: david @ 2011-10-10 13:49 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

The plan is to add the possibility of search terms after the file name,
and the use of -- to stop looking for an output file name.
---
 test/dump-restore |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index 96c4f19..699337c 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -8,6 +8,34 @@ test_expect_success "Dumping all tags" "generate_message &&
 notmuch new &&
 notmuch dump > dump.expected"
 
+test_begin_subtest "dump outfile"
+notmuch dump dump-outfile.actual
+test_expect_equal_file dump.expected dump-outfile.actual
+
+test_begin_subtest "dump outfile --"
+notmuch dump dump-1-arg-dash.actual
+test_expect_equal_file dump.expected dump-1-arg-dash.actual
+
+# Note, we assume all messages from cworth have a message-id
+# containing cworth.org
+
+grep cworth\.org dump.expected > dump-cworth.expected
+
+test_begin_subtest "dump -- from:cworth"
+test_subtest_known_broken
+notmuch dump -- from:cworth > dump-dash-cworth.actual
+test_expect_equal_file dump-cworth.expected dump-dash-cworth.actual
+
+test_begin_subtest "dump outfile from:cworth"
+test_subtest_known_broken
+notmuch dump dump-outfile-cworth.actual from:cworth
+test_expect_equal_file dump-cworth.expected dump-outfile-cworth.actual
+
+test_begin_subtest "dump outfile -- from:cworth"
+test_subtest_known_broken
+notmuch dump dump-outfile-dash-inbox.actual -- from:cworth
+test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual
+
 test_begin_subtest "Clearing all tags"
 sed -e "s/(\([^(]*\))$/()/" < dump.expected > clear.expected
 notmuch restore < clear.expected
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/6] notmuch-dump: update handling of file name argument
  2011-10-10 13:49   ` david
  2011-10-10 13:49     ` [PATCH 1/6] test: update dump-restore to use redirection instead of filename args david
  2011-10-10 13:49     ` [PATCH 2/6] test: add tests for command line arguments to notmuch-dump david
@ 2011-10-10 13:49     ` david
  2011-10-10 13:49     ` [PATCH 4/6] notmuch-dump: treat any remaining arguments after the filename as search terms david
                       ` (3 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: david @ 2011-10-10 13:49 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

We permit -- as an "option processing terminator".

Currently this does not do anything useful, but we plan to add
search terms after the --.
---
 notmuch-dump.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/notmuch-dump.c b/notmuch-dump.c
index 7e7bc17..610144e 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -26,7 +26,7 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
     notmuch_config_t *config;
     notmuch_database_t *notmuch;
     notmuch_query_t *query;
-    FILE *output;
+    FILE *output = stdout;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
     notmuch_tags_t *tags;
@@ -40,23 +40,28 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
     if (notmuch == NULL)
 	return 1;
 
-    query = notmuch_query_create (notmuch, "");
-    if (query == NULL) {
-	fprintf (stderr, "Out of memory\n");
-	return 1;
-    }
-    notmuch_query_set_sort (query, NOTMUCH_SORT_MESSAGE_ID);
-
-    if (argc) {
+    if (argc && strcmp (argv[0], "--") != 0) {
 	output = fopen (argv[0], "w");
 	if (output == NULL) {
 	    fprintf (stderr, "Error opening %s for writing: %s\n",
 		     argv[0], strerror (errno));
 	    return 1;
 	}
-    } else {
-	output = stdout;
+	argc--;
+	argv++;
+    }
+
+    if (argc && strcmp (argv[0], "--") == 0){
+	argc--;
+	argv++;
+    }
+
+    query = notmuch_query_create (notmuch, "");
+    if (query == NULL) {
+	fprintf (stderr, "Out of memory\n");
+	return 1;
     }
+    notmuch_query_set_sort (query, NOTMUCH_SORT_MESSAGE_ID);
 
     for (messages = notmuch_query_search_messages (query);
 	 notmuch_messages_valid (messages);
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 4/6] notmuch-dump: treat any remaining arguments after the filename as search terms
  2011-10-10 13:49   ` david
                       ` (2 preceding siblings ...)
  2011-10-10 13:49     ` [PATCH 3/6] notmuch-dump: update handling of file name argument david
@ 2011-10-10 13:49     ` david
  2011-10-10 13:49     ` [PATCH 5/6] test: all dump-restore tests should be working now david
                       ` (2 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: david @ 2011-10-10 13:49 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

The main motivation here is allow the fast dumping of tag data for
messages having certain tags.  In practice it seems too slow to pipe
dump to grep.
---
 notmuch-dump.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/notmuch-dump.c b/notmuch-dump.c
index 610144e..ac5d074 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -30,6 +30,7 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
     notmuch_messages_t *messages;
     notmuch_message_t *message;
     notmuch_tags_t *tags;
+    const char* query_str = "";
 
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
@@ -56,7 +57,15 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
 	argv++;
     }
 
-    query = notmuch_query_create (notmuch, "");
+    if (argc) {
+	query_str = query_string_from_args (notmuch, argc, argv);
+	if (query_str == NULL) {
+	    fprintf (stderr, "Out of memory.\n");
+	    return 1;
+	}
+    }
+ 
+    query = notmuch_query_create (notmuch, query_str);
     if (query == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return 1;
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 5/6] test: all dump-restore tests should be working now
  2011-10-10 13:49   ` david
                       ` (3 preceding siblings ...)
  2011-10-10 13:49     ` [PATCH 4/6] notmuch-dump: treat any remaining arguments after the filename as search terms david
@ 2011-10-10 13:49     ` david
  2011-10-10 13:49     ` [PATCH 6/6] notmuch-dump: deprecate use of output file argument david
  2011-10-16 20:34     ` Thomas Schwinge
  6 siblings, 0 replies; 23+ messages in thread
From: david @ 2011-10-10 13:49 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

3 x test_subtest_known_broken removed to tidy up the output.
---
 test/dump-restore |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index 699337c..d8db259 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -22,17 +22,14 @@ test_expect_equal_file dump.expected dump-1-arg-dash.actual
 grep cworth\.org dump.expected > dump-cworth.expected
 
 test_begin_subtest "dump -- from:cworth"
-test_subtest_known_broken
 notmuch dump -- from:cworth > dump-dash-cworth.actual
 test_expect_equal_file dump-cworth.expected dump-dash-cworth.actual
 
 test_begin_subtest "dump outfile from:cworth"
-test_subtest_known_broken
 notmuch dump dump-outfile-cworth.actual from:cworth
 test_expect_equal_file dump-cworth.expected dump-outfile-cworth.actual
 
 test_begin_subtest "dump outfile -- from:cworth"
-test_subtest_known_broken
 notmuch dump dump-outfile-dash-inbox.actual -- from:cworth
 test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual
 
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 6/6] notmuch-dump: deprecate use of output file argument.
  2011-10-10 13:49   ` david
                       ` (4 preceding siblings ...)
  2011-10-10 13:49     ` [PATCH 5/6] test: all dump-restore tests should be working now david
@ 2011-10-10 13:49     ` david
  2011-10-16 20:34     ` Thomas Schwinge
  6 siblings, 0 replies; 23+ messages in thread
From: david @ 2011-10-10 13:49 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

We print an intentionally non-specific message on stderr, since it
isn't clear if there will be some global output file argument to
replace.

We update the test suite atomically, since it relies on having the
same text in two files.
---
 notmuch-dump.c    |    1 +
 test/dump-restore |    4 ++++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/notmuch-dump.c b/notmuch-dump.c
index ac5d074..409c86a 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -42,6 +42,7 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
 	return 1;
 
     if (argc && strcmp (argv[0], "--") != 0) {
+	fprintf (stderr, "Warning: the output file argument of dump is deprecated.\n");
 	output = fopen (argv[0], "w");
 	if (output == NULL) {
 	    fprintf (stderr, "Error opening %s for writing: %s\n",
diff --git a/test/dump-restore b/test/dump-restore
index d8db259..5f3f0db 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -12,6 +12,10 @@ test_begin_subtest "dump outfile"
 notmuch dump dump-outfile.actual
 test_expect_equal_file dump.expected dump-outfile.actual
 
+test_begin_subtest "dump outfile # deprecated"
+test_expect_equal "Warning: the output file argument of dump is deprecated."\
+  "$(notmuch dump /dev/null 2>&1)"
+
 test_begin_subtest "dump outfile --"
 notmuch dump dump-1-arg-dash.actual
 test_expect_equal_file dump.expected dump-1-arg-dash.actual
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-09 16:01 ` David Bremner
  2011-10-10 13:49   ` david
@ 2011-10-10 13:59   ` Tomi Ollila
  2011-10-10 14:25     ` David Bremner
  2011-10-10 16:41   ` Jameson Graef Rollins
  2 siblings, 1 reply; 23+ messages in thread
From: Tomi Ollila @ 2011-10-10 13:59 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

On Sun 09 Oct 2011 19:01, David Bremner <bremner@unb.ca> writes:

> On Thu, 06 Oct 2011 21:20:40 -0300, David Bremner <bremner@unb.ca> wrote:
>> 
>> I'd like to add a search term argument to notmuch dump (see
>> id:"87wrcijn1w.fsf@zancas.localnet" and followup for context). The
>> "notmuch" way would be to have
>> 
>>     notmuch dump <search-term>
>> 
>> do the right thing
>
> Another option occured to me that is consistent at least with notmuch
> tag and notmuch show would be to support the following transitional
> syntaxes
>
>         notmuch dump file
>         notmuch dump file [--] search terms
>         notmuch dump -- search terms
>
> the first two could then be deprecated, and eventually the syntax
>
>     notmuch dump search terms 
>
> could be enabled.

In this case there would be 2 transitional syntaxes. IMHO 

'notmuch dump <search-terms>' causes least transitional pain.

> the question of whether to support 
>
>     notmuch dump --file foo.txt
>
> or something like 
>
>    notmuch --stdout=foo.txt dump

who about 

notmuch [--output-file=<file>] <command> ...

> could be dealt with later.
>
> David

Tomi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-10 13:59   ` output file argument to notmuch dump Tomi Ollila
@ 2011-10-10 14:25     ` David Bremner
  0 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2011-10-10 14:25 UTC (permalink / raw)
  To: Tomi Ollila; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

On Mon, 10 Oct 2011 16:59:06 +0300, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> 
> In this case there would be 2 transitional syntaxes. IMHO 
> 
> 'notmuch dump <search-terms>' causes least transitional pain.
> 

I don't understand what you mean here. Can you elaborate?

  notmuch dump <search-terms> 

is not a transitional syntax, but rather a new syntax that breaks
existing scripts. 

The advantage of using -- here is that it allows things like

    notmuch dump --format=something-better -- --output

to search for --output (of course the query is then parsed a bit
surprisingly, but this is a different issue.)

d

[-- Attachment #2: Type: application/pgp-signature, Size: 315 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-09 16:01 ` David Bremner
  2011-10-10 13:49   ` david
  2011-10-10 13:59   ` output file argument to notmuch dump Tomi Ollila
@ 2011-10-10 16:41   ` Jameson Graef Rollins
  2011-10-11  2:12     ` David Bremner
  2 siblings, 1 reply; 23+ messages in thread
From: Jameson Graef Rollins @ 2011-10-10 16:41 UTC (permalink / raw)
  To: David Bremner, notmuch

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

On Sun, 09 Oct 2011 13:01:53 -0300, David Bremner <bremner@unb.ca> wrote:
> Another option occured to me that is consistent at least with notmuch
> tag and notmuch show would be to support the following transitional
> syntaxes
> 
>         notmuch dump file
>         notmuch dump file [--] search terms
>         notmuch dump -- search terms
> 
> the first two could then be deprecated, and eventually the syntax
> 
>     notmuch dump search terms 
> 
> could be enabled.

Hey, David.  I think this is a fine approach, Go with the transitional
"--" separator for one release or so, with a deprecation warning, and
then move to the standard form in the release after that.

> the question of whether to support 
> 
>     notmuch dump --file foo.txt
> 
> or something like 
> 
>    notmuch --stdout=foo.txt dump
> 
> could be dealt with later.

I realize the later probably requires more work, since we would have to
replace all print calls with a new function, but I think it's the
cleaner and more elegant solution.  There's no reason to have an output
redirection option for "dump"s and not for any of the other commands.

jamie.



[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: output file argument to notmuch dump.
  2011-10-10 16:41   ` Jameson Graef Rollins
@ 2011-10-11  2:12     ` David Bremner
  0 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2011-10-11  2:12 UTC (permalink / raw)
  To: Jameson Graef Rollins, notmuch

On Mon, 10 Oct 2011 09:41:47 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:

> >    notmuch --stdout=foo.txt dump
> > 
> > could be dealt with later.
> 
> I realize the later probably requires more work, since we would have to
> replace all print calls with a new function, but I think it's the
> cleaner and more elegant solution.  There's no reason to have an output
> redirection option for "dump"s and not for any of the other commands.

As far as I know, freopen(3) makes this a one-liner. I think the hardest
part of this would be figuring out the name of the option ;)

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re:
  2011-10-10 13:49   ` david
                       ` (5 preceding siblings ...)
  2011-10-10 13:49     ` [PATCH 6/6] notmuch-dump: deprecate use of output file argument david
@ 2011-10-16 20:34     ` Thomas Schwinge
  2011-10-16 23:25       ` Re: David Bremner
  6 siblings, 1 reply; 23+ messages in thread
From: Thomas Schwinge @ 2011-10-16 20:34 UTC (permalink / raw)
  To: david; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 2038 bytes --]

Hi!

On Mon, 10 Oct 2011 10:49:15 -0300, david@tethera.net wrote:
> OK, here is my proposal to add search terms to notmuch dump.

Having worked in the same area ;-), I felt competent to review this.  And
I definitely do like David's approach.  The patches look good, with the
following comments:

What's missing is adding (roughly) the same text to the notmuch manpage,
``notmuch help dump'', NEWS file.  These should be added to the
respective patches, for enhance functionality and deprecation of output
filename.

> 2b7781d test: all dump-restore tests should be working now
> 7a203d6 notmuch-dump: treat any remaining arguments after the filename as search t

I would suggest to combine these two into one patch: enhance
implementation (7a203d6) and update the tests (2b7781d) is one unit.

> d6715d7 test: add tests for command line arguments to notmuch-dump

Specifically:

On Mon, 10 Oct 2011 10:49:17 -0300, david@tethera.net wrote:
> The plan is to add the possibility of search terms after the file name,
> and the use of -- to stop looking for an output file name.
> ---
>  test/dump-restore |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/test/dump-restore b/test/dump-restore
> index 96c4f19..699337c 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -8,6 +8,34 @@ test_expect_success "Dumping all tags" "generate_message &&
>  notmuch new &&
>  notmuch dump > dump.expected"
>  
> +test_begin_subtest "dump outfile"
> +notmuch dump dump-outfile.actual
> +test_expect_equal_file dump.expected dump-outfile.actual
> +
> +test_begin_subtest "dump outfile --"
> +notmuch dump dump-1-arg-dash.actual
> +test_expect_equal_file dump.expected dump-1-arg-dash.actual
> 
> [...]

I don't understand the purpose of the second test above.  Was this meant
to be ``notmuch dump dump-1-arg-dash.actual --'' (as suggested by the
description), or ``notmuch dump -- > dump-1-arg-dash.actual''?


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re:
  2011-10-16 20:34     ` Thomas Schwinge
@ 2011-10-16 23:25       ` David Bremner
  0 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2011-10-16 23:25 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: notmuch

On Sun, 16 Oct 2011 22:34:29 +0200, Thomas Schwinge <thomas@schwinge.name> wrote:

> Having worked in the same area ;-), I felt competent to review this.  And
> I definitely do like David's approach.  The patches look good, with the
> following comments:

Thanks for the review.  I pushed a modified version of the series which
I think fixed all of the things you noticed.

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2011-10-16 23:25 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-07  0:20 output file argument to notmuch dump David Bremner
2011-10-07  0:37 ` Dmitry Kurochkin
2011-10-07  0:53   ` David Bremner
2011-10-07  1:43     ` Tom Prince
2011-10-07  1:14 ` Jameson Graef Rollins
2011-10-07 10:23 ` Jesse Rosenthal
2011-10-07 11:15   ` Tomi Ollila
2011-10-07 17:22     ` Jameson Graef Rollins
2011-10-10  7:32       ` Tomi Ollila
2011-10-09 16:01 ` David Bremner
2011-10-10 13:49   ` david
2011-10-10 13:49     ` [PATCH 1/6] test: update dump-restore to use redirection instead of filename args david
2011-10-10 13:49     ` [PATCH 2/6] test: add tests for command line arguments to notmuch-dump david
2011-10-10 13:49     ` [PATCH 3/6] notmuch-dump: update handling of file name argument david
2011-10-10 13:49     ` [PATCH 4/6] notmuch-dump: treat any remaining arguments after the filename as search terms david
2011-10-10 13:49     ` [PATCH 5/6] test: all dump-restore tests should be working now david
2011-10-10 13:49     ` [PATCH 6/6] notmuch-dump: deprecate use of output file argument david
2011-10-16 20:34     ` Thomas Schwinge
2011-10-16 23:25       ` Re: David Bremner
2011-10-10 13:59   ` output file argument to notmuch dump Tomi Ollila
2011-10-10 14:25     ` David Bremner
2011-10-10 16:41   ` Jameson Graef Rollins
2011-10-11  2:12     ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).