From: Thomas Schwinge <thomas@schwinge.name>
To: david@tethera.net
Subject: Re:
In-Reply-To: <1318254561-25386-1-git-send-email-david@tethera.net>
References: <8739f2qhla.fsf@zancas.localnet>
	<1318254561-25386-1-git-send-email-david@tethera.net>
User-Agent: Notmuch/0.9-101-g81dad07 (http://notmuchmail.org) Emacs/23.3.1
	(i486-pc-linux-gnu)
Date: Sun, 16 Oct 2011 22:34:29 +0200
Message-ID: <87ipno1xre.fsf@kepler.schwinge.homeip.net>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Cc: notmuch@notmuchmail.org
Precedence: list

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

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(-)
>=20
> 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"
>=20=20
> +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
>=20
> [...]

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=C3=BC=C3=9Fe,
 Thomas

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iQEcBAEBAgAGBQJOmz/VAAoJEGe3hdm9kOiiOEMH/19KlTnLdgsxsn7vSU5MoE5W
C9nsQzKgKhst3ChkVAdmLyssURmVcENqHaYtBH8NPIA7qTHUKdGSPo4aY+sCzdnJ
Zaf9UqIqn16bEQZARmataMcFtPeDJBl2qp7h6oe7Ej0o6tIWvpOCU8ESg4qN7Ndn
89Q65Uue7DyrBzk4sqOM4Q88t+gGy/jPMcOxpnTj5NsxW3feEFlZ1AarDVh3DRrt
wbkO9B+y+EAwCnx0GzakZxdK3oPZX9LaqcNjvIOLrefJvVHIu0IHXDx41k3kxJxm
LXuGmk5b5UXy+x50/PgOjNhXYZrKVwHu2yel8YAw+Erfh5fOiSTiO810XVm4ydU=
=BinX
-----END PGP SIGNATURE-----
--=-=-=--