Skip to content

Add explicit CDATA guard cleanup - #39

Merged
zyc9012 merged 3 commits into
serpapi:masterfrom
Joseph-Mutua:fix/issue-31-cdata-text
Sep 27, 2026
Merged

zyc9012 merged 3 commits into
serpapi:masterfrom
Joseph-Mutua:fix/issue-31-cdata-text

Conversation

@Joseph-Mutua

@Joseph-Mutua Joseph-Mutua commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add Node#unwrap_cdata_text as an explicit, non-mutating convenience API
  • remove bare, line-comment, block-comment, and nested block-comment CDATA guards only when they form one complete outer pair
  • preserve text / content, surrounding whitespace, and incomplete, mismatched, or embedded guards unchanged
  • compose with Ruby's String#strip as node.unwrap_cdata_text.strip when generic surrounding-whitespace cleanup is wanted
  • use readable, CDATA-specific guard patterns with greedy outer-body matching
  • cover every guard form discussed in the issue plus boundary, composition, exact-whitespace, and immutability cases

API choice

The CDATA-specific name reflects the method's deliberately narrow responsibility. Whitespace cleanup remains the responsibility of String#strip, so it behaves consistently for guarded and ordinary text.

Validation

  • bundle exec ruby -Itest -Ispec spec/node_spec.rb — 138 runs, 328 assertions
  • bundle exec rake test — 312 runs, 4,660 assertions
  • generated guard matrix — 10,000 cases, 0 failures
  • bundle exec yardoc --no-output lib/nokolexbor/node.rb

@zyc9012 @forthrin, this follows the explicit opt-in direction from #31 while keeping normal DOM accessors lossless.

Fixes #31

@zyc9012 zyc9012 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @Joseph-Mutua!

Comment thread lib/nokolexbor/node.rb Outdated
# and embedded wrappers are returned unchanged.
#
# @return [String]
def unwrapped_text

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the method name is too general.

Maybe unwrapped_cdata_text or unwrap_cdata_text

@Joseph-Mutua Joseph-Mutua Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in aac2c60: the method is now Node#unwrap_cdata_text, matching its CDATA-specific scope.

I also separated the two transformations noted on the issue. The helper now removes only a complete recognized CDATA guard and preserves whitespace; callers can use node.unwrap_cdata_text.strip when they also want generic surrounding-whitespace cleanup.

@Joseph-Mutua Joseph-Mutua changed the title Add explicit CDATA wrapper cleanup Add explicit CDATA guard cleanup Sep 24, 2026

@zyc9012 zyc9012 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you @Joseph-Mutua! LGTM.

@zyc9012
zyc9012 merged commit 0375a7b into serpapi:master Sep 27, 2026
33 of 43 checks passed
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.

Doesn't remove CDATA wrapper like its counterpart

2 participants