Re: [PATCH v4 3/3] cli/reply: make --decrypt take a keyword
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
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
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
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
On Sat 2017-12-23 10:47:52 -0400, David Bremner wrote: > Daniel Kahn Gillmor writes: >> +{ .opt_keyword = (int*)(¶ms.crypto.decrypt), .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
Daniel Kahn Gillmor writes: > + { .opt_keyword = (int*)(¶ms.crypto.decrypt), .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
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 = &format, .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 = &decrypt, .name = "decrypt", .present = &decrypt_set }, + { .opt_keyword = (int*)(¶ms.crypto.decrypt), .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 = decryp