Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ansi-wl-print with prettyprinter #92

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

9999years
Copy link

This builds off of #91.

The workaround for #91 requires adding the prettyprinter and prettyprinter-ansi-terminal packages anyways, so let's remove the deprecated ansi-wl-print.

The `ansi-wl-pprint` changelog for 1.0.2 says that "Using `show` won't
preserve formatting anymore, as `prettyprinter`s `Show Doc` instance is
annotation invariant."

This patch applies their suggested workaround, `Text.unpack . renderLazy
. layoutPretty defaultLayoutOptions`.

See: https://github.com/ekmett/ansi-wl-pprint/blob/master/Changelog.md#102
Copy link
Collaborator

@phadej phadej left a comment

Choose a reason for hiding this comment

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

Apparently this also replaces pretty with prettyprinter, not only ansi-wl-pprint with prettyprinter directly. Why?

wrt expr = BS.writeFile fp $ TE.encodeUtf8 $ T.pack $ showWL (WL.plain (ansiWlExpr expr)) ++ "\n"

showWL :: WL.Doc -> String
showWL doc = WL.displayS (WL.renderSmart 0.4 80 doc) ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

why this change. AFAICT (by using this package very day), ediffGolden colors output?

Copy link
Author

Choose a reason for hiding this comment

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

Correct, there's no change in behavior here. There's a small change in the implementation because the prettyprint libraries have a different interface than the ansi-wl-pprint compatibility layer.

(x == y)
where
render = TL.unpack . renderLazy . layoutPretty defaultLayoutOptions
Copy link
Collaborator

Choose a reason for hiding this comment

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

AFAICT, this is the only relevant change, properties are not coloured with prettyprinter. If so, this is the only relevant change.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, #91 includes only this change, which is why I linked to it in #77 (comment).

@9999years
Copy link
Author

Apparently this also replaces pretty with prettyprinter, not only ansi-wl-pprint with prettyprinter directly. Why?

prettyprinter does not depend on pretty. Ask those maintainers for details about how the library is designed. The use of Text instead of String appears to be a factor, from the README. I'm not involved there, I just want tree-diff to work correctly and I took the time to remove a deprecated library.

@phadej
Copy link
Collaborator

phadej commented Dec 3, 2024

pretty is definitely not deprecated.

@9999years
Copy link
Author

pretty is definitely not deprecated.

That's correct, but ansi-wl-pprint is.

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.

None yet

2 participants