Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for properties with multiple values #5

Open
emersion opened this issue Mar 27, 2017 · 9 comments · May be fixed by #35
Open

Add support for properties with multiple values #5

emersion opened this issue Mar 27, 2017 · 9 comments · May be fixed by #35

Comments

@emersion
Copy link
Owner

e.g. CATEGORIES

@barisere
Copy link
Contributor

barisere commented Oct 1, 2017

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
Copy link
Owner Author

emersion commented Oct 1, 2017

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
Copy link
Contributor

barisere commented Oct 2, 2017

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
Copy link
Contributor

barisere commented Oct 5, 2017

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
Copy link
Owner Author

emersion commented Oct 5, 2017

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
Copy link
Contributor

barisere commented Oct 5, 2017

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
Copy link
Owner Author

emersion commented Oct 6, 2017

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
Copy link

akshaychhajed commented Oct 7, 2017

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

GorthMohogany added a commit to GorthMohogany/go-vcard that referenced this issue Oct 1, 2018
AlmogBaku added a commit to AlmogBaku/go-vcard that referenced this issue Apr 2, 2020
@oliverpool
Copy link
Contributor

#35 is an attempt to address this (quite old) issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants