[PATCH 1/3] test: Add tests for `notmuch-show-clean-address'.

[PATCH 1/3] test: Add tests for `notmuch-show-clean-address'.

[David Edmondson]

---

The address containing UTF-8 still fails. This looks like a failure to
properly round-trip UTF-8 in the test suite - the test passes if run
directly within emacs.

 test/emacs                                         |   19 +++++++++++++++++++
 .../notmuch-address-simplification                 |    9 +++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs.expected-output/notmuch-address-simplification

diff --git a/test/emacs b/test/emacs
index ca82445..bb4a75f 100755
--- a/test/emacs
+++ b/test/emacs
@@ -514,4 +514,23 @@ counter=$(test_emacs \
 )
 test_expect_equal "$counter" 2
 
+test_begin_subtest "notmuch-show address simplification"
+test_emacs '
+(with-temp-buffer
+  (let ((input (list
+"foo@bar.com"
+"<foo@bar.com>"
+"Foo Bar <foo@bar.com>"
+"foo@bar.com <foo@bar.com>"
+"ДБ <db-uknot@stop.me.uk>"
+"foo (at home) <foo@bar.com>"
+"foo [at home] <foo@bar.com>"
+"\"Foo Bar\" <foo@bar.com>"
+"Foo Bar"
+		)))
+    (mapc (lambda (a) (insert (notmuch-show-clean-address a) "\n")) input))
+  (test-output))
+'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-address-simplification
+
 test_done
diff --git a/test/emacs.expected-output/notmuch-address-simplification b/test/emacs.expected-output/notmuch-address-simplification
new file mode 100644
index 0000000..0afe190
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-address-simplification
@@ -0,0 +1,9 @@
+foo@bar.com
+foo@bar.com
+Foo Bar <foo@bar.com>
+foo@bar.com
+ДБ <db-uknot@stop.me.uk>
+foo (at home) <foo@bar.com>
+foo [at home] <foo@bar.com>
+Foo Bar <foo@bar.com>
+Foo Bar
-- 
1.7.7.3

[PATCH 2/3] emacs: Avoid `mail-header-parse-address' in `notmuch-show-clean-address'.

[David Edmondson]

`mail-header-parse-address' expects un-decoded mailbox parts, which is
not what we have at this point. Replace it with simple string
deconstruction.
---
 emacs/notmuch-show.el |   48 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 9328650..afff6a5 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -227,21 +227,43 @@ indentation."
   "Try to clean a single email ADDRESS for display.  Return
 unchanged ADDRESS if parsing fails."
   (condition-case nil
-    (let* ((parsed (mail-header-parse-address address))
-	   (address (car parsed))
-	   (name (cdr parsed)))
-      ;; Remove double quotes. They might be required during transport,
-      ;; but we don't need to see them.
-      (when name
-        (setq name (replace-regexp-in-string "\"" "" name)))
+    (let (p-name p-address)
+      ;; It would be convenient to use `mail-header-parse-address',
+      ;; but that expects un-decoded mailbox parts, whereas our
+      ;; mailbox parts are already decoded (and hence may contain
+      ;; UTF-8). Given that notmuch should handle most of the awkward
+      ;; cases, some simple string deconstruction should be sufficient
+      ;; here.
+      (cond
+       ;; "User <user@dom.ain>" style.
+       ((string-match "\\(.*\\) <\\(.*\\)>" address)
+	(setq p-name (match-string 1 address)
+	      p-address (match-string 2 address)))
+
+       ;; "<user@dom.ain>" style.
+       ((string-match "<\\(.*\\)>" address)
+	(setq p-address (match-string 1 address)))
+
+       ;; Everything else.
+       (t
+	(setq p-address address)))
+      
+      ;; Remove outer double quotes. They might be required during
+      ;; transport, but we don't need to see them.
+      (when (and p-name
+		 (string-match "^\"\\(.*\\)\"$" p-name))
+        (setq p-name (match-string 1 p-name)))
+
       ;; If the address is 'foo@bar.com <foo@bar.com>' then show just
       ;; 'foo@bar.com'.
-      (when (string= name address)
-        (setq name nil))
-
-      (if (not name)
-        address
-        (concat name " <" address ">")))
+      (when (string= p-name p-address)
+        (setq p-name nil))
+
+      ;; If no name results, return just the address.
+      (if (not p-name)
+	  p-address
+	;; Otherwise format the name and address together.
+	(concat p-name " <" p-address ">")))
     (error address)))
 
 (defun notmuch-show-insert-headerline (headers date tags depth)
-- 
1.7.7.3

[PATCH 3/3] test: Correct the expected output of the 'invalid From' test.

[David Edmondson]

---
 test/emacs |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/emacs b/test/emacs
index bb4a75f..0168f85 100755
--- a/test/emacs
+++ b/test/emacs
@@ -78,7 +78,7 @@ thread=$(notmuch search --output=threads subject:message-with-invalid-from)
 test_emacs "(notmuch-show \"$thread\")
 	    (test-output)"
 cat <<EOF >EXPECTED
-"Invalid " From" <test_suite@notmuchmail.org> (2001-01-05) (inbox)
+Invalid " From <test_suite@notmuchmail.org> (2001-01-05) (inbox)
 Subject: message-with-invalid-from
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
 Date: Tue, 05 Jan 2001 15:43:57 -0000
-- 
1.7.7.3

Re: [PATCH 3/3] test: Correct the expected output of the 'invalid From' test.

[Tomi Ollila]

On Tue, 27 Dec 2011 10:15:40 +0000, David Edmondson <dme@dme.org> wrote:

LGTM, all 3.

Tomi

[PATCH 1/3 v2] emacs: Avoid `mail-header-parse-address' in `notmuch-show-clean-address'.

[David Edmondson]

`mail-header-parse-address' expects un-decoded mailbox parts, which is
not what we have at this point. Replace it with simple string
deconstruction.
---

Added removal of backslashes from the mailbox part.

 emacs/notmuch-show.el |   52 ++++++++++++++++++++++++++++++++++++------------
 1 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5502efd..01b774d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -230,21 +230,47 @@ indentation."
   "Try to clean a single email ADDRESS for display.  Return
 unchanged ADDRESS if parsing fails."
   (condition-case nil
-    (let* ((parsed (mail-header-parse-address address))
-	   (address (car parsed))
-	   (name (cdr parsed)))
-      ;; Remove double quotes. They might be required during transport,
-      ;; but we don't need to see them.
-      (when name
-        (setq name (replace-regexp-in-string "\"" "" name)))
+    (let (p-name p-address)
+      ;; It would be convenient to use `mail-header-parse-address',
+      ;; but that expects un-decoded mailbox parts, whereas our
+      ;; mailbox parts are already decoded (and hence may contain
+      ;; UTF-8). Given that notmuch should handle most of the awkward
+      ;; cases, some simple string deconstruction should be sufficient
+      ;; here.
+      (cond
+       ;; "User <user@dom.ain>" style.
+       ((string-match "\\(.*\\) <\\(.*\\)>" address)
+	(setq p-name (match-string 1 address)
+	      p-address (match-string 2 address)))
+
+       ;; "<user@dom.ain>" style.
+       ((string-match "<\\(.*\\)>" address)
+	(setq p-address (match-string 1 address)))
+
+       ;; Everything else.
+       (t
+	(setq p-address address)))
+      
+      ;; Remove elements of the mailbox part that are not relevant for
+      ;; display, even if they are required during transport.
+      (when p-name
+	;; Outer double quotes.
+	(when (string-match "^\"\\(.*\\)\"$" p-name)
+	  (setq p-name (match-string 1 p-name)))
+
+	;; Backslashes.
+	(setq p-name (replace-regexp-in-string "\\\\" "" p-name)))
+	
       ;; If the address is 'foo@bar.com <foo@bar.com>' then show just
       ;; 'foo@bar.com'.
-      (when (string= name address)
-        (setq name nil))
-
-      (if (not name)
-        address
-        (concat name " <" address ">")))
+      (when (string= p-name p-address)
+        (setq p-name nil))
+
+      ;; If no name results, return just the address.
+      (if (not p-name)
+	  p-address
+	;; Otherwise format the name and address together.
+	(concat p-name " <" p-address ">")))
     (error address)))
 
 (defun notmuch-show-insert-headerline (headers date tags depth)
-- 
1.7.7.3

[PATCH 2/3 v2] test: Add tests for `notmuch-show-clean-address'.

[David Edmondson]

---

Added backslash test. UTF8 round-trip still isn't right.

 test/emacs                                         |   20 ++++++++++++++++++++
 .../notmuch-address-simplification                 |   10 ++++++++++
 2 files changed, 30 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs.expected-output/notmuch-address-simplification

diff --git a/test/emacs b/test/emacs
index a06c223..69738cd 100755
--- a/test/emacs
+++ b/test/emacs
@@ -514,4 +514,24 @@ counter=$(test_emacs \
 )
 test_expect_equal "$counter" 2
 
+test_begin_subtest "notmuch-show address simplification"
+test_emacs '
+(with-temp-buffer
+  (let ((input (list
+"foo@bar.com"
+"<foo@bar.com>"
+"Foo Bar <foo@bar.com>"
+"foo@bar.com <foo@bar.com>"
+"ДБ <db-uknot@stop.me.uk>"
+"foo (at home) <foo@bar.com>"
+"foo [at home] <foo@bar.com>"
+"\"Foo Bar\" <foo@bar.com>"
+"Foo Bar"
+"Fred Dibna \\[extraordinaire\\] <fred@dibna.com>"
+		)))
+    (mapc (lambda (a) (insert (notmuch-show-clean-address a) "\n")) input))
+  (test-output))
+'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-address-simplification
+
 test_done
diff --git a/test/emacs.expected-output/notmuch-address-simplification b/test/emacs.expected-output/notmuch-address-simplification
new file mode 100644
index 0000000..4747827
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-address-simplification
@@ -0,0 +1,10 @@
+foo@bar.com
+foo@bar.com
+Foo Bar <foo@bar.com>
+foo@bar.com
+ДБ <db-uknot@stop.me.uk>
+foo (at home) <foo@bar.com>
+foo [at home] <foo@bar.com>
+Foo Bar <foo@bar.com>
+Foo Bar
+Fred Dibna [extraordinaire] <fred@dibna.com>
-- 
1.7.7.3

Re: [PATCH 2/3 v2] test: Add tests for `notmuch-show-clean-address'.

[David Bremner]

On Fri, 30 Dec 2011 09:39:37 +0000, David Edmondson <dme@dme.org> wrote:
> ---
> 
> Added backslash test. UTF8 round-trip still isn't right.
> 

Hi David.

Do you mind explaining a bit better in the commit message what the issue
is here?

d

Re: [PATCH 2/3 v2] test: Add tests for `notmuch-show-clean-address'.

[David Edmondson]

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

On Tue, 10 Jan 2012 07:06:58 -0400, David Bremner <david@tethera.net> wrote:
> On Fri, 30 Dec 2011 09:39:37 +0000, David Edmondson <dme@dme.org> wrote:
> > ---
> > 
> > Added backslash test. UTF8 round-trip still isn't right.
> > 
> 
> Do you mind explaining a bit better in the commit message what the issue
> is here?

I'm not completely sure.

The expected output file is supposed to contain some UTF8 encoded
text. One of the addresses to be tested has a 'human readable' part that
has the same UTF8 text. Testing the functions directly within emacs
retains the text correctly encoded. Pushing the text through the test
suite results in text that does not correctly match the expected
output.

There are many places where things could be wrong:
      - the expected output file doesn't include the correct UTF8 text
        (but it looks fine in emacs),
      - the test wrapper script doesn't correctly pass around the UTF8
        encoded text,
      - the emacs reader is not correctly loading the UTF8 encoded text,
      - <your favourite imagined problem here>.

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

[PATCH 3/3 v2] test: Fix the 'invalid from' test in light of address cleaning changes.

[David Edmondson]

---
 test/emacs |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/emacs b/test/emacs
index 69738cd..32d49ae 100755
--- a/test/emacs
+++ b/test/emacs
@@ -78,7 +78,7 @@ thread=$(notmuch search --output=threads subject:message-with-invalid-from)
 test_emacs "(notmuch-show \"$thread\")
 	    (test-output)"
 cat <<EOF >EXPECTED
-"Invalid " From" <test_suite@notmuchmail.org> (2001-01-05) (inbox)
+Invalid " From <test_suite@notmuchmail.org> (2001-01-05) (inbox)
 Subject: message-with-invalid-from
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
 Date: Fri, 05 Jan 2001 15:43:57 +0000
-- 
1.7.7.3