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

[BREAKING] TextFilter now requires instantiation #398

Merged
merged 6 commits into from
Feb 28, 2024
Merged

[BREAKING] TextFilter now requires instantiation #398

merged 6 commits into from
Feb 28, 2024

Conversation

gjtorikian
Copy link
Owner

This shouldn't be too disruptive for folks using TextFilter, but is a breaking change nonetheless. Yes, SemVer dictates that I bump a major release, but I don't think many, if anybody, uses text filters, and I don't want to signal a major change for arguably the more important functions of this library, which is to take markup and turn it into safe HTML.

None of the behavior related to HTML manipulation or node filters are changing. Instead, TextFilters should now be instantiated just like node filters:

# before
MarkdownPipeline = HTMLPipeline.new (
  text_filters: [HTMLPipeline::TextFilter::ImageFilter],
  convert_filter: HTMLPipeline::ConvertFilter::MarkdownFilter.new,
  node_filters: [
    HTMLPipeline::NodeFilter::HttpsFilter.new,HTMLPipeline::NodeFilter::MentionFilter.new,
  ], context: context)
  
# after

MarkdownPipeline = HTMLPipeline.new (
  text_filters: [HTMLPipeline::TextFilter::ImageFilter.new],
  convert_filter: HTMLPipeline::ConvertFilter::MarkdownFilter.new,
  node_filters: [
    HTMLPipeline::NodeFilter::HttpsFilter.new,HTMLPipeline::NodeFilter::MentionFilter.new,
  ], context: context)

That's it.

@gjtorikian gjtorikian merged commit b9e02e3 into main Feb 28, 2024
3 checks passed
@gjtorikian gjtorikian deleted the classify branch February 28, 2024 22:32
@dentarg
Copy link
Contributor

dentarg commented Feb 28, 2024

Why do you think not many (or none) uses text filters? Curious if I'm doing something wrong (https://github.com/Starkast/wikimum/pull/590/files#diff-f76e24ef86d4ffadf31658baddaf98cdf97a1186cfa61f2a41d735b3a878dac0)

Re: the breaking change, it's okay, I have tests. Promises about SemVer is not to be trusted :)

@gjtorikian
Copy link
Owner Author

Because they don’t really “do” anything special. They’re kind of just elaborately structured gsub statements! But obviously I use them too and I’m happy they work for you too!

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