From: Carl Worth <cworth@cworth.org>
To: camalot@picnicpark.org, notmuch@notmuchmail.org
In-Reply-To: <1260814438-4195-2-git-send-email-camalot@picnicpark.org>
References: <87my1m323m.fsf@yoom.home.cworth.org>
	<1260814438-4195-1-git-send-email-camalot@picnicpark.org>
	<1260814438-4195-2-git-send-email-camalot@picnicpark.org>
Date: Tue, 23 Feb 2010 11:12:36 -0800
Message-ID: <87d3zvwyu3.fsf@yoom.home.cworth.org>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Cc: Keith Amidon <keith@nicira.com>
Subject: Re: [notmuch] [PATCH] Rework saving of attachments.
Precedence: list

--=-=-=

On Mon, 14 Dec 2009 10:13:58 -0800, camalot@picnicpark.org wrote:
> From: Keith Amidon <keith@nicira.com>

Hi Keith,

I apologize for the extraordinarly-late review, but here it is...

I tried this patch out, wanted to like it, and almost pushed it out, but
I decided against it in its current form. Here are some thoughts:

1. The commit message ("rework saving of attachments") is not
   adequate. It doesn't actually say what the commit does, (how can I
   test whether things have been reworked?). If the vagueness of the
   message is because the commit is changing several different aspects
   of behavior, then I would argue that the commit should be split up
   into separate pieces.

2. A binding to save a single attachment (with only a prefix argument to
   select which) just isn't usable. First, there's nothing in the UI to
   indicate the appropriate numbers to pass as the prefix argument,
   (other than manually counting the attachments). And second, the
   functionality is simply too hidden and non-obvious. This is most
   dangerous because in the common case of a single attachment, the 'w'
   binding will seem to be saving all attachments setting up confusion
   if the user tries to save multiple attachments with this same
   keybinding.

   Now, having a function to save a single attachment is just fine,
   (leaving someone else to hook up a binding to a particular button,
   say). So I'd accept a patch that added that, but didn't add a direct
   key-binding for it.

3. For saving multiple attachments, the feature I'd really like to see
   is the ability to specify a single directory and have all the
   attachments saved there.

Obviously, this third feature is just something different than what the
patch does, so not necessarily a reason to reject the patch. So what is
it that the patch actually does again?

I think the big advantage of the patch is getting rid of the initial
prompting "save this attachment (foo)?". It occurs to me that a simpler
way to get rid of that would be to simply not ask that question when the
user hits 'w' and there *is* only a single attachment. Then, with
multiple attachments, 'w' could prompt in turn as currently.

That would leave open the ability to use 'W' for a command to write all
attachments to a particular directory.

So that's one idea, at least. What do you think?

-Carl

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iD8DBQFLhCik6JDdNq8qSWgRAhqXAKCIbMR9jYdOuK4tKPEkPrZCGUXT0ACfXFhD
gx1I/+4jTl1VWzX7rci/8mQ=
=Rld/
-----END PGP SIGNATURE-----
--=-=-=--