Re: [PATCH v4 3/3] cli/reply: make --decrypt take a keyword

[Daniel Kahn Gillmor]

On Sat 2017-12-30 09:05:40 -0400, David Bremner wrote:
> I need more time to think about this, so I'd rather defer till after the
> release in any case. 

are you saying that you want to defer this whole series until after the
release?  that would be a real shame, since it would mean we'd have
both:

notmuch show --decrypt

and

notmuch new --decrypt=true

which seems particularly troubling.  please let's at least make it a
keyword in all cases.

> But at some point we collectively (I think? Maybe Jamie and I browbeat
> you into it) decided that it was OK for --decrypt=true to have context
> dependent behaviour.  It seems to me that "different things in a
> different context" issue already exists.

Oh, i'm not saying that "notmuch show --decrypt=true" must mean exactly
the same thing as "notmuch new --decrypt=true" -- i understand that it
does not mean the same thing, because the contexts are different.  My
complaint was that documenting "notmuch show --decrypt=nostash" seems
like it introduces confusion around the fact that --decrypt=nostash *is*
identical to --decrypt=true in one place, and is functionally
(significantly) different from --decrypt=true in another place.

iow: i'm fine with --decrypt=true being a coherent policy about message
decryption that does different things in different contexts.  I think
explaining about --decrypt=nostash being the same as that policy in some
contexts and different in others is pretty awkward, but if people really
want it, i won't block on it, and i look forward to seeing the patches
to the documentation.

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v4 3/3] cli/reply: make --decrypt take a keyword

[David Bremner]

Daniel Kahn Gillmor  writes:

> It sounds like you were suggesting "--decrypt=nostash" as a synonym for
> "--decrypt=true" on show/reply, which i confess i didn't fully
> understand when i wrote my response.  If it really makes you feel better
> to add the alias/synonym, i wouldn't block such a change.

Yes, that's what I was suggesting.

> But I think the documentation is tricky to write (and trickier to read
> and trickier still to understand!)  if "--decrypt=nostash" means the
> same thing as "--decrypt=true" in one context, but they mean different
> things in a different context.

I need more time to think about this, so I'd rather defer till after the
release in any case. But at some point we collectively (I think? Maybe
Jamie and I browbeat you into it) decided that it was OK for
--decrypt=true to have context dependent behaviour. It seems to me that
"different things in a different context" issue already exists.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v4 3/3] cli/reply: make --decrypt take a keyword

[Daniel Kahn Gillmor]

On Fri 2017-12-29 10:30:00 -0400, David Bremner wrote:
> Daniel Kahn Gillmor  writes:
>
>> No, we should not support --decrypt=nostash for show or reply.  The
>> semantics of the display commands (show, reply) are such that they
>> *never* modify the index or stash anything in there.  The equivalent for
>> the indexing (new, insert, reindex) commands' "--decrypt=nostash" in the
>> display commands is simply "--decrypt=true".
>
> I'm not sure I completely agree, but its a trivial matter to add nostash
> later if desired. And it's always easier to add API / command options
> than to take them away.

yes, true that we can always expand out, and it's more parsimonious to
start small.  :)

It sounds like you were suggesting "--decrypt=nostash" as a synonym for
"--decrypt=true" on show/reply, which i confess i didn't fully
understand when i wrote my response.  If it really makes you feel better
to add the alias/synonym, i wouldn't block such a change.

But I think the documentation is tricky to write (and trickier to read
and trickier still to understand!)  if "--decrypt=nostash" means the
same thing as "--decrypt=true" in one context, but they mean different
things in a different context.

I'm still trying to aim for that sweet spot where the smallest possible
API guides the user to sensible decisions while still understanding
what's going on. :)

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v4 3/3] cli/reply: make --decrypt take a keyword

[David Bremner]

Daniel Kahn Gillmor  writes:

> No, we should not support --decrypt=nostash for show or reply.  The
> semantics of the display commands (show, reply) are such that they
> *never* modify the index or stash anything in there.  The equivalent for
> the indexing (new, insert, reindex) commands' "--decrypt=nostash" in the
> display commands is simply "--decrypt=true".
>

I'm not sure I completely agree, but its a trivial matter to add nostash
later if desired. And it's always easier to add API / command options
than to take them away.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v4 3/3] cli/reply: make --decrypt take a keyword

[Daniel Kahn Gillmor]

On Sat 2017-12-23 10:47:52 -0400, David Bremner wrote:
> Daniel Kahn Gillmor  writes:
>> +{ .opt_keyword = (int*)(), .name = "decrypt",
>> +  .keyword_no_arg_value = "true", .keywords =
>> +  (notmuch_keyword_t []){ { "false", NOTMUCH_DECRYPT_FALSE },
>> +  { "auto", NOTMUCH_DECRYPT_AUTO },
>> +  { "true", NOTMUCH_DECRYPT_NOSTASH },
>> +  { 0, 0 } } },
>
> same question as for show, should we support --decrypt=nostash ?

No, we should not support --decrypt=nostash for show or reply.  The
semantics of the display commands (show, reply) are such that they
*never* modify the index or stash anything in there.  The equivalent for
the indexing (new, insert, reindex) commands' "--decrypt=nostash" in the
display commands is simply "--decrypt=true".

If you take a look at the subsequent series that follows this one,
you'll see that i offer a "--decrypt=stash" command "notmuch show", but
it makes some unusual assumptions (in particular, about opening the
database read/write, which "notmuch show" has never needed to do
before).

So, in case it wasn't clear: i've thought pretty hard about these
commands and the options that users are likely to want or need.

But the current series (under discussion in this thread) should be
applied *without* those additional changes -- i don't want to have the
whole controversy about "notmuch show --decrypt=stash" now. :)

  --dkg
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v4 3/3] cli/reply: make --decrypt take a keyword

[David Bremner]

Daniel Kahn Gillmor  writes:
> + { .opt_keyword = (int*)(), .name = "decrypt",
> +   .keyword_no_arg_value = "true", .keywords =
> +   (notmuch_keyword_t []){ { "false", NOTMUCH_DECRYPT_FALSE },
> +   { "auto", NOTMUCH_DECRYPT_AUTO },
> +   { "true", NOTMUCH_DECRYPT_NOSTASH },
> +   { 0, 0 } } },

same question as for show, should we support --decrypt=nostash ?

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v4 3/3] cli/reply: make --decrypt take a keyword

[Daniel Kahn Gillmor]

This brings the --decrypt argument to "notmuch reply" into line with
the other --decrypt arguments (in "show", "new", "insert", and
"reindex").  This patch is really just about bringing consistency to
the user interface.

We also use the recommended form in the emacs MUA when replying, and
update test T350 to match.
---
 completion/notmuch-completion.bash |  2 +-
 doc/man1/notmuch-reply.rst | 34 --
 emacs/notmuch-mua.el   |  2 +-
 notmuch-reply.c| 11 ++-
 test/T350-crypto.sh|  2 +-
 5 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/completion/notmuch-completion.bash 
b/completion/notmuch-completion.bash
index 17f3c5ec..249b9664 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -351,7 +351,7 @@ _notmuch_reply()
return
;;
--decrypt)
-   COMPREPLY=( $( compgen -W "true false" -- "${cur}" ) )
+   COMPREPLY=( $( compgen -W "true auto false" -- "${cur}" ) )
return
;;
 esac
diff --git a/doc/man1/notmuch-reply.rst b/doc/man1/notmuch-reply.rst
index ede77930..1b62e075 100644
--- a/doc/man1/notmuch-reply.rst
+++ b/doc/man1/notmuch-reply.rst
@@ -72,20 +72,26 @@ Supported options for **reply** include
 in this order, and copy values from the first that contains
 something other than only the user's addresses.
 
-``--decrypt``
-Decrypt any MIME encrypted parts found in the selected content
-(ie. "multipart/encrypted" parts). Status of the decryption will
-be reported (currently only supported with --format=json and
---format=sexp) and on successful decryption the
-multipart/encrypted part will be replaced by the decrypted
-content.
-
-If a session key is already known for the message, then it
-will be decrypted automatically unless the user explicitly
-sets ``--decrypt=false``.
-
-Decryption expects a functioning **gpg-agent(1)** to provide any
-needed credentials. Without one, the decryption will likely fail.
+``--decrypt=(false|auto|true)``
+
+If ``true``, decrypt any MIME encrypted parts found in the
+selected content (i.e., "multipart/encrypted" parts). Status
+of the decryption will be reported (currently only supported
+with --format=json and --format=sexp), and on successful
+decryption the multipart/encrypted part will be replaced by
+the decrypted content.
+
+If ``auto``, and a session key is already known for the
+message, then it will be decrypted, but notmuch will not try
+to access the user's secret keys.
+
+Use ``false`` to avoid even automatic decryption.
+
+Non-automatic decryption expects a functioning
+**gpg-agent(1)** to provide any needed credentials. Without
+one, the decryption will likely fail.
+
+Default: ``auto``
 
 See **notmuch-search-terms(7)** for details of the supported syntax for
 .
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 7a341ebf..59b546a6 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -181,7 +181,7 @@ mutiple parts get a header."
reply
original)
 (when process-crypto
-  (setq args (append args '("--decrypt"
+  (setq args (append args '("--decrypt=true"
 
 (if reply-all
(setq args (append args '("--reply-to=all")))
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 5cdf642b..75cf7ecb 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -704,8 +704,6 @@ notmuch_reply_command (notmuch_config_t *config, int argc, 
char *argv[])
 };
 int format = FORMAT_DEFAULT;
 int reply_all = true;
-bool decrypt = false;
-bool decrypt_set = false;
 
 notmuch_opt_desc_t options[] = {
{ .opt_keyword = , .name = "format", .keywords =
@@ -719,7 +717,12 @@ notmuch_reply_command (notmuch_config_t *config, int argc, 
char *argv[])
  (notmuch_keyword_t []){ { "all", true },
  { "sender", false },
  { 0, 0 } } },
-   { .opt_bool = , .name = "decrypt", .present = _set },
+   { .opt_keyword = (int*)(), .name = "decrypt",
+ .keyword_no_arg_value = "true", .keywords =
+ (notmuch_keyword_t []){ { "false", NOTMUCH_DECRYPT_FALSE },
+ { "auto", NOTMUCH_DECRYPT_AUTO },
+ { "true", NOTMUCH_DECRYPT_NOSTASH },
+ { 0, 0 } } },
{ .opt_inherit = notmuch_shared_options },
{ }
 };
@@ -729,8 +732,6 @@ notmuch_reply_command (notmuch_config_t *config, int argc, 
char *argv[])
return EXIT_FAILURE;
 
 notmuch_process_shared_options (argv[0]);
-if (decrypt_set)
-   params.crypto.decrypt = decrypt ? NOTMUCH_DECRYPT_NOSTASH :