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

Parsing nested extensions #4

Merged

Conversation

matteomelotti
Copy link
Contributor

Add new extensions type, manage parsing nested extensions, updated gpx file example

@We-Gold
Copy link
Owner

We-Gold commented Apr 20, 2024

I like the approach you have taken here, this should improve the behavior of the library. There is a minor bug in the current implementation.

The section below is in the wrong order in the ternary operator.

const value = isNaN(+textContent) ? parseFloat(textContent) : textContent

It should be:

const value = isNaN(+textContent) ? textContent : parseFloat(textContent)

The issue with the current segment is that it attempts to parse the textContent as a float if and only if the textContent is NaN, which is the opposite of the intended behavior. I tried reversing it and got the intended behavior.

I manually tested the code and it seems to be working outside of that issue. I hope to add a better testing system to the library in the future, but for now that will have to do. Anyway, once you make that fix I'll be happy to merge in the change.

@matteomelotti
Copy link
Contributor Author

Yes I confirm the issue, thanks.
I was using a different function isNumber so I forgot to switch

@We-Gold
Copy link
Owner

We-Gold commented Apr 21, 2024

Great. Once you make the fix, I'd be happy to merge it in.

@matteomelotti
Copy link
Contributor Author

@We-Gold pushed the fix, thanks!

@We-Gold We-Gold merged commit c03a18a into We-Gold:main Apr 21, 2024
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