[Patch v5 3/4] emacs: check drafts for encryption tags before saving

[David Bremner]

Mark Walters <markwalters1009 at gmail.com> writes:

>> @@ -109,6 +140,15 @@ This saves the current message in the database with tags
>>  `notmuch-draft-tags` (in addition to any default tags
>>  applied to newly inserted messages)."
>>    (interactive)
>> +  (case notmuch-draft-save-plaintext
>> +    ((ask)
>> +     (unless (notmuch-draft--check-encryption-tag t)
>> +       (error "Save aborted")))
>> +    ((t)
>> +     (ignore))
>> +    ((nil)
>> +     (unless (notmuch-draft--check-encryption-tag nil)
>> +       (error "Refusing to save draft with encryption tags (see `notmuch-draft-save-plaintext')"))))
>
> What would you think of rejigging the logic here? I would prefer that
> the first check was "is there an encryption tag" and then if there is
> such a tag decide what to do. The reason I prefer that is that it makes
> the common case clear.
>
> I realise there are downsides too -- eg in your code you don't even
> check for secure tags if they are going to  be ignored anyway.

OK, I was mainly concerned with not prompting the user uneccesarily, but
the following seems to be equivalent:


-  (case notmuch-draft-save-plaintext
-    ((ask)
-     (unless (notmuch-draft--check-encryption-tag t)
-       (error "Save aborted")))
-    ((t)
-     (ignore))
-    ((nil)
-     (unless (notmuch-draft--check-encryption-tag nil)
+  (unless (notmuch-draft--check-encryption-tag
+          (eq notmuch-draft-save-plaintext 'ask))
+    (case notmuch-draft-save-plaintext
+      ((ask)
+       (error "Save aborted"))
+      ((t)
+       (ignore))
+      ((nil)
        (error "Refusing to save draft with encryption tags (see `notmuch-draft-save-plaintext')"))))