[PATCH 0/3] minor cleanup and improvements

[PATCH 0/3] minor cleanup and improvements

[David Edmondson]

Three minor cleanup or small improvement patches.

David Edmondson (3):
  emacs: Stop the `truncate-string-to-width' madness.
  emacs: Don't mark messages as "unsaved" when printing.
  emacs: Prefer '[No Subject]' to blank subjects.

 emacs/notmuch-lib.el   |    5 +++++
 emacs/notmuch-print.el |    9 +++++++--
 emacs/notmuch-show.el  |    5 ++++-
 emacs/notmuch.el       |   13 ++++---------
 4 files changed, 20 insertions(+), 12 deletions(-)

-- 
1.7.8.3

[PATCH 1/3] emacs: Stop the `truncate-string-to-width' madness.

[David Edmondson]

There's no need to call `truncate-string-to-width' twice in this code
path.
---
 emacs/notmuch.el |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 3ec0816..3f6b977 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -441,18 +441,16 @@ Complete list of currently available key bindings:
   (interactive "P")
   (let ((thread-id (notmuch-search-find-thread-id))
 	(subject (notmuch-search-find-subject)))
+
+    (if (string-match "^[ \t]*$" subject)
+	(setq subject "[No Subject]"))
+
     (if (> (length thread-id) 0)
 	(notmuch-show thread-id
 		      (current-buffer)
 		      notmuch-search-query-string
-		      ;; name the buffer based on notmuch-search-find-subject
-		      (if (string-match "^[ \t]*$" subject)
-			  "[No Subject]"
-			(truncate-string-to-width
-			 (concat "*"
-				 (truncate-string-to-width subject 32 nil nil t)
-				 "*")
-			 32 nil nil t))
+		      ;; Name the buffer based on the subject.
+		      (concat "*" (truncate-string-to-width subject 30 nil nil t) "*")
 		      crypto-switch)
       (message "End of search results."))))
 
-- 
1.7.8.3

[PATCH 2/3] emacs: Don't mark messages as "unsaved" when printing.

[David Edmondson]

`ps-print-buffer' notes that a buffer is unsaved unless
`buffer-modified-p' returns `nil', so ensure that it does.
---
 emacs/notmuch-print.el |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-print.el b/emacs/notmuch-print.el
index f96ccbe..83eb525 100644
--- a/emacs/notmuch-print.el
+++ b/emacs/notmuch-print.el
@@ -80,6 +80,7 @@ Optional OUTPUT allows passing a list of flags to muttprint."
 
 (defun notmuch-print-message (msg)
   "Print a message using the user-selected mechanism."
+  (set-buffer-modified-p nil)
   (funcall notmuch-print-mechanism msg))
 
 (provide 'notmuch-print)
-- 
1.7.8.3

[PATCH 3/3] emacs: Prefer '[No Subject]' to blank subjects.

[David Edmondson]

---
 emacs/notmuch-lib.el   |    5 +++++
 emacs/notmuch-print.el |    8 ++++++--
 emacs/notmuch-show.el  |    5 ++++-
 emacs/notmuch.el       |    5 +----
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 241fe8c..de33575 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -130,6 +130,11 @@ the user hasn't set this variable with the old or new value."
   (interactive)
   (kill-buffer (current-buffer)))
 
+(defun notmuch-prettify-subject (subject)
+  (if (string-match "^[ \t]*$" subject)
+      (setq subject "[No Subject]"))
+  subject)
+
 ;;
 
 (defun notmuch-common-do-stash (text)
diff --git a/emacs/notmuch-print.el b/emacs/notmuch-print.el
index 83eb525..51bb740 100644
--- a/emacs/notmuch-print.el
+++ b/emacs/notmuch-print.el
@@ -19,6 +19,8 @@
 ;;
 ;; Authors: David Edmondson <dme@dme.org>
 
+(require 'notmuch-lib)
+
 (defcustom notmuch-print-mechanism 'notmuch-print-lpr
   "How should printing be done?"
   :group 'notmuch
@@ -56,14 +58,16 @@ 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 (plist-get (notmuch-show-get-prop :headers msg) :Subject)))
+  (let ((subject (notmuch-prettify-subject
+		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
     (rename-buffer subject t)
     (ps-print-buffer)))
 
 (defun notmuch-print-ps-print/evince (msg)
   "Preview a message buffer using ps-print and evince."
   (let ((ps-file (make-temp-file "notmuch"))
-	(subject (plist-get (notmuch-show-get-prop :headers msg) :Subject)))
+	(subject (notmuch-prettify-subject
+		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
     (rename-buffer subject t)
     (ps-print-buffer ps-file)
     (notmuch-print-run-evince ps-file)))
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e6a5b31..c602b3e 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -985,7 +985,7 @@ buffer."
       (notmuch-show-next-open-message))
 
     ;; Set the header line to the subject of the first open message.
-    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
+    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))
 
     (notmuch-show-mark-read)))
 
@@ -1216,6 +1216,9 @@ Some useful entries are:
 (defun notmuch-show-get-depth ()
   (notmuch-show-get-prop :depth))
 
+(defun notmuch-show-get-pretty-subject ()
+  (notmuch-prettify-subject (notmuch-show-get-subject)))
+
 (defun notmuch-show-set-tags (tags)
   "Set the tags of the current message."
   (notmuch-show-set-prop :tags tags)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 3f6b977..ce1e232 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -440,10 +440,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-search-find-subject)))
-
-    (if (string-match "^[ \t]*$" subject)
-	(setq subject "[No Subject]"))
+	(subject (notmuch-prettify-subject (notmuch-search-find-subject))))
 
     (if (> (length thread-id) 0)
 	(notmuch-show thread-id
-- 
1.7.8.3

Re: [PATCH 3/3] emacs: Prefer '[No Subject]' to blank subjects.

[David Edmondson]

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

On Wed, 25 Jan 2012 13:08:33 +0000, David Edmondson <dme@dme.org> wrote:
> ---
>  emacs/notmuch-lib.el   |    5 +++++
>  emacs/notmuch-print.el |    8 ++++++--
>  emacs/notmuch-show.el  |    5 ++++-
>  emacs/notmuch.el       |    5 +----
>  4 files changed, 16 insertions(+), 7 deletions(-)

Don't apply this one. Some odd interaction when I test with Jameson's
patches to re-work movement through a thread.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 0/3 v2] minor cleanup and improvements

[David Edmondson]

Three minor cleanup or small improvement patches.

v2: Fix the case where subject can be `nil' in patch 3.

David Edmondson (3):
  emacs: Stop the `truncate-string-to-width' madness.
  emacs: Don't mark messages as "unsaved" when printing.
  emacs: Prefer '[No Subject]' to blank subjects.

 emacs/notmuch-lib.el   |    6 ++++++
 emacs/notmuch-print.el |    9 +++++++--
 emacs/notmuch-show.el  |    5 ++++-
 emacs/notmuch.el       |   13 ++++---------
 4 files changed, 21 insertions(+), 12 deletions(-)

-- 
1.7.8.3

[PATCH 1/3] emacs: Stop the `truncate-string-to-width' madness.

[David Edmondson]

There's no need to call `truncate-string-to-width' twice in this code
path.
---
 emacs/notmuch.el |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 3ec0816..3f6b977 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -441,18 +441,16 @@ Complete list of currently available key bindings:
   (interactive "P")
   (let ((thread-id (notmuch-search-find-thread-id))
 	(subject (notmuch-search-find-subject)))
+
+    (if (string-match "^[ \t]*$" subject)
+	(setq subject "[No Subject]"))
+
     (if (> (length thread-id) 0)
 	(notmuch-show thread-id
 		      (current-buffer)
 		      notmuch-search-query-string
-		      ;; name the buffer based on notmuch-search-find-subject
-		      (if (string-match "^[ \t]*$" subject)
-			  "[No Subject]"
-			(truncate-string-to-width
-			 (concat "*"
-				 (truncate-string-to-width subject 32 nil nil t)
-				 "*")
-			 32 nil nil t))
+		      ;; Name the buffer based on the subject.
+		      (concat "*" (truncate-string-to-width subject 30 nil nil t) "*")
 		      crypto-switch)
       (message "End of search results."))))
 
-- 
1.7.8.3

Re: [PATCH 1/3] emacs: Stop the `truncate-string-to-width' madness.

[Austin Clements]

Quoth David Edmondson on Jan 25 at  1:48 pm:
> There's no need to call `truncate-string-to-width' twice in this code
> path.

LGTM if what I point out below is okay.  Technically this changes the
behavior of this code, but what it did before was obviously wrong (if
you do roll a new version, I'd mention this in the commit message; but
no need to do that just for this).

> ---
>  emacs/notmuch.el |   14 ++++++--------
>  1 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 3ec0816..3f6b977 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -441,18 +441,16 @@ Complete list of currently available key bindings:
>    (interactive "P")
>    (let ((thread-id (notmuch-search-find-thread-id))
>  	(subject (notmuch-search-find-subject)))
> +
> +    (if (string-match "^[ \t]*$" subject)
> +	(setq subject "[No Subject]"))
> +

Is subject necessarily a string at this point?  Previously this only
ran if the code determined there was a thread at point.

>      (if (> (length thread-id) 0)
>  	(notmuch-show thread-id
>  		      (current-buffer)
>  		      notmuch-search-query-string
> -		      ;; name the buffer based on notmuch-search-find-subject
> -		      (if (string-match "^[ \t]*$" subject)
> -			  "[No Subject]"
> -			(truncate-string-to-width
> -			 (concat "*"
> -				 (truncate-string-to-width subject 32 nil nil t)
> -				 "*")
> -			 32 nil nil t))
> +		      ;; Name the buffer based on the subject.
> +		      (concat "*" (truncate-string-to-width subject 30 nil nil t) "*")
>  		      crypto-switch)
>        (message "End of search results."))))
>  

Re: [PATCH 1/3] emacs: Stop the `truncate-string-to-width' madness.

[David Edmondson]

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

On Sat, 28 Jan 2012 00:09:58 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth David Edmondson on Jan 25 at  1:48 pm:
> > There's no need to call `truncate-string-to-width' twice in this code
> > path.
> 
> LGTM if what I point out below is okay.  Technically this changes the
> behavior of this code, but what it did before was obviously wrong (if
> you do roll a new version, I'd mention this in the commit message; but
> no need to do that just for this).
> 
> > ---
> >  emacs/notmuch.el |   14 ++++++--------
> >  1 files changed, 6 insertions(+), 8 deletions(-)
> > 
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index 3ec0816..3f6b977 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -441,18 +441,16 @@ Complete list of currently available key bindings:
> >    (interactive "P")
> >    (let ((thread-id (notmuch-search-find-thread-id))
> >  	(subject (notmuch-search-find-subject)))
> > +
> > +    (if (string-match "^[ \t]*$" subject)
> > +	(setq subject "[No Subject]"))
> > +
> 
> Is subject necessarily a string at this point?  Previously this only
> ran if the code determined there was a thread at point.

No, it's a bug. It's fixed in the third patch, but should be fixed here
as well.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 2/3] emacs: Don't mark messages as "unsaved" when printing.

[David Edmondson]

`ps-print-buffer' notes that a buffer is unsaved unless
`buffer-modified-p' returns `nil', so ensure that it does.
---
 emacs/notmuch-print.el |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-print.el b/emacs/notmuch-print.el
index f96ccbe..83eb525 100644
--- a/emacs/notmuch-print.el
+++ b/emacs/notmuch-print.el
@@ -80,6 +80,7 @@ Optional OUTPUT allows passing a list of flags to muttprint."
 
 (defun notmuch-print-message (msg)
   "Print a message using the user-selected mechanism."
+  (set-buffer-modified-p nil)
   (funcall notmuch-print-mechanism msg))
 
 (provide 'notmuch-print)
-- 
1.7.8.3

Re: [PATCH 2/3] emacs: Don't mark messages as "unsaved" when printing.

[David Bremner]

On Wed, 25 Jan 2012 13:48:33 +0000, David Edmondson <dme@dme.org> wrote:
> `ps-print-buffer' notes that a buffer is unsaved unless
> `buffer-modified-p' returns `nil', so ensure that it does.

pushed,

d

Re: [PATCH 2/3] emacs: Don't mark messages as "unsaved" when printing.

[David Bremner]

On Wed, 25 Jan 2012 13:48:33 +0000, David Edmondson <dme@dme.org> wrote:
> `ps-print-buffer' notes that a buffer is unsaved unless
> `buffer-modified-p' returns `nil', so ensure that it does.
> ---

Pushed.

d

[PATCH 3/3] emacs: Prefer '[No Subject]' to blank subjects.

[David Edmondson]

---
 emacs/notmuch-lib.el   |    6 ++++++
 emacs/notmuch-print.el |    8 ++++++--
 emacs/notmuch-show.el  |    5 ++++-
 emacs/notmuch.el       |    5 +----
 4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 241fe8c..5b8a41c 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -130,6 +130,12 @@ the user hasn't set this variable with the old or new value."
   (interactive)
   (kill-buffer (current-buffer)))
 
+(defun notmuch-prettify-subject (subject)
+  (if (and subject
+	   (string-match "^[ \t]*$" subject))
+      (setq subject "[No Subject]"))
+  subject)
+
 ;;
 
 (defun notmuch-common-do-stash (text)
diff --git a/emacs/notmuch-print.el b/emacs/notmuch-print.el
index 83eb525..51bb740 100644
--- a/emacs/notmuch-print.el
+++ b/emacs/notmuch-print.el
@@ -19,6 +19,8 @@
 ;;
 ;; Authors: David Edmondson <dme@dme.org>
 
+(require 'notmuch-lib)
+
 (defcustom notmuch-print-mechanism 'notmuch-print-lpr
   "How should printing be done?"
   :group 'notmuch
@@ -56,14 +58,16 @@ 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 (plist-get (notmuch-show-get-prop :headers msg) :Subject)))
+  (let ((subject (notmuch-prettify-subject
+		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
     (rename-buffer subject t)
     (ps-print-buffer)))
 
 (defun notmuch-print-ps-print/evince (msg)
   "Preview a message buffer using ps-print and evince."
   (let ((ps-file (make-temp-file "notmuch"))
-	(subject (plist-get (notmuch-show-get-prop :headers msg) :Subject)))
+	(subject (notmuch-prettify-subject
+		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
     (rename-buffer subject t)
     (ps-print-buffer ps-file)
     (notmuch-print-run-evince ps-file)))
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e6a5b31..c602b3e 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -985,7 +985,7 @@ buffer."
       (notmuch-show-next-open-message))
 
     ;; Set the header line to the subject of the first open message.
-    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
+    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))
 
     (notmuch-show-mark-read)))
 
@@ -1216,6 +1216,9 @@ Some useful entries are:
 (defun notmuch-show-get-depth ()
   (notmuch-show-get-prop :depth))
 
+(defun notmuch-show-get-pretty-subject ()
+  (notmuch-prettify-subject (notmuch-show-get-subject)))
+
 (defun notmuch-show-set-tags (tags)
   "Set the tags of the current message."
   (notmuch-show-set-prop :tags tags)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 3f6b977..ce1e232 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -440,10 +440,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-search-find-subject)))
-
-    (if (string-match "^[ \t]*$" subject)
-	(setq subject "[No Subject]"))
+	(subject (notmuch-prettify-subject (notmuch-search-find-subject))))
 
     (if (> (length thread-id) 0)
 	(notmuch-show thread-id
-- 
1.7.8.3

Re: [PATCH 3/3] emacs: Prefer '[No Subject]' to blank subjects.

[David Edmondson]

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

On Fri, 27 Jan 2012 10:23:07 +0000, Mark Walters <m.walters@qmul.ac.uk> wrote:
> I am very much not a lisp expert

Me neither, so please do continue to review stuff.

> The patch 1/3 seems to set the show buffer line to *[No Subject]* where
> it used to be just [No Subject]. (I have no preference: I just wasn't
> sure if that was intentional.

There was inconsistency before, now the buffer name always has the
surrounding *s.

> Patch 3/3:
> > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> > index e6a5b31..c602b3e 100644
> > --- a/emacs/notmuch-show.el
> > +++ b/emacs/notmuch-show.el
> > @@ -985,7 +985,7 @@ buffer."
> >        (notmuch-show-next-open-message))
> >  
> >      ;; Set the header line to the subject of the first open message.
> > -    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
> > +    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))
> >  
> >      (notmuch-show-mark-read)))
> >  
> > @@ -1216,6 +1216,9 @@ Some useful entries are:
> >  (defun notmuch-show-get-depth ()
> >    (notmuch-show-get-prop :depth))
> >  
> > +(defun notmuch-show-get-pretty-subject ()
> > +  (notmuch-prettify-subject (notmuch-show-get-subject)))
> > +
> >  (defun notmuch-show-set-tags (tags)
> >    "Set the tags of the current message."
> >    (notmuch-show-set-prop :tags tags)
> 
> As far as I can see notmuch-show-get-pretty-subject is only called once
> so I wondered why you bothered with a new function. But as I say I have
> almost zero lisp experience so no feel for lisp style.

Just mirroring the existing structure really.
`notmuch-show-get-<something>' to access details of the message. I've no
strong preference.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 3/3] emacs: Prefer '[No Subject]' to blank subjects.

[Mark Walters]

Oh one other question: I think a search result line in the emacs
interface just has a blank if a thread has no subject. Would it be
appropriate to change that to [No Subject] too? (I have no preference)

Best wishes

Mark

On Fri, 27 Jan 2012 10:28:56 +0000, David Edmondson <dme@dme.org> wrote:
> On Fri, 27 Jan 2012 10:23:07 +0000, Mark Walters <m.walters@qmul.ac.uk> wrote:
> > I am very much not a lisp expert
> 
> Me neither, so please do continue to review stuff.
> 
> > The patch 1/3 seems to set the show buffer line to *[No Subject]* where
> > it used to be just [No Subject]. (I have no preference: I just wasn't
> > sure if that was intentional.
> 
> There was inconsistency before, now the buffer name always has the
> surrounding *s.
> 
> > Patch 3/3:
> > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> > > index e6a5b31..c602b3e 100644
> > > --- a/emacs/notmuch-show.el
> > > +++ b/emacs/notmuch-show.el
> > > @@ -985,7 +985,7 @@ buffer."
> > >        (notmuch-show-next-open-message))
> > >  
> > >      ;; Set the header line to the subject of the first open message.
> > > -    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
> > > +    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))
> > >  
> > >      (notmuch-show-mark-read)))
> > >  
> > > @@ -1216,6 +1216,9 @@ Some useful entries are:
> > >  (defun notmuch-show-get-depth ()
> > >    (notmuch-show-get-prop :depth))
> > >  
> > > +(defun notmuch-show-get-pretty-subject ()
> > > +  (notmuch-prettify-subject (notmuch-show-get-subject)))
> > > +
> > >  (defun notmuch-show-set-tags (tags)
> > >    "Set the tags of the current message."
> > >    (notmuch-show-set-prop :tags tags)
> > 
> > As far as I can see notmuch-show-get-pretty-subject is only called once
> > so I wondered why you bothered with a new function. But as I say I have
> > almost zero lisp experience so no feel for lisp style.
> 
> Just mirroring the existing structure really.
> `notmuch-show-get-<something>' to access details of the message. I've no
> strong preference.

Re: [PATCH 3/3] emacs: Prefer '[No Subject]' to blank subjects.

[David Edmondson]

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

On Fri, 27 Jan 2012 13:31:27 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> Oh one other question: I think a search result line in the emacs
> interface just has a blank if a thread has no subject. Would it be
> appropriate to change that to [No Subject] too? (I have no preference)

Yes, makes sense. In the next version.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 3/3] emacs: Prefer '[No Subject]' to blank subjects.

[Mark Walters]

I am very much not a lisp expert but for what it's worth I read/reviewed
the patches and like them with a couple of minor queries that I am happy
to be overruled on

The patch 1/3 seems to set the show buffer line to *[No Subject]* where
it used to be just [No Subject]. (I have no preference: I just wasn't
sure if that was intentional.

Patch 2/3 is obviously fine and good.

Patch 3/3:
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index e6a5b31..c602b3e 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -985,7 +985,7 @@ buffer."
>        (notmuch-show-next-open-message))
>  
>      ;; Set the header line to the subject of the first open message.
> -    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
> +    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))
>  
>      (notmuch-show-mark-read)))
>  
> @@ -1216,6 +1216,9 @@ Some useful entries are:
>  (defun notmuch-show-get-depth ()
>    (notmuch-show-get-prop :depth))
>  
> +(defun notmuch-show-get-pretty-subject ()
> +  (notmuch-prettify-subject (notmuch-show-get-subject)))
> +
>  (defun notmuch-show-set-tags (tags)
>    "Set the tags of the current message."
>    (notmuch-show-set-prop :tags tags)

As far as I can see notmuch-show-get-pretty-subject is only called once
so I wondered why you bothered with a new function. But as I say I have
almost zero lisp experience so no feel for lisp style.

Best wishes

Mark

(sorry for the resend: I sent from the wrong address the first time)

Re: [PATCH 3/3] emacs: Prefer '[No Subject]' to blank subjects.

[Austin Clements]

LGTM other than what Mark pointed out about this not applying to
subjects in the search buffer.

Quoth David Edmondson on Jan 25 at  1:48 pm:
> ---
>  emacs/notmuch-lib.el   |    6 ++++++
>  emacs/notmuch-print.el |    8 ++++++--
>  emacs/notmuch-show.el  |    5 ++++-
>  emacs/notmuch.el       |    5 +----
>  4 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 241fe8c..5b8a41c 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -130,6 +130,12 @@ the user hasn't set this variable with the old or new value."
>    (interactive)
>    (kill-buffer (current-buffer)))
>  
> +(defun notmuch-prettify-subject (subject)
> +  (if (and subject
> +	   (string-match "^[ \t]*$" subject))
> +      (setq subject "[No Subject]"))
> +  subject)
> +
>  ;;
>  
>  (defun notmuch-common-do-stash (text)
> diff --git a/emacs/notmuch-print.el b/emacs/notmuch-print.el
> index 83eb525..51bb740 100644
> --- a/emacs/notmuch-print.el
> +++ b/emacs/notmuch-print.el
> @@ -19,6 +19,8 @@
>  ;;
>  ;; Authors: David Edmondson <dme@dme.org>
>  
> +(require 'notmuch-lib)
> +
>  (defcustom notmuch-print-mechanism 'notmuch-print-lpr
>    "How should printing be done?"
>    :group 'notmuch
> @@ -56,14 +58,16 @@ 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 (plist-get (notmuch-show-get-prop :headers msg) :Subject)))
> +  (let ((subject (notmuch-prettify-subject
> +		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
>      (rename-buffer subject t)
>      (ps-print-buffer)))
>  
>  (defun notmuch-print-ps-print/evince (msg)
>    "Preview a message buffer using ps-print and evince."
>    (let ((ps-file (make-temp-file "notmuch"))
> -	(subject (plist-get (notmuch-show-get-prop :headers msg) :Subject)))
> +	(subject (notmuch-prettify-subject
> +		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
>      (rename-buffer subject t)
>      (ps-print-buffer ps-file)
>      (notmuch-print-run-evince ps-file)))
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index e6a5b31..c602b3e 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -985,7 +985,7 @@ buffer."
>        (notmuch-show-next-open-message))
>  
>      ;; Set the header line to the subject of the first open message.
> -    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
> +    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))
>  
>      (notmuch-show-mark-read)))
>  
> @@ -1216,6 +1216,9 @@ Some useful entries are:
>  (defun notmuch-show-get-depth ()
>    (notmuch-show-get-prop :depth))
>  
> +(defun notmuch-show-get-pretty-subject ()
> +  (notmuch-prettify-subject (notmuch-show-get-subject)))
> +
>  (defun notmuch-show-set-tags (tags)
>    "Set the tags of the current message."
>    (notmuch-show-set-prop :tags tags)
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 3f6b977..ce1e232 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -440,10 +440,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-search-find-subject)))
> -
> -    (if (string-match "^[ \t]*$" subject)
> -	(setq subject "[No Subject]"))
> +	(subject (notmuch-prettify-subject (notmuch-search-find-subject))))
>  
>      (if (> (length thread-id) 0)
>  	(notmuch-show thread-id

Re: [PATCH 0/3 v2] minor cleanup and improvements

[David Edmondson]

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

On Wed, 25 Jan 2012 13:48:31 +0000, David Edmondson <dme@dme.org> wrote:
>   emacs: Stop the `truncate-string-to-width' madness.
>   emacs: Don't mark messages as "unsaved" when printing.
>   emacs: Prefer '[No Subject]' to blank subjects.

Can I get reviewers for these? The middle one is trivial. The others are
simple.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 0/2 v2] minor cleanup and improvements

[David Edmondson]

Address the comments from Mark and Austin:
 - Don't assume that `subject' is a string,
 - Show [No Subject] in search output

David Edmondson (2):
  emacs: Stop the `truncate-string-to-width' madness.
  emacs: Prefer '[No Subject]' to blank subjects.

 emacs/notmuch-lib.el   |    9 +++++++++
 emacs/notmuch-print.el |    8 ++++++--
 emacs/notmuch-show.el  |    5 ++++-
 emacs/notmuch.el       |   15 +++++----------
 4 files changed, 24 insertions(+), 13 deletions(-)

-- 
1.7.8.3

[PATCH 1/2 v2] emacs: Stop the `truncate-string-to-width' madness.

[David Edmondson]

There's no need to call `truncate-string-to-width' twice in this code
path.
---
 emacs/notmuch.el |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 72f78ed..d4d6904 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -469,18 +469,16 @@ Complete list of currently available key bindings:
   (let ((thread-id (notmuch-search-find-thread-id))
 	(subject (notmuch-search-find-subject)))
     (if (> (length thread-id) 0)
-	(notmuch-show thread-id
-		      (current-buffer)
-		      notmuch-search-query-string
-		      ;; name the buffer based on notmuch-search-find-subject
-		      (if (string-match "^[ \t]*$" subject)
-			  "[No Subject]"
-			(truncate-string-to-width
-			 (concat "*"
-				 (truncate-string-to-width subject 32 nil nil t)
-				 "*")
-			 32 nil nil t))
-		      crypto-switch)
+	(progn
+	  (if (string-match "^[ \t]*$" subject)
+	      (setq subject "[No Subject]"))
+
+	  (notmuch-show thread-id
+			(current-buffer)
+			notmuch-search-query-string
+			;; Name the buffer based on the subject.
+			(concat "*" (truncate-string-to-width subject 30 nil nil t) "*")
+			crypto-switch))
       (message "End of search results."))))
 
 (defun notmuch-search-reply-to-thread (&optional prompt-for-sender)
-- 
1.7.8.3

Re: [PATCH 1/2 v2] emacs: Stop the `truncate-string-to-width' madness.

[Austin Clements]

LGTM.  One comment that you can take or leave.

Quoth David Edmondson on Jan 30 at 10:16 am:
> There's no need to call `truncate-string-to-width' twice in this code
> path.
> ---
>  emacs/notmuch.el |   22 ++++++++++------------
>  1 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 72f78ed..d4d6904 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -469,18 +469,16 @@ Complete list of currently available key bindings:
>    (let ((thread-id (notmuch-search-find-thread-id))
>  	(subject (notmuch-search-find-subject)))
>      (if (> (length thread-id) 0)
> -	(notmuch-show thread-id
> -		      (current-buffer)
> -		      notmuch-search-query-string
> -		      ;; name the buffer based on notmuch-search-find-subject
> -		      (if (string-match "^[ \t]*$" subject)
> -			  "[No Subject]"
> -			(truncate-string-to-width
> -			 (concat "*"
> -				 (truncate-string-to-width subject 32 nil nil t)
> -				 "*")
> -			 32 nil nil t))
> -		      crypto-switch)
> +	(progn
> +	  (if (string-match "^[ \t]*$" subject)
> +	      (setq subject "[No Subject]"))
> +
> +	  (notmuch-show thread-id
> +			(current-buffer)
> +			notmuch-search-query-string
> +			;; Name the buffer based on the subject.
> +			(concat "*" (truncate-string-to-width subject 30 nil nil t) "*")
> +			crypto-switch))
>        (message "End of search results."))))

Should this instead be an `error'?  It that makes sense, it would also
simplify the code, since you could

(if (= (length thread-id) 0)
    (error "End of search results."))
<current progn body>

>  
>  (defun notmuch-search-reply-to-thread (&optional prompt-for-sender)

Re: [PATCH 1/2 v2] emacs: Stop the `truncate-string-to-width' madness.

[David Edmondson]

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

On Mon, 30 Jan 2012 18:46:55 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> >        (message "End of search results."))))
> 
> Should this instead be an `error'?

It was an error in the past (until
id:"1324370714-28545-1-git-send-email-dme@dme.org"), but I found it to
be very annoying and switched it to use `message' because the error
would fire whenever `notmuch-show-archive-thread-internal' walked past
the last message.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[David Edmondson]

---
 emacs/notmuch-lib.el   |    9 +++++++++
 emacs/notmuch-print.el |    8 ++++++--
 emacs/notmuch-show.el  |    5 ++++-
 emacs/notmuch.el       |   21 +++++++++------------
 4 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index c906ca7..d315f76 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -133,6 +133,15 @@ the user hasn't set this variable with the old or new value."
   (interactive)
   (kill-buffer (current-buffer)))
 
+(defun notmuch-prettify-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
+	     (string-match "^[ \t]*$" subject))
+	"[No Subject]"
+      subject)))
+
 ;;
 
 (defun notmuch-common-do-stash (text)
diff --git a/emacs/notmuch-print.el b/emacs/notmuch-print.el
index 880f96d..6653d97 100644
--- a/emacs/notmuch-print.el
+++ b/emacs/notmuch-print.el
@@ -19,6 +19,8 @@
 ;;
 ;; Authors: David Edmondson <dme@dme.org>
 
+(require 'notmuch-lib)
+
 (declare-function notmuch-show-get-prop "notmuch-show" (prop &optional props))
 
 (defcustom notmuch-print-mechanism 'notmuch-print-lpr
@@ -58,14 +60,16 @@ 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 (plist-get (notmuch-show-get-prop :headers msg) :Subject)))
+  (let ((subject (notmuch-prettify-subject
+		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
     (rename-buffer subject t)
     (ps-print-buffer)))
 
 (defun notmuch-print-ps-print/evince (msg)
   "Preview a message buffer using ps-print and evince."
   (let ((ps-file (make-temp-file "notmuch"))
-	(subject (plist-get (notmuch-show-get-prop :headers msg) :Subject)))
+	(subject (notmuch-prettify-subject
+		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
     (rename-buffer subject t)
     (ps-print-buffer ps-file)
     (notmuch-print-run-evince ps-file)))
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 84ac624..7ea9ba2 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1017,7 +1017,7 @@ buffer."
       (notmuch-show-next-open-message))
 
     ;; Set the header line to the subject of the first open message.
-    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
+    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))
 
     (notmuch-show-mark-read)))
 
@@ -1248,6 +1248,9 @@ Some useful entries are:
 (defun notmuch-show-get-depth ()
   (notmuch-show-get-prop :depth))
 
+(defun notmuch-show-get-pretty-subject ()
+  (notmuch-prettify-subject (notmuch-show-get-subject)))
+
 (defun notmuch-show-set-tags (tags)
   "Set the tags of the current message."
   (notmuch-show-set-prop :tags tags)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index d4d6904..2b0a752 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -467,18 +467,14 @@ Complete list of currently available key bindings:
   "Display the currently selected thread."
   (interactive "P")
   (let ((thread-id (notmuch-search-find-thread-id))
-	(subject (notmuch-search-find-subject)))
+	(subject (notmuch-prettify-subject (notmuch-search-find-subject))))
     (if (> (length thread-id) 0)
-	(progn
-	  (if (string-match "^[ \t]*$" subject)
-	      (setq subject "[No Subject]"))
-
-	  (notmuch-show thread-id
-			(current-buffer)
-			notmuch-search-query-string
-			;; Name the buffer based on the subject.
-			(concat "*" (truncate-string-to-width subject 30 nil nil t) "*")
-			crypto-switch))
+	(notmuch-show thread-id
+		      (current-buffer)
+		      notmuch-search-query-string
+		      ;; Name the buffer based on the subject.
+		      (concat "*" (truncate-string-to-width subject 30 nil nil t) "*")
+		      crypto-switch)
       (message "End of search results."))))
 
 (defun notmuch-search-reply-to-thread (&optional prompt-for-sender)
@@ -854,7 +850,8 @@ non-authors is found, assume that all of the authors match."
 		      (if (/= (match-beginning 1) line)
 			  (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 subject tags)
+			(notmuch-search-show-result date count authors
+						    (notmuch-prettify-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

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[Austin Clements]

LGTM.

Quoth David Edmondson on Jan 30 at 10:16 am:
> ---
>  emacs/notmuch-lib.el   |    9 +++++++++
>  emacs/notmuch-print.el |    8 ++++++--
>  emacs/notmuch-show.el  |    5 ++++-
>  emacs/notmuch.el       |   21 +++++++++------------
>  4 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index c906ca7..d315f76 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -133,6 +133,15 @@ the user hasn't set this variable with the old or new value."
>    (interactive)
>    (kill-buffer (current-buffer)))
>  
> +(defun notmuch-prettify-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
> +	     (string-match "^[ \t]*$" subject))
> +	"[No Subject]"
> +      subject)))
> +
>  ;;
>  
>  (defun notmuch-common-do-stash (text)
> diff --git a/emacs/notmuch-print.el b/emacs/notmuch-print.el
> index 880f96d..6653d97 100644
> --- a/emacs/notmuch-print.el
> +++ b/emacs/notmuch-print.el
> @@ -19,6 +19,8 @@
>  ;;
>  ;; Authors: David Edmondson <dme@dme.org>
>  
> +(require 'notmuch-lib)
> +
>  (declare-function notmuch-show-get-prop "notmuch-show" (prop &optional props))
>  
>  (defcustom notmuch-print-mechanism 'notmuch-print-lpr
> @@ -58,14 +60,16 @@ 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 (plist-get (notmuch-show-get-prop :headers msg) :Subject)))
> +  (let ((subject (notmuch-prettify-subject
> +		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
>      (rename-buffer subject t)
>      (ps-print-buffer)))
>  
>  (defun notmuch-print-ps-print/evince (msg)
>    "Preview a message buffer using ps-print and evince."
>    (let ((ps-file (make-temp-file "notmuch"))
> -	(subject (plist-get (notmuch-show-get-prop :headers msg) :Subject)))
> +	(subject (notmuch-prettify-subject
> +		  (plist-get (notmuch-show-get-prop :headers msg) :Subject))))
>      (rename-buffer subject t)
>      (ps-print-buffer ps-file)
>      (notmuch-print-run-evince ps-file)))
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 84ac624..7ea9ba2 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1017,7 +1017,7 @@ buffer."
>        (notmuch-show-next-open-message))
>  
>      ;; Set the header line to the subject of the first open message.
> -    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
> +    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))
>  
>      (notmuch-show-mark-read)))
>  
> @@ -1248,6 +1248,9 @@ Some useful entries are:
>  (defun notmuch-show-get-depth ()
>    (notmuch-show-get-prop :depth))
>  
> +(defun notmuch-show-get-pretty-subject ()
> +  (notmuch-prettify-subject (notmuch-show-get-subject)))
> +
>  (defun notmuch-show-set-tags (tags)
>    "Set the tags of the current message."
>    (notmuch-show-set-prop :tags tags)
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index d4d6904..2b0a752 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -467,18 +467,14 @@ Complete list of currently available key bindings:
>    "Display the currently selected thread."
>    (interactive "P")
>    (let ((thread-id (notmuch-search-find-thread-id))
> -	(subject (notmuch-search-find-subject)))
> +	(subject (notmuch-prettify-subject (notmuch-search-find-subject))))
>      (if (> (length thread-id) 0)
> -	(progn
> -	  (if (string-match "^[ \t]*$" subject)
> -	      (setq subject "[No Subject]"))
> -
> -	  (notmuch-show thread-id
> -			(current-buffer)
> -			notmuch-search-query-string
> -			;; Name the buffer based on the subject.
> -			(concat "*" (truncate-string-to-width subject 30 nil nil t) "*")
> -			crypto-switch))
> +	(notmuch-show thread-id
> +		      (current-buffer)
> +		      notmuch-search-query-string
> +		      ;; Name the buffer based on the subject.
> +		      (concat "*" (truncate-string-to-width subject 30 nil nil t) "*")
> +		      crypto-switch)
>        (message "End of search results."))))
>  
>  (defun notmuch-search-reply-to-thread (&optional prompt-for-sender)
> @@ -854,7 +850,8 @@ non-authors is found, assume that all of the authors match."
>  		      (if (/= (match-beginning 1) line)
>  			  (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 subject tags)
> +			(notmuch-search-show-result date count authors
> +						    (notmuch-prettify-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)

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[Jameson Graef Rollins]

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

Sorry to be so late on this, but I'm not a big fan of this new feature.
I would prefer to always see the subject (or any other field for that
matter) as is.

As a principle I would prefer there not be text replacements unless it's
very clear that text has been replaced.  Buttons work because it's clear
they're buttons.  This is not the case here, though, since this text
replacement could actually be confused with real text.

It's also not clear to me why this feature would be needed.  I have
never found blank subjects confusing.  The field is always clearly
delineated, at least in search and show mode.  If it's not clear
elsewhere, maybe we can make the delineation of the subject field
clearer, but leave the actual subject text string alone.

If some feel this feature is really needed we should at least have a
customization variable.  notmuch-unblank-subject?  I don't have any good
name suggestions, though.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[David Edmondson]

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

On Sun, 05 Feb 2012 23:07:02 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Sorry to be so late on this, but I'm not a big fan of this new feature.
> I would prefer to always see the subject (or any other field for that
> matter) as is.

The Emacs UI always replaced blank subjects with '[No Subject]' in
buffer names. The printing code also needed something other than a blank
subject for buffer renaming.

> As a principle I would prefer there not be text replacements unless it's
> very clear that text has been replaced.  Buttons work because it's clear
> they're buttons.  This is not the case here, though, since this text
> replacement could actually be confused with real text.
> 
> It's also not clear to me why this feature would be needed.  I have
> never found blank subjects confusing.  The field is always clearly
> delineated, at least in search and show mode.  If it's not clear
> elsewhere, maybe we can make the delineation of the subject field
> clearer, but leave the actual subject text string alone.
> 
> If some feel this feature is really needed we should at least have a
> customization variable.  notmuch-unblank-subject?  I don't have any good
> name suggestions, though.

Updating `notmuch-prettify-subject' to use a user configurable string
(that can be set to the empty string) sounds like a good idea. Please
ensure that the various other bits of code that require something other
than a blank subject still work properly.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[Jameson Graef Rollins]

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

On Mon, 06 Feb 2012 07:47:38 +0000, David Edmondson <dme@dme.org> wrote:
> On Sun, 05 Feb 2012 23:07:02 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > Sorry to be so late on this, but I'm not a big fan of this new feature.
> > I would prefer to always see the subject (or any other field for that
> > matter) as is.
> 
> The Emacs UI always replaced blank subjects with '[No Subject]' in
> buffer names. The printing code also needed something other than a blank
> subject for buffer renaming.

I don't much care what the buffer name is.  That seems to be a
completely different issue to me.

> Updating `notmuch-prettify-subject' to use a user configurable string
> (that can be set to the empty string) sounds like a good idea. Please
> ensure that the various other bits of code that require something other
> than a blank subject still work properly.

I am actually perfectly happy to see this patch reverted.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[David Edmondson]

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

On Mon, 06 Feb 2012 00:06:54 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Mon, 06 Feb 2012 07:47:38 +0000, David Edmondson <dme@dme.org> wrote:
> > On Sun, 05 Feb 2012 23:07:02 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > > Sorry to be so late on this, but I'm not a big fan of this new feature.
> > > I would prefer to always see the subject (or any other field for that
> > > matter) as is.
> > 
> > The Emacs UI always replaced blank subjects with '[No Subject]' in
> > buffer names. The printing code also needed something other than a blank
> > subject for buffer renaming.
> 
> I don't much care what the buffer name is.  That seems to be a
> completely different issue to me.

I agree that they could be treated differently.

With blank subjects the printing code generated an error (because
`rename-buffer' doesn't like an empty string as the first argument), so
_some_ change is required.

> > Updating `notmuch-prettify-subject' to use a user configurable string
> > (that can be set to the empty string) sounds like a good idea. Please
> > ensure that the various other bits of code that require something other
> > than a blank subject still work properly.
> 
> I am actually perfectly happy to see this patch reverted.

Whatever bremner decides.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[Jameson Graef Rollins]

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

On Mon, 06 Feb 2012 08:56:34 +0000, David Edmondson <dme@dme.org> wrote:
> With blank subjects the printing code generated an error (because
> `rename-buffer' doesn't like an empty string as the first argument), so
> _some_ change is required.

This sounds like something that could easily be fixed in the printing
code, without changing the show and search behavior.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[David Edmondson]

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

On Mon, 06 Feb 2012 11:19:46 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Mon, 06 Feb 2012 08:56:34 +0000, David Edmondson <dme@dme.org> wrote:
> > With blank subjects the printing code generated an error (because
> > `rename-buffer' doesn't like an empty string as the first argument), so
> > _some_ change is required.
> 
> This sounds like something that could easily be fixed in the printing
> code, without changing the show and search behavior.

Yes.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[Antoine Beaupré]

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

On Sun, 05 Feb 2012 23:07:02 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Sorry to be so late on this, but I'm not a big fan of this new feature.
> I would prefer to always see the subject (or any other field for that
> matter) as is.

I agree. as a native french speaker, for example, it's annoying having
to change those settings everywhere I got...

a.
-- 
To be naive and easily deceived is impermissible, today more than
ever, when the prevailing untruths may lead to a catastrophe because
they blind people to real dangers and real possibilities.
                        - Erich Fromm

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[David Bremner]

On Mon, 30 Jan 2012 10:16:01 +0000, David Edmondson <dme@dme.org> wrote:
> ---
>  emacs/notmuch-lib.el   |    9 +++++++++
>  emacs/notmuch-print.el |    8 ++++++--
>  emacs/notmuch-show.el  |    5 ++++-
>  emacs/notmuch.el       |   21 +++++++++------------
>  4 files changed, 28 insertions(+), 15 deletions(-)

Jameson and Antoine have asked for this patch to be reverted. As far as
I understand it the printing code actually crashes on blank subjects if
this patch is removed, so it seems to me that that (slightly) outweighs
the annoyance caused by printing [No Subject].  I don't find the
argument that different people are annoyed

I'd be willing to be convinced otherwise, but as it turns out the
question is somewhat academic; there have been enough changes that this
patch no longer reverts cleanly. So Someone (TM) will have to do some to
change the behaviour; it sounds like the sensible thing to do would be to move
notmuch-prettify-subject into notmuch-print.el and call it there.

d

Re: [PATCH 0/2 v2] minor cleanup and improvements

[David Edmondson]

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

Oops. That should be 'v3'.

I need a bunch of automation to make posting patches simpler.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 0/2 v2] minor cleanup and improvements

[David Bremner]

On Mon, 30 Jan 2012 10:15:59 +0000, David Edmondson <dme@dme.org> wrote:
> Address the comments from Mark and Austin:
>  - Don't assume that `subject' is a string,
>  - Show [No Subject] in search output

pushed, 

d

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

[David Edmondson]

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.
---
 emacs/notmuch-lib.el   |   20 +++++++++++++++++---
 emacs/notmuch-print.el |    4 ++--
 emacs/notmuch-show.el  |    2 +-
 emacs/notmuch.el       |    4 ++--
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index d315f76..2edc868 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,20 @@ 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 (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)))
 
 ;;
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

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

[Jameson Graef Rollins]

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

On Mon,  6 Feb 2012 21:30:16 +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.

Hey, David.  I still think this is a lot of unnecessary code to deal
with what seems to be an issue with notmuch-print.el only.  I honestly
just don't see the point of replacing blank subjects with some other
string in all these circumstances.

Can't we just revert the original patch and just provide the needed fix
to print.el alone?

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

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

[David Edmondson]

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)))
 
 ;;
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

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

[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