Allow importing KML with empty geometries. #510

[paul121]

A simple fix for the bug identified in #510

Attaching a screenshot that shows the result of parsing the following placemark:

<Placemark>
	<name>Placemark name</name>
      <MultiGeometry></MultiGeometry>
  </Placemark>

[paul121]

Force push to add fix to README.md

[mstenta]

I wonder if it makes sense to add use GeometryCollection; to the top of the file and change this to:

$geometry = new GeometryCollection();

Same effect, but feels like it would be more consistent to add a use statement in this case, even though phayes/GeometryCollection doesn't use namespaces.

I wonder if there's any benefit to consistency in this regard, like IDE integration, code sniffing, etc...

Curious your thoughts on this (very minor) thought @paul121 - doesn't need to be a blocker for merging this, I don't think, but worth considering?

[paul121]

Oh interesting, yeah I don't see any problem with that, as long as it works! Normally PHPStorm would add that automatically for me but it's possible I typed it in differently without using any type helpers.

[mstenta]

Cool! I can make that change and amend your commit.

[mstenta]

Pushed that change - do you want to give it one more test @paul121? I didn't test it, but PHPStorm recognizes GeometryCollection, so I assume it works.

[paul121]

Great, tested & works great!

[paul121]

I wonder if it makes sense to add use GeometryCollection; to the top of the file and change this to:

Hey @mstenta this change actually created a code sniffer violation:

see this failing action

FILE: .../profiles/farm/modules/core/kml/src/Normalizer/KmlNormalizer.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 7 | ERROR | [x] Non-namespaced classes/interfaces/traits should not
   |       |     be referenced with use statements
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 6.38 secs; Memory: 26MB

[mstenta]

Of course it did. 🙄

Should have left well enough alone...

[mstenta]

Pushed a commit that changes this back to the way you had it originally @paul121 👍