Obey message-cite-reply-position from Emacs' message.el

Obey message-cite-reply-position from Emacs' message.el

[Kristoffer Balintona]

[-- Attachment #1: Type: text/plain, Size: 997 bytes --]

Hello,

Message.el has a user option called message-cite-reply-position that
determines where a signature is placed in an email compositino buffer
when replying. Its docstring reads:

    Where the reply should be positioned.
    If ‘traditional’, reply inline.
    If ‘above’, reply above quoted text.
    If ‘below’, reply below quoted text.

    Note: Many newsgroups frown upon nontraditional reply styles.
    You probably want to set this variable only for specific groups,
    e.g. using ‘gnus-posting-styles’:

      (eval (setq-local message-cite-reply-position 'above))

The current behavior of notmuch accords with the user values of below
and traditional but not above.

The notmuch function responsible for the position of the signature (or,
rather, the position of the citations) is notmuch-mua-reply. I have
attached a patch that adds a brief snippet which obeys
message-cite-reply-position when its value is above.

-- 
Best,
Kristoffer

[-- Attachment #2: notmuch-mua-reply-respect-message-cite-reply-position.patch --]
[-- Type: text/x-patch, Size: 1339 bytes --]

 emacs/notmuch-mua.el | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index bf62b656..1d1a1591 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -295,6 +295,20 @@ 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 `above', e.g., for Gmail-like
+      ;; email replies, then before inserting the citation, put the point
+      ;; after the signature and insert a newline for spacing. Also respects
+      ;; if `message-cite-reply-position' is set via `message-cite-style'.
+      (when (or (equal message-cite-reply-position 'above)
+                (and message-cite-style
+                     (eq (eval (cadr
+                                (assoc 'message-cite-reply-position
+                                       (if (symbolp message-cite-style)
+                                           (eval message-cite-style)
+                                         message-cite-style))))
+                         'above)))
+        (goto-char (point-max))
+        (insert "\n"))
       (let ((from (plist-get original-headers :From))
 	    (date (plist-get original-headers :Date))
 	    (start (point)))

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: Obey message-cite-reply-position from Emacs' message.el

[Pengji Zhang]

Kristoffer Balintona <krisbalintona@gmail.com> writes:

> Hello,

Hello!

> The current behavior of notmuch accords with the user values of below
> and traditional but not above.
>
> The notmuch function responsible for the position of the signature
> (or, rather, the position of the citations) is notmuch-mua-reply. I
> have attached a patch that adds a brief snippet which obeys
> message-cite-reply-position when its value is above.

This indeed seems reasonable. Thank you! I am not a maintainer, but here
are some comments of mine.

> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index bf62b656..1d1a1591 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -295,6 +295,20 @@ 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 `above', e.g., for Gmail-like
> +      ;; email replies, then before inserting the citation, put the point
> +      ;; after the signature and insert a newline for spacing. Also respects
> +      ;; if `message-cite-reply-position' is set via `message-cite-style'.
> +      (when (or (equal message-cite-reply-position 'above)

Nit: I think it is slightly better to use 'eq' instead of 'equal' here.

> +                (and message-cite-style
> +                     (eq (eval (cadr
> +                                (assoc 'message-cite-reply-position

Likewise, here we may use 'assq'.

> +                                       (if (symbolp message-cite-style)
> +                                           (eval message-cite-style)

Here I think it is better to use 'symbol-value' instead of 'eval'.

> +                                         message-cite-style))))
> +                         'above)))
> +        (goto-char (point-max))
> +        (insert "\n"))
>        (let ((from (plist-get original-headers :From))
>  	    (date (plist-get original-headers :Date))
>  	    (start (point)))

Regards,
Pengji

Re: Obey message-cite-reply-position from Emacs' message.el

[David Bremner]

Pengji Zhang <me@pengjiz.com> writes:

> Kristoffer Balintona <krisbalintona@gmail.com> writes:
>
>> Hello,
>
> Hello!
>
>> The current behavior of notmuch accords with the user values of below
>> and traditional but not above.
>>
>> The notmuch function responsible for the position of the signature
>> (or, rather, the position of the citations) is notmuch-mua-reply. I
>> have attached a patch that adds a brief snippet which obeys
>> message-cite-reply-position when its value is above.

Since this is a new feature, we'd normally expect a test or two.

you can find some examples in test/T453-emacs-reply.sh

d

Re: Obey message-cite-reply-position from Emacs' message.el

[Kristoffer Balintona]

[-- Attachment #1: Type: text/plain, Size: 1661 bytes --]

On Thu, Oct 17 2024, David Bremner wrote:

> Pengji Zhang <me@pengjiz.com> writes:
> [... 7 lines elided]
>>> The current behavior of notmuch accords with the user values of below
>>> and traditional but not above.
>>>
>>> The notmuch function responsible for the position of the signature
>>> (or, rather, the position of the citations) is notmuch-mua-reply. I
>>> have attached a patch that adds a brief snippet which obeys
>>> message-cite-reply-position when its value is above.
>
> Since this is a new feature, we'd normally expect a test or two.
>
> you can find some examples in test/T453-emacs-reply.sh
>
> d

Hello,

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.

Regarding the expected behavior: a value of above and below are
self-explanatory, however, a value of traditional is unclear. The
docstring for message-cite-reply-position states:

    Where the reply should be positioned.
    If ‘traditional’, reply inline.
    If ‘above’, reply above quoted text.
    If ‘below’, reply below quoted text.

As far as I can tell, there is no effective difference between the
values of traditional and below There is hardly any documentation on the
matter in message.el, but looking at the source code traditional doesn't
seem to be treated specially. So I've treated them as equivalent in my
test cases.

Please let me know if any modifications are in order.

-- 
In gratitude,
Kristoffer

[-- Attachment #2: 001-obey-message-cite-reply-position.patch --]
[-- Type: text/x-patch, Size: 1391 bytes --]

 emacs/notmuch-mua.el | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index bf62b656..f58bdcae 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -295,6 +295,22 @@ 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 `above', e.g., for
+      ;; Gmail-like email replies, then before inserting the citation,
+      ;; put the point after the signature and insert a newline for
+      ;; spacing. Also respects citation position if
+      ;; `message-cite-style' specifies a value for
+      ;; `message-cite-reply-position'.
+      (when (or (eq message-cite-reply-position 'above)
+                (and message-cite-style
+                     (eq (eval (cadr
+                                (assq 'message-cite-reply-position
+                                      (if (symbolp message-cite-style)
+                                          (symbol-value message-cite-style)
+                                        message-cite-style))))
+                         'above)))
+        (goto-char (point-max))
+        (insert "\n"))
       (let ((from (plist-get original-headers :From))
 	    (date (plist-get original-headers :Date))
 	    (start (point)))

[-- Attachment #3: 002-obey-message-cite-reply-position-tests.patch --]
[-- Type: text/x-patch, Size: 4482 bytes --]

 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
+
+ID4=1258471718-6781-2-git-send-email-dottedmag@dottedmag.net
+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 <test_suite@notmuchmail.org>
+To: Mikhail Gusarov <dottedmag@dottedmag.net>
+Subject: Re: [notmuch] [PATCH 2/2] Include <stdint.h> to get uint32_t in C++ file with gcc 4.4
+In-Reply-To: <1258471718-6781-2-git-send-email-dottedmag@dottedmag.net>
+Fcc: MAIL_DIR/sent
+--text follows this line--
+
+-- 
+EMAIL SIGNATURE
+
+"Mikhail Gusarov" <dottedmag@dottedmag.net> 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 <test_suite@notmuchmail.org>
+To: Mikhail Gusarov <dottedmag@dottedmag.net>
+Subject: Re: [notmuch] [PATCH 2/2] Include <stdint.h> to get uint32_t in C++ file with gcc 4.4
+In-Reply-To: <1258471718-6781-2-git-send-email-dottedmag@dottedmag.net>
+Fcc: MAIL_DIR/sent
+--text follows this line--
+"Mikhail Gusarov" <dottedmag@dottedmag.net> 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 <test_suite@notmuchmail.org>
+To: Mikhail Gusarov <dottedmag@dottedmag.net>
+Subject: Re: [notmuch] [PATCH 2/2] Include <stdint.h> to get uint32_t in C++ file with gcc 4.4
+In-Reply-To: <1258471718-6781-2-git-send-email-dottedmag@dottedmag.net>
+Fcc: MAIL_DIR/sent
+--text follows this line--
+"Mikhail Gusarov" <dottedmag@dottedmag.net> 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

[-- Attachment #4: Type: text/plain, Size: 0 bytes --]

Re: Obey message-cite-reply-position from Emacs' message.el

[Kristoffer Balintona]

On Thu, Oct 17 2024, Pengji Zhang wrote:

> Kristoffer Balintona <krisbalintona@gmail.com> writes:
>
>> Hello,
>
> Hello!
>
>> The current behavior of notmuch accords with the user values of below
>> and traditional but not above.
>>
>> The notmuch function responsible for the position of the signature
>> (or, rather, the position of the citations) is notmuch-mua-reply. I
>> have attached a patch that adds a brief snippet which obeys
>> message-cite-reply-position when its value is above.
>
> This indeed seems reasonable. Thank you! I am not a maintainer, but here
> are some comments of mine.
>
>> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
>> index bf62b656..1d1a1591 100644
>> --- a/emacs/notmuch-mua.el
>> +++ b/emacs/notmuch-mua.el
>> @@ -295,6 +295,20 @@ 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 `above', e.g., for Gmail-like
>> +      ;; email replies, then before inserting the citation, put the point
>> +      ;; after the signature and insert a newline for spacing. Also respects
>> +      ;; if `message-cite-reply-position' is set via `message-cite-style'.
>> +      (when (or (equal message-cite-reply-position 'above)
>
> Nit: I think it is slightly better to use 'eq' instead of 'equal' here.
>
>> +                (and message-cite-style
>> +                     (eq (eval (cadr
>> +                                (assoc 'message-cite-reply-position
>
> Likewise, here we may use 'assq'.
>
>> +                                       (if (symbolp message-cite-style)
>> +                                           (eval message-cite-style)
>
> Here I think it is better to use 'symbol-value' instead of 'eval'.
>
>> +                                         message-cite-style))))
>> +                         'above)))
>> +        (goto-char (point-max))
>> +        (insert "\n"))
>>        (let ((from (plist-get original-headers :From))
>>  	    (date (plist-get original-headers :Date))
>>  	    (start (point)))
>
> Regards,
> Pengji

Hi Pengji,

Thank you very much for your suggestions. I think they're valid. I've
incorporated them into my new patches.

-- 
In gratitude,
Kristoffer