version is mandatory at vCard 4.0 #128

helveticadomes · 2018-06-20T06:46:23Z

version is mandatory and at vCard 4.0 and it has to be on the line direct after the Begin Property

… after the Begin Property

jeroendesloovere

We have a Version Property/Parameter.
Maby you can find a way:

to make it "required"?
to place it as the first property?

helveticadomes · 2018-06-20T06:54:58Z

The vCard need to start with:

BEGIN:VCARD
VERSION:4.0  
here comes the first fields

now the vCard begins with

BEGIN:VCARD  
here comes the first fields

Defined by the Specifications:
https://tools.ietf.org/html/rfc6350#section-6.7.9

Special notes: This property MUST be present in the vCard object,
and it must appear immediately after BEGIN:VCARD. The value MUST
be "4.0" if the vCard corresponds to this specification. Note
that earlier versions of vCard allowed this property to be placed
anywhere in the vCard object, or even to be absent.

jeroendesloovere · 2018-06-20T12:58:27Z

@helveticadomes, I do understand what the purpose is.

I'm just not sure if we should go "your solution" or "need to use the already available"

… after the Begin Property Use the Version Property instead of static

helveticadomes · 2018-06-21T09:32:02Z

As Requested i update my pullrequest:
it does now use the Version Formatter/Parser instead of the static Version.
It also make it mandatory and move the Version Property to the first line after begin.
after a few trys i also be able to pass the Travis checks

merge pr to be uptodate

2.0.0 dev

…failed Travis checks)

jeroendesloovere · 2018-06-25T07:54:31Z

src/VCard.php

    }

    public function getParameters(string $filterByPropertyParameterClass = null): array
    {
        if ($filterByPropertyParameterClass === null) {
+
+            $array = $this->parameters;


I would replace your entire "additions" here by an array_map().
In that new function you could check "return ($class instanceof Version) ? -1 : +1".
Using such an easy way we move our "Version" to the frond of the array by using comparisons.

Another (but bigger) addition to this solution could be to add a $priority value to each and every Property.

I am not sure. with help of a similar addition i can solve #113 => https://github.com/helveticadomes/vcard/blob/feature-fix-name-property/src/VCard.php
Lines 167-190
Pullrequest not quite finished yet

jeroendesloovere · 2018-06-25T07:54:58Z

src/Formatter/VcfFormatter.php

@@ -12,6 +12,7 @@
    public const CONTENT_TYPE = 'text/vcard';
    public const FILE_EXTENSION = 'vcf';
    public const VCARD_BEGIN = 'BEGIN:VCARD';
+    //public const VCARD_VERSION = 'VERSION:4.0';


This line can be removed

jeroendesloovere · 2018-06-25T07:55:03Z

src/Formatter/VcfFormatter.php

@@ -21,6 +22,7 @@ public function getContent(array $vCards): string
        /** @var VCard $vCard */
        foreach ($vCards as $vCard) {
            $string .= self::VCARD_BEGIN . "\r\n";
+            //$string .= self::VCARD_VERSION . "\r\n";


This line can be removed

jeroendesloovere · 2018-06-25T07:55:57Z

src/VCard.php

@@ -130,12 +132,24 @@ private function addPropertyParameter(PropertyParameterInterface $propertyParame

    public function getKind(): Kind
    {
-        return $this->getParameters(Kind::class)[0];
+        $kind = $this->getParameters(Kind::class);
+        return reset($kind);


Please change to one line

No => that would lead to:

Notice: Only variables should be passed by reference in ..\vendor\jeroendesloovere\vcard\src\VCard.php on line 136

at least if it is done like that:

return reset($this->getParameters(Kind::class));

Ok, probably return reset($kind = $this->getParameters(Kind::class)); will work then.

But I suggest you leave it like that. Just "add an empty rule above the return statement".

unfortunately your suggested code doesn't work either
why empty rule? because of $this->add($kind ?? Kind::individual()); inside constructor it could not be empty

It is a good practice to add an empty line above a return statement like this:

$kind = $this->getParameters(Kind::class); return reset($kind);

and not like this:

$kind = $this->getParameters(Kind::class); return reset($kind);

… commented lines

melroy89 · 2018-12-27T20:34:49Z

Please, merge this. The version number is required in v4:
https://tools.ietf.org/html/rfc6350#page-48

I think helveticadomes did wonderful work.

melroy89 · 2019-02-27T16:37:36Z

Special notes:  This property MUST be present in the vCard object,
      and it must appear immediately after BEGIN:VCARD.  The value MUST
      be "4.0" if the vCard corresponds to this specification.

melroy89 · 2019-03-02T13:40:27Z

@jeroendesloovere Note: Since I'm validating the actual context. This merge should lead to a UT failures now. Which is good! Since I wrote a good unit testers that ACTUALLY checks the content as well 👍

Nevertheless, this does require some change in the UT to fix it again, see:
https://travis-ci.org/jeroendesloovere/vcard/jobs/500787051

Like this line:
https://github.com/jeroendesloovere/vcard/blob/2.0.0-dev/tests/VCardTest.php#L249
And line:
https://github.com/jeroendesloovere/vcard/blob/2.0.0-dev/tests/VCardTest.php#L271

Likely needs to be extended with this version 4.0 :)

melroy89 · 2019-03-02T16:42:06Z

@jeroendesloovere I will fix the failing test! However, I found another bug in the version parser (or lack of parsing of the version I would say). I will try to fix that as well.

version is mandatory and at vCard 4.0 it has to be on the line direct…

f460a9c

… after the Begin Property

jeroendesloovere requested changes Jun 20, 2018

View reviewed changes

helveticadomes changed the title ~~version is mandatory and at vCard 4.0~~ version is mandatory at vCard 4.0 Jun 20, 2018

version is mandatory and at vCard 4.0 it has to be on the line direct…

0d01280

… after the Begin Property Use the Version Property instead of static

helveticadomes and others added 3 commits June 21, 2018 13:13

Merge pull request #1 from jeroendesloovere/2.0.0-dev

0b95823

merge pr to be uptodate

Merge pull request #2 from helveticadomes/2.0.0-dev

ee5c5d6

2.0.0 dev

Remove Blank Lines, and move use Role to its alphanum order (may fix …

39b758d

…failed Travis checks)

jeroendesloovere requested changes Jun 25, 2018

View reviewed changes

Do Requested changes: add blank line above return and remove abadoned…

76f766c

… commented lines

melroy89 approved these changes Mar 1, 2019

View reviewed changes

jeroendesloovere approved these changes Mar 2, 2019

View reviewed changes

jeroendesloovere merged commit 6346d51 into jeroendesloovere:2.0.0-dev Mar 2, 2019

helveticadomes mentioned this pull request Mar 4, 2019

vCard 4.0: fix Name Property #146

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

version is mandatory at vCard 4.0 #128

version is mandatory at vCard 4.0 #128

helveticadomes commented Jun 20, 2018

jeroendesloovere left a comment

helveticadomes commented Jun 20, 2018 •

edited

Loading

jeroendesloovere commented Jun 20, 2018 •

edited

Loading

helveticadomes commented Jun 21, 2018 •

edited

Loading

jeroendesloovere Jun 25, 2018

helveticadomes Jun 25, 2018 •

edited

Loading

jeroendesloovere Jun 25, 2018

jeroendesloovere Jun 25, 2018

jeroendesloovere Jun 25, 2018

helveticadomes Jun 25, 2018 •

edited

Loading

jeroendesloovere Jun 25, 2018

helveticadomes Jun 25, 2018 •

edited

Loading

jeroendesloovere Jun 26, 2018 •

edited

Loading

melroy89 commented Dec 27, 2018

melroy89 commented Feb 27, 2019

melroy89 commented Mar 2, 2019 •

edited

Loading

melroy89 commented Mar 2, 2019

version is mandatory at vCard 4.0 #128

version is mandatory at vCard 4.0 #128

Conversation

helveticadomes commented Jun 20, 2018

jeroendesloovere left a comment

Choose a reason for hiding this comment

helveticadomes commented Jun 20, 2018 • edited Loading

jeroendesloovere commented Jun 20, 2018 • edited Loading

helveticadomes commented Jun 21, 2018 • edited Loading

jeroendesloovere Jun 25, 2018

Choose a reason for hiding this comment

helveticadomes Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

jeroendesloovere Jun 25, 2018

Choose a reason for hiding this comment

jeroendesloovere Jun 25, 2018

Choose a reason for hiding this comment

jeroendesloovere Jun 25, 2018

Choose a reason for hiding this comment

helveticadomes Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

jeroendesloovere Jun 25, 2018

Choose a reason for hiding this comment

helveticadomes Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

jeroendesloovere Jun 26, 2018 • edited Loading

Choose a reason for hiding this comment

melroy89 commented Dec 27, 2018

melroy89 commented Feb 27, 2019

melroy89 commented Mar 2, 2019 • edited Loading

melroy89 commented Mar 2, 2019

helveticadomes commented Jun 20, 2018 •

edited

Loading

jeroendesloovere commented Jun 20, 2018 •

edited

Loading

helveticadomes commented Jun 21, 2018 •

edited

Loading

helveticadomes Jun 25, 2018 •

edited

Loading

helveticadomes Jun 25, 2018 •

edited

Loading

helveticadomes Jun 25, 2018 •

edited

Loading

jeroendesloovere Jun 26, 2018 •

edited

Loading

melroy89 commented Mar 2, 2019 •

edited

Loading