Re: [PATCH v2] emacs: Leave blank subjects alone by default.

Subject: Re: [PATCH v2] emacs: Leave blank subjects alone by default.

Date: Sun, 12 Feb 2012 08:52:08 +0000

To: David Edmondson, notmuch@notmuchmail.org

Cc:

From: Mark Walters


On Mon,  6 Feb 2012 22:48:10 +0000, David Edmondson <dme@dme.org> wrote:
> Add `notmuch-blank-subject' to control how empty or whitespace only
> subjects are treated. The default behaviour is to leave them
> alone. The user can choose a string to use as a replacement.
> 
> Modify code that cannot handle blank subjects to use a fixed string.
> ---
> 
> Fix the non-string subject case (again, sigh).
> 
>  emacs/notmuch-lib.el   |   21 ++++++++++++++++++---
>  emacs/notmuch-print.el |    4 ++--
>  emacs/notmuch-show.el  |    2 +-
>  emacs/notmuch.el       |    4 ++--
>  4 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index d315f76..95e3c29 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -67,6 +67,13 @@
>    :type 'boolean
>    :group 'notmuch-search)
>  
> +(defcustom notmuch-blank-subject nil
> +  "How should subjects that are empty or whitespace only be
> +treated?"
> +  :type '(choice (const :tag "Leave them alone" nil)
> +		 (string :tag "Replacement string" "[No Subject]"))
> +  :group 'notmuch)
> +
>  ;;
>  
>  (defvar notmuch-search-history nil
> @@ -133,13 +140,21 @@ the user hasn't set this variable with the old or new value."
>    (interactive)
>    (kill-buffer (current-buffer)))
>  
> -(defun notmuch-prettify-subject (subject)
> +(defun notmuch-pretty-non-empty-subject (subject)
> +  (let ((subject (notmuch-pretty-subject subject)))
> +    (if (and subject
> +	     (string-match "^[ \t]*$" subject))
> +	"no subject"
> +      subject)))
> +
> +(defun notmuch-pretty-subject (subject)
>    ;; This function is used by `notmuch-search-process-filter' which
>    ;; requires that we not disrupt its' matching state.
>    (save-match-data
> -    (if (and subject
> +    (if (and (stringp notmuch-blank-subject)
> +	     subject
>  	     (string-match "^[ \t]*$" subject))
> -	"[No Subject]"
> +	notmuch-blank-subject
>        subject)))


Hi

This LGTM but I wonder whether notmuch-pretty-non-empty-subject should
try the defcustom notmuch-blank-subject and only fall back to "no
subject" if not set? (Of course then someone will want an option to
customise the string in print but not in search....)

Best wishes

Mark


>  
>  ;;
> diff --git a/emacs/notmuch-print.el b/emacs/notmuch-print.el
> index 6653d97..dda082a 100644
> --- a/emacs/notmuch-print.el
> +++ b/emacs/notmuch-print.el
> @@ -60,7 +60,7 @@ Optional OUTPUT allows passing a list of flags to muttprint."
>  
>  (defun notmuch-print-ps-print (msg)
>    "Print a message buffer using the ps-print package."
> -  (let ((subject (notmuch-prettify-subject
> +  (let ((subject (notmuch-pretty-non-empty-subject
>  		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
>      (rename-buffer subject t)
>      (ps-print-buffer)))
> @@ -68,7 +68,7 @@ Optional OUTPUT allows passing a list of flags to muttprint."
>  (defun notmuch-print-ps-print/evince (msg)
>    "Preview a message buffer using ps-print and evince."
>    (let ((ps-file (make-temp-file "notmuch"))
> -	(subject (notmuch-prettify-subject
> +	(subject (notmuch-pretty-non-empty-subject
>  		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
>      (rename-buffer subject t)
>      (ps-print-buffer ps-file)
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 7469e2e..4a23cc2 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1261,7 +1261,7 @@ Some useful entries are:
>    (notmuch-show-get-prop :depth))
>  
>  (defun notmuch-show-get-pretty-subject ()
> -  (notmuch-prettify-subject (notmuch-show-get-subject)))
> +  (notmuch-pretty-subject (notmuch-show-get-subject)))
>  
>  (defun notmuch-show-set-tags (tags)
>    "Set the tags of the current message."
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index cd04ffd..aab6946 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -467,7 +467,7 @@ Complete list of currently available key bindings:
>    "Display the currently selected thread."
>    (interactive "P")
>    (let ((thread-id (notmuch-search-find-thread-id))
> -	(subject (notmuch-prettify-subject (notmuch-search-find-subject))))
> +	(subject (notmuch-pretty-non-empty-subject (notmuch-search-find-subject))))
>      (if (> (length thread-id) 0)
>  	(notmuch-show thread-id
>  		      (current-buffer)
> @@ -850,7 +850,7 @@ non-authors is found, assume that all of the authors match."
>  			  (insert (concat "Error: Unexpected output from notmuch search:\n" (substring string line (match-beginning 1)) "\n")))
>  		      (let ((beg (point)))
>  			(notmuch-search-show-result date count authors
> -						    (notmuch-prettify-subject subject) tags)
> +						    (notmuch-pretty-subject subject) tags)
>  			(notmuch-search-color-line beg (point) tag-list)
>  			(put-text-property beg (point) 'notmuch-search-thread-id thread-id)
>  			(put-text-property beg (point) 'notmuch-search-authors authors)
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Thread: