Re: [Patch v5 4/4] emacs: resume messages

[Mark Walters <markwalters1009@gmail.com>]

On Sat, 12 Nov 2016, David Bremner <david@tethera.net> wrote:
> David Bremner <david@tethera.net> writes:
>
>> Provide functionality to resume editing a mesage previously saved with
>> notmuch-draft-save, including decoding the X-Notmuch-Emacs-Secure
>> header.
>
> s/mesage/message/
>
>> +(defun notmuch-draft-unquote-some-mml ()
>> +  "Unquote the mml tags in `notmuch-draft-quoted-tags`."
>> +  (save-excursion
>> +    (when notmuch-draft-quoted-tags
>> +      (let ((re (concat "<#!+/?\\("
>> +			(mapconcat 'identity notmuch-draft-quoted-tags "\\|")
>> +
> Same issue here with regex quoting, I think.

Yes I will change this.

>> +    (let (secure-tag)
>> +      (save-restriction
>> +	(message-narrow-to-headers)
>> +	(setq secure-tag (message-fetch-field "X-Notmuch-Emacs-Secure" 't))
>> +	(message-remove-header "X-Notmuch-Emacs-Secure"))
>> +      (message-goto-body)
>> +      (when secure-tag
>> +	(insert secure-tag "\n")))))
>
> Can the setq inside the let be replaced with
>
> (let ((secure-tag (message-fetch-field "X-Notmuch-Emacs-Secure" 't)))
>      ...
>
> Perhaps by pushing the let inside the save-restriction?

We need to insert the secure-tag in the body, so outside of the
save-restriction. However, message-fetch-field requires the message to
be narrowed to the headers. So I don't see an easy way to avoid the setq.

>
>>  (require 'notmuch-mua)
>>  (require 'notmuch-crypto)
>>  (require 'notmuch-print)
>> +(require 'notmuch-draft)
>
> This line I added.
>
>> +(defun notmuch-show-resume-message ()
>> +  "Resume EDITING the current draft message."
>> +  (interactive)
>> +  (let ((id (notmuch-show-get-message-id)))
>> +    (when id
>> +      (notmuch-draft-resume id))))

Do you mean if we are not on a message? I think this may not be needed:
I think all I intended to check was that we are on a message, but that
seems to always be true in the show buffer (unlike threads in the search
buffer).

Best wishes

Mark


>
> The error handling is not very clear to me
> here. notmuch-show-get-message-id is not documented to return nil on
> error. Should some docstring be changed here?
>
>> +<#secure method=pgpmime mode=sign>
>> +EOF
>> +test_expect_equal_file EXPECTED OUTPUT.clean
>>  test_done
>
> The quoting of the secure tag here is not present in the original test,
> but sure confused me for a few minutes.
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch