Re: [Patch] phone links...

Re: [Patch] phone links...

[Michael Strey]

Daimrod <daimrod@gmail.com> writes:

> While merging a patch (from Feng Shu) I have found a bug in
> `org-contacts-split-property'. Though the docstring says that OMIT-NULLS
> is forced to t when SEPARATORS is nil (just like `split-string'), it
> wasn't the case. I've pushed a fix; could you check on your side that it
> doesn't break anything in your workflow?

I'm fine with your update.  Thank you for the correction!

-- 
Michael Strey
mailto:mstrey@strey.biz
http://www.strey.biz

Re: phone links...

[Robert P. Goldman]

On 4/4/13 Apr 4 -7:15 AM, Bastien wrote:
> Hi Robert,
> 
> Robert Goldman <rpgoldman@sift.info> writes:
> 
>> The two files are below, in hopes of getting suggestions for improvement
>> so that someday this could find itself into contrib/.
> 
> Looks like a good start -- please keep us updated about your progress
> and your will to include this in contrib/, I'm sure many users would
> enjoy it.

I will be happy to include this into contrib (and do the relevant
assignment) at any time when people think that it is sufficiently ready
to go.  I have been testing it in contrib/ in a testing branch in my git
repo.

I think a minor refactoring would make it more general: split the phone
handler into two components: a filter function (like the one for
linphone), defaulting to identity; and a command, probably revised to be
a format string (so that the dialed number could be place in an
arbitrary location).

I will try to have a new version with these changes very soon.

Best,
r

Re: phone links...

[Michael Strey]

Robert,

On Do, Apr 04, 2013 at 09:38:48 -0500, Robert P. Goldman wrote:

[...]

> I will be happy to include this into contrib (and do the relevant
> assignment) at any time when people think that it is sufficiently ready
> to go.  I have been testing it in contrib/ in a testing branch in my git
> repo.

What about integrating these few functions into org-contacts?  Most
users will use telephony functions in conjunction with a contact
database.  The recently applied patches from Feng are already steps into
this direction.  The next logical step would be to define some basic
rules about how to write phone numbers and to support links in the EMAIL
and PHONE properties like in the following example.

#+BEGIN_SRC org
* Surname, Name
  :PROPERTIES:
  :EMAIL:    mailto:foo@bar.com
  :PHONE:    [[phone:+49 351 4129535]]; [[phone:+49 176 6626196]]
  :END:
#+END_SRC

Moreover the definition of the phone link would allow to extend the
current functionality of MobileOrg by a phone call function.

Regards
-- 
Michael Strey 
www.strey.biz

Re: phone links...

[Robert Goldman]

Michael Strey wrote:
> Robert,
> 
> On Do, Apr 04, 2013 at 09:38:48 -0500, Robert P. Goldman wrote:
> 
> [...]
> 
>> I will be happy to include this into contrib (and do the relevant
>> assignment) at any time when people think that it is sufficiently ready
>> to go.  I have been testing it in contrib/ in a testing branch in my git
>> repo.
> 
> What about integrating these few functions into org-contacts?  

I am very reluctant to do this.

I don't use org-contacts at all, but I *do* like to have phone numbers
in my TODO items.

I don't believe a person should have to use org contacts in order to
permit themselves to use phone links.

I think we can get an appropriate combination by having org-phone be
free-standing, and simply let the org-contacts user adopt them if s/he
likes.

Related question:  if the org-contact entries are *known* to be phone
numbers (keyed by the field), why not automagically treat those values
as phone numbers, instead of requiring them to be phone links (which add
cruft to the files)?  And similarly for EMAIL.

I.e., why not simply have:

#+BEGIN_SRC org
* Surname, Name
  :PROPERTIES:
  :EMAIL:    foo@bar.com
  :PHONE:    +49 351 4129535; +49 176 6626196
  :END:
#+END_SRC
?

But as a non-org-contacts user, my opinion is obviously not so important
here...

Re: phone links...

[Michael Strey]

On Mo, Apr 08, 2013 at 07:47:52 -0500, Robert Goldman wrote:
> Michael Strey wrote:

[...]

> > What about integrating these few functions into org-contacts?  
> 
> I am very reluctant to do this.
> 
> I don't use org-contacts at all, but I *do* like to have phone numbers
> in my TODO items.

Actually, I do not use org-contacts either but I use an org file as
contact data base that follows the conventions defined in org-contacts.
Nevertheless, I would like to be able to use some of the newer functions of
org-contacts, namely the VCard export.

Currently org-phone.el as well as my org-dial.el are incompatible with
org-contacts.  The only idea behind my proposal was to make the contributors
of both packages aware of each other.


> I think we can get an appropriate combination by having org-phone be
> free-standing, and simply let the org-contacts user adopt them if s/he
> likes.

Yes, of course, with some persuading.

> Related question:  if the org-contact entries are *known* to be phone
> numbers (keyed by the field), why not automagically treat those values
> as phone numbers, instead of requiring them to be phone links (which add
> cruft to the files)?  And similarly for EMAIL.
> 
> I.e., why not simply have:
> 
> #+BEGIN_SRC org
> * Surname, Name
>   :PROPERTIES:
>   :EMAIL:    foo@bar.com
>   :PHONE:    +49 351 4129535; +49 176 6626196
>   :END:
> #+END_SRC
> ?

That would be convenient indeed.  But considering the given syntax rules
for links in org-mode, I'm not sure whether I really would like it.  Too
much magic can be confusing.

Moreover that would create a dependency between org-contacts and org-phone.
Org-contacts would require org-phone in order to work as intended, thus
creating a strong argument for my proposal of merging both packages.

Regards
-- 
Michael Strey 
www.strey.biz

Re: phone links...

[Robert Goldman]

Michael Strey wrote:
> Currently org-phone.el as well as my org-dial.el are incompatible with
> org-contacts.  The only idea behind my proposal was to make the contributors
> of both packages aware of each other.

Can you explain what makes org-phone incompatible with org-contacts?
Maybe my naming of some function?

If so, I will make sure to remove any incompatiblities on my side.

BTW, I would be happy to push org-phone.el into contrib/lisp...

I *think* I may have commit privileges as a hang-over from once
committing patches to the old generic exporter.....

cheers,
r

Re: phone links...

[Michael Strey]

Robert,

On Mo, Apr 08, 2013 at 09:44:12 -0500, Robert Goldman wrote:
> Michael Strey wrote:
> > Currently org-phone.el as well as my org-dial.el are incompatible with
> > org-contacts.  The only idea behind my proposal was to make the contributors
> > of both packages aware of each other.
> 
> Can you explain what makes org-phone incompatible with org-contacts?
> Maybe my naming of some function?

The problem is on the side of org-contacts.  Org-contacts does not
support links in its properties.  Thus, currently the only solution to
use the advantages of org-contacts and org-phone is to give the
information twice, like in the following example.

#+BEGIN_SRC org
* Strey, Michael
:PROPERTIES:
:EMAIL:    mstrey@strey.biz foo@bar.com
:PHONE:    +493514129535 +491263213
:END:

[[mailto:mstrey@strey.biz]]
[[mailto:foo@bar.com]]
[[phone:+49 (0)351 41295-35]]
[[phone:+49 126 3213]]
#+END_SRC

This shortcoming effects not only the phone links but email links as
well.

Regards
-- 
Michael Strey 
www.strey.biz

Re: phone links...

[Michael Strey]

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

On Tue, Apr 09, 2013 at 09:31:40AM +0200, Michael Strey wrote:

[...]

> The problem is on the side of org-contacts.  Org-contacts does not
> support links in its properties.
[...]

> This shortcoming effects not only the phone links but email links as
> well.

Attached is a patch to allow org links in org-contacts properties.
It allows entries like in the following example without effecting
org-contacts current functions.

#+BEGIN_SRC org
* Surname, Name
:PROPERTIES:
:EMAIL:    mailto:test2@test.de; [[mailto:name@test.de]] foo@bar.biz
:PHONE:    [[tel:+49 351 4129535]], +491766626196 [[+49 (351) 41295-35]]
:END:
#+END_SRC

Phone links of the form [[tel:+49 351 412 95-35][My phone number]] or
[[tel:+49 351 41295-35]] are expected.  `-', `/', `(', `)' and
whitespace characters are allowed in telephone numbers.


-- 
Michael Strey 
www.strey.biz


[-- Attachment #2: 0174-Org-contacts-Allow-org-links-in-properties.patch --]
[-- Type: text/plain, Size: 9954 bytes --]

From 69ae791cd552bacdcbc99af99a82ab699fa16d36 Mon Sep 17 00:00:00 2001
From: Michael Strey <mstrey@strey.biz>
Date: Fri, 12 Apr 2013 12:33:16 +0200
Subject: [PATCH 174/174] Org-contacts: Allow org links in properties

* org-contacts.el (org-contacts-split-property, chomp): Introduce a custom
  version of split-string that keeps org links intact. `chomp' is a
  helper function for `org-contacts-split-property'.
* org-contacts.el (org-contacts-strip-link): Introduce a new function
  that removes brackets, description, link type and colon from an org
  link string returning the pure link key.
* org-contacts.el (org-contacts-complete-group)
	(org-contacts-complete-name, org-contacts-view-send-email)
	(org-contacts-get-icon, org-contacts-vcard-format): Apply the
  new functions to the already existing functions extracting telephone numbers
  and email addresses from the properties.

Allowed separators for email addresses and phone numbers are `,', `;'
and whitespace.  `:' is not allowed anymore as separator to avoid
confusion with implizit links.

Examples of properties that are possible after those changes:

* Surname, Name
:PROPERTIES:
:EMAIL:    mailto:test2@test.de; [[mailto:name@test.de]] foo@bar.biz
:PHONE:    [[tel:+49 351 4129535]], +491766626196 [[+49 (351) 41295-35]]
:END:

Phone links of the form [[tel:+49 351 412 95-35][My phone number]] or
[[tel:+49 351 41295-35]] are expected.  `-', `/', `(', `)' and
whitespace characters are allowed in telephone numbers.
---
 contrib/lisp/org-contacts.el | 83 ++++++++++++++++++++++++++++++++++++--------
 5 files changed, 106 insertions(+), 28 deletions(-)

diff --git a/contrib/lisp/org-contacts.el b/contrib/lisp/org-contacts.el
index 65eeea8..a502674 100644
--- a/contrib/lisp/org-contacts.el
+++ b/contrib/lisp/org-contacts.el
@@ -218,7 +218,7 @@ This overrides `org-email-link-description-format' if set."
     org-contacts-db))
 
 (defun org-contacts-filter (&optional name-match tags-match)
-  "Search for a contact maching NAME-MATCH and TAGS-MATCH.
+  "Search for a contact matching NAME-MATCH and TAGS-MATCH.
 If both match values are nil, return all contacts."
   (if (and (null name-match)
 	   (null tags-match))
@@ -426,7 +426,7 @@ A group FOO is composed of contacts with the tag FOO."
 				    (or (cdr (assoc-string "ALLTAGS" (caddr contact))) "") ":")))))))
 	(list start end
 	      (if (= (length completion-list) 1)
-		  ;; We've foudn the correct group, returns the address
+		  ;; We've found the correct group, returns the address
 		  (lexical-let ((tag (get-text-property 0 'org-contacts-group
 							(car completion-list))))
 		    (lambda (string pred &optional to-ignore)
@@ -438,11 +438,11 @@ A group FOO is composed of contacts with the tag FOO."
 				       ;; returned by `org-contacts-filter'.
 				       for contact-name = (car contact)
 				       ;; Grab the first email of the contact
-				       for email = (car (split-string
+				       for email = (org-contacts-strip-link (car (org-contacts-split-property
 							 (or
 							  (cdr (assoc-string org-contacts-email-property
 									     (caddr contact)))
-							  "")))
+							  ""))))
 				       ;; If the user has an email address, append USER <EMAIL>.
 				       if email collect (org-contacts-format-email contact-name email))
 				 ", ")))
@@ -459,14 +459,14 @@ A group FOO is composed of contacts with the tag FOO."
 		;; returned by `org-contacts-filter'.
 		for contact-name = (car contact)
 		;; Build the list of the user email addresses.
-		for email-list = (split-string (or
+		for email-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>.
 		nconc (loop for email in email-list
-			    collect (org-contacts-format-email contact-name email))))
+			    collect (org-contacts-format-email contact-name (org-contacts-strip-link email)))))
 	 (completion-list (org-contacts-all-completions-prefix
 			   string
 			   (org-uniquify completion-list))))
@@ -731,11 +731,12 @@ address."
     (org-with-point-at marker
       (let ((emails (org-entry-get (point) org-contacts-email-property)))
         (if emails
-            (let ((email-list (split-string emails)))
+            (let ((email-list (org-contacts-split-property emails)))
               (if (and (= (length email-list) 1) (not ask))
                   (compose-mail (org-contacts-format-email
                                  (org-get-heading t) emails))
                 (let ((email (completing-read "Send mail to which address: " email-list)))
+		  (setq email (org-contacts-strip-link email))
                   (org-contacts-check-mail-address email)
                   (compose-mail (org-contacts-format-email (org-get-heading t) email)))))
           (error (format "This contact has no mail address set (no %s property)."
@@ -759,8 +760,8 @@ address."
              (email-list (org-entry-get pom org-contacts-email-property))
              (gravatar
               (when email-list
-                (loop for email in (split-string email-list)
-                      for gravatar = (gravatar-retrieve-synchronously email)
+                (loop for email in (org-contacts-split-property email-list)
+                      for gravatar = (gravatar-retrieve-synchronously (org-contacts-strip-link email))
                       if (and gravatar
                               (not (eq gravatar 'error)))
                       return gravatar))))
@@ -842,19 +843,19 @@ to do our best."
 	 (head (format "BEGIN:VCARD\nVERSION:3.0\nN:%s\nFN:%s\n" n name)))
     (concat head
 	    (when email (progn
-			  (setq emails-list (split-string email "[,;: ]+"))
+			  (setq emails-list (org-contacts-split-property email "[,; ]+"))
 			  (setq result "")
 			  (while emails-list
-			    (setq result (concat result  "EMAIL:" (car emails-list) "\n"))
+			    (setq result (concat result  "EMAIL:" (org-contacts-strip-link (car emails-list)) "\n"))
 			    (setq emails-list (cdr emails-list)))
 			  result))
 	    (when addr
 	      (format "ADR:;;%s\n" (replace-regexp-in-string "\\, ?" ";" addr)))
 	    (when tel (progn
-			(setq phones-list (split-string tel "[,;: ]+"))
+			(setq phones-list (org-contacts-split-property tel "[,; ]+"))
 			(setq result "")
 			(while phones-list
-			  (setq result (concat result  "TEL:" (car phones-list) "\n"))
+			  (setq result (concat result  "TEL:" (org-contacts-strip-link (car phones-list)) "\n"))
 			  (setq phones-list (cdr phones-list)))
 			result))
 	    (when bday
@@ -903,7 +904,61 @@ Requires google-maps-el."
     if addr
     collect (cons (list addr) (list :label (string-to-char (car contact)))))))
 
-(provide 'org-contacts)
+(defun org-contacts-strip-link (link)
+  "Remove brackets, description, link type and colon from an org link string and return the pure link key."
+   (let (startpos colonpos endpos)
+     (setq startpos (string-match (regexp-opt '("[[tel:" "[[mailto:")) link))
+     (if startpos
+         (progn
+            (setq colonpos (string-match ":" link))
+            (setq endpos (string-match "\\]" link))
+            (if endpos (substring link (1+ colonpos) endpos) link))
+         (progn
+            (setq startpos (string-match "mailto:" link))
+            (setq colonpos (string-match ":" link))
+            (if startpos (substring link (1+ colonpos)) link)))))
+
+(defun org-contacts-split-property (string &optional separators omit-nulls)
+  "Custom version of `split-string'.
+Split a property STRING into sub-strings bounded by matches
+for SEPARATORS but keep Org links intact.
+
+The beginning and end of STRING, and each match for SEPARATORS, are
+splitting points.  The substrings matching SEPARATORS are removed, and
+the substrings between the splitting points are collected as a list,
+which is returned.
+
+If SEPARATORS is non-nil, it should be a regular expression matching text
+which separates, but is not part of, the substrings.  If nil it defaults to
+`split-string-default-separators', normally \"[ \\f\\t\\n\\r\\v]+\", and
+OMIT-NULLS is forced to t.
+
+If OMIT-NULLS is t, zero-length substrings are omitted from the list \(so
+that for the default value of SEPARATORS leading and trailing whitespace
+are effectively trimmed).  If nil, all zero-length substrings are retained."
+(let* ((keep-nulls (or nil omit-nulls))
+         (rexp (or separators split-string-default-separators))
+         (inputlist (split-string string rexp keep-nulls))
+         (linkstring "")
+         (bufferstring "")
+         (proplist (list "")))
+    (while inputlist
+      (setq bufferstring (pop inputlist))
+      (if (string-match "\\[\\[" bufferstring)
+          (progn
+            (setq linkstring (concat bufferstring " "))
+            (while (not (string-match "\\]\\]" bufferstring))
+              (setq bufferstring (pop inputlist))
+              (setq linkstring (concat  linkstring bufferstring " ")))
+            (setq proplist (cons (chomp linkstring) proplist)))
+        (setq proplist (cons bufferstring proplist))))
+    (cdr (reverse proplist))))
+
+(defun chomp (str)
+  "Chomp leading and tailing whitespace from STR."
+  (while (string-match "\\`\n+\\|^\\s-+\\|\\s-+$\\|\n+\\'" str)
+    (setq str (replace-match "" t t str)))
+  str)
 
 (provide 'org-contacts)
 
-;; This program is distaributed in the hope that it will be useful,
+;; This program is distributed in the hope that it will be useful,
 ;; but WITHOUT ANY WARRANTY; without even the implied warranty of
 ;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 ;; GNU General Public License for more details.
-- 
1.8.2

Re: phone links...

[Daimrod]

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

Michael Strey <mstrey@strey.biz> writes:

Hello Michael,

> On Tue, Apr 09, 2013 at 09:31:40AM +0200, Michael Strey wrote:
>
> [...]
>
>> The problem is on the side of org-contacts.  Org-contacts does not
>> support links in its properties.
> [...]
>
>> This shortcoming effects not only the phone links but email links as
>> well.
>
> Attached is a patch to allow org links in org-contacts properties.
> It allows entries like in the following example without effecting
> org-contacts current functions.
>
> #+BEGIN_SRC org
> * Surname, Name
> :PROPERTIES:
> :EMAIL:    mailto:test2@test.de; [[mailto:name@test.de]] foo@bar.biz
> :PHONE:    [[tel:+49 351 4129535]], +491766626196 [[+49 (351) 41295-35]]
> :END:
> #+END_SRC
>
> Phone links of the form [[tel:+49 351 412 95-35][My phone number]] or
> [[tel:+49 351 41295-35]] are expected.  `-', `/', `(', `)' and
> whitespace characters are allowed in telephone numbers.

Thank you for your patch, though here are a few suggestions:
- It looks like `chomp' does the same thing the `org-trim' (in `org.el')
  if so you should use it.

- You should use `org-link-display-format' instead of
  `org-contacts-strip-link'.

- You have done some unrelated changes (fix some typos, ...), could you
  provide a separated patches for them?

Regarding `org-contacts-split-property', I haven't found anything about
multiple values within a node property in `org-element' and the syntax
description doesn't mention it, so you were right to roll your own. :)
However, I think it would be better to store the separators in a
variable (like `org-contacts-property-values-separator') and maybe even
to use it by default instead of `split-string-default-separators'
because we use it more and because it's easy to forget.

> +                (loop for email in (org-contacts-split-property email-list)
                                                                           ^^^^
> +                      for gravatar = (gravatar-retrieve-synchronously (org-contacts-strip-link email))

What do you think?


Thanks again for your time!

Regards,

-- 
Daimrod/Greg

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

Re: phone links...

[Michael Strey]

Hi Greg,

Thank you for looking into my debut feature!

On Wed, Apr 17, 2013 at 12:22:14AM +0200, Daimrod wrote:
> Michael Strey <mstrey@strey.biz> writes:

[...]

> > It allows entries like in the following example without effecting
> > org-contacts current functions.
> >
> > #+BEGIN_SRC org
> > * Surname, Name
> > :PROPERTIES:
> > :EMAIL:    mailto:test2@test.de; [[mailto:name@test.de]] foo@bar.biz
> > :PHONE:    [[tel:+49 351 4129535]], +491766626196 [[+49 (351) 41295-35]]
                                                        ^
should be
:PHONE:    [[tel:+49 351 4129535]], +491766626196 [[tel:+49 (351) 41295-35]]
> > :END:
> > #+END_SRC

[...]

> Thank you for your patch, though here are a few suggestions:
> - It looks like `chomp' does the same thing the `org-trim' (in `org.el')
>   if so you should use it.

Done.  Thanks for the hint.

> - You should use `org-link-display-format' instead of
>   `org-contacts-strip-link'.

I don't think so.  `org-link-display-format' returns the description of
the link if there is one.  My `org-contacts-strip-link' always returns
the target.  Using `org-link-display-format' would lead to wrong results
with links like
     [[mailto:foo@bar.com][foo (at) bar (dot) com]]
     [[tel:+49 351 4129535][+49 (0)351 4129535]]

> - You have done some unrelated changes (fix some typos, ...), could you
>   provide a separated patches for them?

Oh =:-|, another struggle with Git.  I'm still learning and will do my best.

> Regarding `org-contacts-split-property', I haven't found anything about
> multiple values within a node property in `org-element' and the syntax
> description doesn't mention it, so you were right to roll your own. :)

That wasn't me.  This (disputable) feature was already there for the
:EMAIL: proprerty.  Actually, I don't like multiple values within a node
property and would prefer a VCard-like syntax like

#+BEGIN_SRC org
*** Strey, Michael
:PROPERTIES:
:KIND:     individual
:ORG:      STREY Consult
:FN:       Michael Strey
:N:        Strey;Michael;;
:ADR;TYPE=home:;;my street;my city;federal state;post code;my country
:EMAIL:           mailto:strey@myprovider.de
:EMAIL;PREF=1:    mailto:me@mycompany.biz
:TEL;TYPE="fax,work":[[tel:0321 21104568]]
:TEL;TYPE="fax,home":[[tel:0351 4129535]]
:TEL;TYPE="voice,home":[[tel:0351 4129535]]
:LANG:     de
:ICON:     ~/GTD/Icons/icon-strey_michael.jpg
:END:
#+END_SRC

> However, I think it would be better to store the separators in a
> variable (like `org-contacts-property-values-separator') and maybe even
> to use it by default instead of `split-string-default-separators'
> because we use it more and because it's easy to forget.

That was already hard-coded before in Feng's
`org-contacts-vcard-format'.  But yes, you are right.  Since it is
limited by some constraints we should make it an extra variable.

> 
> > +                (loop for email in (org-contacts-split-property email-list)
>                                                                            ^^^^
> > +                      for gravatar = (gravatar-retrieve-synchronously (org-contacts-strip-link email))

This should be correctly and worked for me during my tests.
Confusingly `email-list' is not a list but a string here.

Regards
-- 
Michael Strey
http://www.strey.biz

Re: phone links...

[Daimrod]

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

Michael Strey <mstrey@strey.biz> writes:

>> - You should use `org-link-display-format' instead of
>>   `org-contacts-strip-link'.
>
> I don't think so.  `org-link-display-format' returns the description of
> the link if there is one.  My `org-contacts-strip-link' always returns
> the target.  Using `org-link-display-format' would lead to wrong results
> with links like
>      [[mailto:foo@bar.com][foo (at) bar (dot) com]]
>      [[tel:+49 351 4129535][+49 (0)351 4129535]]

You're right, I had misunderstood the function.

>> - You have done some unrelated changes (fix some typos, ...), could you
>>   provide a separated patches for them?
>
> Oh =:-|, another struggle with Git.  I'm still learning and will do my best.

If you're not already using magit[1] I highly recommend it to you. You
can use it to commit only a portion of the changes (you can also do it
with "git commit --interactive" but I find it easier with magit).

>> Regarding `org-contacts-split-property', I haven't found anything about
>> multiple values within a node property in `org-element' and the syntax
>> description doesn't mention it, so you were right to roll your own. :)
>
> That wasn't me.  This (disputable) feature was already there for the
> :EMAIL: proprerty.  Actually, I don't like multiple values within a node
> property and would prefer a VCard-like syntax like
>
> #+BEGIN_SRC org
> *** Strey, Michael
> :PROPERTIES:
> :KIND:     individual
> :ORG:      STREY Consult
> :FN:       Michael Strey
> :N:        Strey;Michael;;
> :ADR;TYPE=home:;;my street;my city;federal state;post code;my country
> :EMAIL:           mailto:strey@myprovider.de
> :EMAIL;PREF=1:    mailto:me@mycompany.biz
> :TEL;TYPE="fax,work":[[tel:0321 21104568]]
> :TEL;TYPE="fax,home":[[tel:0351 4129535]]
> :TEL;TYPE="voice,home":[[tel:0351 4129535]]
> :LANG:     de
> :ICON:     ~/GTD/Icons/icon-strey_michael.jpg
> :END:
> #+END_SRC

I must admit I am unsure about this. On one side it does solve some
problems, on the other side I don't really like the idea of adding this
extra syntax on top of properties.

I think it's fine to use semicolons to separate values in node
properties, but I would like to expose a clean API to manipulate users
and user's properties so that we don't have to use `*-split-*'
everywhere.

For example it could expose your previous example into this:
#+BEGIN_SRC lisp
  '((kind "individual")
    (org "STREY Consult")
    (fn "Michael Strey")
    (n ("Stry" "Michael" ""))
    (adr ("" "" "my street" "my city" "federal state" "post code" "my country") :type home)
    (email "mailto:strey@myprovider.de")
    (email "mailto:me@mycompany.biz" :pref 1)
    (tel "[[tel:0321 21104568]]" :type "fax,work")
    (tel "[[tel:0351 4129535]]" :type "fax,home")
    (tel "[[tel:0351 4129535]]" :type "voice,home")
    (lang "de")
    (icon "~/GTD/Icons/icon-strey_michael.jpg"))  
#+END_SRC

>> However, I think it would be better to store the separators in a
>> variable (like `org-contacts-property-values-separator') and maybe even
>> to use it by default instead of `split-string-default-separators'
>> because we use it more and because it's easy to forget.
>
> That was already hard-coded before in Feng's
> `org-contacts-vcard-format'.  But yes, you are right.  Since it is
> limited by some constraints we should make it an extra variable.
>
>> 
>> > +                (loop for email in (org-contacts-split-property email-list)
>>                                                                            ^^^^
>> > +                      for gravatar = (gravatar-retrieve-synchronously (org-contacts-strip-link email))
>
> This should be correctly and worked for me during my tests.
> Confusingly `email-list' is not a list but a string here.

My bad (again :/).

Regards,

1. https://github.com/magit/magit

-- 
Daimrod/Greg

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

Re: [Patch] phone links...

[Michael Strey]

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

Greg,

Here are the reworked patches.

0390: fixes two typos
0391: contains all the changes from my last patch and your corrections
0392: introduces a constant for the default value of separators

Best regards
Michael Strey

On Sat, Apr 20, 2013 at 05:59:39PM +0200, Daimrod wrote:
> Michael Strey <mstrey@strey.biz> writes:

> > Oh =:-|, another struggle with Git.  I'm still learning and will do my best.
> 
> If you're not already using magit[1] I highly recommend it to you. You
> can use it to commit only a portion of the changes (you can also do it
> with "git commit --interactive" but I find it easier with magit).

Thanks for the hint.  It helped a lot.

-- 
Michael Strey 
www.strey.biz

[-- Attachment #2: 0390-org-contacts-Fix-two-typos-in-comments.patch --]
[-- Type: text/plain, Size: 1593 bytes --]

From 2b59a2800de7066dde72121bc72ba75ea88f3c5a Mon Sep 17 00:00:00 2001
From: Michael Strey <mstrey@strey.biz>
Date: Fri, 26 Apr 2013 12:06:34 +0200
Subject: [PATCH 390/392] org-contacts: Fix two typos in comments

* contrib/lisp/org-contacts.el (org-contacts-db): fix typo in the
  word `matching'.
* contrib/lisp/org-contacts.el (org-contacts-complete-group): fix typo
  in the word `found'.

TINYCHANGE
---
 contrib/lisp/org-contacts.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/lisp/org-contacts.el b/contrib/lisp/org-contacts.el
index a3c4aed..626ad7a 100644
--- a/contrib/lisp/org-contacts.el
+++ b/contrib/lisp/org-contacts.el
@@ -225,7 +225,7 @@ This overrides `org-email-link-description-format' if set."
     org-contacts-db))
 
 (defun org-contacts-filter (&optional name-match tags-match)
-  "Search for a contact maching NAME-MATCH and TAGS-MATCH.
+  "Search for a contact matching NAME-MATCH and TAGS-MATCH.
 If both match values are nil, return all contacts."
   (if (and (null name-match)
 	   (null tags-match))
@@ -433,7 +433,7 @@ A group FOO is composed of contacts with the tag FOO."
 				    (or (cdr (assoc-string "ALLTAGS" (caddr contact))) "") ":")))))))
 	(list start end
 	      (if (= (length completion-list) 1)
-		  ;; We've foudn the correct group, returns the address
+		  ;; We've found the correct group, returns the address
 		  (lexical-let ((tag (get-text-property 0 'org-contacts-group
 							(car completion-list))))
 		    (lambda (string pred &optional to-ignore)
-- 
1.8.2


[-- Attachment #3: 0391-Org-contacts-Allow-org-links-in-properties.patch --]
[-- Type: text/plain, Size: 8610 bytes --]

From 68145d055b7a48821d9aec308cd44764a5b889ec Mon Sep 17 00:00:00 2001
From: Michael Strey <mstrey@strey.biz>
Date: Fri, 26 Apr 2013 12:29:55 +0200
Subject: [PATCH 391/392] Org-contacts: Allow org links in properties

* contrib/lisp/org-contacts.el (org-contacts-split-property):
  Introduce a custom version of split-string that keeps org links
  intact.

* contrib/lisp/org-contacts.el (org-contacts-strip-link): Introduce a
  new function that removes brackets, description, link type and colon
  from an org link string returning the pure link target.

* contrib/lisp/org-contacts.el (provide 'org-contacts): Remove a
  redundant line.

* contrib/lisp/org-contacts.el (org-contacts-complete-group,
  org-contacts-complete-name, org-contacts-view-send-email,
  org-contacts-get-icon, org-contacts-vcard-format): Apply the new
  functions to the already existing functions extracting telephone
  numbers and email addresses from the properties.

Allowed separators for email addresses and phone numbers are `,', `;'
and whitespace.  `:' is not allowed anymore as separator to avoid
confusion with implizit links.

Examples of properties that are possible after those changes:

* Surname, Name
:PROPERTIES:
:EMAIL:    mailto:test2@test.de; [[mailto:name@test.de]] foo@bar.biz
:PHONE:    [[tel:+49 351 4129535]], +491766626196 [[+49 (351) 41295-35]]
:END:

Phone links of the form [[tel:+49 351 412 95-35][My phone number]] or
[[tel:+49 351 41295-35]] are expected.  `-', `/', `(', `)' and
whitespace characters are allowed in telephone numbers.
---
 contrib/lisp/org-contacts.el | 73 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/contrib/lisp/org-contacts.el b/contrib/lisp/org-contacts.el
index 626ad7a..eb3fc09 100644
--- a/contrib/lisp/org-contacts.el
+++ b/contrib/lisp/org-contacts.el
@@ -445,11 +445,11 @@ A group FOO is composed of contacts with the tag FOO."
 				       ;; returned by `org-contacts-filter'.
 				       for contact-name = (car contact)
 				       ;; Grab the first email of the contact
-				       for email = (car (split-string
+				       for email = (org-contacts-strip-link (car (org-contacts-split-property
 							 (or
 							  (cdr (assoc-string org-contacts-email-property
 									     (caddr contact)))
-							  "")))
+							  ""))))
 				       ;; If the user has an email address, append USER <EMAIL>.
 				       if email collect (org-contacts-format-email contact-name email))
 				 ", ")))
@@ -466,14 +466,14 @@ A group FOO is composed of contacts with the tag FOO."
 		;; returned by `org-contacts-filter'.
 		for contact-name = (car contact)
 		;; Build the list of the user email addresses.
-		for email-list = (split-string (or
+		for email-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>.
 		nconc (loop for email in email-list
-			    collect (org-contacts-format-email contact-name email))))
+			    collect (org-contacts-format-email contact-name (org-contacts-strip-link email)))))
 	 (completion-list (org-contacts-all-completions-prefix
 			   string
 			   (org-uniquify completion-list))))
@@ -738,11 +738,12 @@ address."
     (org-with-point-at marker
       (let ((emails (org-entry-get (point) org-contacts-email-property)))
         (if emails
-            (let ((email-list (split-string emails)))
+            (let ((email-list (org-contacts-split-property emails)))
               (if (and (= (length email-list) 1) (not ask))
                   (compose-mail (org-contacts-format-email
                                  (org-get-heading t) emails))
                 (let ((email (completing-read "Send mail to which address: " email-list)))
+		  (setq email (org-contacts-strip-link email))
                   (org-contacts-check-mail-address email)
                   (compose-mail (org-contacts-format-email (org-get-heading t) email)))))
           (error (format "This contact has no mail address set (no %s property)."
@@ -766,8 +767,8 @@ address."
              (email-list (org-entry-get pom org-contacts-email-property))
              (gravatar
               (when email-list
-                (loop for email in (split-string email-list)
-                      for gravatar = (gravatar-retrieve-synchronously email)
+                (loop for email in (org-contacts-split-property email-list)
+                      for gravatar = (gravatar-retrieve-synchronously (org-contacts-strip-link email))
                       if (and gravatar
                               (not (eq gravatar 'error)))
                       return gravatar))))
@@ -849,19 +850,19 @@ to do our best."
 	 (head (format "BEGIN:VCARD\nVERSION:3.0\nN:%s\nFN:%s\n" n name)))
     (concat head
 	    (when email (progn
-			  (setq emails-list (split-string email "[,;: ]+"))
+			  (setq emails-list (org-contacts-split-property email "[,; ]+"))
 			  (setq result "")
 			  (while emails-list
-			    (setq result (concat result  "EMAIL:" (car emails-list) "\n"))
+			    (setq result (concat result  "EMAIL:" (org-contacts-strip-link (car emails-list)) "\n"))
 			    (setq emails-list (cdr emails-list)))
 			  result))
 	    (when addr
 	      (format "ADR:;;%s\n" (replace-regexp-in-string "\\, ?" ";" addr)))
 	    (when tel (progn
-			(setq phones-list (split-string tel "[,;: ]+"))
+			(setq phones-list (org-contacts-split-property tel "[,; ]+"))
 			(setq result "")
 			(while phones-list
-			  (setq result (concat result  "TEL:" (car phones-list) "\n"))
+			  (setq result (concat result  "TEL:" (org-contacts-strip-link (car phones-list)) "\n"))
 			  (setq phones-list (cdr phones-list)))
 			result))
 	    (when bday
@@ -910,7 +911,55 @@ Requires google-maps-el."
     if addr
     collect (cons (list addr) (list :label (string-to-char (car contact)))))))
 
-(provide 'org-contacts)
+(defun org-contacts-strip-link (link)
+  "Remove brackets, description, link type and colon from an org link string and return the pure link target."
+   (let (startpos colonpos endpos)
+     (setq startpos (string-match (regexp-opt '("[[tel:" "[[mailto:")) link))
+     (if startpos
+         (progn
+            (setq colonpos (string-match ":" link))
+            (setq endpos (string-match "\\]" link))
+            (if endpos (substring link (1+ colonpos) endpos) link))
+         (progn
+            (setq startpos (string-match "mailto:" link))
+            (setq colonpos (string-match ":" link))
+            (if startpos (substring link (1+ colonpos)) link)))))
+
+(defun org-contacts-split-property (string &optional separators omit-nulls)
+  "Custom version of `split-string'.
+Split a property STRING into sub-strings bounded by matches
+for SEPARATORS but keep Org links intact.
+
+The beginning and end of STRING, and each match for SEPARATORS, are
+splitting points.  The substrings matching SEPARATORS are removed, and
+the substrings between the splitting points are collected as a list,
+which is returned.
+
+If SEPARATORS is non-nil, it should be a regular expression matching text
+which separates, but is not part of, the substrings.  If nil it defaults to
+`split-string-default-separators', normally \"[ \\f\\t\\n\\r\\v]+\", and
+OMIT-NULLS is forced to t.
+
+If OMIT-NULLS is t, zero-length substrings are omitted from the list \(so
+that for the default value of SEPARATORS leading and trailing whitespace
+are effectively trimmed).  If nil, all zero-length substrings are retained."
+(let* ((keep-nulls (or nil omit-nulls))
+         (rexp (or separators split-string-default-separators))
+         (inputlist (split-string string rexp keep-nulls))
+         (linkstring "")
+         (bufferstring "")
+         (proplist (list "")))
+    (while inputlist
+      (setq bufferstring (pop inputlist))
+      (if (string-match "\\[\\[" bufferstring)
+          (progn
+            (setq linkstring (concat bufferstring " "))
+            (while (not (string-match "\\]\\]" bufferstring))
+              (setq bufferstring (pop inputlist))
+              (setq linkstring (concat  linkstring bufferstring " ")))
+            (setq proplist (cons (org-trim linkstring) proplist)))
+        (setq proplist (cons bufferstring proplist))))
+    (cdr (reverse proplist))))
 
 (provide 'org-contacts)
 
-- 
1.8.2


[-- Attachment #4: 0392-Org-contacts-Introduce-the-constant-org-contacts-pro.patch --]
[-- Type: text/plain, Size: 4322 bytes --]

From a4018be0b34c3c40ad6b43f2380e0fd52a171591 Mon Sep 17 00:00:00 2001
From: Michael Strey <mstrey@strey.biz>
Date: Fri, 26 Apr 2013 13:57:46 +0200
Subject: [PATCH 392/392] Org-contacts: Introduce the constant
 `org-contacts-property-values-separators'

* contrib/lisp/org-contacts.el (org-contacts-property-values-separators):
  Definition of a new constant that will be used as default value of
  separators for `org-contacts-split-property'.

* contrib/lisp/org-contacts.el (org-contacts-vcard-format): We are now
  using the default separator in general.

* contrib/lisp/org-contacts.el (org-contacts-show-map): Line break
  corrected.

* contrib/lisp/org-contacts.el (org-contacts-split-property): Correct
  the comment describing the function.  Application of the newly
  defined constant `org-contacts-property-values-separators'.
---
 contrib/lisp/org-contacts.el | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/contrib/lisp/org-contacts.el b/contrib/lisp/org-contacts.el
index eb3fc09..84ebc9d 100644
--- a/contrib/lisp/org-contacts.el
+++ b/contrib/lisp/org-contacts.el
@@ -174,6 +174,11 @@ This overrides `org-email-link-description-format' if set."
 (declare-function std11-narrow-to-header "ext:std11")
 (declare-function std11-fetch-field "ext:std11")
 
+(defconst org-contacts-property-values-separators "[,; \f\t\n\r\v]+"
+  "The default value of separators for `org-contacts-split-property'.
+
+A regexp matching strings of whitespace, `,' and `;'.")
+
 (defvar org-contacts-keymap
   (let ((map (make-sparse-keymap)))
     (define-key map "M" 'org-contacts-view-send-email)
@@ -850,7 +855,7 @@ to do our best."
 	 (head (format "BEGIN:VCARD\nVERSION:3.0\nN:%s\nFN:%s\n" n name)))
     (concat head
 	    (when email (progn
-			  (setq emails-list (org-contacts-split-property email "[,; ]+"))
+			  (setq emails-list (org-contacts-split-property email))
 			  (setq result "")
 			  (while emails-list
 			    (setq result (concat result  "EMAIL:" (org-contacts-strip-link (car emails-list)) "\n"))
@@ -859,7 +864,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-split-property tel))
 			(setq result "")
 			(while phones-list
 			  (setq result (concat result  "TEL:" (org-contacts-strip-link (car phones-list)) "\n"))
@@ -912,7 +917,8 @@ Requires google-maps-el."
     collect (cons (list addr) (list :label (string-to-char (car contact)))))))
 
 (defun org-contacts-strip-link (link)
-  "Remove brackets, description, link type and colon from an org link string and return the pure link target."
+  "Remove brackets, description, link type and colon from an org
+link string and return the pure link target."
    (let (startpos colonpos endpos)
      (setq startpos (string-match (regexp-opt '("[[tel:" "[[mailto:")) link))
      (if startpos
@@ -935,16 +941,16 @@ splitting points.  The substrings matching SEPARATORS are removed, and
 the substrings between the splitting points are collected as a list,
 which is returned.
 
-If SEPARATORS is non-nil, it should be a regular expression matching text
-which separates, but is not part of, the substrings.  If nil it defaults to
-`split-string-default-separators', normally \"[ \\f\\t\\n\\r\\v]+\", and
-OMIT-NULLS is forced to t.
+If SEPARATORS is non-nil, it should be a regular expression
+matching text which separates, but is not part of, the
+substrings.  If nil it defaults to `org-contacts-property-values-separators',
+normally \"[,; \f\t\n\r\v]+\", and OMIT-NULLS is forced to t.
 
 If OMIT-NULLS is t, zero-length substrings are omitted from the list \(so
 that for the default value of SEPARATORS leading and trailing whitespace
 are effectively trimmed).  If nil, all zero-length substrings are retained."
 (let* ((keep-nulls (or nil omit-nulls))
-         (rexp (or separators split-string-default-separators))
+         (rexp (or separators org-contacts-property-values-separators))
          (inputlist (split-string string rexp keep-nulls))
          (linkstring "")
          (bufferstring "")
-- 
1.8.2

Re: [Patch] phone links...

[Daimrod]

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

Michael Strey <mstrey@strey.biz> writes:

> Greg,
>
> Here are the reworked patches.
>
> 0390: fixes two typos
> 0391: contains all the changes from my last patch and your corrections
> 0392: introduces a constant for the default value of separators

Excellent.

> On Sat, Apr 20, 2013 at 05:59:39PM +0200, Daimrod wrote:
>> Michael Strey <mstrey@strey.biz> writes:
>
>> > Oh =:-|, another struggle with Git.  I'm still learning and will do my best.
>> 
>> If you're not already using magit[1] I highly recommend it to you. You
>> can use it to commit only a portion of the changes (you can also do it
>> with "git commit --interactive" but I find it easier with magit).
>
> Thanks for the hint.  It helped a lot.

Merged & pulled. Thanks!

-- 
Daimrod/Greg

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

Re: [Patch] phone links...

[Daimrod]

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

Michael Strey <mstrey@strey.biz> writes:

Hello Michael,

While merging a patch (from Feng Shu) I have found a bug in
`org-contacts-split-property'. Though the docstring says that OMIT-NULLS
is forced to t when SEPARATORS is nil (just like `split-string'), it
wasn't the case. I've pushed a fix; could you check on your side that it
doesn't break anything in your workflow?

Thanks,

-- 
Daimrod/Greg

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