wish: more informative citations

wish: more informative citations

[David Bremner]

Wouldn't it be nice if citations showed the first line or so of the text
being cited?

Stealing text from another thread, 

 > In case of a citation following immediately new contents. When the citation
 > was collapsed:
 > 
 >     [1-line citation. Click/Enter to show.]
 > Lorem ipsum dolor sit amet, consectetur adipisicin

Would be displayed as something like:

> In case of a citation following [ Click/Enter to show 3 more lines ]

Actually I'm not too sure about the format, but I thought I'd through
that out there.

Happy hacking,

David

Re: wish: more informative citations

[Carl Worth]

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

On Fri, 18 Dec 2009 20:47:20 -0400, David Bremner <david@tethera.net> wrote:
> Would be displayed as something like:
> 
> > In case of a citation following [ Click/Enter to show 3 more lines ]
> 
> Actually I'm not too sure about the format, but I thought I'd through
> that out there.

That's a fine idea. Along with this would be getting rid of the
stupidity of displaying [1 line citation] rather than just displaying
the citation itself!

And I really want my keybinding for displaying all the citations in the
current message. And code to recognize top-posted copies as citations
and hiding that. And, and...

-Carl

...and more time to do all this stuff. I've got an ever-growing TODO
list and a backlog of patches to be reviewed that goes back several
weeks now. I'm hoping that I'll be able to sneak some time over the
upcoming holidays...

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

[PATCH] notmuch.el: Refactor citation markup. Variables for minimum size, button text.

[david]

From: David Bremner <bremner@unb.ca>

This is a fairly intrusive rewrite.

- I pulled the common code for the signature and citation case out
  into a separate function. This is not so much shorter, but I think it
  will be easier to maintain.

- I replaced the sequence of (looking-at blah) (forward-line)  with a single
  re-search-forward per citation.

New variables

- notmuch-show-signature-button-format, notmuch-show-citation-button-format
  Allow customization of button text.

- notmuch-show-citation-lines-min
  Do not buttonize citations below the given threshold.
---

I decided to start with a simple threshold, and only convert
sufficiently long citations to buttons.  Once I started messing with
that code, I decided to rework it to be more maintainable and
hopefully faster.

This patch also fixes the problem of merging cites separated by a
single line of whitespace This is also fixed by the much simpler
<1260769295-12924-3-git-send-email-kanru@kanru.info>, but unlike that
patch there _shouldn't_ be a problem with eating the blank line after
a citation.  I'm not sure about the other bugs fixed by Kan-Ru's
series; perhaps Kan-Ru can comment.

 notmuch.el | 130
 ++++++++++++++++++++++++++++++++++++++---------------------- 1 files
 changed, 83 insertions(+), 47 deletions(-)

diff --git a/notmuch.el b/notmuch.el
index 97914f2..6a5ceea 100644
--- a/notmuch.el
+++ b/notmuch.el
@@ -92,9 +92,24 @@ for indentation at the beginning of the line. But notmuch will
 move past the indentation when testing this pattern, (so that the
 pattern can still test against the entire line).")
 
+(defvar notmuch-show-signature-button-format 
+  "[ %d-line hidden signature. Click/Enter to show ]"
+  "String used to construct button text for hidden signatures
+
+Can use up to one integer format parameter, i.e. %d")
+
+(defvar notmuch-show-citation-button-format 
+  "[ %d-line hidden citation. Click/Enter to show ]"
+  "String used to construct button text for hidden citations.
+
+Can use up to one integer format parameter, i.e. %d")
+
 (defvar notmuch-show-signature-lines-max 12
   "Maximum length of signature that will be hidden by default.")
 
+(defvar notmuch-show-citation-lines-min 4
+  "Minimum length of citation that will be hidden.")
+
 (defvar notmuch-command "notmuch"
   "Command to run the notmuch binary.")
 
@@ -593,54 +608,75 @@ which this thread was originally shown."
   'face 'notmuch-message-summary-face
   :supertype 'notmuch-button-invisibility-toggle-type)
 
+(defun notmuch-show-citation-regexp (depth)
+  "Build a regexp for matching citations at a given DEPTH (indent)"
+  (let ((line-regexp (format "[[:space:]]\\{%d\\}*>.*\n" depth)))
+    (concat "\\(?:^" line-regexp 
+	    "\\(?:[[:space:]]*\n" line-regexp
+	    "\\)?\\)+")))
+
+(defun notmuch-show-region-to-button (beg end type prefix button-text)
+  "Auxilary function to do the actual making of overlays and buttons
+
+BEG and END are buffer locations. TYPE should a string, either
+\"citation\" or \"signature\". PREFIX is some arbitrary text to
+insert before the button, probably for indentation.  BUTTON-TEXT
+is what to put on the button."
+
+;; This uses some slightly tricky conversions between strings and
+;; symbols because of the way the button code works. Note that
+;; replacing intern-soft with make-symbol will cause this to fail, 
+;; since the newly created symbol has no plist.
+
+  (let ((overlay (make-overlay beg end))
+	(invis-spec (make-symbol (concat "notmuch-" type "-region")))
+	(button-type (intern-soft (concat "notmuch-button-" 
+					  type "-toggle-type"))))
+    (add-to-invisibility-spec invis-spec)
+    (overlay-put overlay 'invisible invis-spec)
+    (goto-char (1+ end))
+    (save-excursion 
+      (goto-char (1- beg))
+      (insert prefix)
+      (insert-button button-text
+		     'invisibility-spec invis-spec
+		     :type button-type)
+      )))
+
+						 
 (defun notmuch-show-markup-citations-region (beg end depth)
-  (goto-char beg)
-  (beginning-of-line)
-  (while (< (point) end)
-    (let ((beg-sub (point-marker))
-	  (indent (make-string depth ? ))
-	  (citation ">"))
-      (move-to-column depth)
-      (if (looking-at citation)
-	  (progn
-	    (while (looking-at citation)
-	      (forward-line)
-	      (move-to-column depth))
-	    (let ((overlay (make-overlay beg-sub (point)))
-                  (invis-spec (make-symbol "notmuch-citation-region")))
-              (add-to-invisibility-spec invis-spec)
-	      (overlay-put overlay 'invisible invis-spec)
-              (let ((p (point-marker))
-                    (cite-button-text
-                     (concat "["  (number-to-string (count-lines beg-sub (point)))
-                             "-line citation. Click/Enter to show.]")))
-                (goto-char (- beg-sub 1))
-                (insert (concat "\n" indent))
-                (insert-button cite-button-text
-                               'invisibility-spec invis-spec
-                               :type 'notmuch-button-citation-toggle-type)
-                (forward-line)
-              ))))
-      (move-to-column depth)
-      (if (looking-at notmuch-show-signature-regexp)
-	  (let ((sig-lines (- (count-lines beg-sub end) 1)))
-	    (if (<= sig-lines notmuch-show-signature-lines-max)
-		(progn
-                  (let ((invis-spec (make-symbol "notmuch-signature-region")))
-                    (add-to-invisibility-spec invis-spec)
-                    (overlay-put (make-overlay beg-sub end)
-                                 'invisible invis-spec)
-                  
-                    (goto-char (- beg-sub 1))
-                    (insert (concat "\n" indent))
-                    (let ((sig-button-text (concat "[" (number-to-string sig-lines)
-                                                   "-line signature. Click/Enter to show.]")))
-                      (insert-button sig-button-text 'invisibility-spec invis-spec
-                                     :type 'notmuch-button-signature-toggle-type)
-                     )
-                    (insert "\n")
-                    (goto-char end))))))
-      (forward-line))))
+  "Markup citations, and up to one signature in the given region"
+  
+  (let ((citation-regexp (notmuch-show-citation-regexp depth))
+	(signature-regexp (concat (format "^[[:space:]]\\{%d\\}" depth) 
+				  notmuch-show-signature-regexp))
+	(indent (make-string depth ? )))
+    (goto-char beg)
+    (beginning-of-line)
+    (while (and (< (point) end) 
+		(re-search-forward citation-regexp end t))
+      (let* ((cite-start (match-beginning 0))
+	     (cite-end 	(match-end 0))
+	     (cite-lines (count-lines cite-start cite-end)))
+	(if (>= cite-lines notmuch-show-citation-lines-min)
+	    (notmuch-show-region-to-button 
+	     cite-start cite-end
+	     "citation"
+	     indent
+	     (format notmuch-show-citation-button-format cite-lines)
+	     ))))
+    (if (re-search-forward signature-regexp end t)
+	(let* ((sig-start (match-beginning 0))
+	       (sig-end (match-end 0))
+	       (sig-lines (1- (count-lines sig-start end))))
+	  (if (<= sig-lines notmuch-show-signature-lines-max)
+	      (notmuch-show-region-to-button 
+	       sig-start
+	       end
+	       "signature"
+	       indent
+	       (format notmuch-show-signature-button-format sig-lines)
+	       ))))))
 
 (defun notmuch-show-markup-part (beg end depth)
   (if (re-search-forward notmuch-show-part-begin-regexp nil t)
-- 
1.6.5.7

[PATCH] notmuch.el: Refactor citation markup. Variables for minimum size, button text.

[david]

From: David Bremner <bremner@unb.ca>

This is a fairly intrusive rewrite.

- I pulled the common code for the signature and citation case out
  into a separate function. This is not so much shorter, but I think it
  will be easier to maintain.

- I replaced the sequence of (looking-at blah) (forward-line)  with a single
  re-search-forward per citation.

New variables

- notmuch-show-signature-button-format, notmuch-show-citation-button-format
  Allow customization of button text.

- notmuch-show-citation-lines-min
  Do not buttonize citations below the given threshold.
---

This is an updated (already!) version of the patch. It fixes 3
bugs. Let's forget they were my silly bugs, and be impressed with my
fixing :).  This version has a call to untabify on the whole region,
which is a bit annoying, but it avoids getting really fancy with the
regexp to match tabs as x-spaces. Another alternative would be to
convince indent-rigidly to only use spaces, but I didn't see an
immediate way to configure that.


 notmuch.el |  133 ++++++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 86 insertions(+), 47 deletions(-)

diff --git a/notmuch.el b/notmuch.el
index 97914f2..9707c25 100644
--- a/notmuch.el
+++ b/notmuch.el
@@ -92,9 +92,24 @@ for indentation at the beginning of the line. But notmuch will
 move past the indentation when testing this pattern, (so that the
 pattern can still test against the entire line).")
 
+(defvar notmuch-show-signature-button-format 
+  "[ %d-line hidden signature. Click/Enter to show ]"
+  "String used to construct button text for hidden signatures
+
+Can use up to one integer format parameter, i.e. %d")
+
+(defvar notmuch-show-citation-button-format 
+  "[ %d-line hidden citation. Click/Enter to show ]"
+  "String used to construct button text for hidden citations.
+
+Can use up to one integer format parameter, i.e. %d")
+
 (defvar notmuch-show-signature-lines-max 12
   "Maximum length of signature that will be hidden by default.")
 
+(defvar notmuch-show-citation-lines-min 4
+  "Minimum length of citation that will be hidden.")
+
 (defvar notmuch-command "notmuch"
   "Command to run the notmuch binary.")
 
@@ -593,54 +608,78 @@ which this thread was originally shown."
   'face 'notmuch-message-summary-face
   :supertype 'notmuch-button-invisibility-toggle-type)
 
+(defun notmuch-show-citation-regexp (depth)
+  "Build a regexp for matching citations at a given DEPTH (indent)"
+  (let ((line-regexp (format "[[:space:]]\\{%d\\}*>.*\n" depth)))
+    (concat "\\(?:^" line-regexp 
+	    "\\(?:[[:space:]]*\n" line-regexp
+	    "\\)?\\)+")))
+
+(defun notmuch-show-region-to-button (beg end type prefix button-text)
+  "Auxilary function to do the actual making of overlays and buttons
+
+BEG and END are buffer locations. TYPE should a string, either
+\"citation\" or \"signature\". PREFIX is some arbitrary text to
+insert before the button, probably for indentation.  BUTTON-TEXT
+is what to put on the button."
+
+;; This uses some slightly tricky conversions between strings and
+;; symbols because of the way the button code works. Note that
+;; replacing intern-soft with make-symbol will cause this to fail, 
+;; since the newly created symbol has no plist.
+
+  (let ((overlay (make-overlay beg end))
+	(invis-spec (make-symbol (concat "notmuch-" type "-region")))
+	(button-type (intern-soft (concat "notmuch-button-" 
+					  type "-toggle-type"))))
+    (add-to-invisibility-spec invis-spec)
+    (overlay-put overlay 'invisible invis-spec)
+    (goto-char (1+ end))
+    (save-excursion 
+      (goto-char (1- beg))
+      (insert prefix)
+      (insert-button button-text
+		     'invisibility-spec invis-spec
+		     :type button-type)
+      )))
+
+						 
 (defun notmuch-show-markup-citations-region (beg end depth)
-  (goto-char beg)
-  (beginning-of-line)
-  (while (< (point) end)
-    (let ((beg-sub (point-marker))
-	  (indent (make-string depth ? ))
-	  (citation ">"))
-      (move-to-column depth)
-      (if (looking-at citation)
-	  (progn
-	    (while (looking-at citation)
-	      (forward-line)
-	      (move-to-column depth))
-	    (let ((overlay (make-overlay beg-sub (point)))
-                  (invis-spec (make-symbol "notmuch-citation-region")))
-              (add-to-invisibility-spec invis-spec)
-	      (overlay-put overlay 'invisible invis-spec)
-              (let ((p (point-marker))
-                    (cite-button-text
-                     (concat "["  (number-to-string (count-lines beg-sub (point)))
-                             "-line citation. Click/Enter to show.]")))
-                (goto-char (- beg-sub 1))
-                (insert (concat "\n" indent))
-                (insert-button cite-button-text
-                               'invisibility-spec invis-spec
-                               :type 'notmuch-button-citation-toggle-type)
-                (forward-line)
-              ))))
-      (move-to-column depth)
-      (if (looking-at notmuch-show-signature-regexp)
-	  (let ((sig-lines (- (count-lines beg-sub end) 1)))
-	    (if (<= sig-lines notmuch-show-signature-lines-max)
-		(progn
-                  (let ((invis-spec (make-symbol "notmuch-signature-region")))
-                    (add-to-invisibility-spec invis-spec)
-                    (overlay-put (make-overlay beg-sub end)
-                                 'invisible invis-spec)
-                  
-                    (goto-char (- beg-sub 1))
-                    (insert (concat "\n" indent))
-                    (let ((sig-button-text (concat "[" (number-to-string sig-lines)
-                                                   "-line signature. Click/Enter to show.]")))
-                      (insert-button sig-button-text 'invisibility-spec invis-spec
-                                     :type 'notmuch-button-signature-toggle-type)
-                     )
-                    (insert "\n")
-                    (goto-char end))))))
-      (forward-line))))
+  "Markup citations, and up to one signature in the given region"
+  ;; it would be nice if the untabify was not required, but 
+  ;; that would require notmuch to indent with spaces.
+  (untabify beg end)
+  (let ((citation-regexp (notmuch-show-citation-regexp depth))
+	(signature-regexp (concat (format "^[[:space:]]\\{%d\\}" depth) 
+				  notmuch-show-signature-regexp))
+	(indent (concat "\n" (make-string depth ? ))))
+    (goto-char beg)
+    (beginning-of-line)
+    (while (and (< (point) end) 
+		(re-search-forward citation-regexp end t))
+      (let* ((cite-start (match-beginning 0))
+	     (cite-end 	(match-end 0))
+	     (cite-lines (count-lines cite-start cite-end)))
+	(if (>= cite-lines notmuch-show-citation-lines-min)
+	    (notmuch-show-region-to-button 
+	     cite-start cite-end
+	     "citation"
+	     indent
+	     (format notmuch-show-citation-button-format cite-lines)
+	     ))))
+    (if (and (< (point) end) 
+	     (re-search-forward signature-regexp end t))
+	(let* ((sig-start (match-beginning 0))
+	       (sig-end (match-end 0))
+	       (sig-lines (1- (count-lines sig-start end))))
+	  (if (<= sig-lines notmuch-show-signature-lines-max)
+	      (notmuch-show-region-to-button 
+	       sig-start
+	       end
+	       "signature"
+	       indent
+	       (format notmuch-show-signature-button-format sig-lines)
+	       ))))))
 
 (defun notmuch-show-markup-part (beg end depth)
   (if (re-search-forward notmuch-show-part-begin-regexp nil t)
-- 
1.6.5.7

Re: [PATCH] notmuch.el: Refactor citation markup. Variables for minimum size, button text.

[Kan-Ru Chen]

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

On Thu, 24 Dec 2009 10:38:54 -0400, david@tethera.net wrote:
> From: David Bremner <bremner@unb.ca>
> 
> This is a fairly intrusive rewrite.
> 
> - I pulled the common code for the signature and citation case out
>   into a separate function. This is not so much shorter, but I think it
>   will be easier to maintain.
> 
> - I replaced the sequence of (looking-at blah) (forward-line)  with a single
>   re-search-forward per citation.
> 
> New variables
> 
> - notmuch-show-signature-button-format, notmuch-show-citation-button-format
>   Allow customization of button text.
> 
> - notmuch-show-citation-lines-min
>   Do not buttonize citations below the given threshold.

I like this idea, but this patch hides all citations larger than the
threshold. I'd like to see limited lines of citations been displayed.

For example:

    > Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do
    > eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enimad
    > minim veniam, quis nostrud exercitation ullamco laboris nisi ut
    > aliquip ex ea commodo consequat. Duis aute irure dolor in
    [ 6-line hidden citation. Click/Enter to show ]

And click the button shows the rest.

> ---
> 
> I decided to start with a simple threshold, and only convert
> sufficiently long citations to buttons.  Once I started messing with
> that code, I decided to rework it to be more maintainable and
> hopefully faster.
> 
> This patch also fixes the problem of merging cites separated by a
> single line of whitespace This is also fixed by the much simpler
> <1260769295-12924-3-git-send-email-kanru@kanru.info>, but unlike that
> patch there _shouldn't_ be a problem with eating the blank line after
> a citation.  I'm not sure about the other bugs fixed by Kan-Ru's
> series; perhaps Kan-Ru can comment.
> 
>  notmuch.el | 130
>  ++++++++++++++++++++++++++++++++++++++---------------------- 1 files
>  changed, 83 insertions(+), 47 deletions(-)
> 
> diff --git a/notmuch.el b/notmuch.el
> index 97914f2..6a5ceea 100644
> --- a/notmuch.el
> +++ b/notmuch.el
> @@ -92,9 +92,24 @@ for indentation at the beginning of the line. But notmuch will
>  move past the indentation when testing this pattern, (so that the
>  pattern can still test against the entire line).")
>  
> +(defvar notmuch-show-signature-button-format 
> +  "[ %d-line hidden signature. Click/Enter to show ]"
> +  "String used to construct button text for hidden signatures
> +
> +Can use up to one integer format parameter, i.e. %d")
> +
> +(defvar notmuch-show-citation-button-format 
> +  "[ %d-line hidden citation. Click/Enter to show ]"
> +  "String used to construct button text for hidden citations.
> +
> +Can use up to one integer format parameter, i.e. %d")
> +
>  (defvar notmuch-show-signature-lines-max 12
>    "Maximum length of signature that will be hidden by default.")
>  
> +(defvar notmuch-show-citation-lines-min 4
> +  "Minimum length of citation that will be hidden.")
> +
>  (defvar notmuch-command "notmuch"
>    "Command to run the notmuch binary.")
>  
> @@ -593,54 +608,75 @@ which this thread was originally shown."
>    'face 'notmuch-message-summary-face
>    :supertype 'notmuch-button-invisibility-toggle-type)
>  
> +(defun notmuch-show-citation-regexp (depth)
> +  "Build a regexp for matching citations at a given DEPTH (indent)"
> +  (let ((line-regexp (format "[[:space:]]\\{%d\\}*>.*\n" depth)))
> +    (concat "\\(?:^" line-regexp 
> +	    "\\(?:[[:space:]]*\n" line-regexp
> +	    "\\)?\\)+")))
> +
> +(defun notmuch-show-region-to-button (beg end type prefix button-text)
> +  "Auxilary function to do the actual making of overlays and buttons
> +
> +BEG and END are buffer locations. TYPE should a string, either
> +\"citation\" or \"signature\". PREFIX is some arbitrary text to
> +insert before the button, probably for indentation.  BUTTON-TEXT
> +is what to put on the button."
> +
> +;; This uses some slightly tricky conversions between strings and
> +;; symbols because of the way the button code works. Note that
> +;; replacing intern-soft with make-symbol will cause this to fail, 
> +;; since the newly created symbol has no plist.
> +
> +  (let ((overlay (make-overlay beg end))
> +	(invis-spec (make-symbol (concat "notmuch-" type "-region")))
> +	(button-type (intern-soft (concat "notmuch-button-" 
> +					  type "-toggle-type"))))
> +    (add-to-invisibility-spec invis-spec)
> +    (overlay-put overlay 'invisible invis-spec)
> +    (goto-char (1+ end))
> +    (save-excursion 
> +      (goto-char (1- beg))
> +      (insert prefix)
> +      (insert-button button-text
> +		     'invisibility-spec invis-spec
> +		     :type button-type)
> +      )))
> +
> +						 
>  (defun notmuch-show-markup-citations-region (beg end depth)
> -  (goto-char beg)
> -  (beginning-of-line)
> -  (while (< (point) end)
> -    (let ((beg-sub (point-marker))
> -	  (indent (make-string depth ? ))
> -	  (citation ">"))
> -      (move-to-column depth)
> -      (if (looking-at citation)
> -	  (progn
> -	    (while (looking-at citation)
> -	      (forward-line)
> -	      (move-to-column depth))
> -	    (let ((overlay (make-overlay beg-sub (point)))
> -                  (invis-spec (make-symbol "notmuch-citation-region")))
> -              (add-to-invisibility-spec invis-spec)
> -	      (overlay-put overlay 'invisible invis-spec)
> -              (let ((p (point-marker))
> -                    (cite-button-text
> -                     (concat "["  (number-to-string (count-lines beg-sub (point)))
> -                             "-line citation. Click/Enter to show.]")))
> -                (goto-char (- beg-sub 1))
> -                (insert (concat "\n" indent))
> -                (insert-button cite-button-text
> -                               'invisibility-spec invis-spec
> -                               :type 'notmuch-button-citation-toggle-type)
> -                (forward-line)
> -              ))))
> -      (move-to-column depth)
> -      (if (looking-at notmuch-show-signature-regexp)
> -	  (let ((sig-lines (- (count-lines beg-sub end) 1)))
> -	    (if (<= sig-lines notmuch-show-signature-lines-max)
> -		(progn
> -                  (let ((invis-spec (make-symbol "notmuch-signature-region")))
> -                    (add-to-invisibility-spec invis-spec)
> -                    (overlay-put (make-overlay beg-sub end)
> -                                 'invisible invis-spec)
> -                  
> -                    (goto-char (- beg-sub 1))
> -                    (insert (concat "\n" indent))
> -                    (let ((sig-button-text (concat "[" (number-to-string sig-lines)
> -                                                   "-line signature. Click/Enter to show.]")))
> -                      (insert-button sig-button-text 'invisibility-spec invis-spec
> -                                     :type 'notmuch-button-signature-toggle-type)
> -                     )
> -                    (insert "\n")
> -                    (goto-char end))))))
> -      (forward-line))))
> +  "Markup citations, and up to one signature in the given region"
> +  
> +  (let ((citation-regexp (notmuch-show-citation-regexp depth))
> +	(signature-regexp (concat (format "^[[:space:]]\\{%d\\}" depth) 
> +				  notmuch-show-signature-regexp))
> +	(indent (make-string depth ? )))
> +    (goto-char beg)
> +    (beginning-of-line)
> +    (while (and (< (point) end) 
> +		(re-search-forward citation-regexp end t))
> +      (let* ((cite-start (match-beginning 0))
> +	     (cite-end 	(match-end 0))
> +	     (cite-lines (count-lines cite-start cite-end)))
> +	(if (>= cite-lines notmuch-show-citation-lines-min)
> +	    (notmuch-show-region-to-button 
> +	     cite-start cite-end
> +	     "citation"
> +	     indent
> +	     (format notmuch-show-citation-button-format cite-lines)
> +	     ))))
> +    (if (re-search-forward signature-regexp end t)
> +	(let* ((sig-start (match-beginning 0))
> +	       (sig-end (match-end 0))
> +	       (sig-lines (1- (count-lines sig-start end))))
> +	  (if (<= sig-lines notmuch-show-signature-lines-max)
> +	      (notmuch-show-region-to-button 
> +	       sig-start
> +	       end
> +	       "signature"
> +	       indent
> +	       (format notmuch-show-signature-button-format sig-lines)
> +	       ))))))
>  
>  (defun notmuch-show-markup-part (beg end depth)
>    (if (re-search-forward notmuch-show-part-begin-regexp nil t)
> -- 
> 1.6.5.7
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

-- 
Kan-Ru Chen | http://kanru.info

Q: Why are my replies five sentences or less?
A: http://five.sentenc.es/

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

Update Patch Series. Only collapse part of citations.

[david]

On Fri, 25 Dec 2009 09:47:02 +0800, Kan-Ru Chen <kanru@kanru.info> wrote:

> I like this idea, but this patch hides all citations larger than the
> threshold. I'd like to see limited lines of citations been displayed.

Since you ask nicely, OK :).

I kept it as two patches so that people could compare the usability of
both versions.  

[PATCH 1/2] notmuch.el: Refactor citation markup. Variables for minimum size, button text.

This is as before, except one stray '*' deleted from the regex for
matching citations. This bug was only detected when I started
partially hiding citations.

[PATCH 2/2] notmuch.el: show some of citation even when hiding.  

This implements Kan-Ru's suggested format. I also changed the default
text on the citation buttons so that it made a bit more sense when the
citation was expanded.

At first I thought having the buttons in the middle of an expanded
citation would be really irritating, but I got used to it pretty
quickly. So give it a chance ;).  Hiding buttons or replacing them
looks a bit messy to me.

David

[PATCH 1/2] notmuch.el: Refactor citation markup. Variables for minimum size, button text.

[david]

From: David Bremner <bremner@unb.ca>

This is a fairly intrusive rewrite.

- I pulled the common code for the signature and citation case out
  into a separate function. This is not so much shorter, but I think it
  will be easier to maintain.

- I replaced the sequence of (looking-at blah) (forward-line)  with a single
  re-search-forward per citation.

New variables

- notmuch-show-signature-button-format, notmuch-show-citation-button-format
  Allow customization of button text.

- notmuch-show-citation-lines-min
  Do not buttonize citations below the given threshold.
---
 notmuch.el |  133 ++++++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 86 insertions(+), 47 deletions(-)

diff --git a/notmuch.el b/notmuch.el
index 97914f2..8a51d27 100644
--- a/notmuch.el
+++ b/notmuch.el
@@ -92,9 +92,24 @@ for indentation at the beginning of the line. But notmuch will
 move past the indentation when testing this pattern, (so that the
 pattern can still test against the entire line).")
 
+(defvar notmuch-show-signature-button-format 
+  "[ %d-line hidden signature. Click/Enter to show ]"
+  "String used to construct button text for hidden signatures
+
+Can use up to one integer format parameter, i.e. %d")
+
+(defvar notmuch-show-citation-button-format 
+  "[ %d-line hidden citation. Click/Enter to show ]"
+  "String used to construct button text for hidden citations.
+
+Can use up to one integer format parameter, i.e. %d")
+
 (defvar notmuch-show-signature-lines-max 12
   "Maximum length of signature that will be hidden by default.")
 
+(defvar notmuch-show-citation-lines-min 4
+  "Minimum length of citation that will be hidden.")
+
 (defvar notmuch-command "notmuch"
   "Command to run the notmuch binary.")
 
@@ -593,54 +608,78 @@ which this thread was originally shown."
   'face 'notmuch-message-summary-face
   :supertype 'notmuch-button-invisibility-toggle-type)
 
+(defun notmuch-show-citation-regexp (depth)
+  "Build a regexp for matching citations at a given DEPTH (indent)"
+  (let ((line-regexp (format "[[:space:]]\\{%d\\}>.*\n" depth)))
+    (concat "\\(?:^" line-regexp 
+	    "\\(?:[[:space:]]*\n" line-regexp
+	    "\\)?\\)+")))
+
+(defun notmuch-show-region-to-button (beg end type prefix button-text)
+  "Auxilary function to do the actual making of overlays and buttons
+
+BEG and END are buffer locations. TYPE should a string, either
+\"citation\" or \"signature\". PREFIX is some arbitrary text to
+insert before the button, probably for indentation.  BUTTON-TEXT
+is what to put on the button."
+
+;; This uses some slightly tricky conversions between strings and
+;; symbols because of the way the button code works. Note that
+;; replacing intern-soft with make-symbol will cause this to fail, 
+;; since the newly created symbol has no plist.
+
+  (let ((overlay (make-overlay beg end))
+	(invis-spec (make-symbol (concat "notmuch-" type "-region")))
+	(button-type (intern-soft (concat "notmuch-button-" 
+					  type "-toggle-type"))))
+    (add-to-invisibility-spec invis-spec)
+    (overlay-put overlay 'invisible invis-spec)
+    (goto-char (1+ end))
+    (save-excursion 
+      (goto-char (1- beg))
+      (insert prefix)
+      (insert-button button-text
+		     'invisibility-spec invis-spec
+		     :type button-type)
+      )))
+
+						 
 (defun notmuch-show-markup-citations-region (beg end depth)
-  (goto-char beg)
-  (beginning-of-line)
-  (while (< (point) end)
-    (let ((beg-sub (point-marker))
-	  (indent (make-string depth ? ))
-	  (citation ">"))
-      (move-to-column depth)
-      (if (looking-at citation)
-	  (progn
-	    (while (looking-at citation)
-	      (forward-line)
-	      (move-to-column depth))
-	    (let ((overlay (make-overlay beg-sub (point)))
-                  (invis-spec (make-symbol "notmuch-citation-region")))
-              (add-to-invisibility-spec invis-spec)
-	      (overlay-put overlay 'invisible invis-spec)
-              (let ((p (point-marker))
-                    (cite-button-text
-                     (concat "["  (number-to-string (count-lines beg-sub (point)))
-                             "-line citation. Click/Enter to show.]")))
-                (goto-char (- beg-sub 1))
-                (insert (concat "\n" indent))
-                (insert-button cite-button-text
-                               'invisibility-spec invis-spec
-                               :type 'notmuch-button-citation-toggle-type)
-                (forward-line)
-              ))))
-      (move-to-column depth)
-      (if (looking-at notmuch-show-signature-regexp)
-	  (let ((sig-lines (- (count-lines beg-sub end) 1)))
-	    (if (<= sig-lines notmuch-show-signature-lines-max)
-		(progn
-                  (let ((invis-spec (make-symbol "notmuch-signature-region")))
-                    (add-to-invisibility-spec invis-spec)
-                    (overlay-put (make-overlay beg-sub end)
-                                 'invisible invis-spec)
-                  
-                    (goto-char (- beg-sub 1))
-                    (insert (concat "\n" indent))
-                    (let ((sig-button-text (concat "[" (number-to-string sig-lines)
-                                                   "-line signature. Click/Enter to show.]")))
-                      (insert-button sig-button-text 'invisibility-spec invis-spec
-                                     :type 'notmuch-button-signature-toggle-type)
-                     )
-                    (insert "\n")
-                    (goto-char end))))))
-      (forward-line))))
+  "Markup citations, and up to one signature in the given region"
+  ;; it would be nice if the untabify was not required, but 
+  ;; that would require notmuch to indent with spaces.
+  (untabify beg end)
+  (let ((citation-regexp (notmuch-show-citation-regexp depth))
+	(signature-regexp (concat (format "^[[:space:]]\\{%d\\}" depth) 
+				  notmuch-show-signature-regexp))
+	(indent (concat "\n" (make-string depth ? ))))
+    (goto-char beg)
+    (beginning-of-line)
+    (while (and (< (point) end) 
+		(re-search-forward citation-regexp end t))
+      (let* ((cite-start (match-beginning 0))
+	     (cite-end 	(match-end 0))
+	     (cite-lines (count-lines cite-start cite-end)))
+	(if (>= cite-lines notmuch-show-citation-lines-min)
+	    (notmuch-show-region-to-button 
+	     cite-start cite-end
+	     "citation"
+	     indent
+	     (format notmuch-show-citation-button-format cite-lines)
+	     ))))
+    (if (and (< (point) end) 
+	     (re-search-forward signature-regexp end t))
+	(let* ((sig-start (match-beginning 0))
+	       (sig-end (match-end 0))
+	       (sig-lines (1- (count-lines sig-start end))))
+	  (if (<= sig-lines notmuch-show-signature-lines-max)
+	      (notmuch-show-region-to-button 
+	       sig-start
+	       end
+	       "signature"
+	       indent
+	       (format notmuch-show-signature-button-format sig-lines)
+	       ))))))
 
 (defun notmuch-show-markup-part (beg end depth)
   (if (re-search-forward notmuch-show-part-begin-regexp nil t)
-- 
1.6.5.7

[PATCH 2/2] notmuch.el: show some of citation even when hiding.

[david]

From: David Bremner <bremner@unb.ca>

- rename notmuch-show-citation-lines-min to n-s-c-l-prefix
- call forward-line with the appropriate parameter to adjust
  region to be hidden.
- change citation button text so that it makes (some) sense when citation is shown
---
 notmuch.el |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/notmuch.el b/notmuch.el
index 8a51d27..c280411 100644
--- a/notmuch.el
+++ b/notmuch.el
@@ -99,7 +99,7 @@ pattern can still test against the entire line).")
 Can use up to one integer format parameter, i.e. %d")
 
 (defvar notmuch-show-citation-button-format 
-  "[ %d-line hidden citation. Click/Enter to show ]"
+  "[ %d more citation lines. Click/Enter to toggle visibility. ]"
   "String used to construct button text for hidden citations.
 
 Can use up to one integer format parameter, i.e. %d")
@@ -107,8 +107,11 @@ Can use up to one integer format parameter, i.e. %d")
 (defvar notmuch-show-signature-lines-max 12
   "Maximum length of signature that will be hidden by default.")
 
-(defvar notmuch-show-citation-lines-min 4
-  "Minimum length of citation that will be hidden.")
+(defvar notmuch-show-citation-lines-prefix 4
+  "Always show at least this many lines of a citation.
+
+If there is one more line, show that, otherwise collapse
+remaining lines into a button.")
 
 (defvar notmuch-command "notmuch"
   "Command to run the notmuch binary.")
@@ -660,12 +663,15 @@ is what to put on the button."
       (let* ((cite-start (match-beginning 0))
 	     (cite-end 	(match-end 0))
 	     (cite-lines (count-lines cite-start cite-end)))
-	(if (>= cite-lines notmuch-show-citation-lines-min)
+	(when (>  cite-lines (1+ notmuch-show-citation-lines-prefix))
+	    (goto-char cite-start)
+	    (forward-line notmuch-show-citation-lines-prefix)
 	    (notmuch-show-region-to-button 
-	     cite-start cite-end
+	     (point) cite-end
 	     "citation"
 	     indent
-	     (format notmuch-show-citation-button-format cite-lines)
+	     (format notmuch-show-citation-button-format 
+		     (- cite-lines notmuch-show-citation-lines-prefix))
 	     ))))
     (if (and (< (point) end) 
 	     (re-search-forward signature-regexp end t))
-- 
1.6.5.7

Re: Update Patch Series. Only collapse part of citations.

[Kan-Ru Chen]

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

On Fri, 25 Dec 2009 16:09:06 -0400, david@tethera.net wrote:
> On Fri, 25 Dec 2009 09:47:02 +0800, Kan-Ru Chen <kanru@kanru.info> wrote:
> 
> > I like this idea, but this patch hides all citations larger than the
> > threshold. I'd like to see limited lines of citations been displayed.
> 
> Since you ask nicely, OK :).

Neat!

> [PATCH 2/2] notmuch.el: show some of citation even when hiding.  
> 
> This implements Kan-Ru's suggested format. I also changed the default
> text on the citation buttons so that it made a bit more sense when the
> citation was expanded.

Works great here.

Reviewed-by: Kan-Ru Chen <kanru@kanru.info>

-- 
Kan-Ru Chen | http://kanru.info

Q: Why are my replies five sentences or less?
A: http://five.sentenc.es/

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

Re: Update Patch Series. Only collapse part of citations.

[Carl Worth]

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

> On Fri, 25 Dec 2009 16:09:06 -0400, david@tethera.net wrote:
> > On Fri, 25 Dec 2009 09:47:02 +0800, Kan-Ru Chen <kanru@kanru.info> wrote:
> > > I like this idea, but this patch hides all citations larger than the
> > > threshold. I'd like to see limited lines of citations been displayed.
> > 
> > Since you ask nicely, OK :).

Very nice work, David!

I've just used this for a few minutes here, and it's very nice. (The
previous code was hiding far too much of the citations and I found
myself having to open them all the time just to be able to read things.)

On Sun, 27 Dec 2009 00:22:21 +0800, Kan-Ru Chen <kanru@kanru.info> wrote:
> Works great here.
>
> Reviewed-by: Kan-Ru Chen <kanru@kanru.info>

And thanks for the review. I've now committed and pushed these changes,
(with the reviewed-by added and some trailing whitespace removed).

-Carl

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