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

Implement new context option: ignored_ancestor_tags to accept more ignored tags. #170

Merged
merged 1 commit into from
Feb 11, 2015
Merged

Implement new context option: ignored_ancestor_tags to accept more ignored tags. #170

merged 1 commit into from
Feb 11, 2015

Conversation

JuanitoFatas
Copy link
Contributor

Hello maintainer,

I attempt to fix #163 by implemented ignored_pattern.

This filter is designed to run on HTML fragments, so we should assume that tags is a reasonable input to work with.

And while reading conversations in #163,

The EmojiFilter has something similar, but it doesn't use a constant.
@simeonwillbanks

above quote inspired me to implement ignored_ancestor_tags.

Then we could specify more tags to stop the emojification process if we want.

Please take a look. 🙇 🙏

Thanks!!

def call
doc.search('text()').each do |node|
content = node.to_html
next unless content.include?(':')
next if has_ancestor?(node, %w(pre code tt))
next if has_ancestor?(node, ignored_ancestor_tags)
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this change, but I'd skip the ignored_pattern. This filter is designed to run on HTML fragments, so we should assume that tags is a reasonable input to work with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Got it. Removed ignored_pattern.

@JuanitoFatas JuanitoFatas changed the title Implement ignored_ancestor_tags and ignored_pattern. Implement new context option: ignored_ancestor_tags to accept more ignored tags. Jan 25, 2015
@JuanitoFatas
Copy link
Contributor Author

Error due to Nokogiri 1.6.6.2 & 1.6.6.1, works on 1.6.5:

Nokogiri::XML::XPath::SyntaxError: Invalid expression: .//child::text() | self::child::text()
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/searchable.rb:165:in `evaluate'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/searchable.rb:165:in `block in xpath'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/searchable.rb:156:in `map'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/searchable.rb:156:in `xpath'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/searchable.rb:193:in `css_internal'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/node_set.rb:76:in `block in css'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/node_set.rb:187:in `block in each'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/node_set.rb:186:in `upto'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/node_set.rb:186:in `each'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/node_set.rb:75:in `inject'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/node_set.rb:75:in `css'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/document_fragment.rb:108:in `block in search'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/document_fragment.rb:104:in `each'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/document_fragment.rb:104:in `inject'
    /home/travis/.rvm/gems/ruby-2.2.0/gems/nokogiri-1.6.6.2/lib/nokogiri/xml/document_fragment.rb:104:in `search'
    /home/travis/build/jch/html-pipeline/lib/html/pipeline/emoji_filter.rb:22:in `call'

@flavorjones
Copy link

Team Nokogiri is looking into this at sparklemotion/nokogiri#1233

simeonwillbanks pushed a commit that referenced this pull request Feb 9, 2015
- As identified in #170,
  nokogiri 1.6.x is buggy
  - Team nokogiri is working on the fix
    sparklemotion/nokogiri#1233
- Until the bug is fixed, define a range of working nokogiri gems
@JuanitoFatas
Copy link
Contributor Author

since #176 pinned the nokogiri version, could someone please restart the tests please 😄 ? Thanks!

@jch
Copy link
Contributor

jch commented Feb 10, 2015

@JuanitoFatas you'll need to merge the latest master into your branch and push. That'll restart the build and have the changes from #176.

@JuanitoFatas
Copy link
Contributor Author

@jch Ah right I need to rebase 😛, rebased and force pushed, please check again! 😃

@simeonwillbanks
Copy link
Contributor

👍

@jch
Copy link
Contributor

jch commented Feb 11, 2015

@JuanitoFatas I usually just do a merge from master rather than rebase and force push, but it's a matter of preference. Thanks for updating.

jch added a commit that referenced this pull request Feb 11, 2015
…tern-for-emoji-filter

Implement new context option: ignored_ancestor_tags to accept more ignored tags.
@jch jch merged commit 2f52b08 into gjtorikian:master Feb 11, 2015
@JuanitoFatas JuanitoFatas deleted the fix/configurable-ignore-pattern-for-emoji-filter branch February 22, 2015 06:02
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.

Do not mention or emojify in a codeblock
4 participants