* [PATCH v3 1/9] emacs/show: Re-arrange determination if a part header is necessary
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
@ 2014-05-12 12:29 ` David Edmondson
2014-05-12 22:09 ` Mark Walters
2014-05-12 12:29 ` [PATCH v3 2/9] emacs/show: Allow the user to decide when part headers should be inserted David Edmondson
` (9 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2014-05-12 12:29 UTC (permalink / raw)
To: notmuch
Move the determination of whether a part header is required to a
distinct function.
---
emacs/notmuch-show.el | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 10fc872..ec99141 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -871,6 +871,21 @@ message at DEPTH in the current thread."
;; showable this returns nil.
(notmuch-show-create-part-overlays button part-beg part-end))))
+(defun notmuch-show-mime-type (part)
+ "Return the correct mime-type to use for PART."
+ (let ((content-type (downcase (plist-get part :content-type))))
+ (or (and (string= content-type "application/octet-stream")
+ (notmuch-show-get-mime-type-of-application/octet-stream part))
+ (and (string= content-type "inline patch")
+ "text/x-diff")
+ content-type)))
+
+(defun notmuch-show-insert-header-p (part)
+ "Return non-NIL if a header button should be inserted for this part."
+ (let ((mime-type (notmuch-show-mime-type part)))
+ (not (and (string= mime-type "text/plain")
+ (<= (plist-get part :id) 1)))))
+
(defun notmuch-show-insert-bodypart (msg part depth &optional hide)
"Insert the body part PART at depth DEPTH in the current thread.
@@ -881,19 +896,15 @@ is t, hide the part initially and show the button. If HIDE is
useful for quoting in replies)."
(let* ((content-type (downcase (plist-get part :content-type)))
- (mime-type (or (and (string= content-type "application/octet-stream")
- (notmuch-show-get-mime-type-of-application/octet-stream part))
- (and (string= content-type "inline patch")
- "text/x-diff")
- content-type))
+ (mime-type (notmuch-show-mime-type part))
(nth (plist-get part :id))
(beg (point))
;; Hide the part initially if HIDE is t.
(show-part (not (equal hide t)))
;; We omit the part button for the first (or only) part if
;; this is text/plain, or HIDE is 'no-buttons.
- (button (unless (or (equal hide 'no-buttons)
- (and (string= mime-type "text/plain") (<= nth 1)))
+ (button (when (and (not (equal hide 'no-buttons))
+ (notmuch-show-insert-header-p part))
(notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
(content-beg (point)))
--
2.0.0.rc0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 1/9] emacs/show: Re-arrange determination if a part header is necessary
2014-05-12 12:29 ` [PATCH v3 1/9] emacs/show: Re-arrange determination if a part header is necessary David Edmondson
@ 2014-05-12 22:09 ` Mark Walters
2014-05-13 9:47 ` David Edmondson
0 siblings, 1 reply; 34+ messages in thread
From: Mark Walters @ 2014-05-12 22:09 UTC (permalink / raw)
To: David Edmondson, notmuch
On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
> Move the determination of whether a part header is required to a
> distinct function.
> ---
> emacs/notmuch-show.el | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 10fc872..ec99141 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -871,6 +871,21 @@ message at DEPTH in the current thread."
> ;; showable this returns nil.
> (notmuch-show-create-part-overlays button part-beg part-end))))
>
> +(defun notmuch-show-mime-type (part)
> + "Return the correct mime-type to use for PART."
> + (let ((content-type (downcase (plist-get part :content-type))))
> + (or (and (string= content-type "application/octet-stream")
> + (notmuch-show-get-mime-type-of-application/octet-stream part))
> + (and (string= content-type "inline patch")
> + "text/x-diff")
> + content-type)))
> +
> +(defun notmuch-show-insert-header-p (part)
> + "Return non-NIL if a header button should be inserted for this part."
> + (let ((mime-type (notmuch-show-mime-type part)))
> + (not (and (string= mime-type "text/plain")
> + (<= (plist-get part :id) 1)))))
My only query here is whether a notmuch-show-hide-header-p (part) might
make some of the logic clearer?
MW
> +
> (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
> "Insert the body part PART at depth DEPTH in the current thread.
>
> @@ -881,19 +896,15 @@ is t, hide the part initially and show the button. If HIDE is
> useful for quoting in replies)."
>
> (let* ((content-type (downcase (plist-get part :content-type)))
> - (mime-type (or (and (string= content-type "application/octet-stream")
> - (notmuch-show-get-mime-type-of-application/octet-stream part))
> - (and (string= content-type "inline patch")
> - "text/x-diff")
> - content-type))
> + (mime-type (notmuch-show-mime-type part))
> (nth (plist-get part :id))
> (beg (point))
> ;; Hide the part initially if HIDE is t.
> (show-part (not (equal hide t)))
> ;; We omit the part button for the first (or only) part if
> ;; this is text/plain, or HIDE is 'no-buttons.
> - (button (unless (or (equal hide 'no-buttons)
> - (and (string= mime-type "text/plain") (<= nth 1)))
> + (button (when (and (not (equal hide 'no-buttons))
> + (notmuch-show-insert-header-p part))
> (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
> (content-beg (point)))
>
> --
> 2.0.0.rc0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/9] emacs/show: Re-arrange determination if a part header is necessary
2014-05-12 22:09 ` Mark Walters
@ 2014-05-13 9:47 ` David Edmondson
0 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2014-05-13 9:47 UTC (permalink / raw)
To: Mark Walters, notmuch
[-- Attachment #1: Type: text/plain, Size: 3019 bytes --]
On Mon, May 12 2014, Mark Walters wrote:
> On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
>> Move the determination of whether a part header is required to a
>> distinct function.
>> ---
>> emacs/notmuch-show.el | 25 ++++++++++++++++++-------
>> 1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index 10fc872..ec99141 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -871,6 +871,21 @@ message at DEPTH in the current thread."
>> ;; showable this returns nil.
>> (notmuch-show-create-part-overlays button part-beg part-end))))
>>
>> +(defun notmuch-show-mime-type (part)
>> + "Return the correct mime-type to use for PART."
>> + (let ((content-type (downcase (plist-get part :content-type))))
>> + (or (and (string= content-type "application/octet-stream")
>> + (notmuch-show-get-mime-type-of-application/octet-stream part))
>> + (and (string= content-type "inline patch")
>> + "text/x-diff")
>> + content-type)))
>> +
>> +(defun notmuch-show-insert-header-p (part)
>> + "Return non-NIL if a header button should be inserted for this part."
>> + (let ((mime-type (notmuch-show-mime-type part)))
>> + (not (and (string= mime-type "text/plain")
>> + (<= (plist-get part :id) 1)))))
>
> My only query here is whether a notmuch-show-hide-header-p (part) might
> make some of the logic clearer?
You're suggesting to simply invert the sense of the function?
>
> MW
>
>
>> +
>> (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
>> "Insert the body part PART at depth DEPTH in the current thread.
>>
>> @@ -881,19 +896,15 @@ is t, hide the part initially and show the button. If HIDE is
>> useful for quoting in replies)."
>>
>> (let* ((content-type (downcase (plist-get part :content-type)))
>> - (mime-type (or (and (string= content-type "application/octet-stream")
>> - (notmuch-show-get-mime-type-of-application/octet-stream part))
>> - (and (string= content-type "inline patch")
>> - "text/x-diff")
>> - content-type))
>> + (mime-type (notmuch-show-mime-type part))
>> (nth (plist-get part :id))
>> (beg (point))
>> ;; Hide the part initially if HIDE is t.
>> (show-part (not (equal hide t)))
>> ;; We omit the part button for the first (or only) part if
>> ;; this is text/plain, or HIDE is 'no-buttons.
>> - (button (unless (or (equal hide 'no-buttons)
>> - (and (string= mime-type "text/plain") (<= nth 1)))
>> + (button (when (and (not (equal hide 'no-buttons))
>> + (notmuch-show-insert-header-p part))
>> (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
>> (content-beg (point)))
>>
>> --
>> 2.0.0.rc0
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 310 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 2/9] emacs/show: Allow the user to decide when part headers should be inserted
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
2014-05-12 12:29 ` [PATCH v3 1/9] emacs/show: Re-arrange determination if a part header is necessary David Edmondson
@ 2014-05-12 12:29 ` David Edmondson
2014-05-12 22:15 ` Mark Walters
2014-05-12 12:29 ` [PATCH v3 3/9] emacs/show: Accommodate the lack of part header buttons David Edmondson
` (8 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2014-05-12 12:29 UTC (permalink / raw)
To: notmuch
Make the function that determines whether a part header should be
inserted a user controlled, with some example functions.
---
emacs/notmuch-show.el | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index ec99141..f78a0ab 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -135,6 +135,17 @@ indentation."
:type 'boolean
:group 'notmuch-show)
+(defcustom notmuch-show-insert-header-p-function 'notmuch-show-insert-header-p-smart
+ "A function to call to determine whether a MIME part should have a header button.
+
+The function is passed one argument, PART - the MIME part in
+question."
+ :type 'function
+ :group 'notmuch-show
+ :options '(notmuch-show-insert-header-p-smart
+ notmuch-show-insert-header-p-always
+ notmuch-show-insert-header-p-never))
+
(defvar notmuch-show-thread-id nil)
(make-variable-buffer-local 'notmuch-show-thread-id)
(put 'notmuch-show-thread-id 'permanent-local t)
@@ -880,12 +891,18 @@ message at DEPTH in the current thread."
"text/x-diff")
content-type)))
-(defun notmuch-show-insert-header-p (part)
+(defun notmuch-show-insert-header-p-smart (part)
"Return non-NIL if a header button should be inserted for this part."
(let ((mime-type (notmuch-show-mime-type part)))
(not (and (string= mime-type "text/plain")
(<= (plist-get part :id) 1)))))
+(defun notmuch-show-insert-header-p-always (part)
+ t)
+
+(defun notmuch-show-insert-header-p-never (part)
+ nil)
+
(defun notmuch-show-insert-bodypart (msg part depth &optional hide)
"Insert the body part PART at depth DEPTH in the current thread.
@@ -904,7 +921,7 @@ useful for quoting in replies)."
;; We omit the part button for the first (or only) part if
;; this is text/plain, or HIDE is 'no-buttons.
(button (when (and (not (equal hide 'no-buttons))
- (notmuch-show-insert-header-p part))
+ (funcall notmuch-show-insert-header-p-function part))
(notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
(content-beg (point)))
--
2.0.0.rc0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 2/9] emacs/show: Allow the user to decide when part headers should be inserted
2014-05-12 12:29 ` [PATCH v3 2/9] emacs/show: Allow the user to decide when part headers should be inserted David Edmondson
@ 2014-05-12 22:15 ` Mark Walters
2014-05-13 9:49 ` David Edmondson
0 siblings, 1 reply; 34+ messages in thread
From: Mark Walters @ 2014-05-12 22:15 UTC (permalink / raw)
To: David Edmondson, notmuch
On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
> Make the function that determines whether a part header should be
> inserted a user controlled, with some example functions.
> ---
> emacs/notmuch-show.el | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index ec99141..f78a0ab 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -135,6 +135,17 @@ indentation."
> :type 'boolean
> :group 'notmuch-show)
>
> +(defcustom notmuch-show-insert-header-p-function 'notmuch-show-insert-header-p-smart
> + "A function to call to determine whether a MIME part should have a header button.
> +
> +The function is passed one argument, PART - the MIME part in
> +question."
> + :type 'function
> + :group 'notmuch-show
> + :options '(notmuch-show-insert-header-p-smart
> + notmuch-show-insert-header-p-always
> + notmuch-show-insert-header-p-never))
> +
I think this is nice as a defcustom for testing/seeing what happens, but I am
not sure we want it exposed longer term: for example never makes it
nearly impossible to view pdf parts.
Best wishes
Mark
> (defvar notmuch-show-thread-id nil)
> (make-variable-buffer-local 'notmuch-show-thread-id)
> (put 'notmuch-show-thread-id 'permanent-local t)
> @@ -880,12 +891,18 @@ message at DEPTH in the current thread."
> "text/x-diff")
> content-type)))
>
> -(defun notmuch-show-insert-header-p (part)
> +(defun notmuch-show-insert-header-p-smart (part)
> "Return non-NIL if a header button should be inserted for this part."
> (let ((mime-type (notmuch-show-mime-type part)))
> (not (and (string= mime-type "text/plain")
> (<= (plist-get part :id) 1)))))
>
> +(defun notmuch-show-insert-header-p-always (part)
> + t)
> +
> +(defun notmuch-show-insert-header-p-never (part)
> + nil)
> +
> (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
> "Insert the body part PART at depth DEPTH in the current thread.
>
> @@ -904,7 +921,7 @@ useful for quoting in replies)."
> ;; We omit the part button for the first (or only) part if
> ;; this is text/plain, or HIDE is 'no-buttons.
> (button (when (and (not (equal hide 'no-buttons))
> - (notmuch-show-insert-header-p part))
> + (funcall notmuch-show-insert-header-p-function part))
> (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
> (content-beg (point)))
>
> --
> 2.0.0.rc0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/9] emacs/show: Allow the user to decide when part headers should be inserted
2014-05-12 22:15 ` Mark Walters
@ 2014-05-13 9:49 ` David Edmondson
0 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2014-05-13 9:49 UTC (permalink / raw)
To: Mark Walters, notmuch
[-- Attachment #1: Type: text/plain, Size: 2898 bytes --]
On Mon, May 12 2014, Mark Walters wrote:
> On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
>> Make the function that determines whether a part header should be
>> inserted a user controlled, with some example functions.
>> ---
>> emacs/notmuch-show.el | 21 +++++++++++++++++++--
>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index ec99141..f78a0ab 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -135,6 +135,17 @@ indentation."
>> :type 'boolean
>> :group 'notmuch-show)
>>
>> +(defcustom notmuch-show-insert-header-p-function 'notmuch-show-insert-header-p-smart
>> + "A function to call to determine whether a MIME part should have a header button.
>> +
>> +The function is passed one argument, PART - the MIME part in
>> +question."
>> + :type 'function
>> + :group 'notmuch-show
>> + :options '(notmuch-show-insert-header-p-smart
>> + notmuch-show-insert-header-p-always
>> + notmuch-show-insert-header-p-never))
>> +
>
> I think this is nice as a defcustom for testing/seeing what happens, but I am
> not sure we want it exposed longer term: for example never makes it
> nearly impossible to view pdf parts.
Okay.
>
> Best wishes
>
> Mark
>
>
>
>> (defvar notmuch-show-thread-id nil)
>> (make-variable-buffer-local 'notmuch-show-thread-id)
>> (put 'notmuch-show-thread-id 'permanent-local t)
>> @@ -880,12 +891,18 @@ message at DEPTH in the current thread."
>> "text/x-diff")
>> content-type)))
>>
>> -(defun notmuch-show-insert-header-p (part)
>> +(defun notmuch-show-insert-header-p-smart (part)
>> "Return non-NIL if a header button should be inserted for this part."
>> (let ((mime-type (notmuch-show-mime-type part)))
>> (not (and (string= mime-type "text/plain")
>> (<= (plist-get part :id) 1)))))
>>
>> +(defun notmuch-show-insert-header-p-always (part)
>> + t)
>> +
>> +(defun notmuch-show-insert-header-p-never (part)
>> + nil)
>> +
>> (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
>> "Insert the body part PART at depth DEPTH in the current thread.
>>
>> @@ -904,7 +921,7 @@ useful for quoting in replies)."
>> ;; We omit the part button for the first (or only) part if
>> ;; this is text/plain, or HIDE is 'no-buttons.
>> (button (when (and (not (equal hide 'no-buttons))
>> - (notmuch-show-insert-header-p part))
>> + (funcall notmuch-show-insert-header-p-function part))
>> (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
>> (content-beg (point)))
>>
>> --
>> 2.0.0.rc0
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 310 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 3/9] emacs/show: Accommodate the lack of part header buttons
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
2014-05-12 12:29 ` [PATCH v3 1/9] emacs/show: Re-arrange determination if a part header is necessary David Edmondson
2014-05-12 12:29 ` [PATCH v3 2/9] emacs/show: Allow the user to decide when part headers should be inserted David Edmondson
@ 2014-05-12 12:29 ` David Edmondson
2014-05-12 22:16 ` Mark Walters
2014-05-12 12:29 ` [PATCH v3 4/9] emacs/mua: Generate improved cited text for replies David Edmondson
` (7 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2014-05-12 12:29 UTC (permalink / raw)
To: notmuch
Various pieces of code assumed (reasonably) that part header buttons
are present. Modify them to avoid problems if no part headers were
inserted.
---
emacs/notmuch-show.el | 88 ++++++++++++++++++++++++++++-----------------------
1 file changed, 48 insertions(+), 40 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index f78a0ab..e511655 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -494,36 +494,37 @@ message at DEPTH in the current thread."
(defun notmuch-show-toggle-part-invisibility (&optional button)
(interactive)
- (let* ((button (or button (button-at (point))))
- (overlay (button-get button 'overlay))
- (lazy-part (button-get button :notmuch-lazy-part)))
- ;; We have a part to toggle if there is an overlay or if there is a lazy part.
- ;; If neither is present we cannot toggle the part so we just return nil.
- (when (or overlay lazy-part)
- (let* ((show (button-get button :notmuch-part-hidden))
- (new-start (button-start button))
- (button-label (button-get button :base-label))
- (old-point (point))
- (properties (text-properties-at (button-start button)))
- (inhibit-read-only t))
- ;; Toggle the button itself.
- (button-put button :notmuch-part-hidden (not show))
- (goto-char new-start)
- (insert "[ " button-label (if show " ]" " (hidden) ]"))
- (set-text-properties new-start (point) properties)
- (let ((old-end (button-end button)))
- (move-overlay button new-start (point))
- (delete-region (point) old-end))
- (goto-char (min old-point (1- (button-end button))))
- ;; Return nil if there is a lazy-part, it is empty, and we are
- ;; trying to show it. In all other cases return t.
- (if lazy-part
- (when show
- (button-put button :notmuch-lazy-part nil)
- (notmuch-show-lazy-part lazy-part button))
- ;; else there must be an overlay.
- (overlay-put overlay 'invisible (not show))
- t)))))
+ (let ((button (or button (button-at (point)))))
+ (when button
+ (let ((overlay (button-get button 'overlay))
+ (lazy-part (button-get button :notmuch-lazy-part)))
+ ;; We have a part to toggle if there is an overlay or if there is a lazy part.
+ ;; If neither is present we cannot toggle the part so we just return nil.
+ (when (or overlay lazy-part)
+ (let* ((show (button-get button :notmuch-part-hidden))
+ (new-start (button-start button))
+ (button-label (button-get button :base-label))
+ (old-point (point))
+ (properties (text-properties-at (button-start button)))
+ (inhibit-read-only t))
+ ;; Toggle the button itself.
+ (button-put button :notmuch-part-hidden (not show))
+ (goto-char new-start)
+ (insert "[ " button-label (if show " ]" " (hidden) ]"))
+ (set-text-properties new-start (point) properties)
+ (let ((old-end (button-end button)))
+ (move-overlay button new-start (point))
+ (delete-region (point) old-end))
+ (goto-char (min old-point (1- (button-end button))))
+ ;; Return nil if there is a lazy-part, it is empty, and we are
+ ;; trying to show it. In all other cases return t.
+ (if lazy-part
+ (when show
+ (button-put button :notmuch-lazy-part nil)
+ (notmuch-show-lazy-part lazy-part button))
+ ;; else there must be an overlay.
+ (overlay-put overlay 'invisible (not show))
+ t)))))))
;; MIME part renderers
@@ -632,14 +633,17 @@ message at DEPTH in the current thread."
t)
(defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth button)
- (button-put button 'face 'notmuch-crypto-part-header)
- ;; add signature status button if sigstatus provided
+ (when button
+ (button-put button 'face 'notmuch-crypto-part-header))
+ ;; Add signature status button if sigstatus provided.
(if (plist-member part :sigstatus)
(let* ((from (notmuch-show-get-header :From msg))
(sigstatus (car (plist-get part :sigstatus))))
(notmuch-crypto-insert-sigstatus-button sigstatus from))
- ;; if we're not adding sigstatus, tell the user how they can get it
- (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))
+ ;; If we're not adding the signature status, tell the user how
+ ;; they can get it.
+ (when button
+ (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts.")))
(let ((inner-parts (plist-get part :content))
(start (point)))
@@ -653,17 +657,20 @@ message at DEPTH in the current thread."
t)
(defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth button)
- (button-put button 'face 'notmuch-crypto-part-header)
- ;; add encryption status button if encstatus specified
+ (when button
+ (button-put button 'face 'notmuch-crypto-part-header))
+ ;; Add encryption status button if encryption status is specified.
(if (plist-member part :encstatus)
(let ((encstatus (car (plist-get part :encstatus))))
(notmuch-crypto-insert-encstatus-button encstatus)
- ;; add signature status button if sigstatus specified
+ ;; Add signature status button if signature status is
+ ;; specified.
(if (plist-member part :sigstatus)
(let* ((from (notmuch-show-get-header :From msg))
(sigstatus (car (plist-get part :sigstatus))))
(notmuch-crypto-insert-sigstatus-button sigstatus from))))
- ;; if we're not adding encstatus, tell the user how they can get it
+ ;; If we're not adding the encryption status, tell the user how
+ ;; they can get it.
(button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))
(let ((inner-parts (plist-get part :content))
@@ -930,8 +937,9 @@ useful for quoting in replies)."
(if show-part
(notmuch-show-insert-bodypart-internal msg part mime-type nth depth button)
- (button-put button :notmuch-lazy-part
- (list msg part mime-type nth depth button)))
+ (when button
+ (button-put button :notmuch-lazy-part
+ (list msg part mime-type nth depth button))))
;; Some of the body part handlers leave point somewhere up in the
;; part, so we make sure that we're down at the end.
--
2.0.0.rc0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 3/9] emacs/show: Accommodate the lack of part header buttons
2014-05-12 12:29 ` [PATCH v3 3/9] emacs/show: Accommodate the lack of part header buttons David Edmondson
@ 2014-05-12 22:16 ` Mark Walters
0 siblings, 0 replies; 34+ messages in thread
From: Mark Walters @ 2014-05-12 22:16 UTC (permalink / raw)
To: David Edmondson, notmuch
On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
> Various pieces of code assumed (reasonably) that part header buttons
> are present. Modify them to avoid problems if no part headers were
> inserted.
Despite your reservations on irc this looks fine to me.
MW
> ---
> emacs/notmuch-show.el | 88 ++++++++++++++++++++++++++++-----------------------
> 1 file changed, 48 insertions(+), 40 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index f78a0ab..e511655 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -494,36 +494,37 @@ message at DEPTH in the current thread."
>
> (defun notmuch-show-toggle-part-invisibility (&optional button)
> (interactive)
> - (let* ((button (or button (button-at (point))))
> - (overlay (button-get button 'overlay))
> - (lazy-part (button-get button :notmuch-lazy-part)))
> - ;; We have a part to toggle if there is an overlay or if there is a lazy part.
> - ;; If neither is present we cannot toggle the part so we just return nil.
> - (when (or overlay lazy-part)
> - (let* ((show (button-get button :notmuch-part-hidden))
> - (new-start (button-start button))
> - (button-label (button-get button :base-label))
> - (old-point (point))
> - (properties (text-properties-at (button-start button)))
> - (inhibit-read-only t))
> - ;; Toggle the button itself.
> - (button-put button :notmuch-part-hidden (not show))
> - (goto-char new-start)
> - (insert "[ " button-label (if show " ]" " (hidden) ]"))
> - (set-text-properties new-start (point) properties)
> - (let ((old-end (button-end button)))
> - (move-overlay button new-start (point))
> - (delete-region (point) old-end))
> - (goto-char (min old-point (1- (button-end button))))
> - ;; Return nil if there is a lazy-part, it is empty, and we are
> - ;; trying to show it. In all other cases return t.
> - (if lazy-part
> - (when show
> - (button-put button :notmuch-lazy-part nil)
> - (notmuch-show-lazy-part lazy-part button))
> - ;; else there must be an overlay.
> - (overlay-put overlay 'invisible (not show))
> - t)))))
> + (let ((button (or button (button-at (point)))))
> + (when button
> + (let ((overlay (button-get button 'overlay))
> + (lazy-part (button-get button :notmuch-lazy-part)))
> + ;; We have a part to toggle if there is an overlay or if there is a lazy part.
> + ;; If neither is present we cannot toggle the part so we just return nil.
> + (when (or overlay lazy-part)
> + (let* ((show (button-get button :notmuch-part-hidden))
> + (new-start (button-start button))
> + (button-label (button-get button :base-label))
> + (old-point (point))
> + (properties (text-properties-at (button-start button)))
> + (inhibit-read-only t))
> + ;; Toggle the button itself.
> + (button-put button :notmuch-part-hidden (not show))
> + (goto-char new-start)
> + (insert "[ " button-label (if show " ]" " (hidden) ]"))
> + (set-text-properties new-start (point) properties)
> + (let ((old-end (button-end button)))
> + (move-overlay button new-start (point))
> + (delete-region (point) old-end))
> + (goto-char (min old-point (1- (button-end button))))
> + ;; Return nil if there is a lazy-part, it is empty, and we are
> + ;; trying to show it. In all other cases return t.
> + (if lazy-part
> + (when show
> + (button-put button :notmuch-lazy-part nil)
> + (notmuch-show-lazy-part lazy-part button))
> + ;; else there must be an overlay.
> + (overlay-put overlay 'invisible (not show))
> + t)))))))
>
> ;; MIME part renderers
>
> @@ -632,14 +633,17 @@ message at DEPTH in the current thread."
> t)
>
> (defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth button)
> - (button-put button 'face 'notmuch-crypto-part-header)
> - ;; add signature status button if sigstatus provided
> + (when button
> + (button-put button 'face 'notmuch-crypto-part-header))
> + ;; Add signature status button if sigstatus provided.
> (if (plist-member part :sigstatus)
> (let* ((from (notmuch-show-get-header :From msg))
> (sigstatus (car (plist-get part :sigstatus))))
> (notmuch-crypto-insert-sigstatus-button sigstatus from))
> - ;; if we're not adding sigstatus, tell the user how they can get it
> - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))
> + ;; If we're not adding the signature status, tell the user how
> + ;; they can get it.
> + (when button
> + (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts.")))
>
> (let ((inner-parts (plist-get part :content))
> (start (point)))
> @@ -653,17 +657,20 @@ message at DEPTH in the current thread."
> t)
>
> (defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth button)
> - (button-put button 'face 'notmuch-crypto-part-header)
> - ;; add encryption status button if encstatus specified
> + (when button
> + (button-put button 'face 'notmuch-crypto-part-header))
> + ;; Add encryption status button if encryption status is specified.
> (if (plist-member part :encstatus)
> (let ((encstatus (car (plist-get part :encstatus))))
> (notmuch-crypto-insert-encstatus-button encstatus)
> - ;; add signature status button if sigstatus specified
> + ;; Add signature status button if signature status is
> + ;; specified.
> (if (plist-member part :sigstatus)
> (let* ((from (notmuch-show-get-header :From msg))
> (sigstatus (car (plist-get part :sigstatus))))
> (notmuch-crypto-insert-sigstatus-button sigstatus from))))
> - ;; if we're not adding encstatus, tell the user how they can get it
> + ;; If we're not adding the encryption status, tell the user how
> + ;; they can get it.
> (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))
>
> (let ((inner-parts (plist-get part :content))
> @@ -930,8 +937,9 @@ useful for quoting in replies)."
>
> (if show-part
> (notmuch-show-insert-bodypart-internal msg part mime-type nth depth button)
> - (button-put button :notmuch-lazy-part
> - (list msg part mime-type nth depth button)))
> + (when button
> + (button-put button :notmuch-lazy-part
> + (list msg part mime-type nth depth button))))
>
> ;; Some of the body part handlers leave point somewhere up in the
> ;; part, so we make sure that we're down at the end.
> --
> 2.0.0.rc0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 4/9] emacs/mua: Generate improved cited text for replies
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
` (2 preceding siblings ...)
2014-05-12 12:29 ` [PATCH v3 3/9] emacs/show: Accommodate the lack of part header buttons David Edmondson
@ 2014-05-12 12:29 ` David Edmondson
2014-05-12 22:30 ` Mark Walters
2014-05-12 12:29 ` [PATCH v3 5/9] emacs/show: Remove the 'no-buttons option of `notmuch-show-insert-bodypart' David Edmondson
` (6 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2014-05-12 12:29 UTC (permalink / raw)
To: notmuch
Use the message display code to generate message text to cite in
replies.
---
emacs/notmuch-mua.el | 38 ++++++++------------------------------
1 file changed, 8 insertions(+), 30 deletions(-)
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 95e4a4d..09c922f 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -28,7 +28,7 @@
(eval-when-compile (require 'cl))
-(declare-function notmuch-show-insert-bodypart "notmuch-show" (msg part depth &optional hide))
+(declare-function notmuch-show-insert-body "notmuch-show" (msg body depth))
;;
@@ -123,31 +123,6 @@ list."
else if (notmuch-match-content-type (plist-get part :content-type) "multipart/*")
do (notmuch-mua-reply-crypto (plist-get part :content))))
-(defun notmuch-mua-get-quotable-parts (parts)
- (loop for part in parts
- if (notmuch-match-content-type (plist-get part :content-type) "multipart/alternative")
- collect (let* ((subparts (plist-get part :content))
- (types (mapcar (lambda (part) (plist-get part :content-type)) subparts))
- (chosen-type (car (notmuch-multipart/alternative-choose types))))
- (loop for part in (reverse subparts)
- if (notmuch-match-content-type (plist-get part :content-type) chosen-type)
- return part))
- else if (notmuch-match-content-type (plist-get part :content-type) "multipart/*")
- append (notmuch-mua-get-quotable-parts (plist-get part :content))
- else if (notmuch-match-content-type (plist-get part :content-type) "text/*")
- collect part))
-
-(defun notmuch-mua-insert-quotable-part (message part)
- ;; We don't want text properties leaking from the show renderer into
- ;; the reply so we use a temp buffer. Also we don't want hooks, such
- ;; as notmuch-wash-*, to be run on the quotable part so we set
- ;; notmuch-show-insert-text/plain-hook to nil.
- (insert (with-temp-buffer
- (let ((notmuch-show-insert-text/plain-hook nil))
- ;; Show the part but do not add buttons.
- (notmuch-show-insert-bodypart message part 0 'no-buttons))
- (buffer-substring-no-properties (point-min) (point-max)))))
-
;; There is a bug in emacs 23's message.el that results in a newline
;; not being inserted after the References header, so the next header
;; is concatenated to the end of it. This function fixes the problem,
@@ -225,10 +200,13 @@ list."
(insert "From: " from "\n")
(insert "Date: " date "\n\n")
- ;; Get the parts of the original message that should be quoted; this includes
- ;; all the text parts, except the non-preferred ones in a multipart/alternative.
- (let ((quotable-parts (notmuch-mua-get-quotable-parts (plist-get original :body))))
- (mapc (apply-partially 'notmuch-mua-insert-quotable-part original) quotable-parts))
+ (insert (with-temp-buffer
+ ;; Don't attempt to clean up messages, excerpt
+ ;; citations, etc. in the original message before
+ ;; quoting.
+ (let ((notmuch-show-insert-text/plain-hook nil))
+ (notmuch-show-insert-body original (plist-get original :body) 0)
+ (buffer-substring-no-properties (point-min) (point-max)))))
(set-mark (point))
(goto-char start)
--
2.0.0.rc0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 4/9] emacs/mua: Generate improved cited text for replies
2014-05-12 12:29 ` [PATCH v3 4/9] emacs/mua: Generate improved cited text for replies David Edmondson
@ 2014-05-12 22:30 ` Mark Walters
2014-05-13 10:06 ` David Edmondson
0 siblings, 1 reply; 34+ messages in thread
From: Mark Walters @ 2014-05-12 22:30 UTC (permalink / raw)
To: David Edmondson, notmuch
On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
> Use the message display code to generate message text to cite in
> replies.
So this is the key change. I am trying to work out what the actual
changes are here: in your commit message for the test update 7/9 you say
that the old code only output the first part. My impression of the
deleted code is that that is not the case (but I don't grok cl very
well).
I think the test change is because in show we do some content-type
guessing of application/octet-stream which the below doesn't do.
But we may also have some things with mm-inlined-types as mentioned in
my earlier reply. Anyway if you can point out any other cases where it
is changed that would be great.
If you go for a function deciding which parts to include then it might
be possible to have a midpoint where we are the same as before, and then
tweak the function to get whatever behaviour we think is best. That
might make it easy to see what is tidying/unification and what is
enhancement.
Incidentally, thank you for splitting this series so finely: I did find
that made it a lot easier to review.
Best wishes
Mark
> ---
> emacs/notmuch-mua.el | 38 ++++++++------------------------------
> 1 file changed, 8 insertions(+), 30 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 95e4a4d..09c922f 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -28,7 +28,7 @@
>
> (eval-when-compile (require 'cl))
>
> -(declare-function notmuch-show-insert-bodypart "notmuch-show" (msg part depth &optional hide))
> +(declare-function notmuch-show-insert-body "notmuch-show" (msg body depth))
>
> ;;
>
> @@ -123,31 +123,6 @@ list."
> else if (notmuch-match-content-type (plist-get part :content-type) "multipart/*")
> do (notmuch-mua-reply-crypto (plist-get part :content))))
>
> -(defun notmuch-mua-get-quotable-parts (parts)
> - (loop for part in parts
> - if (notmuch-match-content-type (plist-get part :content-type) "multipart/alternative")
> - collect (let* ((subparts (plist-get part :content))
> - (types (mapcar (lambda (part) (plist-get part :content-type)) subparts))
> - (chosen-type (car (notmuch-multipart/alternative-choose types))))
> - (loop for part in (reverse subparts)
> - if (notmuch-match-content-type (plist-get part :content-type) chosen-type)
> - return part))
> - else if (notmuch-match-content-type (plist-get part :content-type) "multipart/*")
> - append (notmuch-mua-get-quotable-parts (plist-get part :content))
> - else if (notmuch-match-content-type (plist-get part :content-type) "text/*")
> - collect part))
> -
> -(defun notmuch-mua-insert-quotable-part (message part)
> - ;; We don't want text properties leaking from the show renderer into
> - ;; the reply so we use a temp buffer. Also we don't want hooks, such
> - ;; as notmuch-wash-*, to be run on the quotable part so we set
> - ;; notmuch-show-insert-text/plain-hook to nil.
> - (insert (with-temp-buffer
> - (let ((notmuch-show-insert-text/plain-hook nil))
> - ;; Show the part but do not add buttons.
> - (notmuch-show-insert-bodypart message part 0 'no-buttons))
> - (buffer-substring-no-properties (point-min) (point-max)))))
> -
> ;; There is a bug in emacs 23's message.el that results in a newline
> ;; not being inserted after the References header, so the next header
> ;; is concatenated to the end of it. This function fixes the problem,
> @@ -225,10 +200,13 @@ list."
> (insert "From: " from "\n")
> (insert "Date: " date "\n\n")
>
> - ;; Get the parts of the original message that should be quoted; this includes
> - ;; all the text parts, except the non-preferred ones in a multipart/alternative.
> - (let ((quotable-parts (notmuch-mua-get-quotable-parts (plist-get original :body))))
> - (mapc (apply-partially 'notmuch-mua-insert-quotable-part original) quotable-parts))
> + (insert (with-temp-buffer
> + ;; Don't attempt to clean up messages, excerpt
> + ;; citations, etc. in the original message before
> + ;; quoting.
> + (let ((notmuch-show-insert-text/plain-hook nil))
> + (notmuch-show-insert-body original (plist-get original :body) 0)
> + (buffer-substring-no-properties (point-min) (point-max)))))
>
> (set-mark (point))
> (goto-char start)
> --
> 2.0.0.rc0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 4/9] emacs/mua: Generate improved cited text for replies
2014-05-12 22:30 ` Mark Walters
@ 2014-05-13 10:06 ` David Edmondson
0 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2014-05-13 10:06 UTC (permalink / raw)
To: Mark Walters, notmuch
[-- Attachment #1: Type: text/plain, Size: 2051 bytes --]
On Mon, May 12 2014, Mark Walters wrote:
> On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
>> Use the message display code to generate message text to cite in
>> replies.
>
> So this is the key change. I am trying to work out what the actual
> changes are here: in your commit message for the test update 7/9 you say
> that the old code only output the first part. My impression of the
> deleted code is that that is not the case (but I don't grok cl very
> well).
The comment in the test change was intended only to refer to the content
displayed in that specific test. You are correct that text from several
parts could previously have been used (and all smashed together in a way
that made it difficult to see the boundaries between the parts...).
> I think the test change is because in show we do some content-type
> guessing of application/octet-stream which the below doesn't do.
>
> But we may also have some things with mm-inlined-types as mentioned in
> my earlier reply. Anyway if you can point out any other cases where it
> is changed that would be great.
message/rfc822 was the initial driver. If you now reply to a
multipart/mixed that contains text/plain and message/rfc822 parts, you
will see a significant difference.
I'm not sure if you have it, but
id:mailman.1.1399324386.31850.notmuch@notmuchmail.org was the message I
used often when playing around.
> If you go for a function deciding which parts to include then it might
> be possible to have a midpoint where we are the same as before, and then
> tweak the function to get whatever behaviour we think is best. That
> might make it easy to see what is tidying/unification and what is
> enhancement.
I will look into that - it's a good suggestion, though it actually runs
counter to:
> Incidentally, thank you for splitting this series so finely: I did find
> that made it a lot easier to review.
Making the new code (which relies on the 'show' rendering code) behave
just like the old code will involve adding a bunch of complication to
this specific patch.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 310 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 5/9] emacs/show: Remove the 'no-buttons option of `notmuch-show-insert-bodypart'
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
` (3 preceding siblings ...)
2014-05-12 12:29 ` [PATCH v3 4/9] emacs/mua: Generate improved cited text for replies David Edmondson
@ 2014-05-12 12:29 ` David Edmondson
2014-05-12 22:18 ` Mark Walters
2014-05-12 12:29 ` [PATCH v3 6/9] emacs/mua: Don't insert part headers in citations David Edmondson
` (5 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2014-05-12 12:29 UTC (permalink / raw)
To: notmuch
No code uses the 'no-buttons argument to
`notmuch-show-insert-bodypart', so remove it.
---
emacs/notmuch-show.el | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e511655..981b922 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -915,9 +915,7 @@ message at DEPTH in the current thread."
HIDE determines whether to show or hide the part and the button
as follows: If HIDE is nil, show the part and the button. If HIDE
-is t, hide the part initially and show the button. If HIDE is
-'no-buttons, show the part but do not add any buttons (this is
-useful for quoting in replies)."
+is t, hide the part initially and show the button."
(let* ((content-type (downcase (plist-get part :content-type)))
(mime-type (notmuch-show-mime-type part))
@@ -926,9 +924,8 @@ useful for quoting in replies)."
;; Hide the part initially if HIDE is t.
(show-part (not (equal hide t)))
;; We omit the part button for the first (or only) part if
- ;; this is text/plain, or HIDE is 'no-buttons.
- (button (when (and (not (equal hide 'no-buttons))
- (funcall notmuch-show-insert-header-p-function part))
+ ;; this is text/plain.
+ (button (when (funcall notmuch-show-insert-header-p-function part)
(notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
(content-beg (point)))
--
2.0.0.rc0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 5/9] emacs/show: Remove the 'no-buttons option of `notmuch-show-insert-bodypart'
2014-05-12 12:29 ` [PATCH v3 5/9] emacs/show: Remove the 'no-buttons option of `notmuch-show-insert-bodypart' David Edmondson
@ 2014-05-12 22:18 ` Mark Walters
0 siblings, 0 replies; 34+ messages in thread
From: Mark Walters @ 2014-05-12 22:18 UTC (permalink / raw)
To: David Edmondson, notmuch
On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
> No code uses the 'no-buttons argument to
> `notmuch-show-insert-bodypart', so remove it.
> ---
> emacs/notmuch-show.el | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index e511655..981b922 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -915,9 +915,7 @@ message at DEPTH in the current thread."
>
> HIDE determines whether to show or hide the part and the button
> as follows: If HIDE is nil, show the part and the button. If HIDE
> -is t, hide the part initially and show the button. If HIDE is
> -'no-buttons, show the part but do not add any buttons (this is
> -useful for quoting in replies)."
> +is t, hide the part initially and show the button."
>
> (let* ((content-type (downcase (plist-get part :content-type)))
> (mime-type (notmuch-show-mime-type part))
> @@ -926,9 +924,8 @@ useful for quoting in replies)."
> ;; Hide the part initially if HIDE is t.
> (show-part (not (equal hide t)))
Just a triviality: we might as well make the above (not hide) now.
MW
> ;; We omit the part button for the first (or only) part if
> - ;; this is text/plain, or HIDE is 'no-buttons.
> - (button (when (and (not (equal hide 'no-buttons))
> - (funcall notmuch-show-insert-header-p-function part))
> + ;; this is text/plain.
> + (button (when (funcall notmuch-show-insert-header-p-function part)
> (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
> (content-beg (point)))
>
> --
> 2.0.0.rc0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 6/9] emacs/mua: Don't insert part headers in citations
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
` (4 preceding siblings ...)
2014-05-12 12:29 ` [PATCH v3 5/9] emacs/show: Remove the 'no-buttons option of `notmuch-show-insert-bodypart' David Edmondson
@ 2014-05-12 12:29 ` David Edmondson
2014-05-12 12:29 ` [PATCH v3 7/9] test: Update the test output to accord with the reply changes David Edmondson
` (4 subsequent siblings)
10 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2014-05-12 12:29 UTC (permalink / raw)
To: notmuch
Avoid inserting part headers when rendering a message body for
citation purposes.
---
emacs/notmuch-mua.el | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 09c922f..c800c38 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -201,10 +201,13 @@ list."
(insert "Date: " date "\n\n")
(insert (with-temp-buffer
- ;; Don't attempt to clean up messages, excerpt
- ;; citations, etc. in the original message before
- ;; quoting.
- (let ((notmuch-show-insert-text/plain-hook nil))
+ (let
+ ;; Don't attempt to clean up messages, excerpt
+ ;; citations, etc. in the original message before
+ ;; quoting.
+ ((notmuch-show-insert-text/plain-hook nil)
+ ;; Don't insert part buttons.
+ (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-never))
(notmuch-show-insert-body original (plist-get original :body) 0)
(buffer-substring-no-properties (point-min) (point-max)))))
--
2.0.0.rc0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v3 7/9] test: Update the test output to accord with the reply changes
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
` (5 preceding siblings ...)
2014-05-12 12:29 ` [PATCH v3 6/9] emacs/mua: Don't insert part headers in citations David Edmondson
@ 2014-05-12 12:29 ` David Edmondson
2014-05-13 12:16 ` Mark Walters
2014-05-12 12:29 ` [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message David Edmondson
` (3 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2014-05-12 12:29 UTC (permalink / raw)
To: notmuch
Replying to a message with multiple parts will now typically include
content from several parts (whereas previously only the first part was
used). Update the expected output from the emacs reply tests
accordingly.
---
test/T310-emacs.sh | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index ac966e5..95bb67e 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -473,6 +473,38 @@ Alex Botero-Lowry <alex.boterolowry@gmail.com> writes:
> and http://mail-index.netbsd.org/pkgsrc-bugs/2006/06/07/msg016808.htmlspecifically
> uses 64 as the
> buffer size.
+> From e3bc4bbd7b9d0d086816ab5f8f2d6ffea1dd3ea4 Mon Sep 17 00:00:00 2001
+> From: Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+> Date: Tue, 17 Nov 2009 11:30:39 -0800
+> Subject: [PATCH] Deal with situation where sysconf(_SC_GETPW_R_SIZE_MAX) returns -1
+>
+> ---
+> notmuch-config.c | 2 ++
+> 1 files changed, 2 insertions(+), 0 deletions(-)
+>
+> diff --git a/notmuch-config.c b/notmuch-config.c
+> index 248149c..e7220d8 100644
+> --- a/notmuch-config.c
+> +++ b/notmuch-config.c
+> @@ -77,6 +77,7 @@ static char *
+> get_name_from_passwd_file (void *ctx)
+> {
+> long pw_buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
+> + if (pw_buf_size == -1) pw_buf_size = 64;
+> char *pw_buf = talloc_zero_size (ctx, pw_buf_size);
+> struct passwd passwd, *ignored;
+> char *name;
+> @@ -101,6 +102,7 @@ static char *
+> get_username_from_passwd_file (void *ctx)
+> {
+> long pw_buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
+> + if (pw_buf_size == -1) pw_buf_size = 64;
+> char *pw_buf = talloc_zero_size (ctx, pw_buf_size);
+> struct passwd passwd, *ignored;
+> char *name;
+> --
+> 1.6.5.2
+>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
--
2.0.0.rc0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 7/9] test: Update the test output to accord with the reply changes
2014-05-12 12:29 ` [PATCH v3 7/9] test: Update the test output to accord with the reply changes David Edmondson
@ 2014-05-13 12:16 ` Mark Walters
0 siblings, 0 replies; 34+ messages in thread
From: Mark Walters @ 2014-05-13 12:16 UTC (permalink / raw)
To: David Edmondson, notmuch
On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
> Replying to a message with multiple parts will now typically include
> content from several parts (whereas previously only the first part was
> used). Update the expected output from the emacs reply tests
> accordingly.
I will reply here rather than to id:cunr43y56wy.fsf@hotblack-desiato.hh.sledj.net
My point was that the change in this test is not because we are now
looking beyond the first part. It is because a part with content-type
application/octet-stream (which we didn't include in replies in the old
scheme, and wouldn't generally in the new scheme) is redesignated by
notmuch-show as a content-type text/x-diff, and thus displayed in the
buffer. Hence the new code includes it in the reply.
You can see this more clearly in patch 9/9 when this part gets a header button
+> [ 0001-Deal-with-situation-where-sysconf-_SC_GETPW_R_SIZE_M.patch: application/octet-stream (as text/x-diff) ]
Anyway, my point is that this is a *good* thing: the change is not a
large intrusive thing of showing lots more parts (as this commit message
sort of suggests) but a small change of it now correctly including a
part which got sent with the wrong content-type.
Best wishes
Mark
> ---
> test/T310-emacs.sh | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
> index ac966e5..95bb67e 100755
> --- a/test/T310-emacs.sh
> +++ b/test/T310-emacs.sh
> @@ -473,6 +473,38 @@ Alex Botero-Lowry <alex.boterolowry@gmail.com> writes:
> > and http://mail-index.netbsd.org/pkgsrc-bugs/2006/06/07/msg016808.htmlspecifically
> > uses 64 as the
> > buffer size.
> +> From e3bc4bbd7b9d0d086816ab5f8f2d6ffea1dd3ea4 Mon Sep 17 00:00:00 2001
> +> From: Alexander Botero-Lowry <alex.boterolowry@gmail.com>
> +> Date: Tue, 17 Nov 2009 11:30:39 -0800
> +> Subject: [PATCH] Deal with situation where sysconf(_SC_GETPW_R_SIZE_MAX) returns -1
> +>
> +> ---
> +> notmuch-config.c | 2 ++
> +> 1 files changed, 2 insertions(+), 0 deletions(-)
> +>
> +> diff --git a/notmuch-config.c b/notmuch-config.c
> +> index 248149c..e7220d8 100644
> +> --- a/notmuch-config.c
> +> +++ b/notmuch-config.c
> +> @@ -77,6 +77,7 @@ static char *
> +> get_name_from_passwd_file (void *ctx)
> +> {
> +> long pw_buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
> +> + if (pw_buf_size == -1) pw_buf_size = 64;
> +> char *pw_buf = talloc_zero_size (ctx, pw_buf_size);
> +> struct passwd passwd, *ignored;
> +> char *name;
> +> @@ -101,6 +102,7 @@ static char *
> +> get_username_from_passwd_file (void *ctx)
> +> {
> +> long pw_buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
> +> + if (pw_buf_size == -1) pw_buf_size = 64;
> +> char *pw_buf = talloc_zero_size (ctx, pw_buf_size);
> +> struct passwd passwd, *ignored;
> +> char *name;
> +> --
> +> 1.6.5.2
> +>
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> --
> 2.0.0.rc0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
` (6 preceding siblings ...)
2014-05-12 12:29 ` [PATCH v3 7/9] test: Update the test output to accord with the reply changes David Edmondson
@ 2014-05-12 12:29 ` David Edmondson
2015-01-19 20:06 ` Mark Walters
2014-05-12 12:29 ` [PATCH v3 9/9] test: Update the test output to accord with more reply changes David Edmondson
` (2 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2014-05-12 12:29 UTC (permalink / raw)
To: notmuch
Whether to insert part headers should depend on the details of the
message being cited.
---
emacs/notmuch-mua.el | 45 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index c800c38..239cc1a 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -132,6 +132,47 @@ list."
(funcall original-func header references)
(unless (bolp) (insert "\n")))
+(defun notmuch-mua-reply-determine-part-function (message)
+ "Determine the part header rendering function to use when
+citing MESSAGE."
+
+ (let* ((body (plist-get message :body))
+ (first-part (car body)))
+ (cond
+ ;; If there are multiple top-level parts, we need the
+ ;; headers. Will this ever happen?
+ ((> (length body) 1)
+ #'notmuch-show-insert-header-p-always)
+
+ ;; If the type of the part is multipart/mixed, we need to see the
+ ;; part headers.
+ ((notmuch-match-content-type (plist-get first-part :content-type) "multipart/mixed")
+ #'notmuch-show-insert-header-p-always)
+
+ ;; If it is multipart/alternative the renderer will choose a
+ ;; default part and render it. There's no need to show the
+ ;; alternative part buttons, as they are not active.
+ ((notmuch-match-content-type (plist-get first-part :content-type) "multipart/alternative")
+ #'notmuch-show-insert-header-p-never)
+
+ ;; If it's a multipart/signed with a single text/* part and a
+ ;; signature, we don't need to see the headers.
+ ((let ((inner-content (plist-get first-part :content)))
+ (and (notmuch-match-content-type (plist-get first-part :content-type) "multipart/signed")
+ (eq (length inner-content) 2)
+ (notmuch-match-content-type (plist-get (nth 0 inner-content) :content-type) "text/*")
+ (notmuch-match-content-type (plist-get (nth 1 inner-content) :content-type) "application/pgp-signature")))
+ #'notmuch-show-insert-header-p-never)
+
+ ;; If the type of the part is text/*, we don't need to see the
+ ;; part headers.
+ ((notmuch-match-content-type (plist-get first-part :content-type) "text/*")
+ #'notmuch-show-insert-header-p-never)
+
+ ;; Otherwise insert the part headers.
+ (t
+ #'notmuch-show-insert-header-p-always))))
+
(defun notmuch-mua-reply (query-string &optional sender reply-all)
(let ((args '("reply" "--format=sexp" "--format-version=1"))
(process-crypto notmuch-show-process-crypto)
@@ -206,8 +247,8 @@ list."
;; citations, etc. in the original message before
;; quoting.
((notmuch-show-insert-text/plain-hook nil)
- ;; Don't insert part buttons.
- (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-never))
+ ;; Determine how to insert part headers.
+ (notmuch-show-insert-header-p-function (notmuch-mua-reply-determine-part-function original)))
(notmuch-show-insert-body original (plist-get original :body) 0)
(buffer-substring-no-properties (point-min) (point-max)))))
--
2.0.0.rc0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message
2014-05-12 12:29 ` [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message David Edmondson
@ 2015-01-19 20:06 ` Mark Walters
2015-01-20 8:44 ` David Edmondson
0 siblings, 1 reply; 34+ messages in thread
From: Mark Walters @ 2015-01-19 20:06 UTC (permalink / raw)
To: David Edmondson, notmuch
On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
> Whether to insert part headers should depend on the details of the
> message being cited.
Hi
Overall I like this series and it does fix two annoying bugs (not being
able to reply to ref822 messages and (correctly) including parts
which have application/octet-stream but are actually text parts).
The one problem is getting the right choice for part headers in the
reply text. I think getting this wrong will irritate users even if the
overall result is better.
My guess at the correct logic is:
1) omit the part header for any empty part: (ie a part we don't display
such as a pdf file).
2) omit multipart/* part headers
3) include all other part headers
4) except omit the first part header (perhaps only in the case it is text/plain)
My reasoning for each is
1) there is no point in saying we had a part which we are omitting.
2) all the subparts of multipart/* will get there own header which
should be sufficient.
3) we want to keep the parts distinguished
4) except we don't need to do that with the first part.
Note for 4) it would be good to have a multipart/alternative with
subparts text/plain and text/html just give the text/plain with no part
header.
I include a patch below which does all of these apart from 4) as I
couldn't see a clean way of implementing it. Any suggestions?
It should apply on top of patch 6 or 7 instead of 8. The key change is
that it always puts in a button and then deletes it if unwanted: this
makes doing 1) above easy.
It does break some tests, nothing unexpected except an interaction with
the way we wash text/plain parts: we remove leading blank lines from the
first text/plain part (because it doesn't have a button) but not from
subsequent ones (because they do). Because this code always has the
second case it doesn't remove a leading blank line of the first part.
Best wishes
Mark
From 8f198b38e76e050ae8d20d866748c41ccf79f3d4 Mon Sep 17 00:00:00 2001
From: Mark Walters <markwalters1009@gmail.com>
Date: Mon, 19 Jan 2015 14:39:25 +0000
Subject: [PATCH] emacs show/reply modify part handling
Modify the part handling so that we always insert the button and
delete it afterwards if not wanted. The advantage is that we can
decide whether to keep the part button based on what the insertion
code does. In particular the reply code can omit the button for all
parts with no displayable content.
---
emacs/notmuch-mua.el | 5 +++--
emacs/notmuch-show.el | 39 +++++++++++++++++++++++++--------------
2 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 0ca9eed..6060f33 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -29,6 +29,7 @@
(eval-when-compile (require 'cl))
(declare-function notmuch-show-insert-body "notmuch-show" (msg body depth))
+(declare-function notmuch-show-insert-header-p-reply "notmuch-show" (part empty-part))
;;
@@ -223,8 +224,8 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
;; citations, etc. in the original message before
;; quoting.
((notmuch-show-insert-text/plain-hook nil)
- ;; Don't insert part buttons.
- (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-never))
+ ;; Insert part buttons appropriate for a reply.
+ (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-reply))
(notmuch-show-insert-body original (plist-get original :body) 0)
(buffer-substring-no-properties (point-min) (point-max)))))
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 4a0899f..2cdb5a8 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -909,16 +909,24 @@ message at DEPTH in the current thread."
"text/x-diff")
content-type)))
-(defun notmuch-show-insert-header-p-smart (part)
+(defun notmuch-show-insert-header-p-smart (part empty-part)
"Return non-NIL if a header button should be inserted for this part."
(let ((mime-type (notmuch-show-mime-type part)))
(not (and (string= mime-type "text/plain")
(<= (plist-get part :id) 1)))))
-(defun notmuch-show-insert-header-p-always (part)
+(defun notmuch-show-insert-header-p-reply (part empty-part)
+ "Return non-NIL if a header button should be inserted for this part."
+ (let ((mime-type (notmuch-show-mime-type part)))
+ (not (or empty-part
+ (notmuch-match-content-type mime-type "multipart/*")
+ (and (string= mime-type "text/plain")
+ (<= (plist-get part :id) 1))))))
+
+(defun notmuch-show-insert-header-p-always (part empty-part)
t)
-(defun notmuch-show-insert-header-p-never (part)
+(defun notmuch-show-insert-header-p-never (part empty-part)
nil)
(defun notmuch-show-insert-bodypart (msg part depth &optional hide)
@@ -936,8 +944,8 @@ is t, hide the part initially and show the button."
(show-part (not (equal hide t)))
;; We omit the part button for the first (or only) part if
;; this is text/plain.
- (button (when (funcall notmuch-show-insert-header-p-function part)
- (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
+ (button-beg (point))
+ (button (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename)))
(content-beg (point)))
;; Store the computed mime-type for later use (e.g. by attachment handlers).
@@ -952,15 +960,18 @@ is t, hide the part initially and show the button."
;; Some of the body part handlers leave point somewhere up in the
;; part, so we make sure that we're down at the end.
(goto-char (point-max))
- ;; Ensure that the part ends with a carriage return.
- (unless (bolp)
- (insert "\n"))
- ;; We do not create the overlay for hidden (lazy) parts until
- ;; they are inserted.
- (if show-part
- (notmuch-show-create-part-overlays button content-beg (point))
- (save-excursion
- (notmuch-show-toggle-part-invisibility button)))
+ (let ((empty-part (equal (point) content-beg)))
+ (if (not (funcall notmuch-show-insert-header-p-function part empty-part))
+ (delete-region button-beg content-beg)
+ ;; Ensure that the part ends with a carriage return.
+ (unless (bolp)
+ (insert "\n"))
+ ;; We do not create the overlay for hidden (lazy) parts until
+ ;; they are inserted.
+ (if show-part
+ (notmuch-show-create-part-overlays button content-beg (point))
+ (save-excursion
+ (notmuch-show-toggle-part-invisibility button)))))
(notmuch-show-record-part-information part beg (point))))
(defun notmuch-show-insert-body (msg body depth)
--
2.1.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message
2015-01-19 20:06 ` Mark Walters
@ 2015-01-20 8:44 ` David Edmondson
0 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2015-01-20 8:44 UTC (permalink / raw)
To: Mark Walters, notmuch
On Mon, Jan 19 2015, Mark Walters <markwalters1009@gmail.com> wrote:
> On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
>> Whether to insert part headers should depend on the details of the
>> message being cited.
>
> Hi
>
> Overall I like this series and it does fix two annoying bugs (not being
> able to reply to ref822 messages and (correctly) including parts
> which have application/octet-stream but are actually text parts).
>
> The one problem is getting the right choice for part headers in the
> reply text. I think getting this wrong will irritate users even if the
> overall result is better.
>
> My guess at the correct logic is:
> 1) omit the part header for any empty part: (ie a part we don't display
> such as a pdf file).
This seems wrong to me (i.e. it's not what I would want ;-).
Showing the part header in the reply acknowledges that it was part of
the message that I'm replying to.
Even more, consider a message:
Hi, attached are the two PDF documents that we discussed.
This is the version with Fred's suggested comments:
[ application/pdf: document 1 ]
This is the version with my proposed alternative edits and much more
content added:
[ application/pdf: document 2 ]
The part headers form a significant part of the content. In a reply I'd
like to see them, so that I can add comments appropriately. (I realise
that commenting 'in-line' is out of fashion in many places now, but for
more complex discussions I still prefer it.)
I still like the original rule that I proposed: the reply should include
whatever is in the 'show' buffer, modulo content that was elided due to
washing.
> 2) omit multipart/* part headers
> 3) include all other part headers
> 4) except omit the first part header (perhaps only in the case it is text/plain)
>
> My reasoning for each is
> 1) there is no point in saying we had a part which we are omitting.
> 2) all the subparts of multipart/* will get there own header which
> should be sufficient.
> 3) we want to keep the parts distinguished
> 4) except we don't need to do that with the first part.
>
> Note for 4) it would be good to have a multipart/alternative with
> subparts text/plain and text/html just give the text/plain with no part
> header.
>
> I include a patch below which does all of these apart from 4) as I
> couldn't see a clean way of implementing it. Any suggestions?
>
> It should apply on top of patch 6 or 7 instead of 8. The key change is
> that it always puts in a button and then deletes it if unwanted: this
> makes doing 1) above easy.
>
> It does break some tests, nothing unexpected except an interaction with
> the way we wash text/plain parts: we remove leading blank lines from the
> first text/plain part (because it doesn't have a button) but not from
> subsequent ones (because they do). Because this code always has the
> second case it doesn't remove a leading blank line of the first part.
>
> Best wishes
>
> Mark
>
>
> From 8f198b38e76e050ae8d20d866748c41ccf79f3d4 Mon Sep 17 00:00:00 2001
> From: Mark Walters <markwalters1009@gmail.com>
> Date: Mon, 19 Jan 2015 14:39:25 +0000
> Subject: [PATCH] emacs show/reply modify part handling
>
> Modify the part handling so that we always insert the button and
> delete it afterwards if not wanted. The advantage is that we can
> decide whether to keep the part button based on what the insertion
> code does. In particular the reply code can omit the button for all
> parts with no displayable content.
> ---
> emacs/notmuch-mua.el | 5 +++--
> emacs/notmuch-show.el | 39 +++++++++++++++++++++++++--------------
> 2 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 0ca9eed..6060f33 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -29,6 +29,7 @@
> (eval-when-compile (require 'cl))
>
> (declare-function notmuch-show-insert-body "notmuch-show" (msg body depth))
> +(declare-function notmuch-show-insert-header-p-reply "notmuch-show" (part empty-part))
>
> ;;
>
> @@ -223,8 +224,8 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
> ;; citations, etc. in the original message before
> ;; quoting.
> ((notmuch-show-insert-text/plain-hook nil)
> - ;; Don't insert part buttons.
> - (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-never))
> + ;; Insert part buttons appropriate for a reply.
> + (notmuch-show-insert-header-p-function #'notmuch-show-insert-header-p-reply))
> (notmuch-show-insert-body original (plist-get original :body) 0)
> (buffer-substring-no-properties (point-min) (point-max)))))
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 4a0899f..2cdb5a8 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -909,16 +909,24 @@ message at DEPTH in the current thread."
> "text/x-diff")
> content-type)))
>
> -(defun notmuch-show-insert-header-p-smart (part)
> +(defun notmuch-show-insert-header-p-smart (part empty-part)
> "Return non-NIL if a header button should be inserted for this part."
> (let ((mime-type (notmuch-show-mime-type part)))
> (not (and (string= mime-type "text/plain")
> (<= (plist-get part :id) 1)))))
>
> -(defun notmuch-show-insert-header-p-always (part)
> +(defun notmuch-show-insert-header-p-reply (part empty-part)
> + "Return non-NIL if a header button should be inserted for this part."
> + (let ((mime-type (notmuch-show-mime-type part)))
> + (not (or empty-part
> + (notmuch-match-content-type mime-type "multipart/*")
> + (and (string= mime-type "text/plain")
> + (<= (plist-get part :id) 1))))))
> +
> +(defun notmuch-show-insert-header-p-always (part empty-part)
> t)
>
> -(defun notmuch-show-insert-header-p-never (part)
> +(defun notmuch-show-insert-header-p-never (part empty-part)
> nil)
>
> (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
> @@ -936,8 +944,8 @@ is t, hide the part initially and show the button."
> (show-part (not (equal hide t)))
> ;; We omit the part button for the first (or only) part if
> ;; this is text/plain.
> - (button (when (funcall notmuch-show-insert-header-p-function part)
> - (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
> + (button-beg (point))
> + (button (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename)))
> (content-beg (point)))
>
> ;; Store the computed mime-type for later use (e.g. by attachment handlers).
> @@ -952,15 +960,18 @@ is t, hide the part initially and show the button."
> ;; Some of the body part handlers leave point somewhere up in the
> ;; part, so we make sure that we're down at the end.
> (goto-char (point-max))
> - ;; Ensure that the part ends with a carriage return.
> - (unless (bolp)
> - (insert "\n"))
> - ;; We do not create the overlay for hidden (lazy) parts until
> - ;; they are inserted.
> - (if show-part
> - (notmuch-show-create-part-overlays button content-beg (point))
> - (save-excursion
> - (notmuch-show-toggle-part-invisibility button)))
> + (let ((empty-part (equal (point) content-beg)))
> + (if (not (funcall notmuch-show-insert-header-p-function part empty-part))
> + (delete-region button-beg content-beg)
> + ;; Ensure that the part ends with a carriage return.
> + (unless (bolp)
> + (insert "\n"))
> + ;; We do not create the overlay for hidden (lazy) parts until
> + ;; they are inserted.
> + (if show-part
> + (notmuch-show-create-part-overlays button content-beg (point))
> + (save-excursion
> + (notmuch-show-toggle-part-invisibility button)))))
> (notmuch-show-record-part-information part beg (point))))
>
> (defun notmuch-show-insert-body (msg body depth)
> --
> 2.1.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 9/9] test: Update the test output to accord with more reply changes
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
` (7 preceding siblings ...)
2014-05-12 12:29 ` [PATCH v3 8/9] emacs/mua: Insert part headers depending on the message David Edmondson
@ 2014-05-12 12:29 ` David Edmondson
2014-05-12 22:08 ` [PATCH v3 0/9] emacs: Improve the cited message included in replies Mark Walters
2016-02-06 21:27 ` David Edmondson
10 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2014-05-12 12:29 UTC (permalink / raw)
To: notmuch
When replying to complex messages, some part headers may be
shown. Update the expected test output accordingly.
---
test/T310-emacs.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 95bb67e..fa15e03 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -394,6 +394,10 @@ User-Agent: Notmuch/XXX Emacs/XXX
--text follows this line--
Adrian Perez de Castro <aperez@igalia.com> writes:
+> [ multipart/mixed ]
+> [ multipart/signed ]
+> [ text/plain ]
+>
> Hello to all,
>
> I have just heard about Not Much today in some random Linux-related news
@@ -433,6 +437,8 @@ Adrian Perez de Castro <aperez@igalia.com> writes:
> --
> Adrian Perez de Castro <aperez@igalia.com>
> Igalia - Free Software Engineering
+> [ signature.asc: application/pgp-signature ]
+> [ text/plain ]
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
@@ -457,6 +463,9 @@ User-Agent: Notmuch/XXX Emacs/XXX
--text follows this line--
Alex Botero-Lowry <alex.boterolowry@gmail.com> writes:
+> [ multipart/mixed ]
+> [ multipart/alternative ]
+> [ text/plain ]
> I saw the announcement this morning, and was very excited, as I had been
> hoping sup would be turned into a library,
> since I like the concept more than the UI (I'd rather an emacs interface).
@@ -473,6 +482,8 @@ Alex Botero-Lowry <alex.boterolowry@gmail.com> writes:
> and http://mail-index.netbsd.org/pkgsrc-bugs/2006/06/07/msg016808.htmlspecifically
> uses 64 as the
> buffer size.
+> [ text/html (hidden) ]
+> [ 0001-Deal-with-situation-where-sysconf-_SC_GETPW_R_SIZE_M.patch: application/octet-stream (as text/x-diff) ]
> From e3bc4bbd7b9d0d086816ab5f8f2d6ffea1dd3ea4 Mon Sep 17 00:00:00 2001
> From: Alexander Botero-Lowry <alex.boterolowry@gmail.com>
> Date: Tue, 17 Nov 2009 11:30:39 -0800
@@ -505,6 +516,7 @@ Alex Botero-Lowry <alex.boterolowry@gmail.com> writes:
> --
> 1.6.5.2
>
+> [ text/plain ]
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
--
2.0.0.rc0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v3 0/9] emacs: Improve the cited message included in replies
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
` (8 preceding siblings ...)
2014-05-12 12:29 ` [PATCH v3 9/9] test: Update the test output to accord with more reply changes David Edmondson
@ 2014-05-12 22:08 ` Mark Walters
2014-05-13 10:01 ` David Edmondson
2016-02-06 21:27 ` David Edmondson
10 siblings, 1 reply; 34+ messages in thread
From: Mark Walters @ 2014-05-12 22:08 UTC (permalink / raw)
To: David Edmondson, notmuch
On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
> emacs: Improve the cited message included in replies
>
> I tried to do things in small increments to make it easier to review.
>
> v2:
> - Don't run the text/plain hooks when generating the message to quote.
>
> v3:
> - Remove the 'no-button code, as it's no longer used.
> - Control the insertion of part headers using a function.
> - Fix the tests.
I think I broadly like this series. I haven't thought through all the
ramifications yet so this is just some first thoughts. I will also send
some comments on individual patches.
In notmuch-show we go to notmuch-show-insert-part-*/* to
notmuch-mm-display-part-inline and then leave the decision to inline to
mm-inlined-types. I think this means that, by default, we will not
inline signatures amongst other things.
So at the least I think we should decide whether we want to override
mm-inlined-types. Alternatively, and in my view preferably, we could
have a function or variable of our own which says which parts to
include. Indeed, if do it with a function we might be able to make an
option to reply to mean "include parts currently shown in the
notmuch-show buffer" which might be nice.
There is a related question and possible bug that we might be able to
do something about at the same time: should we include text parts in the
reply if they have content-disposition attachment? I have been caught
about by this on one occasion replying to a message with a 500K log file
attached (and notmuch-show/wash becomes very slow with a 500K message!)
Finally, I am not sure whether I like having buttons in the reply. My
instinct is against, but they do add context.
Best wishes
Mark
>
>
> David Edmondson (9):
> emacs/show: Re-arrange determination if a part header is necessary
> emacs/show: Allow the user to decide when part headers should be
> inserted
> emacs/show: Accommodate the lack of part header buttons
> emacs/mua: Generate improved cited text for replies
> emacs/show: Remove the 'no-buttons option of
> `notmuch-show-insert-bodypart'
> emacs/mua: Don't insert part headers in citations
> test: Update the test output to accord with the reply changes
> emacs/mua: Insert part headers depending on the message
> test: Update the test output to accord with more reply changes
>
> emacs/notmuch-mua.el | 82 +++++++++++++++++++-----------
> emacs/notmuch-show.el | 135 +++++++++++++++++++++++++++++++-------------------
> test/T310-emacs.sh | 44 ++++++++++++++++
> 3 files changed, 180 insertions(+), 81 deletions(-)
>
> --
> 2.0.0.rc0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v3 0/9] emacs: Improve the cited message included in replies
2014-05-12 22:08 ` [PATCH v3 0/9] emacs: Improve the cited message included in replies Mark Walters
@ 2014-05-13 10:01 ` David Edmondson
2014-05-13 12:05 ` Mark Walters
0 siblings, 1 reply; 34+ messages in thread
From: David Edmondson @ 2014-05-13 10:01 UTC (permalink / raw)
To: Mark Walters, notmuch
[-- Attachment #1: Type: text/plain, Size: 4129 bytes --]
Firstly, I don't think that the code resulting from this patch series is
beyond improvement - the intention was really only that it be better
than the current implementation.
On Mon, May 12 2014, Mark Walters wrote:
> On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
>> emacs: Improve the cited message included in replies
>>
>> I tried to do things in small increments to make it easier to review.
>>
>> v2:
>> - Don't run the text/plain hooks when generating the message to quote.
>>
>> v3:
>> - Remove the 'no-button code, as it's no longer used.
>> - Control the insertion of part headers using a function.
>> - Fix the tests.
>
> I think I broadly like this series. I haven't thought through all the
> ramifications yet so this is just some first thoughts. I will also send
> some comments on individual patches.
Thanks!
> In notmuch-show we go to notmuch-show-insert-part-*/* to
> notmuch-mm-display-part-inline and then leave the decision to inline to
> mm-inlined-types. I think this means that, by default, we will not
> inline signatures amongst other things.
The rule is essentially: whatever text would be shown when the message
is displayed for reading, without any of the washing.
> So at the least I think we should decide whether we want to override
> mm-inlined-types.
I'm not really clear on the benefits of this. Could you explain?
> Alternatively, and in my view preferably, we could have a function or
> variable of our own which says which parts to include. Indeed, if do
> it with a function we might be able to make an option to reply to mean
> "include parts currently shown in the notmuch-show buffer" which might
> be nice.
That seems over complicated to me. The rule (above) from this series
is easy to understand and work with. Other mechanisms could be
implemented later, of course.
> There is a related question and possible bug that we might be able to
> do something about at the same time: should we include text parts in the
> reply if they have content-disposition attachment? I have been caught
> about by this on one occasion replying to a message with a 500K log file
> attached (and notmuch-show/wash becomes very slow with a 500K
> message!)
This is really a question of what happens in `show' mode. If it is
currently displaying text parts with content-disposition attachment,
that sounds like a bug that should be fixed (which would mean that the
cited message wouldn't include that part either).
> Finally, I am not sure whether I like having buttons in the reply. My
> instinct is against, but they do add context.
The last patch in the series is an example of trying to do the right
thing - show the part headers when they are necessary for proper
understanding, but elide them in all other cases.
The mechanism used to do this is pretty crude in the patch. One could
imagine a better implementation that examines the depth of the part
tree, etc.
> Best wishes
>
> Mark
>
>
>
>
>>
>>
>> David Edmondson (9):
>> emacs/show: Re-arrange determination if a part header is necessary
>> emacs/show: Allow the user to decide when part headers should be
>> inserted
>> emacs/show: Accommodate the lack of part header buttons
>> emacs/mua: Generate improved cited text for replies
>> emacs/show: Remove the 'no-buttons option of
>> `notmuch-show-insert-bodypart'
>> emacs/mua: Don't insert part headers in citations
>> test: Update the test output to accord with the reply changes
>> emacs/mua: Insert part headers depending on the message
>> test: Update the test output to accord with more reply changes
>>
>> emacs/notmuch-mua.el | 82 +++++++++++++++++++-----------
>> emacs/notmuch-show.el | 135 +++++++++++++++++++++++++++++++-------------------
>> test/T310-emacs.sh | 44 ++++++++++++++++
>> 3 files changed, 180 insertions(+), 81 deletions(-)
>>
>> --
>> 2.0.0.rc0
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 310 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 0/9] emacs: Improve the cited message included in replies
2014-05-13 10:01 ` David Edmondson
@ 2014-05-13 12:05 ` Mark Walters
0 siblings, 0 replies; 34+ messages in thread
From: Mark Walters @ 2014-05-13 12:05 UTC (permalink / raw)
To: David Edmondson, notmuch
On Tue, 13 May 2014, David Edmondson <dme@dme.org> wrote:
> Firstly, I don't think that the code resulting from this patch series is
> beyond improvement - the intention was really only that it be better
> than the current implementation.
>
> On Mon, May 12 2014, Mark Walters wrote:
>> On Mon, 12 May 2014, David Edmondson <dme@dme.org> wrote:
>>> emacs: Improve the cited message included in replies
>>>
>>> I tried to do things in small increments to make it easier to review.
>>>
>>> v2:
>>> - Don't run the text/plain hooks when generating the message to quote.
>>>
>>> v3:
>>> - Remove the 'no-button code, as it's no longer used.
>>> - Control the insertion of part headers using a function.
>>> - Fix the tests.
>>
>> I think I broadly like this series. I haven't thought through all the
>> ramifications yet so this is just some first thoughts. I will also send
>> some comments on individual patches.
>
> Thanks!
>
>> In notmuch-show we go to notmuch-show-insert-part-*/* to
>> notmuch-mm-display-part-inline and then leave the decision to inline to
>> mm-inlined-types. I think this means that, by default, we will not
>> inline signatures amongst other things.
>
> The rule is essentially: whatever text would be shown when the message
> is displayed for reading, without any of the washing.
Yes I think that is a good rule. That means that essentially all my
comments end up being more a (future) feature request that we make
choosing which parts to view in notmuch-show more configurable.
I do have a slight worry about whether there are any cases that people
will want parts shown in their emacs-show but not in any reply but I
can't think of any.
>> So at the least I think we should decide whether we want to override
>> mm-inlined-types.
>
> I'm not really clear on the benefits of this. Could you explain?
This was just the above slight worry.
>> Alternatively, and in my view preferably, we could have a function or
>> variable of our own which says which parts to include. Indeed, if do
>> it with a function we might be able to make an option to reply to mean
>> "include parts currently shown in the notmuch-show buffer" which might
>> be nice.
>
> That seems over complicated to me. The rule (above) from this series
> is easy to understand and work with. Other mechanisms could be
> implemented later, of course.
Yes, as you say, leave this for later (if ever).
>
>> There is a related question and possible bug that we might be able to
>> do something about at the same time: should we include text parts in the
>> reply if they have content-disposition attachment? I have been caught
>> about by this on one occasion replying to a message with a 500K log file
>> attached (and notmuch-show/wash becomes very slow with a 500K
>> message!)
>
> This is really a question of what happens in `show' mode. If it is
> currently displaying text parts with content-disposition attachment,
> that sounds like a bug that should be fixed (which would mean that the
> cited message wouldn't include that part either).
Yes I agree.
>> Finally, I am not sure whether I like having buttons in the reply. My
>> instinct is against, but they do add context.
>
> The last patch in the series is an example of trying to do the right
> thing - show the part headers when they are necessary for proper
> understanding, but elide them in all other cases.
>
> The mechanism used to do this is pretty crude in the patch. One could
> imagine a better implementation that examines the depth of the part
> tree, etc.
OK I will try and review these soon.
Best wishes
Mark
>
>> Best wishes
>>
>> Mark
>>
>>
>>
>>
>>>
>>>
>>> David Edmondson (9):
>>> emacs/show: Re-arrange determination if a part header is necessary
>>> emacs/show: Allow the user to decide when part headers should be
>>> inserted
>>> emacs/show: Accommodate the lack of part header buttons
>>> emacs/mua: Generate improved cited text for replies
>>> emacs/show: Remove the 'no-buttons option of
>>> `notmuch-show-insert-bodypart'
>>> emacs/mua: Don't insert part headers in citations
>>> test: Update the test output to accord with the reply changes
>>> emacs/mua: Insert part headers depending on the message
>>> test: Update the test output to accord with more reply changes
>>>
>>> emacs/notmuch-mua.el | 82 +++++++++++++++++++-----------
>>> emacs/notmuch-show.el | 135 +++++++++++++++++++++++++++++++-------------------
>>> test/T310-emacs.sh | 44 ++++++++++++++++
>>> 3 files changed, 180 insertions(+), 81 deletions(-)
>>>
>>> --
>>> 2.0.0.rc0
>>>
>>> _______________________________________________
>>> notmuch mailing list
>>> notmuch@notmuchmail.org
>>> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 0/9] emacs: Improve the cited message included in replies
2014-05-12 12:29 ` [PATCH v3 0/9] " David Edmondson
` (9 preceding siblings ...)
2014-05-12 22:08 ` [PATCH v3 0/9] emacs: Improve the cited message included in replies Mark Walters
@ 2016-02-06 21:27 ` David Edmondson
10 siblings, 0 replies; 34+ messages in thread
From: David Edmondson @ 2016-02-06 21:27 UTC (permalink / raw)
To: notmuch
I'd like to resurrect this set of changes. There was discussion last
time around, but I'm unclear about any conclusion. Should I just merge
them forward and re-post?
On Mon, May 12 2014, David Edmondson <dme@dme.org> wrote:
> emacs: Improve the cited message included in replies
>
> I tried to do things in small increments to make it easier to review.
>
> v2:
> - Don't run the text/plain hooks when generating the message to quote.
>
> v3:
> - Remove the 'no-button code, as it's no longer used.
> - Control the insertion of part headers using a function.
> - Fix the tests.
>
>
> David Edmondson (9):
> emacs/show: Re-arrange determination if a part header is necessary
> emacs/show: Allow the user to decide when part headers should be
> inserted
> emacs/show: Accommodate the lack of part header buttons
> emacs/mua: Generate improved cited text for replies
> emacs/show: Remove the 'no-buttons option of
> `notmuch-show-insert-bodypart'
> emacs/mua: Don't insert part headers in citations
> test: Update the test output to accord with the reply changes
> emacs/mua: Insert part headers depending on the message
> test: Update the test output to accord with more reply changes
>
> emacs/notmuch-mua.el | 82 +++++++++++++++++++-----------
> emacs/notmuch-show.el | 135 +++++++++++++++++++++++++++++++-------------------
> test/T310-emacs.sh | 44 ++++++++++++++++
> 3 files changed, 180 insertions(+), 81 deletions(-)
>
> --
> 2.0.0.rc0
^ permalink raw reply [flat|nested] 34+ messages in thread