unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer
@ 2012-01-30 12:24 Dmitry Kurochkin
  2012-01-30 12:24 ` [PATCH 1/2] test: add test for hiding Original Message region at beginning of a message Dmitry Kurochkin
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-01-30 12:24 UTC (permalink / raw)
  To: notmuch

This patch series is an attempt to fix the issue found by David [1] in
a cleaner way.

Regards,
  Dmitry

[1] id:"1327503908-17226-1-git-send-email-dme@dme.org"

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] test: add test for hiding Original Message region at beginning of a message
  2012-01-30 12:24 [PATCH 0/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
@ 2012-01-30 12:24 ` Dmitry Kurochkin
  2012-02-04  1:22   ` David Bremner
  2012-01-30 12:24 ` [PATCH 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
  2012-02-04  7:36 ` [PATCH v2 0/2] " Dmitry Kurochkin
  2 siblings, 1 reply; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-01-30 12:24 UTC (permalink / raw)
  To: notmuch

The test is currently broken and will be fixed by a subsequent patch.

The patch adds a new file for tests of Emacs notmuch-show view.

Based on patch by David Edmondson [1].

[1] id:"1327562380-12894-4-git-send-email-dme@dme.org"
---
 test/emacs-show   |   28 ++++++++++++++++++++++++++++
 test/notmuch-test |    1 +
 2 files changed, 29 insertions(+), 0 deletions(-)
 create mode 100755 test/emacs-show

diff --git a/test/emacs-show b/test/emacs-show
new file mode 100755
index 0000000..9800575
--- /dev/null
+++ b/test/emacs-show
@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+
+test_description="Testing emacs notmuch-show view"
+. test-lib.sh
+
+test_begin_subtest "Hiding Original Message region at beginning of a message"
+test_subtest_known_broken
+message_id='OriginalMessageHiding.1@notmuchmail.org'
+add_message \
+    [id]="$message_id" \
+    '[subject]="Hiding Original Message region at beginning of a message"' \
+    '[body]="-----Original Message-----
+Text here."'
+
+cat <<EOF >EXPECTED
+Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (inbox)
+Subject: Hiding Original Message region at beginning of a message
+To: Notmuch Test Suite <test_suite@notmuchmail.org>
+Date: Fri, 05 Jan 2001 15:43:57 +0000
+
+[ 2-line hidden original message. Click/Enter to show. ]
+EOF
+
+test_emacs "(notmuch-show \"id:$message_id\")
+	    (test-visible-output)"
+test_expect_equal_file OUTPUT EXPECTED
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index 3f1740c..c814be9 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -54,6 +54,7 @@ TESTS="
   argument-parsing
   emacs-test-functions.sh
   emacs-address-cleaning.sh
+  emacs-show
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}
 
-- 
1.7.8.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer
  2012-01-30 12:24 [PATCH 0/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
  2012-01-30 12:24 ` [PATCH 1/2] test: add test for hiding Original Message region at beginning of a message Dmitry Kurochkin
@ 2012-01-30 12:24 ` Dmitry Kurochkin
  2012-01-30 12:54   ` David Edmondson
  2012-01-30 14:11   ` Tomi Ollila
  2012-02-04  7:36 ` [PATCH v2 0/2] " Dmitry Kurochkin
  2 siblings, 2 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-01-30 12:24 UTC (permalink / raw)
  To: notmuch

`Notmuch-wash-region-to-button' is the function that creates hidden
regions with buttons for signatures, citations and original messages.
Before the change, it did not work correctly if the to-be-hidden
region started at the beginning of a message: the visibility toggle
button was hidden as well.  The patch fixes this.  There are two parts
in the fix:

* Use `insert-before-markers' instead of `insert' for creating the
  button, so that it does not get added to the hidden overlay.

* Stop using PREFIX argument for adding a newline before the button.
  The newline should not be added before a button at the beginning of
  buffer.

The corresponding test is fixed now.
---
 emacs/notmuch-wash.el |   24 ++++++++++++++----------
 test/emacs-show       |    1 -
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 5c1e830..9d3d13f 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -136,12 +136,13 @@ collapse the remaining lines into a button.")
 	 (lines-count (count-lines (overlay-start overlay) (overlay-end overlay))))
     (format label-format lines-count)))
 
-(defun notmuch-wash-region-to-button (msg beg end type prefix)
+(defun notmuch-wash-region-to-button (msg beg end type &optional prefix)
   "Auxiliary 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."
+\"citation\" or \"signature\". Optional PREFIX is some arbitrary
+text to insert before the button, probably for indentation.  Note
+that PREFIX should not include a newline."
 
   ;; This uses some slightly tricky conversions between strings and
   ;; symbols because of the way the button code works. Note that
@@ -160,12 +161,15 @@ insert before the button, probably for indentation."
     (overlay-put overlay 'type type)
     (goto-char (1+ end))
     (save-excursion
-      (goto-char (1- beg))
-      (insert prefix)
-      (insert-button (notmuch-wash-button-label overlay)
+      (goto-char beg)
+      (if prefix
+	  (insert-before-markers prefix))
+      (let ((button-beg (point)))
+	(insert-before-markers (notmuch-wash-button-label overlay) "\n")
+	(make-button button-beg (1- (point))
 		     'invisibility-spec invis-spec
 		     'overlay overlay
-		     :type button-type))))
+		     :type button-type)))))
 
 (defun notmuch-wash-excerpt-citations (msg depth)
   "Excerpt citations and up to one signature."
@@ -177,7 +181,7 @@ insert before the button, probably for indentation."
 	     (msg-end (point-max))
 	     (msg-lines (count-lines msg-start msg-end)))
 	(notmuch-wash-region-to-button
-	 msg msg-start msg-end "original" "\n")))
+	 msg msg-start msg-end "original")))
   (while (and (< (point) (point-max))
 	      (re-search-forward notmuch-wash-citation-regexp nil t))
     (let* ((cite-start (match-beginning 0))
@@ -194,7 +198,7 @@ insert before the button, probably for indentation."
 	  (forward-line (- notmuch-wash-citation-lines-suffix))
 	  (notmuch-wash-region-to-button
 	   msg hidden-start (point-marker)
-	   "citation" "\n")))))
+	   "citation")))))
   (if (and (not (eobp))
 	   (re-search-forward notmuch-wash-signature-regexp nil t))
       (let* ((sig-start (match-beginning 0))
@@ -208,7 +212,7 @@ insert before the button, probably for indentation."
 	      (overlay-put (make-overlay sig-start-marker sig-end-marker) 'face 'message-cited-text)
 	      (notmuch-wash-region-to-button
 	       msg sig-start-marker sig-end-marker
-	       "signature" "\n"))))))
+	       "signature"))))))
 
 ;;
 
diff --git a/test/emacs-show b/test/emacs-show
index 9800575..5700d2e 100755
--- a/test/emacs-show
+++ b/test/emacs-show
@@ -4,7 +4,6 @@ test_description="Testing emacs notmuch-show view"
 . test-lib.sh
 
 test_begin_subtest "Hiding Original Message region at beginning of a message"
-test_subtest_known_broken
 message_id='OriginalMessageHiding.1@notmuchmail.org'
 add_message \
     [id]="$message_id" \
-- 
1.7.8.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer
  2012-01-30 12:24 ` [PATCH 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
@ 2012-01-30 12:54   ` David Edmondson
  2012-01-30 14:11   ` Tomi Ollila
  1 sibling, 0 replies; 10+ messages in thread
From: David Edmondson @ 2012-01-30 12:54 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

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

Good fix, +1 (and for the test).

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer
  2012-01-30 12:24 ` [PATCH 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
  2012-01-30 12:54   ` David Edmondson
@ 2012-01-30 14:11   ` Tomi Ollila
  1 sibling, 0 replies; 10+ messages in thread
From: Tomi Ollila @ 2012-01-30 14:11 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

On Mon, 30 Jan 2012 16:24:46 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> `Notmuch-wash-region-to-button' is the function that creates hidden
> regions with buttons for signatures, citations and original messages.
> Before the change, it did not work correctly if the to-be-hidden
> region started at the beginning of a message: the visibility toggle
> button was hidden as well.  The patch fixes this.  There are two parts
> in the fix:
> 
> * Use `insert-before-markers' instead of `insert' for creating the
>   button, so that it does not get added to the hidden overlay.
> 
> * Stop using PREFIX argument for adding a newline before the button.
>   The newline should not be added before a button at the beginning of
>   buffer.
> 
> The corresponding test is fixed now.
> ---

+1 -- for test and fix.


Tomi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] test: add test for hiding Original Message region at beginning of a message
  2012-01-30 12:24 ` [PATCH 1/2] test: add test for hiding Original Message region at beginning of a message Dmitry Kurochkin
@ 2012-02-04  1:22   ` David Bremner
  0 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2012-02-04  1:22 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

On Mon, 30 Jan 2012 16:24:45 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> The test is currently broken and will be fixed by a subsequent patch.
> 
> The patch adds a new file for tests of Emacs notmuch-show view.
> 

This patch needs to be rebased against master.

d

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 0/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer
  2012-01-30 12:24 [PATCH 0/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
  2012-01-30 12:24 ` [PATCH 1/2] test: add test for hiding Original Message region at beginning of a message Dmitry Kurochkin
  2012-01-30 12:24 ` [PATCH 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
@ 2012-02-04  7:36 ` Dmitry Kurochkin
  2012-02-04  7:36   ` [PATCH v2 1/2] test: add test for hiding Original Message region at beginning of a message Dmitry Kurochkin
                     ` (2 more replies)
  2 siblings, 3 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-02-04  7:36 UTC (permalink / raw)
  To: notmuch

Changes since v1:

* rebase on the current master, just one trivial conflict so no need
  for another round of review

Regards,
  Dmitry

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] test: add test for hiding Original Message region at beginning of a message
  2012-02-04  7:36 ` [PATCH v2 0/2] " Dmitry Kurochkin
@ 2012-02-04  7:36   ` Dmitry Kurochkin
  2012-02-04  7:36   ` [PATCH v2 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
  2012-02-04 12:36   ` [PATCH v2 0/2] " David Bremner
  2 siblings, 0 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-02-04  7:36 UTC (permalink / raw)
  To: notmuch

The test is currently broken and will be fixed by a subsequent patch.

The patch adds a new file for tests of Emacs notmuch-show view.

Based on patch by David Edmondson [1].

[1] id:"1327562380-12894-4-git-send-email-dme@dme.org"
---
 test/emacs-show   |   28 ++++++++++++++++++++++++++++
 test/notmuch-test |    1 +
 2 files changed, 29 insertions(+), 0 deletions(-)
 create mode 100755 test/emacs-show

diff --git a/test/emacs-show b/test/emacs-show
new file mode 100755
index 0000000..9800575
--- /dev/null
+++ b/test/emacs-show
@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+
+test_description="Testing emacs notmuch-show view"
+. test-lib.sh
+
+test_begin_subtest "Hiding Original Message region at beginning of a message"
+test_subtest_known_broken
+message_id='OriginalMessageHiding.1@notmuchmail.org'
+add_message \
+    [id]="$message_id" \
+    '[subject]="Hiding Original Message region at beginning of a message"' \
+    '[body]="-----Original Message-----
+Text here."'
+
+cat <<EOF >EXPECTED
+Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (inbox)
+Subject: Hiding Original Message region at beginning of a message
+To: Notmuch Test Suite <test_suite@notmuchmail.org>
+Date: Fri, 05 Jan 2001 15:43:57 +0000
+
+[ 2-line hidden original message. Click/Enter to show. ]
+EOF
+
+test_emacs "(notmuch-show \"id:$message_id\")
+	    (test-visible-output)"
+test_expect_equal_file OUTPUT EXPECTED
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index ced6b47..e14d34e 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -54,6 +54,7 @@ TESTS="
   argument-parsing
   emacs-test-functions
   emacs-address-cleaning
+  emacs-show
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}
 
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer
  2012-02-04  7:36 ` [PATCH v2 0/2] " Dmitry Kurochkin
  2012-02-04  7:36   ` [PATCH v2 1/2] test: add test for hiding Original Message region at beginning of a message Dmitry Kurochkin
@ 2012-02-04  7:36   ` Dmitry Kurochkin
  2012-02-04 12:36   ` [PATCH v2 0/2] " David Bremner
  2 siblings, 0 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-02-04  7:36 UTC (permalink / raw)
  To: notmuch

`Notmuch-wash-region-to-button' is the function that creates hidden
regions with buttons for signatures, citations and original messages.
Before the change, it did not work correctly if the to-be-hidden
region started at the beginning of a message: the visibility toggle
button was hidden as well.  The patch fixes this.  There are two parts
in the fix:

* Use `insert-before-markers' instead of `insert' for creating the
  button, so that it does not get added to the hidden overlay.

* Stop using PREFIX argument for adding a newline before the button.
  The newline should not be added before a button at the beginning of
  buffer.

The corresponding test is fixed now.
---
 emacs/notmuch-wash.el |   24 ++++++++++++++----------
 test/emacs-show       |    1 -
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 67143e5..56981d0 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -136,12 +136,13 @@ collapse the remaining lines into a button.")
 	 (lines-count (count-lines (overlay-start overlay) (overlay-end overlay))))
     (format label-format lines-count)))
 
-(defun notmuch-wash-region-to-button (msg beg end type prefix)
+(defun notmuch-wash-region-to-button (msg beg end type &optional prefix)
   "Auxiliary 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."
+\"citation\" or \"signature\". Optional PREFIX is some arbitrary
+text to insert before the button, probably for indentation.  Note
+that PREFIX should not include a newline."
 
   ;; This uses some slightly tricky conversions between strings and
   ;; symbols because of the way the button code works. Note that
@@ -160,12 +161,15 @@ insert before the button, probably for indentation."
     (overlay-put overlay 'type type)
     (goto-char (1+ end))
     (save-excursion
-      (goto-char (1- beg))
-      (insert prefix)
-      (insert-button (notmuch-wash-button-label overlay)
+      (goto-char beg)
+      (if prefix
+	  (insert-before-markers prefix))
+      (let ((button-beg (point)))
+	(insert-before-markers (notmuch-wash-button-label overlay) "\n")
+	(make-button button-beg (1- (point))
 		     'invisibility-spec invis-spec
 		     'overlay overlay
-		     :type button-type))))
+		     :type button-type)))))
 
 (defun notmuch-wash-excerpt-citations (msg depth)
   "Excerpt citations and up to one signature."
@@ -177,7 +181,7 @@ insert before the button, probably for indentation."
 	     (msg-end (point-max))
 	     (msg-lines (count-lines msg-start msg-end)))
 	(notmuch-wash-region-to-button
-	 msg msg-start msg-end "original" "\n")))
+	 msg msg-start msg-end "original")))
   (while (and (< (point) (point-max))
 	      (re-search-forward notmuch-wash-citation-regexp nil t))
     (let* ((cite-start (match-beginning 0))
@@ -194,7 +198,7 @@ insert before the button, probably for indentation."
 	  (forward-line (- notmuch-wash-citation-lines-suffix))
 	  (notmuch-wash-region-to-button
 	   msg hidden-start (point-marker)
-	   "citation" "\n")))))
+	   "citation")))))
   (if (and (not (eobp))
 	   (re-search-forward notmuch-wash-signature-regexp nil t))
       (let* ((sig-start (match-beginning 0))
@@ -208,7 +212,7 @@ insert before the button, probably for indentation."
 	      (overlay-put (make-overlay sig-start-marker sig-end-marker) 'face 'message-cited-text)
 	      (notmuch-wash-region-to-button
 	       msg sig-start-marker sig-end-marker
-	       "signature" "\n"))))))
+	       "signature"))))))
 
 ;;
 
diff --git a/test/emacs-show b/test/emacs-show
index 9800575..5700d2e 100755
--- a/test/emacs-show
+++ b/test/emacs-show
@@ -4,7 +4,6 @@ test_description="Testing emacs notmuch-show view"
 . test-lib.sh
 
 test_begin_subtest "Hiding Original Message region at beginning of a message"
-test_subtest_known_broken
 message_id='OriginalMessageHiding.1@notmuchmail.org'
 add_message \
     [id]="$message_id" \
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer
  2012-02-04  7:36 ` [PATCH v2 0/2] " Dmitry Kurochkin
  2012-02-04  7:36   ` [PATCH v2 1/2] test: add test for hiding Original Message region at beginning of a message Dmitry Kurochkin
  2012-02-04  7:36   ` [PATCH v2 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
@ 2012-02-04 12:36   ` David Bremner
  2 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2012-02-04 12:36 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

On Sat,  4 Feb 2012 11:36:35 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Changes since v1:
> 
> * rebase on the current master, just one trivial conflict so no need
>   for another round of review
> 

pushed,

d

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-02-04 12:36 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-30 12:24 [PATCH 0/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
2012-01-30 12:24 ` [PATCH 1/2] test: add test for hiding Original Message region at beginning of a message Dmitry Kurochkin
2012-02-04  1:22   ` David Bremner
2012-01-30 12:24 ` [PATCH 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
2012-01-30 12:54   ` David Edmondson
2012-01-30 14:11   ` Tomi Ollila
2012-02-04  7:36 ` [PATCH v2 0/2] " Dmitry Kurochkin
2012-02-04  7:36   ` [PATCH v2 1/2] test: add test for hiding Original Message region at beginning of a message Dmitry Kurochkin
2012-02-04  7:36   ` [PATCH v2 2/2] emacs: fix `notmuch-wash-region-to-button' to work at beginning of buffer Dmitry Kurochkin
2012-02-04 12:36   ` [PATCH v2 0/2] " David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).