fix: avoid panics in ParseNode() #21
Conversation
ParseNode(node, nil) should be able to avoid panic even if node does not contain a valid absolute base path in its <head>, since there can be usable microformats with absolute paths or no paths. ParseNode(nil, base) should return nil instead of panic, because why panic?
microformats.go
Outdated
| @@ -118,13 +118,19 @@ func Parse(r io.Reader, baseURL *url.URL) *Data { | |||
| // ParseNode parses the microformats found in doc. baseURL is the URL this | |||
| // document was retrieved from and is used to resolve any relative URLs. | |||
There was a problem hiding this comment.
I think it's worth updating the godoc to clarify the behavior when baseURL is nil. Something like If baseURL is nil, relative URLs are not expanded. If you don't mind, maybe also change "resolve" to "expanded" in that last sentence as well, since we're not actually doing any URL resolution?
There was a problem hiding this comment.
In fact, as long as an absolute base URL is in the document (i.e. <base href="https://example.com"> we do expand the relative URLs even if baseURL is nil.
I tried to document this (also in Parse() for consistency), but the wording remains clumsy IMO.
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #21 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 837 843 +6
=========================================
+ Hits 837 843 +6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
awesome, thanks! |
this fixes #20
ParseNode(node, nil)should be able to avoid panic even ifnodedoes not contain a valid absolute base path in its<head>, since there can be usable microformats with absolute paths or no paths.ParseNode(nil, base)should returnnilinstead of panic, because why panic?