Soften Nokogiri dependency to versions ">= 1.4" #208
Conversation
| @@ -70,7 +70,7 @@ def self.mentioned_logins_in(text, username_pattern=UsernamePattern) | |||
| def call | |||
| result[:mentioned_usernames] ||= [] | |||
|
|
|||
| doc.search('text()').each do |node| | |||
| doc.search('.//text() | text()').each do |node| | |||
There was a problem hiding this comment.
Why this change here? Since the tests pass, the behavior is the same, but I'm curious if this is the preferred convention.
There was a problem hiding this comment.
Without this change, tests failed with Nokogiri 1.6.6.x:
Nokogiri::XML::XPath::SyntaxError:
Invalid expression: .//child::text() | self::child::text()
Please, as a workaround, use #xpath with a real XPath expression (as above) to avoid having to convert CSS into a hacky XPath expression to work around unrelated DocumentFragment issues.
sparklemotion/nokogiri#1233 (comment)
Nokogiri author suggested use real XPath to avoid this.
|
@JuanitoFatas 👍 to your task list. Sorry it took a while for me to get around to this. |
|
@JuanitoFatas Thanks for the PR. This one is actual for me. I cannot use the latest |
|
The CI failure is due to failed to install Nokogiri 1.4.7. @jch I wrote a script to show that this pull request won't failed in most Nokogiri versions, as per #208 (comment). Do you think it is worth to setup https://github.com/thoughtbot/appraisal for testing many nokogiri versions? If you think this Pull Request is good, I will remove 39d9488 and 940959a. Then finish my todo list listed in #208 (comment). |
|
@JuanitoFatas 👍 to getting this PR merged. There's some test failures in travis you should fix before merging, but I think your plan is good.
You could open a separate PR to experiment with it. I'm open to it. |
JuanitoFatas
force-pushed
the
feature/soften-nokogiri-dependency
branch
3 times, most recently
from
c432583
to
d052ca7
Compare
Many Ruby / Rails projects already use Nokogiri 1.6.6.x series. <= 1.6.5 make these projects impossible to integrate html-pipeline. Use real XPath with search() in emoji_filter and mention_filter Use #search with a real XPath expression to avoid the need to convert CSS into a hacky XPath expression. This workaround mentioned in http://git.io/vmp2F
JuanitoFatas
force-pushed
the
feature/soften-nokogiri-dependency
branch
from
d052ca7
to
c26c429
Compare
|
@jch I think is good to merge and release. I'll work on the apprasial thing for testing multiple Nokogiris this weekend. Thank you and sorry this took so long... |
No, thank you. It's open source, and I'm glad to you as a contributor 😉 |
Soften Nokogiri dependency to versions ">= 1.4"
Many Ruby / Rails projects already use Nokogiri 1.6.6.x series.
<= 1.6.5constraint make these projects impossible to integrate html-pipeline.This pull request uses the workaround mentioned in sparklemotion/nokogiri#1233 (comment).
If this change is okay, then will need:
Backport this change to 1.11.0, Release 1.11.1 + an CHANGELOG entryFound out no need to do this because at v1.10.0, the Nokogiri dependency is locked at ~> 1.4.Related: #176