* [PATCH] emacs: Split the function notmuch-show-clean-address
@ 2012-07-28 12:19 Mark Walters
2012-07-31 7:47 ` Tomi Ollila
2012-12-01 12:45 ` [PATCH] " David Bremner
0 siblings, 2 replies; 6+ messages in thread
From: Mark Walters @ 2012-07-28 12:19 UTC (permalink / raw)
To: notmuch
This function is also used by pick so split it out. Since, pick and show
want a slightly different combination of name and email make the
separated function return them as a pair, and let show or pick extract
the combination they want from that.
---
This change allows the removal of about 50 lines of duplicated code from
notmuch-pick. Later, we may want to move the split out function to lib.
emacs/notmuch-show.el | 19 ++++++++++++++-----
1 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 6335d45..0d8569d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -354,6 +354,19 @@ operation on the contents of the current buffer."
(defun notmuch-show-clean-address (address)
"Try to clean a single email ADDRESS for display. Return
unchanged ADDRESS if parsing fails."
+ (let* ((clean-address (notmuch-clean-address address))
+ (p-address (car clean-address))
+ (p-name (car (cdr clean-address))))
+ ;; If no name, return just the address.
+ (if (not p-name)
+ p-address
+ ;; Otherwise format the name and address together.
+ (concat p-name " <" p-address ">"))))
+
+(defun notmuch-clean-address (address)
+ "Try to clean a single email ADDRESS for display.
+Return (AUTHOR_EMAIL AUTHOR_NAME). Return (ADDRESS nil) if
+parsing fails."
(condition-case nil
(let (p-name p-address)
;; It would be convenient to use `mail-header-parse-address',
@@ -401,11 +414,7 @@ unchanged ADDRESS if parsing fails."
(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 ">")))
+ (list p-address p-name))
(error address)))
(defun notmuch-show-insert-headerline (headers date tags depth)
--
1.7.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] emacs: Split the function notmuch-show-clean-address
2012-07-28 12:19 [PATCH] emacs: Split the function notmuch-show-clean-address Mark Walters
@ 2012-07-31 7:47 ` Tomi Ollila
2012-07-31 16:25 ` Mark Walters
2012-12-01 12:45 ` [PATCH] " David Bremner
1 sibling, 1 reply; 6+ messages in thread
From: Tomi Ollila @ 2012-07-31 7:47 UTC (permalink / raw)
To: Mark Walters, notmuch
On Sat, Jul 28 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> This function is also used by pick so split it out. Since, pick and show
> want a slightly different combination of name and email make the
> separated function return them as a pair, and let show or pick extract
> the combination they want from that.
> ---
Put notmuch-clean-address before notmuch-show-clean-address so that
byte compilation don't complain about missing function.
(or move it to another file --- maybe later if it is sensible thing to do)
You could return the name-address pair as cons cell instead of list (?).
(If you continue to use list you can replace (car (cdr ...)) with (cadr ...))
Anyway, LGTM.
Tomi
>
> This change allows the removal of about 50 lines of duplicated code from
> notmuch-pick. Later, we may want to move the split out function to lib.
>
> emacs/notmuch-show.el | 19 ++++++++++++++-----
> 1 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 6335d45..0d8569d 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -354,6 +354,19 @@ operation on the contents of the current buffer."
> (defun notmuch-show-clean-address (address)
> "Try to clean a single email ADDRESS for display. Return
> unchanged ADDRESS if parsing fails."
> + (let* ((clean-address (notmuch-clean-address address))
> + (p-address (car clean-address))
> + (p-name (car (cdr clean-address))))
> + ;; If no name, return just the address.
> + (if (not p-name)
> + p-address
> + ;; Otherwise format the name and address together.
> + (concat p-name " <" p-address ">"))))
> +
> +(defun notmuch-clean-address (address)
> + "Try to clean a single email ADDRESS for display.
> +Return (AUTHOR_EMAIL AUTHOR_NAME). Return (ADDRESS nil) if
> +parsing fails."
> (condition-case nil
> (let (p-name p-address)
> ;; It would be convenient to use `mail-header-parse-address',
> @@ -401,11 +414,7 @@ unchanged ADDRESS if parsing fails."
> (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 ">")))
> + (list p-address p-name))
> (error address)))
>
> (defun notmuch-show-insert-headerline (headers date tags depth)
> --
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] emacs: Split the function notmuch-show-clean-address
2012-07-31 7:47 ` Tomi Ollila
@ 2012-07-31 16:25 ` Mark Walters
2012-07-31 16:29 ` [PATCH v2] " Mark Walters
0 siblings, 1 reply; 6+ messages in thread
From: Mark Walters @ 2012-07-31 16:25 UTC (permalink / raw)
To: Tomi Ollila, notmuch
On Tue, 31 Jul 2012, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sat, Jul 28 2012, Mark Walters <markwalters1009@gmail.com> wrote:
>
>> This function is also used by pick so split it out. Since, pick and show
>> want a slightly different combination of name and email make the
>> separated function return them as a pair, and let show or pick extract
>> the combination they want from that.
>> ---
Many thanks for the review all these look clear improvements: I will
post a new version as a reply to this email.
> Put notmuch-clean-address before notmuch-show-clean-address so that
> byte compilation don't complain about missing function.
Fixed: I am curious though because I did not get any complaints from
make. Are there some debug or similar things I should be setting?
> (or move it to another file --- maybe later if it is sensible thing to do)
I was vaguely thinking of moving it to lib after the split.
> You could return the name-address pair as cons cell instead of list (?).
This is a clear improvement.
Thanks again
Mark
> (If you continue to use list you can replace (car (cdr ...)) with (cadr ...))
>
> Anyway, LGTM.
>
> Tomi
>
>>
>> This change allows the removal of about 50 lines of duplicated code from
>> notmuch-pick. Later, we may want to move the split out function to lib.
>>
>> emacs/notmuch-show.el | 19 ++++++++++++++-----
>> 1 files changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index 6335d45..0d8569d 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -354,6 +354,19 @@ operation on the contents of the current buffer."
>> (defun notmuch-show-clean-address (address)
>> "Try to clean a single email ADDRESS for display. Return
>> unchanged ADDRESS if parsing fails."
>> + (let* ((clean-address (notmuch-clean-address address))
>> + (p-address (car clean-address))
>> + (p-name (car (cdr clean-address))))
>> + ;; If no name, return just the address.
>> + (if (not p-name)
>> + p-address
>> + ;; Otherwise format the name and address together.
>> + (concat p-name " <" p-address ">"))))
>> +
>> +(defun notmuch-clean-address (address)
>> + "Try to clean a single email ADDRESS for display.
>> +Return (AUTHOR_EMAIL AUTHOR_NAME). Return (ADDRESS nil) if
>> +parsing fails."
>> (condition-case nil
>> (let (p-name p-address)
>> ;; It would be convenient to use `mail-header-parse-address',
>> @@ -401,11 +414,7 @@ unchanged ADDRESS if parsing fails."
>> (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 ">")))
>> + (list p-address p-name))
>> (error address)))
>>
>> (defun notmuch-show-insert-headerline (headers date tags depth)
>> --
>> 1.7.9.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] emacs: Split the function notmuch-show-clean-address
2012-07-31 16:25 ` Mark Walters
@ 2012-07-31 16:29 ` Mark Walters
2012-07-31 17:59 ` Tomi Ollila
0 siblings, 1 reply; 6+ messages in thread
From: Mark Walters @ 2012-07-31 16:29 UTC (permalink / raw)
To: Tomi Ollila, notmuch
This function is also used by pick so split it out in preperation for
moving to lib. In fact, pick and show want a slightly different
combination of name and email on return so make the separated function
return them as a pair, and let show or pick extract the combination
they want from that.
---
emacs/notmuch-show.el | 27 ++++++++++++++++++---------
1 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 6335d45..4ab6f92 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -351,9 +351,10 @@ operation on the contents of the current buffer."
'face 'notmuch-tag-face)
")"))))))
-(defun notmuch-show-clean-address (address)
- "Try to clean a single email ADDRESS for display. Return
-unchanged ADDRESS if parsing fails."
+(defun notmuch-clean-address (address)
+ "Try to clean a single email ADDRESS for display. Return a cons
+cell of (AUTHOR_EMAIL AUTHOR_NAME). Return (ADDRESS nil) if
+parsing fails."
(condition-case nil
(let (p-name p-address)
;; It would be convenient to use `mail-header-parse-address',
@@ -401,12 +402,20 @@ unchanged ADDRESS if parsing fails."
(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)))
+ (cons p-address p-name))
+ (error (cons address nil))))
+
+(defun notmuch-show-clean-address (address)
+ "Try to clean a single email ADDRESS for display. Return
+unchanged ADDRESS if parsing fails."
+ (let* ((clean-address (notmuch-clean-address address))
+ (p-address (car clean-address))
+ (p-name (cdr clean-address)))
+ ;; If no name, return just the address.
+ (if (not p-name)
+ p-address
+ ;; Otherwise format the name and address together.
+ (concat p-name " <" p-address ">"))))
(defun notmuch-show-insert-headerline (headers date tags depth)
"Insert a notmuch style headerline based on HEADERS for a
--
1.7.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] emacs: Split the function notmuch-show-clean-address
2012-07-31 16:29 ` [PATCH v2] " Mark Walters
@ 2012-07-31 17:59 ` Tomi Ollila
0 siblings, 0 replies; 6+ messages in thread
From: Tomi Ollila @ 2012-07-31 17:59 UTC (permalink / raw)
To: Mark Walters, notmuch
On Tue, Jul 31 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> This function is also used by pick so split it out in preperation for
> moving to lib. In fact, pick and show want a slightly different
> combination of name and email on return so make the separated function
> return them as a pair, and let show or pick extract the combination
> they want from that.
> ---
> emacs/notmuch-show.el | 27 ++++++++++++++++++---------
> 1 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 6335d45..4ab6f92 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -351,9 +351,10 @@ operation on the contents of the current buffer."
> 'face 'notmuch-tag-face)
> ")"))))))
>
> -(defun notmuch-show-clean-address (address)
> - "Try to clean a single email ADDRESS for display. Return
> -unchanged ADDRESS if parsing fails."
> +(defun notmuch-clean-address (address)
> + "Try to clean a single email ADDRESS for display. Return a cons
> +cell of (AUTHOR_EMAIL AUTHOR_NAME). Return (ADDRESS nil) if
> +parsing fails."
Looks good to me.
It took a few iterations to understand that the error case in expression
below can be experimented with:
(condition-case nil (error "caught") (error (cons "car" "cdr"))) c-x c-e
Tomi
> (condition-case nil
> (let (p-name p-address)
> ;; It would be convenient to use `mail-header-parse-address',
> @@ -401,12 +402,20 @@ unchanged ADDRESS if parsing fails."
> (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)))
> + (cons p-address p-name))
> + (error (cons address nil))))
> +
> +(defun notmuch-show-clean-address (address)
> + "Try to clean a single email ADDRESS for display. Return
> +unchanged ADDRESS if parsing fails."
> + (let* ((clean-address (notmuch-clean-address address))
> + (p-address (car clean-address))
> + (p-name (cdr clean-address)))
> + ;; If no name, return just the address.
> + (if (not p-name)
> + p-address
> + ;; Otherwise format the name and address together.
> + (concat p-name " <" p-address ">"))))
>
> (defun notmuch-show-insert-headerline (headers date tags depth)
> "Insert a notmuch style headerline based on HEADERS for a
> --
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] emacs: Split the function notmuch-show-clean-address
2012-07-28 12:19 [PATCH] emacs: Split the function notmuch-show-clean-address Mark Walters
2012-07-31 7:47 ` Tomi Ollila
@ 2012-12-01 12:45 ` David Bremner
1 sibling, 0 replies; 6+ messages in thread
From: David Bremner @ 2012-12-01 12:45 UTC (permalink / raw)
To: Mark Walters, notmuch
Mark Walters <markwalters1009@gmail.com> writes:
> This function is also used by pick so split it out. Since, pick and show
> want a slightly different combination of name and email make the
> separated function return them as a pair, and let show or pick extract
> the combination they want from that.
Pushed,
d
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-12-01 12:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-28 12:19 [PATCH] emacs: Split the function notmuch-show-clean-address Mark Walters
2012-07-31 7:47 ` Tomi Ollila
2012-07-31 16:25 ` Mark Walters
2012-07-31 16:29 ` [PATCH v2] " Mark Walters
2012-07-31 17:59 ` Tomi Ollila
2012-12-01 12:45 ` [PATCH] " 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).