Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Mf2 Improvements #85

Closed
wants to merge 13 commits into from
Closed

Mf2 Improvements #85

wants to merge 13 commits into from

Conversation

dshanske
Copy link
Collaborator

@dshanske dshanske commented May 1, 2017

This fixes the misclassification of other properties as mentions, but also starts the process of reconfiguring the mf2 portion of the plugin to store properties in comment meta.

Assuming this particular improvement is going in the right direction, future commits would continue enhancing the authorship code(to pull a remote author URL if needed), as well as other enhancements to allow for enhanced display and additional types in the future, as appropriate.

@pfefferle
Copy link
Owner

This example https://github.com/voxpelli/node-webmention-testpinger/blob/master/templates/notizblog-org.html results in

[...]
Großartige Idee!
Wer sein eigenes Bridgy betreiben will… der Code ist Open Source!

via Array

@pfefferle
Copy link
Owner

@dshanske
Copy link
Collaborator Author

dshanske commented May 1, 2017

Needs improvement on the authorship side, as noted. Will continue there. How do the changes look otherwise?

@pfefferle pfefferle changed the title Mf2 Improvements [WIP] Mf2 Improvements May 1, 2017
@dshanske
Copy link
Collaborator Author

dshanske commented May 1, 2017

I built in the ?sldebug query to test examples also.

@pfefferle
Copy link
Owner

If it is working at the end, I am fine with all your changes ;)

@pfefferle
Copy link
Owner

Thanks for your work!

@dshanske
Copy link
Collaborator Author

dshanske commented May 1, 2017

I am going to keep working at authorship to improve it. So far, in testing the URLs that people reported issues with, it is now correctly classifying things, so we'll move from there.

@dshanske
Copy link
Collaborator Author

dshanske commented May 8, 2017

Still working the scenario of an h-cite nested in a like-of. But the rest should be improved.

@pfefferle
Copy link
Owner

OK, just remove the [WIP] if you think it is ready to be merged.

@dshanske dshanske changed the title [WIP] Mf2 Improvements Mf2 Improvements May 10, 2017
@dshanske
Copy link
Collaborator Author

I think this is ready to go for now. I threw a bunch of different URLs at it and it was able to correctly set the properties. There may be some mf2 markup I haven't accounted for though. I know I need to improve mine.

@pfefferle
Copy link
Owner

nice, thanks!

I will check my links too and merge it if will also work.

@dshanske
Copy link
Collaborator Author

Hoping to close a lot of issues with this

// check u-* params
if ( in_array( $key, array_keys( $classes ) ) ) {
// check "normal" links
if ( self::compare_urls( $target, $values ) ) {
// if ( self::compare_urls( $target, $values ) ) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can't remove this line, because if you do, the function returns the first matching class, if the link matches or not.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has to be checked, because this example for target: http://example.com/webmention/target/placeholder is returning reply but it should return like.

<span class="h-entry">
  <a class="u-in-reply-to" href="http://example.com/">A Cool Post</a><a class="u-url" href="/"></a>
  <a class="u-like-of" href="http://example.com/webmention/target/placeholder">A Cool Post</a><a class="u-url" href="/"></a>
  <a class="u-in-reply-to" href="http://example.com/">A Cool Post</a><a class="u-url" href="/"></a>
</span>

Copy link
Owner

@pfefferle pfefferle May 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or this example returns bookmark but it should be a mention (target: http://example.com/webmention/target/placeholder):

<span class="h-entry">
  <a class="u-bookmark-of" href="http://example.com/">A Cool Post</a><a class="u-url" href="/"></a>
  <a href="http://example.com/webmention/target/placeholder">A Cool Post</a><a class="u-url" href="/"></a>
</span>

public static function get_entries( $mf_array ) {
$entries = array();

public static function get_representative_item( $mf_array, $target ) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is called for getting the representative h-entry and h-card, so we shouldn't mix both. This might cause errors because it might return the author as h-entry and the other way around. The author stuff should be part of get_representative_author.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which is being called for both the h-card and the h-entry?

@dshanske
Copy link
Collaborator Author

I made some minor modifications. Do you have any URLs to test it against? The ones I threw at it, it seemed to work.

@pfefferle
Copy link
Owner

I tested your changes with the testpinger (https://github.com/voxpelli/node-webmention-testpinger) examples and nearly no type is detected correctly.

@pfefferle
Copy link
Owner

The author detection seems to work better though.

@dshanske
Copy link
Collaborator Author

Odd. Think maybe I should close this and go back to the drawing board with a simpler design?

@dshanske
Copy link
Collaborator Author

I'm going to go back to the drawing board and integrate the changes in smaller steps.

@dshanske dshanske closed this May 31, 2017
@pfefferle
Copy link
Owner

pfefferle commented May 31, 2017

what do you think about the following steps:

  • I commit my testcases to the master
  • we add all missing tests for the markups we want to support
  • we refactor the type detection so it will work with all types and markups
  • we re-add your refactorings

@dshanske
Copy link
Collaborator Author

I'm working on a slightly less intrusive version of the refactorings. But if you commit your tests, I can always rebase.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants