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

Skip empty track segments in GPX #40

Merged
merged 1 commit into from
Sep 14, 2019
Merged

Skip empty track segments in GPX #40

merged 1 commit into from
Sep 14, 2019

Conversation

briedis
Copy link

@briedis briedis commented Sep 14, 2019

Endomondo sometimes export empty track segments. This will handle cases when empty segments may exist.

@Sibyx
Copy link
Owner

Sibyx commented Sep 14, 2019

Hello, thank you very much for your PR. Can you please add also a configuration option for this feature? You can still skip empty segments by default, but we should add possibility to bypass this behavior.

@briedis
Copy link
Author

briedis commented Sep 14, 2019

Hmm, are you sure? Because if a segment is empty, it fails with a fatal error when you try to access time on a null node.

This is the error:

1) phpGPX\Tests\LoadRouteFileTest::testRouteFileWithSmoothedStats
Trying to get property 'time' of non-object

/Users/briedis/web/phpGPX/src/phpGPX/Models/Track.php:118
/Users/briedis/web/phpGPX/src/phpGPX/Parsers/TrackParser.php:93
/Users/briedis/web/phpGPX/src/phpGPX/phpGPX.php:128
/Users/briedis/web/phpGPX/src/phpGPX/phpGPX.php:104
/Users/briedis/web/phpGPX/tests/LoadRouteFileTest.php:34

@Sibyx
Copy link
Owner

Sibyx commented Sep 14, 2019

Okay than, thank you very much for contribution!

@Sibyx Sibyx merged commit 1657fcb into Sibyx:master Sep 14, 2019
@briedis
Copy link
Author

briedis commented Sep 14, 2019

Would you mind if I auto-format the source to PSR1/2 standard, and rename the class to StudlyCase ? (first letter uppercase). In a separate PR, of course.

@Sibyx
Copy link
Owner

Sibyx commented Sep 14, 2019

Sure, go for it ;) Also, run the php-cs-fixer there was an issue in your last PR

@Sibyx Sibyx mentioned this pull request Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants