[PATCH 1/1] emacs: fix notmuch-mua-reply point placement when signature involved

[PATCH 1/1] emacs: fix notmuch-mua-reply point placement when signature involved

[Tomi Ollila]

When composing a reply, notmuch-mua-reply attempts to  cite the
the original message by inserting it before the user signature, if
one is present. The existing method used to search the signature
separator backward from the end of the buffer and then move one
line up. In case of variable `message-signature-insert-empty-line'
being nil this caused point to go to the beginning of
'--text follows this line--'
separator line, and citation was inserted there.
This change checks the value of `message-signature-insert-empty-line'
and doesn't move point if that is nil. Additional narrowing to
the body region ensures that point never goes to the separator line
(or beyond).
`message-signature-setup-hook' or `message-setup-hook' may already have
added some other content to the message body, therefore using simply
(message-goto-body) to move point to the beginning of body might lead
to unexpected results.

Original patch from "Geoffrey H. Ferrari", continued with iterations
from Jani and Mark.
---

This is update to

id:1378322458-30159-1-git-send-email-tomi.ollila@iki.fi

with comment changes only.

Tomi

 emacs/notmuch-mua.el | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index ff8149b..d41c0b3 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -196,11 +196,16 @@ list."
 			    nil (notmuch-mua-get-switch-function))))
 
       ;; Insert the message body - but put it in front of the signature
-      ;; if one is present
-      (goto-char (point-max))
-      (if (re-search-backward message-signature-separator nil t)
-	  (forward-line -1)
-	(goto-char (point-max)))
+      ;; if one is present, and after any other content
+      ;; message*setup-hooks may have added to the message body already.
+      (save-restriction
+	(message-goto-body)
+	(narrow-to-region (point) (point-max))
+	(goto-char (point-max))
+	(if (re-search-backward message-signature-separator nil t)
+	    (if message-signature-insert-empty-line
+		(forward-line -1))
+	  (goto-char (point-max))))
 
       (let ((from (plist-get original-headers :From))
 	    (date (plist-get original-headers :Date))
-- 
1.8.0

Re: [PATCH 1/1] emacs: fix notmuch-mua-reply point placement when signature involved

[Mark Walters]

This LGTM +1

Mark

On Fri, 06 Sep 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> When composing a reply, notmuch-mua-reply attempts to  cite the
> the original message by inserting it before the user signature, if
> one is present. The existing method used to search the signature
> separator backward from the end of the buffer and then move one
> line up. In case of variable `message-signature-insert-empty-line'
> being nil this caused point to go to the beginning of
> '--text follows this line--'
> separator line, and citation was inserted there.
> This change checks the value of `message-signature-insert-empty-line'
> and doesn't move point if that is nil. Additional narrowing to
> the body region ensures that point never goes to the separator line
> (or beyond).
> `message-signature-setup-hook' or `message-setup-hook' may already have
> added some other content to the message body, therefore using simply
> (message-goto-body) to move point to the beginning of body might lead
> to unexpected results.
>
> Original patch from "Geoffrey H. Ferrari", continued with iterations
> from Jani and Mark.
> ---
>
> This is update to
>
> id:1378322458-30159-1-git-send-email-tomi.ollila@iki.fi
>
> with comment changes only.
>
> Tomi
>
>  emacs/notmuch-mua.el | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index ff8149b..d41c0b3 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -196,11 +196,16 @@ list."
>  			    nil (notmuch-mua-get-switch-function))))
>  
>        ;; Insert the message body - but put it in front of the signature
> -      ;; if one is present
> -      (goto-char (point-max))
> -      (if (re-search-backward message-signature-separator nil t)
> -	  (forward-line -1)
> -	(goto-char (point-max)))
> +      ;; if one is present, and after any other content
> +      ;; message*setup-hooks may have added to the message body already.
> +      (save-restriction
> +	(message-goto-body)
> +	(narrow-to-region (point) (point-max))
> +	(goto-char (point-max))
> +	(if (re-search-backward message-signature-separator nil t)
> +	    (if message-signature-insert-empty-line
> +		(forward-line -1))
> +	  (goto-char (point-max))))
>  
>        (let ((from (plist-get original-headers :From))
>  	    (date (plist-get original-headers :Date))
> -- 
> 1.8.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/1] emacs: fix notmuch-mua-reply point placement when signature involved

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> Original patch from "Geoffrey H. Ferrari", continued with iterations
> from Jani and Mark.

pushed,

d

[PATCH 1/1] emacs: fix notmuch-mua-reply point placement when signature involved

[Tomi.Ollila]

From: Tomi Ollila <tomi.ollila@iki.fi>

When composing a reply, notmuch-mua-reply attempts to  cite the
the original message by inserting it before the user signature, if
one is present. The existing method used to search the signature
separator backward from the end of the buffer and then move one
line up. In case of variable `message-signature-insert-empty-line'
being nil (and `message-signature-setup-hook' not intervening) this
caused point to go to the beginning of '--text follows this line--'
separator line, and citation was inserted there.
This change checks the value of `message-signature-insert-empty-line'
and doesn't move point if that is nil. Additional narrowing to
the body region ensures that point never goes to the separator line
(or beyond).

Original patch from "Geoffrey H. Ferrari", continued with iterations
from Jani and Mark.
---
 emacs/notmuch-mua.el | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 2baae5f..0280c9f 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -192,10 +192,14 @@ list."
 
       ;; Insert the message body - but put it in front of the signature
       ;; if one is present
-      (goto-char (point-max))
-      (if (re-search-backward message-signature-separator nil t)
-	  (forward-line -1)
-	(goto-char (point-max)))
+      (save-restriction
+	(message-goto-body)
+	(narrow-to-region (point) (point-max))
+	(goto-char (point-max))
+	(if (re-search-backward message-signature-separator nil t)
+	    (if message-signature-insert-empty-line
+		(forward-line -1))
+	  (goto-char (point-max))))
 
       (let ((from (plist-get original-headers :From))
 	    (date (plist-get original-headers :Date))
-- 
1.8.0

Re: [PATCH 1/1] emacs: fix notmuch-mua-reply point placement when signature involved

[Mark Walters]

LGTM +1

Best wishes

Mark

On Wed, 04 Sep 2013, "Tomi.Ollila" <tomi.ollila@iki.fi> wrote:
> From: Tomi Ollila <tomi.ollila@iki.fi>
>
> When composing a reply, notmuch-mua-reply attempts to  cite the
> the original message by inserting it before the user signature, if
> one is present. The existing method used to search the signature
> separator backward from the end of the buffer and then move one
> line up. In case of variable `message-signature-insert-empty-line'
> being nil (and `message-signature-setup-hook' not intervening) this
> caused point to go to the beginning of '--text follows this line--'
> separator line, and citation was inserted there.
> This change checks the value of `message-signature-insert-empty-line'
> and doesn't move point if that is nil. Additional narrowing to
> the body region ensures that point never goes to the separator line
> (or beyond).
>
> Original patch from "Geoffrey H. Ferrari", continued with iterations
> from Jani and Mark.
> ---
>  emacs/notmuch-mua.el | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 2baae5f..0280c9f 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -192,10 +192,14 @@ list."
>  
>        ;; Insert the message body - but put it in front of the signature
>        ;; if one is present
> -      (goto-char (point-max))
> -      (if (re-search-backward message-signature-separator nil t)
> -	  (forward-line -1)
> -	(goto-char (point-max)))
> +      (save-restriction
> +	(message-goto-body)
> +	(narrow-to-region (point) (point-max))
> +	(goto-char (point-max))
> +	(if (re-search-backward message-signature-separator nil t)
> +	    (if message-signature-insert-empty-line
> +		(forward-line -1))
> +	  (goto-char (point-max))))
>  
>        (let ((from (plist-get original-headers :From))
>  	    (date (plist-get original-headers :Date))
> -- 
> 1.8.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/1] emacs: fix notmuch-mua-reply point placement when signature involved

[Jani Nikula]

On Wed, 04 Sep 2013, "Tomi.Ollila" <tomi.ollila@iki.fi> wrote:
> From: Tomi Ollila <tomi.ollila@iki.fi>
>
> When composing a reply, notmuch-mua-reply attempts to  cite the
> the original message by inserting it before the user signature, if
> one is present. The existing method used to search the signature
> separator backward from the end of the buffer and then move one
> line up. In case of variable `message-signature-insert-empty-line'
> being nil (and `message-signature-setup-hook' not intervening) this
> caused point to go to the beginning of '--text follows this line--'
> separator line, and citation was inserted there.
> This change checks the value of `message-signature-insert-empty-line'
> and doesn't move point if that is nil. Additional narrowing to
> the body region ensures that point never goes to the separator line
> (or beyond).
>
> Original patch from "Geoffrey H. Ferrari", continued with iterations
> from Jani and Mark.

Hi Tomi, I don't think you've sufficiently convinced me why we need all
this complexity instead of just doing [1]. And if you do get me
convinced, I'd like the reason to be in the commit message and in a
comment above the piece of code in question.

Cheers,
Jani.


[1] id:cover.1377718199.git.jani@nikula.org




> ---
>  emacs/notmuch-mua.el | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 2baae5f..0280c9f 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -192,10 +192,14 @@ list."
>  
>        ;; Insert the message body - but put it in front of the signature
>        ;; if one is present
> -      (goto-char (point-max))
> -      (if (re-search-backward message-signature-separator nil t)
> -	  (forward-line -1)
> -	(goto-char (point-max)))
> +      (save-restriction
> +	(message-goto-body)
> +	(narrow-to-region (point) (point-max))
> +	(goto-char (point-max))
> +	(if (re-search-backward message-signature-separator nil t)
> +	    (if message-signature-insert-empty-line
> +		(forward-line -1))
> +	  (goto-char (point-max))))
>  
>        (let ((from (plist-get original-headers :From))
>  	    (date (plist-get original-headers :Date))
> -- 
> 1.8.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/1] emacs: fix notmuch-mua-reply point placement when signature involved

[Tomi Ollila]

On Thu, Sep 05 2013, Jani Nikula <jani@nikula.org> wrote:

> On Wed, 04 Sep 2013, "Tomi.Ollila" <tomi.ollila@iki.fi> wrote:
>> From: Tomi Ollila <tomi.ollila@iki.fi>
>>
>> When composing a reply, notmuch-mua-reply attempts to  cite the
>> the original message by inserting it before the user signature, if
>> one is present. The existing method used to search the signature
>> separator backward from the end of the buffer and then move one
>> line up. In case of variable `message-signature-insert-empty-line'
>> being nil (and `message-signature-setup-hook' not intervening) this
>> caused point to go to the beginning of '--text follows this line--'
>> separator line, and citation was inserted there.
>> This change checks the value of `message-signature-insert-empty-line'
>> and doesn't move point if that is nil. Additional narrowing to
>> the body region ensures that point never goes to the separator line
>> (or beyond).
>>
>> Original patch from "Geoffrey H. Ferrari", continued with iterations
>> from Jani and Mark.
>
> Hi Tomi, I don't think you've sufficiently convinced me why we need all
> this complexity instead of just doing [1]. And if you do get me
> convinced, I'd like the reason to be in the commit message and in a
> comment above the piece of code in question.

(add-hook 'message-signature-setup-hook 
   (lambda () (insert "Insert this text before the citation not after.\n")))

From emacs documentation:

message-signature-setup-hook is a variable defined in `message.el'.
Its value is nil

  This variable may be risky if used as a file-local variable.

Documentation:
Normal hook, run each time a new outgoing message is initialized.
It is run after the headers have been inserted and before
the signature is inserted.

You can customize this variable.


Adding the comments is a good idea -- if the above convinces you
I'll prepare comments along this idea.

> Cheers,
> Jani.

Tomi

>
>
> [1] id:cover.1377718199.git.jani@nikula.org
>
>
>
>
>> ---
>>  emacs/notmuch-mua.el | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
>> index 2baae5f..0280c9f 100644
>> --- a/emacs/notmuch-mua.el
>> +++ b/emacs/notmuch-mua.el
>> @@ -192,10 +192,14 @@ list."
>>  
>>        ;; Insert the message body - but put it in front of the signature
>>        ;; if one is present
>> -      (goto-char (point-max))
>> -      (if (re-search-backward message-signature-separator nil t)
>> -	  (forward-line -1)
>> -	(goto-char (point-max)))
>> +      (save-restriction
>> +	(message-goto-body)
>> +	(narrow-to-region (point) (point-max))
>> +	(goto-char (point-max))
>> +	(if (re-search-backward message-signature-separator nil t)
>> +	    (if message-signature-insert-empty-line
>> +		(forward-line -1))
>> +	  (goto-char (point-max))))
>>  
>>        (let ((from (plist-get original-headers :From))
>>  	    (date (plist-get original-headers :Date))
>> -- 
>> 1.8.0
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
>

-- 
"kaik on mänt!"

Re: [PATCH 1/1] emacs: fix notmuch-mua-reply point placement when signature involved

[Jani Nikula]

On Thu, 05 Sep 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Thu, Sep 05 2013, Jani Nikula <jani@nikula.org> wrote:
>
>> On Wed, 04 Sep 2013, "Tomi.Ollila" <tomi.ollila@iki.fi> wrote:
>>> From: Tomi Ollila <tomi.ollila@iki.fi>
>>>
>>> When composing a reply, notmuch-mua-reply attempts to  cite the
>>> the original message by inserting it before the user signature, if
>>> one is present. The existing method used to search the signature
>>> separator backward from the end of the buffer and then move one
>>> line up. In case of variable `message-signature-insert-empty-line'
>>> being nil (and `message-signature-setup-hook' not intervening) this
>>> caused point to go to the beginning of '--text follows this line--'
>>> separator line, and citation was inserted there.
>>> This change checks the value of `message-signature-insert-empty-line'
>>> and doesn't move point if that is nil. Additional narrowing to
>>> the body region ensures that point never goes to the separator line
>>> (or beyond).
>>>
>>> Original patch from "Geoffrey H. Ferrari", continued with iterations
>>> from Jani and Mark.
>>
>> Hi Tomi, I don't think you've sufficiently convinced me why we need all
>> this complexity instead of just doing [1]. And if you do get me
>> convinced, I'd like the reason to be in the commit message and in a
>> comment above the piece of code in question.
>
> (add-hook 'message-signature-setup-hook 
>    (lambda () (insert "Insert this text before the citation not after.\n")))
>
> From emacs documentation:
>
> message-signature-setup-hook is a variable defined in `message.el'.
> Its value is nil
>
>   This variable may be risky if used as a file-local variable.
>
> Documentation:
> Normal hook, run each time a new outgoing message is initialized.
> It is run after the headers have been inserted and before
> the signature is inserted.
>
> You can customize this variable.
>
>
> Adding the comments is a good idea -- if the above convinces you
> I'll prepare comments along this idea.

Meh. Fair enough. Thanks for the explanation.

Cheers,
Jani.