Hello,

I sincerely apologize for the delay in an updated patch.

On Sat, Mar 01 2025, David Bremner wrote:

> Kristoffer Balintona <[email protected]> writes:
>
>> On Thu, Oct 17 2024, David Bremner wrote:
>>
>> Thank you for letting me know about Notmuch's test suite. I've attached
>> two patches. 001 implements the code for the feature, and 002 includes
>> the tests for each of the three possible values of
>> message-cite-reply-position: above, traditional, and below.
>>
>
> thanks for writing some tests.
>
>>
>> Please let me know if any modifications are in order.
>
> Your patches don't seem to have commit messages. You can
> use git-format-patch to include them. See also
>
>    https://notmuchmail.org/contributing/#index5h2
>
> for hints about appropriate commit messages.

Sorry about that. I've attached proper patches this time around, with
hopefully informative and idiomatic commit messages.

>> +      (when (or (eq message-cite-reply-position 'above)
>> +                (and message-cite-style
>> +                     (eq (eval (cadr
>
> Is it possible to avoid the eval here? I am nervous about the
> security implications (side effects e.g.) of using eval.

Good point. My updated patches avoid eval now. I've tried to add a
comment that explains this since it looks strange otherwise.

An additional note on this updated patch: taking a look at
message-yank-original and message--yank-original-internal, it looks like
if message-cite-style specifies a value for message-cite-reply-position,
it'll use that instead. Therefore, I've made message-cite-style take
precedence over message-cite-reply-position.

A final concern: I do not know when these message.el options were added
nor which Emacs version notmuch.el supports up to. As such, I don't know
if this breaks backwards compatibility with (very) old Emacs versions.

What do you think? Are any other changes in order? Is there
documentation that should be updated?

-- 
Kind regards,
Kristoffer
From 1c4b830c21bf6352294c2e7e5a77563fc8014f6b Mon Sep 17 00:00:00 2001
From: Kristoffer Balintona <[email protected]>
Date: Mon, 4 Nov 2024 21:22:39 -0600
Subject: [PATCH 1/3] emacs: Have email reply positions respect relevant
 message.el options

Previously, when composing email replies with `notmuch-mua-new-mail',
the email signature would always be placed below the email citation.
However, there are two message.el user options that affect the
position of the signature and email body:
`message-cite-reply-position` and `message-cite-style-gmail'.
Previously, neither of these user options were respected.

Respect these user options. If `message-cite-reply-position' is
'traditional or 'below, place the email signature below the citation
(the previous behavior is retained). If `message-cite-reply-position'
is 'above, place the email signature above the citation (like in
Gmail-style email replies).

`message-cite-style-gmail' may specify a value for
`message-cite-reply-position'. If it does, that value takes precedence
over `message-cite-reply-position'.
---
 emacs/notmuch-mua.el | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index bf62b656..e567e01b 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -295,6 +295,29 @@ Typically this is added to `notmuch-mua-send-hook'."
 	    (when message-signature-insert-empty-line
 	      (forward-line -1))
 	  (goto-char (point-max))))
+      ;; If `message-cite-reply-position' is the symbol 'above, then
+      ;; before inserting the citation, put the point after the
+      ;; signature and insert a newline for spacing.  This emulates
+      ;; the Gmail-style of email replies, where the signature and
+      ;; email reply body are above the email citation.  If
+      ;; `message-cite-style'is non-nil and specifies a value for
+      ;; `message-cite-reply-position', then use that value instead.
+      ;;
+      ;; Regarding the use of `cadadr' instead of `cdr' on the result
+      ;; of `assoc' below: the value stored in `message-cite-style' is
+      ;; itself a quoted form, e.g., (quote above).  Using `cdr' once
+      ;; returns that entire quoted form.  Using `cadadr' skips the
+      ;; `quote' symbol and returns the underlying symbol
+      ;; (e.g. `above').  (We do this to avoid a call to `eval'
+      ;; because of security concerns.)
+      (when (eq (or (cadadr (assoc 'message-cite-reply-position
+				   (if (symbolp message-cite-style)
+				       (symbol-value message-cite-style)
+				     message-cite-style)))
+		    message-cite-reply-position)
+		'above)
+	(goto-char (point-max))
+	(insert "\n"))
       (let ((from (plist-get original-headers :From))
 	    (date (plist-get original-headers :Date))
 	    (start (point)))
-- 
2.51.0

From 1cd56d4a0e3ad0d76e1a9171f9c6be615ef8a747 Mon Sep 17 00:00:00 2001
From: Kristoffer Balintona <[email protected]>
Date: Mon, 4 Nov 2024 23:38:07 -0600
Subject: [PATCH 2/3] test/emacs: Add tests for `message-cite-reply-position'

`message-cite-reply-position' affects the position of the email body
and signature relative to the email citation in email replies.

Test each of the three possible values of
`message-cite-reply-position'. If `message-cite-reply-position' is
'traditional or 'below, place the email signature below the
citation. If `message-cite-reply-position' is 'above, place the email
signature above the citation (as in Gmail-style email replies).
---
 test/T453-emacs-reply.sh | 131 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 131 insertions(+)

diff --git a/test/T453-emacs-reply.sh b/test/T453-emacs-reply.sh
index 0a27d066..f02a2fb9 100755
--- a/test/T453-emacs-reply.sh
+++ b/test/T453-emacs-reply.sh
@@ -68,4 +68,135 @@ test_emacs "(notmuch-show \"id:${ID3}\")
 notmuch_dir_sanitize < OUTPUT.raw > OUTPUT
 test_expect_equal_file_nonempty $EXPECTED/notmuch-reply-duplicate-4 OUTPUT
 
+add_email_corpus default
+
[email protected]
+test_begin_subtest "if message-cite-reply-position is 'above, position citation below signature"
+test_emacs "(let ((message-cite-reply-position 'above)
+             (message-signature \"EMAIL SIGNATURE\"))
+         (notmuch-mua-new-reply \"id:${ID4}\")
+         (test-visible-output \"OUTPUT.raw\"))"
+cat <<EOF > EXPECTED
+From: Notmuch Test Suite <[email protected]>
+To: Mikhail Gusarov <[email protected]>
+Subject: Re: [notmuch] [PATCH 2/2] Include <stdint.h> to get uint32_t in C++ file with gcc 4.4
+In-Reply-To: <[email protected]>
+Fcc: MAIL_DIR/sent
+--text follows this line--
+
+-- 
+EMAIL SIGNATURE
+
+"Mikhail Gusarov" <[email protected]> writes:
+
+> Signed-off-by: Mikhail Gusarov <dottedmag at dottedmag.net>
+> ---
+>  lib/message.cc |    2 ++
+>  1 files changed, 2 insertions(+), 0 deletions(-)
+>
+> diff --git a/lib/message.cc b/lib/message.cc
+> index 72c350f..a4b090b 100644
+> --- a/lib/message.cc
+> +++ b/lib/message.cc
+> @@ -21,6 +21,8 @@
+>  #include "notmuch-private.h"
+>  #include "database-private.h"
+>  
+> +#include <stdint.h>
+> +
+>  #include <gmime/gmime.h>
+>  
+>  #include <xapian.h>
+> -- 
+> 1.6.3.3
+EOF
+notmuch_dir_sanitize < OUTPUT.raw > OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "if message-cite-reply-position is 'traditional, position citation above signature"
+test_emacs "(let ((message-cite-reply-position 'traditional)
+             (message-signature \"EMAIL SIGNATURE\"))
+         (notmuch-mua-new-reply \"id:${ID4}\")
+         (test-visible-output \"OUTPUT.raw\"))"
+cat <<EOF > EXPECTED
+From: Notmuch Test Suite <[email protected]>
+To: Mikhail Gusarov <[email protected]>
+Subject: Re: [notmuch] [PATCH 2/2] Include <stdint.h> to get uint32_t in C++ file with gcc 4.4
+In-Reply-To: <[email protected]>
+Fcc: MAIL_DIR/sent
+--text follows this line--
+"Mikhail Gusarov" <[email protected]> writes:
+
+> Signed-off-by: Mikhail Gusarov <dottedmag at dottedmag.net>
+> ---
+>  lib/message.cc |    2 ++
+>  1 files changed, 2 insertions(+), 0 deletions(-)
+>
+> diff --git a/lib/message.cc b/lib/message.cc
+> index 72c350f..a4b090b 100644
+> --- a/lib/message.cc
+> +++ b/lib/message.cc
+> @@ -21,6 +21,8 @@
+>  #include "notmuch-private.h"
+>  #include "database-private.h"
+>  
+> +#include <stdint.h>
+> +
+>  #include <gmime/gmime.h>
+>  
+>  #include <xapian.h>
+> -- 
+> 1.6.3.3
+>
+>
+
+-- 
+EMAIL SIGNATURE
+EOF
+notmuch_dir_sanitize < OUTPUT.raw > OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "if message-cite-reply-position is 'below, position citation above signature"
+test_emacs "(let ((message-cite-reply-position 'below)
+             (message-signature \"EMAIL SIGNATURE\"))
+         (notmuch-mua-new-reply \"id:${ID4}\")
+         (test-visible-output \"OUTPUT.raw\"))"
+cat <<EOF > EXPECTED
+From: Notmuch Test Suite <[email protected]>
+To: Mikhail Gusarov <[email protected]>
+Subject: Re: [notmuch] [PATCH 2/2] Include <stdint.h> to get uint32_t in C++ file with gcc 4.4
+In-Reply-To: <[email protected]>
+Fcc: MAIL_DIR/sent
+--text follows this line--
+"Mikhail Gusarov" <[email protected]> writes:
+
+> Signed-off-by: Mikhail Gusarov <dottedmag at dottedmag.net>
+> ---
+>  lib/message.cc |    2 ++
+>  1 files changed, 2 insertions(+), 0 deletions(-)
+>
+> diff --git a/lib/message.cc b/lib/message.cc
+> index 72c350f..a4b090b 100644
+> --- a/lib/message.cc
+> +++ b/lib/message.cc
+> @@ -21,6 +21,8 @@
+>  #include "notmuch-private.h"
+>  #include "database-private.h"
+>  
+> +#include <stdint.h>
+> +
+>  #include <gmime/gmime.h>
+>  
+>  #include <xapian.h>
+> -- 
+> 1.6.3.3
+>
+>
+
+-- 
+EMAIL SIGNATURE
+EOF
+notmuch_dir_sanitize < OUTPUT.raw > OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.51.0

_______________________________________________
notmuch mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to