search

emersion / go-vcard

A Go library to parse and format vCard
MIT License
107 stars 31 forks source link

Add support for properties with multiple values #5

Open emersion opened 7 years ago

emersion commented 7 years ago

e.g. CATEGORIES

barisere commented 6 years ago

I'd like to have a go at this. Hopefully I'm up to it. Also, you can add a Hacktoberfest tag to this issue so it shows up in searches for issues with the tag. More info at https://hacktoberfest.digitalocean.com/

emersion commented 6 years ago

Ahah, added the label. :)

This is a bit tricky since some properties might have multiple values, some won't. Also, it's more difficult than just creating one Field per value, since parsing a card and then formatting it wouldn't produce the same card as the original one:

CATEGORIES: cat 1, cat 2

vs

CATEGORIES: cat 1
CATEGORIES: cat 2

I'm not sure if that matters that much. Maybe reading the relevant parts of the RFC could help to decide.

Also, escaping must be done properly.

barisere commented 6 years ago

So tricky. That RFC is the most boring thing I've seen so far. I'll try. Maybe there might be insight in other vCard libraries.

barisere commented 6 years ago

It seems this feature is already present in this library. According to RFC we have to escape comma-separated values in such properties that have multiple values, and in fact, in all properties.

3.4. Property Value Escaping

Some properties may contain one or more values delimited by a COMMA character (U+002C). Therefore, a COMMA character in a value MUST be escaped with a BACKSLASH character (U+005C), even for properties that don’t allow multiple instances (for consistency).

I tried encoding such a field and it resulted in a backslash-escaped commas.

CATEGORIES: cat 1\, cat 2

But decoding it worked. Importing the vCard in Evolution also worked. Then used the ExampleNewDecoder to decode the card, replacing the preferred value thus

- log.Println(card.PreferredValue(vcard.FieldFormattedName))
+ log.Println(card.PreferredValue(vcard.FieldCategories))

The output was

CATEGORIES: cat 1, cat 2

Have I misunderstood the escaping rule?

emersion commented 6 years ago

Yeah, this is done by https://github.com/emersion/go-vcard/blob/master/encoder.go#L74 and https://github.com/emersion/go-vcard/blob/master/decoder.go#L224. But this issue is not about fields with a single value containing a , character, but rather about fields with multiple values (separated by non-escaped ,).

barisere commented 6 years ago

Does this imply that the comma in CATEGORIES: cat 1, cat 2 should not be escaped, since it serves as a separator for the field's values? Would that mean such fields as CATEGORIES and NICKNAME are special cases with their own escaping rules?

emersion commented 6 years ago

No. It means that for now, users cannot put multiple values in one field. They have to create two fields:

CATEGORIES: cat 1
CATEGORIES: cat 2

Also, as the RFC says, all commas must be escaped in field values (CATEGORIES and NICKNAME are not special cases).

akshaychhajed commented 6 years ago

I think we are interpreting incorrectly.

CATEGORIES: cat1, cat2

Some properties may contain one or more values delimited by a COMMA character (U+002C). Therefore, a COMMA character in a value MUST be escaped with a BACKSLASH character (U+005C), even for properties that don’t allow multiple instances (for consistency).

This statement talks about value in multiple value context.

i.e it should escape "," if value is "cat1, cat2" (single value with "," within) It should not escape if values are "cat1" and "cat2". (multiple values where "," acts as seperator)

The comment talks about escaping "," within values and not when it acts as separator. I think @latentgenius is correct in assuming NICKNAME and CATEGORIES are special cases due to their text-list type.

Encoded CATEGORIES: cat 1\, cat 2 represent single value "cat1, cat2" Encoded CATEGORIES: cat 1, cat 2 represents multiple values