search

senny / sablon

Ruby Document Template Processor based on docx templates and Mail Merge fields.
MIT License
447 stars 128 forks source link

gems, upgrade nokogiri #169

Closed senny closed 10 months ago

senny commented 3 years ago

Tried to upgrade Nokogiri but ended up with a test error that I'm not completely sure how to resolve:

  1) Error:
ContentWordMLTest#test_inserting_word_ml_multiple_times_into_same_paragraph:
RuntimeError: Cannot add sibling to a node with no parent
    /Users/senny/.asdf/installs/ruby/2.6.6/lib/ruby/gems/2.6.0/gems/nokogiri-1.11.4-x86_64-darwin/lib/nokogiri/xml/node.rb:1183:in `add_sibling'
    /Users/senny/.asdf/installs/ruby/2.6.6/lib/ruby/gems/2.6.0/gems/nokogiri-1.11.4-x86_64-darwin/lib/nokogiri/xml/node.rb:202:in `add_next_sibling'
    /Users/senny/Projects/sablon/lib/sablon/content.rb:134:in `block in add_siblings_to'
    /Users/senny/.asdf/installs/ruby/2.6.6/lib/ruby/gems/2.6.0/gems/nokogiri-1.11.4-x86_64-darwin/lib/nokogiri/xml/node_set.rb:239:in `block in each'
    /Users/senny/.asdf/installs/ruby/2.6.6/lib/ruby/gems/2.6.0/gems/nokogiri-1.11.4-x86_64-darwin/lib/nokogiri/xml/node_set.rb:238:in `upto'
    /Users/senny/.asdf/installs/ruby/2.6.6/lib/ruby/gems/2.6.0/gems/nokogiri-1.11.4-x86_64-darwin/lib/nokogiri/xml/node_set.rb:238:in `each'
    /Users/senny/Projects/sablon/lib/sablon/content.rb:133:in `add_siblings_to'
    /Users/senny/Projects/sablon/lib/sablon/content.rb:85:in `append_to'
    /Users/senny/Projects/sablon/test/content_test.rb:201:in `test_inserting_word_ml_multiple_times_into_same_paragraph'

The test that fails looks like this: https://github.com/senny/sablon/blob/883de2ecb8c0e8fe92b549ca654f444160f67561/test/content_test.rb#L197-L217

The part where it's failing like this: https://github.com/senny/sablon/blob/883de2ecb8c0e8fe92b549ca654f444160f67561/lib/sablon/content.rb#L132-L139

I suspect that this is related to the .remove here: https://github.com/senny/sablon/blob/883de2ecb8c0e8fe92b549ca654f444160f67561/lib/sablon/content.rb#L89

senny commented 3 years ago

@stadelmanma let me know if you have any thoughts in this. I have been away from the code base a bit too long to know the right resolution without digging into all the details again.

stadelmanma commented 3 years ago

Hey @senny I recall running into a problem when trying to upgrade nokogiri awhile back but couldn't get it resolved. Unfortnately I've been away from this code for awhile now as well.

senny commented 3 years ago

@stadelmanma thanks for the heads up, completely understandable.

I'll see if I can manage to set aside some time to investigate this subject.

spovich commented 1 year ago

Did some digging and this error was introduced in Nokogiri 1.11.0. https://nokogiri.org/CHANGELOG.html#notes_1 due to changes around the node methods when there is no parent node found. Specifically add_next_sibiling is called and raises RuntimeError: Cannot add sibling to a node with no parent.

Is this just a test-suite issue, or is this a problem with the library that needs to be fixed? I don't know enough about the internals and how it is using nokogiri to even guess.

The nokogiri changelog describes why this changed:

The Node methods add_previous_sibling, previous=, before, add_next_sibling, next=, after, replace, and swap now correctly use their parent as the context node for parsing markup. These methods now also raise a RuntimeError if they are called on a node with no parent. [[nokogumbo#160](https://github.com/rubys/nokogumbo/issues/160)]
senny commented 10 months ago

Finished by #188