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

Context-sensitive relative URL rewrite #97

Merged
merged 1 commit into from
May 4, 2018

Conversation

kornelski
Copy link
Contributor

Similar to #96, but with a critical difference that this is a preprocessing step, before relative URLs are resolved. This allows different relative URLs for images than links.

I've named it url_filter_map, after a similar iterator filter in std.

I've opted for storing callback directly rather than via trait, but this required a custom Debug, since derive couldn't skip the callback.

src/lib.rs Outdated
link_rel: Option<&'a str>,
allowed_classes: HashMap<&'a str, HashSet<&'a str>>,
strip_comments: bool,
id_prefix: Option<&'a str>,
}

type UrlFilterCallback = for<'a> Fn(&str, &str, &'a str) -> Option<Cow<'a, str>> + Send + Sync + 'static;

impl<'a> fmt::Debug for Builder<'a> {
Copy link
Member

Choose a reason for hiding this comment

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

Better idea: write a wrapper type around Box<UrlFilterCallback> that implements Debug itself, so that the main Builder class can stick to the derived implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately that won't work due to orphan rules (it's an alias, not a real type).

src/lib.rs Outdated
}
let replace_with = if let Some(new) = filter_map(&*name.local, &*attr.name.local, &*attr.value) {
if *new != *attr.value {
Some(format_tendril!("{}", new))
Copy link
Member

Choose a reason for hiding this comment

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

Tendril::from_str() is faster.

src/lib.rs Outdated
link_rel: Option<&'a str>,
allowed_classes: HashMap<&'a str, HashSet<&'a str>>,
strip_comments: bool,
id_prefix: Option<&'a str>,
}

type UrlFilterCallback = for<'a> Fn(&str, &str, &'a str) -> Option<Cow<'a, str>> + Send + Sync + 'static;
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a trait, like RelativeUrlEvaluate, so that our users can write their own structs to act as callbacks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, done

Copy link
Member

@notriddle notriddle left a comment

Choose a reason for hiding this comment

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

bors r+

That looks good to me!

bors bot added a commit that referenced this pull request May 4, 2018
97: Context-sensitive relative URL rewrite r=notriddle a=kornelski

Similar to #96, but with a critical difference that this is a preprocessing step, before relative URLs are resolved. This allows different relative URLs for images than links.

I've named it `url_filter_map`, after a similar iterator filter in std.

I've opted for storing callback directly rather than via trait, but this required a custom `Debug`, since `derive` couldn't skip the callback.


Co-authored-by: Kornel <kornel@geekhood.net>
@bors
Copy link
Contributor

bors bot commented May 4, 2018

Build succeeded

@bors bors bot merged commit 63975c9 into rust-ammonia:master May 4, 2018
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