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

convert_filter only executed when node_filters are present #391

Closed
grekko opened this issue Jan 6, 2024 · 3 comments · Fixed by #392
Closed

convert_filter only executed when node_filters are present #391

grekko opened this issue Jan 6, 2024 · 3 comments · Fixed by #392

Comments

@grekko
Copy link
Contributor

grekko commented Jan 6, 2024

Hey hey, I'm playing around with html-pipeline and noticed that the following snippet does not run the convert_filter:

require "html_pipeline/convert_filter/markdown_filter"
require "selma"
markdown = "1. Foo\n2. Bar"
result = HTMLPipeline.new(convert_filter: HTMLPipeline::ConvertFilter::MarkdownFilter.new).call(markdown)
{
    :output => "1. Foo\n2. Bar"
}

vs. when defining any node_filter like so:

require "html_pipeline/convert_filter/markdown_filter"
require "html_pipeline/node_filter/https_filter"
require "selma"
markdown = "1. Foo\n2. Bar"
result = HTMLPipeline.new(
  convert_filter: HTMLPipeline::ConvertFilter::MarkdownFilter.new,
  node_filters: [HTMLPipeline::NodeFilter::HttpsFilter.new],
).call(markdown)
{
    :output => "<ol>\n<li>Foo</li>\n<li>Bar</li>\n</ol>"
}

AFAIU the html-result will not be used unless any @node_filters are present. See

html = @convert_filter.call(text) unless @convert_filter.nil?
unless @node_filters.empty?
payload = default_payload({
node_filters: @node_filters.map { |f| f.class.name },
context: context,
result: result,
})
instrument("call_node_filters.html_pipeline", payload) do
result[:output] = Selma::Rewriter.new(sanitizer: @sanitization_config, handlers: @node_filters).rewrite(html)
end
end

@grekko
Copy link
Contributor Author

grekko commented Jan 6, 2024

Just let me know if you like to have a PR for that – only if this is unintended behaviour of course.

In practice this isn't an issue for me since I'll be using some node_filters.

@franciscodiazydiaz
Copy link

Hey @grekko, I'm in the process of updating to v3 and just encountered the same issue. I was wondering if this was an intentional behavior. In my case, I only need the convert_filter.

@grekko grekko changed the title convert_filter not ran when no node_filters are present convert_filter only executed when node_filters are present Jan 7, 2024
@gjtorikian
Copy link
Owner

Completely unintentional—thank you for the report. Will be fixed monetarily in 3.0.2!

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 a pull request may close this issue.

3 participants