From: Mark Walters <markwalters1009@gmail.com>
To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Subject: Re: [DRAFT PATCH] emacs: show local date next to Date: in case
 value	differs
In-Reply-To: <1427132722-20346-1-git-send-email-tomi.ollila@iki.fi>
References: <1427132722-20346-1-git-send-email-tomi.ollila@iki.fi>
User-Agent: Notmuch/0.18.1+86~gef5e66a (http://notmuchmail.org) Emacs/23.4.1
 (x86_64-pc-linux-gnu)
Date: Mon, 06 Apr 2015 09:02:02 +0100
Message-ID: <876199sn0l.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: tomi.ollila@iki.fi, Tomi Ollila <too@iki.fi>
Precedence: list

On Mon, 23 Mar 2015, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> From: Tomi Ollila <too@iki.fi>
>
> When adding Date: header of a message to notmuch-show buffer, compare the
> date string with local representation of it and if these differ, output
> Date: {original-date-string}  ({local-date-representation})
>
> This is useful e.g. when mail system provides Date: strings with
> different timezone information than the sender is located at.

I think this could be useful. I have to admit that until David pointed
it out in his reply I hadn't noticed the date in the headerline and I do
think the date header is a natural place to display this.

Jani made a comment on irc that the display could be rather cluttered
so I think it should be customizable. Ideally I would suggest having the
options of none (ie display just the sender date as now), local full
date (as your patch), and  relative (to put the relative date in the
date header).

I have some small comments on the implementation: 

> ---
>  emacs/notmuch-show.el | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index f15f981..7e81859 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -460,15 +460,28 @@ (defun notmuch-show-insert-header (header header-value)
>    "Insert a single header."
>    (insert header ": " (notmuch-sanitize header-value) "\n"))
>  
> -(defun notmuch-show-insert-headers (headers)
> +(defun notmuch--make-date (timestamp)
> +  (if (> timestamp 2147483647)
> +      (message-make-date (seconds-to-time timestamp))
> +    (message-make-date (encode-time timestamp 0 0 1 1 1970 t))))
> +
> +(defun notmuch-show-insert-headers (headers &optional timestamp)

I suggest passing msg as an  optional argument instead of timestamp. I
think it will be more flexible if people want to have other
header modification functions in the future (for example we could add
the senders name to the from line if we know it from other messages).

>    "Insert the headers of the current message."
> -  (let ((start (point)))
> +  (let ((start (point))
> +	date-local)
>      (mapc (lambda (header)
>  	    (let* ((header-symbol (intern (concat ":" header)))
>  		   (header-value (plist-get headers header-symbol)))

> -	      (if (and header-value
> -		       (not (string-equal "" header-value)))
> -		  (notmuch-show-insert-header header header-value))))

> +	      (when (and header-value
> +			 (not (string-equal "" header-value)))
> +		(if (and timestamp
> +			 (string-equal header "Date")
> +			 (not (string-equal
> +			       (setq date-local (notmuch--make-date timestamp))
> +			       header-value)))
> +		    (setq header-value
> +			  (format "%s  (%s)" header-value date-local)))

I think I would extract this block as a date-header-wash function. Then
the one special case doesn't interrupt the main function. Something like

    (let ((washed-header-value (if (string-equal header "Date")
                                    (notmuch-date-header-wash header msg)
                                  header-value)))

(This could go inside or outside the "when")

Best wishes

Mark

> +		(notmuch-show-insert-header header header-value))))
>  	  notmuch-message-headers)
>      (save-excursion
>        (save-restriction
> @@ -1012,7 +1025,7 @@ (defun notmuch-show-insert-msg (msg depth)
>      ;; Set `headers-start' to point after the 'Subject:' header to be
>      ;; compatible with the existing implementation. This just sets it
>      ;; to after the first header.
> -    (notmuch-show-insert-headers headers)
> +    (notmuch-show-insert-headers headers (plist-get msg :timestamp))
>      (save-excursion
>        (goto-char content-start)
>        ;; If the subject of this message is the same as that of the
> -- 
> 1.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch