Skip to content

Migrate use of Expect test patdiff to Myers - #180

Merged
mbarbin merged 13 commits into
mainfrom
myers-diff
Mar 17, 2026
Merged

mbarbin merged 13 commits into
mainfrom
myers-diff

Conversation

@mbarbin

@mbarbin mbarbin commented Feb 24, 2026

Copy link
Copy Markdown
Owner

No description provided.

@mbarbin

mbarbin commented Feb 24, 2026

Copy link
Copy Markdown
Owner Author

cc @tmattio - This PR experiments with the new windtrap.myers package.

Diff looks good! There are a few cases of an empty diff whose rendering has become a bit more verbose now. See in the PR review files:

Empty diffs used to be rendered like this (empty):

  [%expect {||}];

They are now rendered like this:

  [%expect
    {|
    --- expected
    +++ actual
    |}];

I think the empty output is probably a more ergonomic default? Probably also what diff does when run from the terminal. Could that be a good request to submit to the myers packages?

@tmattio

tmattio commented Feb 24, 2026

Copy link
Copy Markdown

Nice!!
Yes I agree, empty default is much better, generally, I'm keen to get closer to ppx_expect, although I found that LLM sometime get confused with what is expected vs actual, so the hint might be an acceptable divergence on non-empty diff

@mbarbin

mbarbin commented Feb 24, 2026

Copy link
Copy Markdown
Owner Author

Hi! I didn't think much about the cases where the diff is non-empty, I don't feel strongly. I don't think I would mind that divergence indeed. Thanks for looking! I have open a few PRs like this one in a few projects, I have to go offline for a bit, but I'll send you soon a list if you'd like to look just for reference in case there's something interesting to note, but from what I can tell, all is working well. Thanks!

@mbarbin

mbarbin commented Mar 17, 2026

Copy link
Copy Markdown
Owner Author

Hi @tmattio so I ended up going the vendoring route, and carry some minimal changes to make the integration easier in the tests. So far I have 4 projects where I'd end up doing this. Something doesn't really feel right to me, so maybe in a little while I'll try requesting again (as a gentle insistance) that you'd release myers as a standalone package! ( 😄 ). Team Vendor it is for now! Thanks again for windtrap!

@mbarbin
mbarbin merged commit c101c78 into main Mar 17, 2026
11 of 12 checks passed
@mbarbin
mbarbin deleted the myers-diff branch March 17, 2026 17:47

This branch was previously deployed

1 inactive deployment
github-pages — c101c78c Deployed Mar 17, 2026 by mbarbin via Deploy to GitHub Pages #221
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.

2 participants