Add opt-in binary string diff support#301
Merged
jas14 merged 2 commits intosplitwise:mainfrom Apr 30, 2026
Merged
Conversation
gschlager
force-pushed
the
binary-diff
branch
2 times, most recently
from
20e51bb to
d7e4cc6
Compare
Contributor
Author
|
@jas14 I wanted to follow up on my PR to see if there’s anything I should address or if you haven’t had the chance to review it yet. No worries either way. |
Collaborator
|
Hi @gschlager , I haven't forgotten about this, but haven't had the chance to review yet. Apologies for the delay! |
jas14
requested changes
Mar 26, 2026
Collaborator
jas14
left a comment
jas14
left a comment
There was a problem hiding this comment.
Hey @gschlager , thank you for your contribution and for your patience! I finally had some time to give this PR the time it deserves.
In general, looks excellent. Back to you with a question about the OperationTree prepend and a request to remove the OperationTreeBuilder prepend.
P.S. don't forget to add an entry to the CHANGELOG! 😉
Add a `SuperDiff::BinaryString` extension that renders `ASCII-8BIT` strings as hex dumps. Users opt-in by requiring the module: ``` require 'super_diff/binary_string' ``` Includes a dedicated differ, inspection tree builder, operation tree builder/tree/flattener, and updates SuperDiff::RSpec::Differ to allow diffs for binary strings (skipping the single-line string short- circuit). Hex dumps use 16 bytes per line with offset, hex pairs, and ASCII representation (xxd-style output).
gschlager
added a commit
to gschlager/super_diff
that referenced
this pull request
Apr 18, 2026
- Drop redundant should_compare? override; the MultilineString superclass already returns false as of splitwise#304. - Stop registering OperationTreeBuilders::BinaryString and OperationTrees::BinaryString as extras. Binary strings nested in collections now render via the BinaryString inspection tree builder (e.g. `<binary string (32 bytes)>`) instead of producing inline hex dumps, and have_attributes no longer risks picking up the binary operation tree for a binary-string actual. Top-level binary diffs keep working because Differs::BinaryString references the builder directly. - Add CHANGELOG entry.
gschlager
added a commit
to gschlager/super_diff
that referenced
this pull request
Apr 18, 2026
- Drop redundant should_compare? override; the MultilineString superclass already returns false as of splitwise#304. - Stop registering OperationTreeBuilders::BinaryString and OperationTrees::BinaryString as extras. Binary strings nested in collections now render via the BinaryString inspection tree builder (e.g. `<binary string (32 bytes)>`) instead of producing inline hex dumps, and have_attributes no longer risks picking up the binary operation tree for a binary-string actual. Top-level binary diffs keep working because Differs::BinaryString references the builder directly. - Add CHANGELOG entry.
gschlager
added a commit
to gschlager/super_diff
that referenced
this pull request
Apr 18, 2026
- Drop redundant should_compare? override; the MultilineString superclass already returns false as of splitwise#304. - Stop registering OperationTreeBuilders::BinaryString and OperationTrees::BinaryString as extras. Binary strings nested in collections now render via the BinaryString inspection tree builder (e.g. `<binary string (32 bytes)>`) instead of producing inline hex dumps, and have_attributes no longer risks picking up the binary operation tree for a binary-string actual. Top-level binary diffs keep working because Differs::BinaryString references the builder directly. - Add CHANGELOG entry.
- Drop redundant should_compare? override; the MultilineString superclass already returns false. - Stop registering OperationTreeBuilders::BinaryString and OperationTrees::BinaryString as extras. Binary strings nested in collections now render via the BinaryString inspection tree builder (e.g. `<binary string (32 bytes)>`) instead of producing inline hex dumps, and have_attributes no longer risks picking up the binary operation tree for a binary-string actual. Top-level binary diffs keep working because Differs::BinaryString references the builder directly. - Add CHANGELOG entry.
Collaborator
|
Hey @gschlager , just got back from vacation yesterday and am looking forward to reviewing your changes later this week! |
jas14
approved these changes
Apr 30, 2026
Collaborator
jas14
left a comment
jas14
left a comment
There was a problem hiding this comment.
Fantastic. Thank you again for this contribution!
Contributor
Author
|
Thanks for reviewing, giving constructive feedback, and finally, for merging! Much appreciated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #300
Add a
SuperDiff::BinaryStringextension that rendersASCII-8BITstrings as hex dumps. Users opt-in by requiring the module:Includes a dedicated differ, inspection tree builder, operation tree builder/tree/flattener, and updates SuperDiff::RSpec::Differ to allow diffs for binary strings (skipping the single-line string short-
circuit).
Hex dumps use 16 bytes per line with offset, hex pairs, and ASCII representation (xxd-style output).