From: Thomas Jost <schnouki@schnouki.net>
To: David Edmondson <dme@dme.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH 1/2] emacs: Avoid `mail-header-parse-address' in
	`notmuch-show-clean-address'.
In-Reply-To: <1327491904-28416-2-git-send-email-dme@dme.org>
References: <1327491904-28416-1-git-send-email-dme@dme.org>
	<1327491904-28416-2-git-send-email-dme@dme.org>
User-Agent: Notmuch/0.11+132~gf915e3a (http://notmuchmail.org) Emacs/24.0.92.1
	(x86_64-unknown-linux-gnu)
Date: Wed, 25 Jan 2012 13:51:46 +0100
Message-ID: <878vkwc5ct.fsf@thor.loria.fr>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

On Wed, 25 Jan 2012 11:45:03 +0000, David Edmondson <dme@dme.org> wrote:
> `mail-header-parse-address' expects un-decoded mailbox parts, which is
> not what we have at this point. Replace it with simple string
> deconstruction.
>=20
> Mark the corresponding test as no longer broken.
>=20
> Minor whitespace cleanup.
> ---
>  emacs/notmuch-show.el          |   50 ++++++++++++++++++++++++++++------=
-----
>  test/emacs-address-cleaning.sh |    1 -
>  2 files changed, 36 insertions(+), 15 deletions(-)
>=20
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index e6a5b31..1fd2524 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -289,21 +289,43 @@ operation on the contents of the current buffer."
>    "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=3D name address)
> -        (setq name nil))
> -
> -      (if (not name)
> -        address
> -        (concat name " <" address ">")))
> +      (when (string=3D 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)))
>=20=20
>  (defun notmuch-show-insert-headerline (headers date tags depth)
> @@ -1417,7 +1439,7 @@ than only the current message."
>    (interactive "P\nsPipe message to command: ")
>    (let (shell-command)
>      (if entire-thread
> -	(setq shell-command=20
> +	(setq shell-command
>  	      (concat notmuch-command " show --format=3Dmbox "
>  		      (shell-quote-argument
>  		       (mapconcat 'identity (notmuch-show-get-message-ids-for-open-mes=
sages) " OR "))
> diff --git a/test/emacs-address-cleaning.sh b/test/emacs-address-cleaning=
.sh
> index 0d85bdc..51018fe 100755
> --- a/test/emacs-address-cleaning.sh
> +++ b/test/emacs-address-cleaning.sh
> @@ -12,7 +12,6 @@ test_emacs_expect_t \
>      '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-2=
)'
>=20=20
>  test_begin_subtest "notmuch-test-address-clean part 3"
> -test_subtest_known_broken
>  test_emacs_expect_t \
>      '(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-3=
)'
>=20=20
> --=20
> 1.7.8.3

Hello David,

Works fine for me, but this also breaks en Emacs test:

 FAIL   notmuch-show for message with invalid From
	--- emacs.10.expected	2012-01-25 12:50:00.310786410 +0000
	+++ emacs.10.output	2012-01-25 12:50:00.310786410 +0000
	@@ -1,4 +1,4 @@
	-"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
nil

Thanks for this patch anyway :)

Regards,

=2D-=20
Thomas/Schnouki

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iQEcBAEBAgAGBQJPH/riAAoJEMPdciX+bh5IyWQH/iablZAbREuMGWochZi5wUqp
QqvODN5n9ejDHtYYKI3y3YjbrOTJu2MVU1pPlYrvnL3tQWHfu0lsDrhMnrqOn5NV
B3sx/VLj6BJiC1Pvh9oDtDk6zy5rgc5H2iugCucu9083SL/eC1HMfbyEEpEQHBtq
6JZG8fOqxqFxIddqSca+QPEQKHS2+zz+HQhO+IMGQWAQVhcD3aNV59EVVxlb7YKt
CeLG3H+oNTGGum3CqLXu1WHmkgcfGGRziMeMFmen1QXjHTuSUFIr2SkTDbJ0FmMP
79cQsUqaO1R7lYtghnOa9Fg8bbL3/P81IGfhAO/Qr0NqYcTqeohCm+lfm0dz+Fg=
=h3x4
-----END PGP SIGNATURE-----
--=-=-=--