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

Added new entry content and while hooks #41

Merged
merged 5 commits into from
Aug 5, 2015
Merged

Added new entry content and while hooks #41

merged 5 commits into from
Aug 5, 2015

Conversation

robneu
Copy link
Contributor

@robneu robneu commented Jun 8, 2015

This fixes #40

@joshuadavidnelson
Copy link

Wouldn't the_content filter be a better method in place of the tha_entry_content hook? Also, there should be a tha_content_endwhile_before hook to be consistent.

@robneu
Copy link
Contributor Author

robneu commented Jun 8, 2015

Filtering the content comes with its own set of issues because it's used by plugins to do all kinds of things. It's also a little weird to isolate the_content from the_excerpt whereas having this hook means you can just check things like is_archive etc depending on how your theme is coded. Having a standard hook location makes life a lot easier.

I'm not opposed to an endwhile before hook, but if we go that route it might make more sense to change it from endwhile to while and place it after the if statement but before the opening of the while loop.

@joshuadavidnelson
Copy link

I think having a while hook that's after the if statement, but before the while statement - as well as the endwhile one you have currently - would be great. There are plenty of uses for both of those locations.

Can you not check things like is_archive in the filter? You should be able to do just about everything with the content filter. If you added a tha_content_before and tha_content_after perhaps, that seems more in-line with hooking methods used here and elsewhere.

@robneu
Copy link
Contributor Author

robneu commented Jun 8, 2015

The primary use case for having a content hook would be hooking and unhooking the entire content area conditionally. Basically you'd hook any markup for the_content as well as the_content itself to this hook and then it can be conditionally removed by plugins and child themes.

I suppose technically can do this by filtering the_content, but the wrapping markup in particular would be tricky if not impossible. The other problem with this method is if I filter the content to be empty, I'm probably going to have to do a ton of filter juggling to hook it somewhere other than where it was originally. I'd have to filter it on hook A, then remove the filter on hook B then hook the_content to hook C or something along those lines.

I'm kind of thinking out loud here, so if any of this sounds ridiculous or you have an idea of how to do it without filter gymnastics I'd love to hear it.

@joshuadavidnelson
Copy link

If you wanted to conditionally remove the content or override it with another value based on its location, you could do that with the filter by conditionally returning the content or an empty string. In terms of where you hook in, that would likely just require some testing on what to set the filter priority on the content filter, right? You could hook into the new tha_while_start hook or another tha_ hook prior to the content. But if you're using the hook for markup, you'd want a _before and an _after hook, so you can close any potential markup meant to wrap the content? That could be done with the filter, too.

I just noticed that if you're hooking the_content into tha_entry_content, then you'd have a double entry, or you'd have to remove the_content from the markup.

Perhaps I'm missing something, which is entirely possible - let me know if I'm just not seeing it.

@robneu
Copy link
Contributor Author

robneu commented Jun 8, 2015

In the example it would cause a double entry if the content were hooked, but I wasn't sure how to handle that since probably not all theme authors would want to hook the_content. If we did go the before/after route and leave the content unhooked by default then emptying it with a filter on before and removing that filter on after would probably solve most of the usual weirdness I've run into when dealing with this.

Once that was in place, it should be safe to then hook the_content and its containing markup in any other location which would make me happy. Soooo... with that in mind I'll go ahead and update the PR with these changes and hopefully it'll wind up rolled into THA in the future. :D

@joshuadavidnelson
Copy link

Awesome! Yea, I think the before and after route has more universal applications, in addition to being consistent with the hooks used elsewhere in THA. Thanks for hearing me out!

@robneu
Copy link
Contributor Author

robneu commented Jun 8, 2015

One last thing... what do you think we should do about the before/after while naming convention? Should we keep after endwhile and make before while or make them both while?

I think after while still makes sense, especially since not all developers will be using a literal endwhile and may prefer a closing bracket.

@joshuadavidnelson
Copy link

Yea, I would just use the while with _before and _after to be consistent. I agree, endwhile would be confusing.

@robneu robneu changed the title Added entry content and after endwhile hooks Added new entry content and while hooks Jun 8, 2015
@zamoose
Copy link
Owner

zamoose commented Aug 5, 2015

I like this approach.

zamoose added a commit that referenced this pull request Aug 5, 2015
Added new entry content and while hooks
@zamoose zamoose merged commit 969c949 into zamoose:master Aug 5, 2015
@robneu
Copy link
Contributor Author

robneu commented Aug 5, 2015

🎉

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.

No entry content "inside" hook or after endwhile hook
3 participants