Skip to content

Textual representation of different values at the top level#18

Merged
fpringle merged 10 commits into
mainfrom
fpringle/top-level-show
Apr 30, 2026
Merged

Textual representation of different values at the top level#18
fpringle merged 10 commits into
mainfrom
fpringle/top-level-show

Conversation

@fpringle
Copy link
Copy Markdown
Owner

Closes #16.

Adds a new DiffError constructor TopLevelNotEqualShow. This is similar to TopLevelNotEqual, except that it includes 2 Text arguments that are the result of calling show on the compared values.

Whereas before we would have

printDiffResult $ diff (Atom 1) (Atom 2)
Both values use constructor Atom but fields don't match
In field 0 (0-indexed):
  Not equal

Now we have:

printDiffResult $ diff (Atom 1) (Atom 2)
Both values use constructor Atom but fields don't match
In field 0 (0-indexed):
  Not equal
  Left value:  1
  Right value: 2

@fpringle fpringle marked this pull request as ready for review April 28, 2026 20:27
@fpringle
Copy link
Copy Markdown
Owner Author

@ozkutuk there are 2 differences between this approach and yours. The first is simple, I decided to add a new constructor TopLevelNotEqualShow for backwards-compatability. Secondly, the arguments are Texts rather than as. This makes the Show and Eq instances trivial, and is equivalent for our use case (since we're only going to call show on them anyway).

An alternative solution which we discussed would have been this:

TopLevelNotEqual :: (Show a, Eq a) => a -> a -> DiffError a

However this ended up being less elegant, and would mean removing some of the existing instances (like MVar, as you mentioned). With the approach I took, we can use the original constructor (without Show) for those types, and the new one for the rest.

Copy link
Copy Markdown
Contributor

@ozkutuk ozkutuk left a comment

Choose a reason for hiding this comment

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

Thanks for the super-fast PR! LGTM, just some typos in the Haddocks. Otherwise, the approach seems solid. I can see how using Text instead of a in the constructor makes for a far simpler implementation.

Comment thread generic-diff/src/Generics/Diff/Class.hs Outdated
Comment thread generic-diff/src/Generics/Diff/Class.hs Outdated
Comment thread generic-diff/src/Generics/Diff/Class.hs Outdated
Co-authored-by: Berk Özkütük <berk@ozkutuk.me>
@fpringle
Copy link
Copy Markdown
Owner Author

@ozkutuk Thanks! I shouldn't write documentation late at night 😝

@fpringle fpringle merged commit 472b6d1 into main Apr 30, 2026
20 checks passed
@fpringle fpringle deleted the fpringle/top-level-show branch April 30, 2026 18:44
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.

Displaying values in top-level inequalities

2 participants