- First commit to issue fork.
- last update
over 1 year ago 488 pass - last update
over 1 year ago 488 pass - Status changed to Needs work
over 1 year ago 4:38am 14 June 2023 - 🇳🇿New Zealand ericgsmith
Gave this a rebase and review - still a bit more work needed
- Status changed to Needs review
5 months ago 7:25pm 1 September 2024 - 🇳🇱Netherlands megachriz
I hope I've addressed all the issues. Will look again later.
- 🇳🇱Netherlands megachriz
I've checked again. I added code to support base urls that do not include the scheme. So previously, "example.com" was marked as incorrect, because
parse_url()
does not see that as a url that contains a domain name. With the regular expression/^([^\/]+\.[a-z]+)\//i
, the code can accept "example.com" as a base url. Tests cases are added for this and it is checked that "www.example.com", "example.com/cat" and "example.com/foo.bar" are correctly parsed as well.There is one remaining unresolved thread and that is that there is a inconsistency with other Tamper plugins in that it tries to convert the incoming data to a string, where other plugins would throw a TamperException. I'm not sure if that is a problem or if it is worth it to let this issue hang because of that. In my opinion, at least in the context of Feeds, I think it is a good thing if the Tamper plugin tries to be "friendly" and just see if it can use the data. Perhaps the other Tamper plugins need to be changed to become friendlier to or maybe there should exist a strict mode if it is important that the source data is of the correct data type. Changing other Tamper plugins are out of scope for this issue, however.
I think that this is ready to go, although it is possible that a small change is needed after ✨ Improve handling of empty data Active is resolved.