alexrothenberg / motion-addressbook

MIT License
89 stars 30 forks source link

full support for all the multi-valued elements #20

Closed jmay closed 11 years ago

jmay commented 11 years ago

Supports all the remaining multi-valued elements: URLs, Social Profiles, IM Profiles.

I converted the support for email addresses and phone numbers to use the same MultiValued class as these others. This means that I had to remove the unitary "email" and "phone_number" methods, you much fetch a list of email_values or phone_values.

And I've broken the ability to assign values for individual email/phone entries like @ab_person.mobile_phone = xxxx. You have to assign the entire list of values with :label parameters for each.

Search on email address does work - it will compare against all the email addresses on each Person record. But search on phone, URL etc. are not currently functional.

Not really satisfied with this, but I haven't come up with a better idea that can handle all the possible stuff that can accumulate in AB records.

alexrothenberg commented 11 years ago

At a glance it looks pretty impressive, but it'll take a few days before I can take a look at this more carefully.

I'm just wondering about the unary email and phone_number. I was depending on those and am not sure if anyone else was. I'd prefer to see if we can keep them at least for now and if we do get rid of them do it in a major version bump to follow semantic versioning.

jmay commented 11 years ago

Perhaps the unary methods should just return the first entry? It's always possible that the OS will reorder multiple values, but being able to get something useful back is probably a good idea. Same for address, url etc.

On 19 Mar, 2013, at 10:30 AM, Alex Rothenberg notifications@github.com wrote:

At a glance it looks pretty impressive, but it'll take a few days before I can take a look at this more carefully.

I'm just wondering about the unary email and phone_number. I was depending on those and am not sure if anyone else was. I'd prefer to see if we can keep them at least for now and if we do get rid of them do it in a major version bump to follow semantic versioning.

— Reply to this email directly or view it on GitHub.

alexrothenberg commented 11 years ago

Another great improvement! Thanks @jmay for taking the time to keep making this better.