Skip to content

Added microformat test suite #54

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

Merged
merged 2 commits into from
Aug 16, 2014
Merged

Conversation

gRegorLove
Copy link
Member

In general, this test suite is working. However, I ran into some whitespace issues when comparing the parsed output to the expected output.

For example, test adr/simpleproperties indicates the (implied) name output should be 665 3rd St. Suite 207 San Francisco, CA 94107 U.S.A.

But the Mf2 parser returns 665 3rd St. \r\n Suite 207 \r\n San Francisco, \r\n CA \r\n 94107 \r\n U.S.A. - which appears correct according to the specifications; whitespace should be preserved within the contents since it follows the DOM textContent. http://microformats.org/wiki/microformats2-parsing#parsing_for_implied_properties and chat logs with tantek's comments http://indiewebcamp.com/irc/2014-07-20#t1405898141.

I also think this test suite shows some changes to be made to php-mf2, e.g. for adr microformats not inside an hcard. That is a separate issue, though. :) Refer to the chat logs here: http://indiewebcamp.com/irc/2014-07-20#t1405891518

Issue #50

…hough the cURL options are not returning a header.

The response from the cURL request is only the HTML so shouldn't need any trimming.
@gRegorLove
Copy link
Member Author

Just added a small fix for Mf2\fetch() where header size was being trimmed even though the cURL options are not returning a header. The response from the cURL request is only the HTML so shouldn't need any trimming.

@barnabywalters
Copy link
Collaborator

This is looking great! As it’s already working fine, I’m going to go ahead and merge this straight into master. There are some things I want to add and change, but I see no reason to do so here — better to merge now rather than let the conflicts accumulate.

@barnabywalters barnabywalters merged commit 6e97408 into microformats:master Aug 16, 2014
@barnabywalters
Copy link
Collaborator

Merged. Thanks for all your hard work and sorry for the long wait, looking forward to working on this more; integrating it better with the existing test suite, and making more of it pass :)

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