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