emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Daimrod <daimrod@gmail.com>
To: Feng Shu <tumashu@gmail.com>
Cc: orgmode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] org-contacts.el: add expire feature
Date: Wed, 29 May 2013 23:20:46 +0200	[thread overview]
Message-ID: <87wqqhlchq.fsf@tanger.home> (raw)
In-Reply-To: 87li6yx8u9.fsf@news.tumashu-localhost.org

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

Feng Shu <tumashu@gmail.com> writes:

Hello Feng,

Thanks for this interesting feature, here are some remarks:

> From e974db131d88acf06bb6b250eac2fae8c7d0a96e Mon Sep 17 00:00:00 2001
> From: Feng Shu <tumashu@gmail.com>
> Date: Wed, 29 May 2013 20:30:43 +0800
> Subject: [PATCH] * contrib/lisp/org-contacts.el:   Add a feature which can
>  expire   emails and phones

Could you use the org-mode convention for commit messages?[1]

It should be:

#+BEGIN_SRC change-log
  contrib/lisp/org-contacts.el: Add a feature which can expire emails and phones
  
  ,* contrib/lisp/org-contacts.el: Add a feature which can expire
  emails and phones.
  
  ...[rest of the commit message]...
#+END_SRC

> * test
>   :PROPERTIES:
>   :EMAIL: test1@gmail.com  test2@gmail.com  test3@gmail.com
>   :PHONE:  123456  123457 123458
>   :EXPIRE:  test2@gmail.com 123457
>   :END:
>
> when completing or exporting to vcard,  the emails and  phones in the
> expire property (test2@gmail.com and 123457) will be ignore

Since the purpose of this property is to ignore some values when
exporting to vcard, don't you think it would be better to name it IGNORE
or VCARD_IGNORE? (and of course to rename all functions accordingly)

> ---
>  contrib/lisp/org-contacts.el |   32 +++++++++++++++++++++++++++-----
>  1 个文件被修改,插入 27 行(+),删除 5 行(-)
>
> diff --git a/contrib/lisp/org-contacts.el b/contrib/lisp/org-contacts.el
> index 7b0b603..ae6c6f1 100644
> --- a/contrib/lisp/org-contacts.el
> +++ b/contrib/lisp/org-contacts.el
> @@ -86,6 +86,11 @@ When set to nil, all your Org files will be used."
>    :type 'string
>    :group 'org-contacts)
>  
> +(defcustom org-contacts-expire-property "EXPIRE"
> +  "Name of the property for emails or phones which will be expired"
> +  :type 'string
> +  :group 'org-contacts)
> +
>  
>  (defcustom org-contacts-birthday-format "Birthday: %l (%Y)"
>    "Format of the anniversary agenda entry.
> @@ -476,6 +481,14 @@ A group FOO is composed of contacts with the tag FOO."
>  		(completion-table-case-fold completion-list
>  					    (not org-contacts-completion-ignore-case))))))))
>  
> +
> +(defun org-contacts-remove-expired-property (expire-list list)
> +  "Remove emails or phones in list-expired from list"

Don't forget the dot at the end of the sentence.

> +    (while expire-list
> +      (setq list (remove (car expire-list) list))
> +      (setq expire-list (cdr expire-list)))
> +    list)
> +

This is not very idiomatic elisp, I would write something more like:
#+BEGIN_SRC emacs-lisp
  (remove-if (lambda (el)
               (member el expire-list)) 
             list)
#+END_SRC

>  (defun org-contacts-complete-name (start end string)
>    "Complete text at START with a user name and email."
>    (let* ((completion-ignore-case org-contacts-completion-ignore-case)
> @@ -484,10 +497,17 @@ A group FOO is composed of contacts with the tag FOO."
>  		;; The contact name is always the car of the assoc-list
>  		;; returned by `org-contacts-filter'.
>  		for contact-name = (car contact)
> +
> +		;; Build the list of the email addresses which has
> +		;; been expired
> +		for expire-list = (org-contacts-split-property (or
> +								(cdr (assoc-string org-contacts-expire-property
> +										   (caddr contact))) ""))
>  		;; Build the list of the user email addresses.
> -		for email-list = (org-contacts-split-property (or
> -						(cdr (assoc-string org-contacts-email-property
> -								   (caddr contact))) ""))
> +		for email-list = (org-contacts-remove-expired-property expire-list
> +								       (org-contacts-split-property (or
> +												     (cdr (assoc-string org-contacts-email-property
> +															(caddr contact))) "")))
>  		;; If the user has email addresses…
>  		if email-list
>  		;; … append a list of USER <EMAIL>.
> @@ -869,15 +889,17 @@ to do our best."
>  	 (n (org-contacts-vcard-encode-name name))
>  	 (email (cdr (assoc-string org-contacts-email-property properties)))
>  	 (tel  (cdr (assoc-string org-contacts-tel-property properties)))
> +	 (expire  (cdr (assoc-string org-contacts-expire-property properties)))
>  	 (note (cdr (assoc-string org-contacts-note-property properties)))
>  	 (bday (org-contacts-vcard-escape (cdr (assoc-string org-contacts-birthday-property properties))))
>  	 (addr (cdr (assoc-string org-contacts-address-property properties)))
>  	 (nick (org-contacts-vcard-escape (cdr (assoc-string org-contacts-nickname-property properties))))
>  	 (head (format "BEGIN:VCARD\nVERSION:3.0\nN:%s\nFN:%s\n" n name))
> +	 (expire-list (when expire (setq expire-list (org-contacts-split-property expire))))
>  	 emails-list result phones-list)
>      (concat head
>  	    (when email (progn
> -			  (setq emails-list (org-contacts-split-property email))
> +			  (setq emails-list (org-contacts-remove-expired-property expire-list (org-contacts-split-property email))) ;

There is a leading semi-colon.                                                                                        ^^^^

>  			  (setq result "")
>  			  (while emails-list
>  			    (setq result (concat result  "EMAIL:" (org-contacts-strip-link (car emails-list)) "\n"))
> @@ -886,7 +908,7 @@ to do our best."
>  	    (when addr
>  	      (format "ADR:;;%s\n" (replace-regexp-in-string "\\, ?" ";" addr)))
>  	    (when tel (progn
> -			(setq phones-list (org-contacts-split-property tel))
> +			(setq phones-list (org-contacts-remove-expired-property expire-list (org-contacts-split-property tel)))
>  			(setq result "")
>  			(while phones-list
>  			  (setq result (concat result  "TEL:" (org-contacts-strip-link (car phones-list)) "\n"))
> -- 
> 1.7.10.4

[1] http://orgmode.org/worg/org-contribute.html#sec-5

-- 
Daimrod/Greg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 835 bytes --]

  reply	other threads:[~2013-05-29 21:18 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-29 12:35 [PATCH] org-contacts.el: add expire feature Feng Shu
2013-05-29 21:20 ` Daimrod [this message]
2013-05-30  0:32   ` Feng Shu
2013-05-30  0:38   ` Feng Shu
2013-05-30 10:36     ` Daimrod
2013-05-30 10:49       ` Feng Shu
2013-05-30 11:32       ` Feng Shu
2013-05-30 23:59         ` Daimrod
2013-05-31  4:22           ` Feng Shu
2013-05-30 17:37   ` Karl Voit
2013-05-30 22:56     ` Feng Shu
2013-05-31  9:38       ` Karl Voit
2013-05-31  0:05     ` Daimrod
2013-05-31  9:42       ` Handling outdated contact information (was: [PATCH] org-contacts.el: add expire feature) Karl Voit
2013-05-31 11:00         ` Handling outdated contact information Daimrod
2013-05-31 12:12           ` Feng Shu
2013-05-31 13:15         ` Feng Shu
2013-05-31 21:41           ` Org-mode contacts to Android (was: Handling outdated contact information) Karl Voit
2013-05-31 22:28             ` Org-mode contacts to Android Feng Shu
2013-06-01  0:23               ` Feng Shu
2013-05-29 21:30 ` [PATCH] org-contacts.el: add expire feature Daimrod
2013-05-29 23:15   ` Feng Shu
2013-05-30  0:42 ` [PATCH] org-contacts.el: ignore emails or phones with ignore property Feng Shu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wqqhlchq.fsf@tanger.home \
    --to=daimrod@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=tumashu@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.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).