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

Add support for image and link URL rewriting #481

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

liamwhite
Copy link
Contributor

This adds support for stateful URL rewriting during HTML generation. I attempted to follow the broken_link_callback in ParseOptions as an example, but needed to wrap the type because there isn't a blanket implementation for Debug on dyn Fn.

@liamwhite
Copy link
Contributor Author

Hmm it might be better to do this as a dyn trait instead of a naked callback.

@liamwhite liamwhite marked this pull request as draft November 1, 2024 12:39
@liamwhite liamwhite marked this pull request as ready for review November 1, 2024 13:27
@liamwhite
Copy link
Contributor Author

Much cleaner now and no longer requires Mutex.

@liamwhite liamwhite force-pushed the rewriter branch 4 times, most recently from fae1aa2 to 6ed2455 Compare November 1, 2024 16:53
@kivikakk
Copy link
Owner

kivikakk commented Nov 7, 2024

Awesome, LGTM! I am not fully across how (Ref)UnwindSafe work, but I can see that it's required to have the trait Sync/Send. THanks so much!

@kivikakk kivikakk merged commit d0e9e7e into kivikakk:main Nov 7, 2024
20 checks passed
@liamwhite liamwhite deleted the rewriter branch November 7, 2024 01:51
@liamwhite
Copy link
Contributor Author

RefUnwindSafe is a weaker assertion than Sync; every type that is Sync should also imply RefUnwindSafe, it is filed upstream issue that it is not. If you remove it you will see that the tests do not compile due to a problem with catch_unwind.

@kivikakk
Copy link
Owner

kivikakk commented Nov 7, 2024

If you remove it you will see that the tests do not compile due to a problem with catch_unwind.

Indeed, that's exactly what I tried, just to see what kinds of things happened without. Thanks for the link to the upstream issue, that's handy!

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.

3 participants