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

Need prettyprinter exports to convert between String and Text #207

Open
newhoggy opened this issue Sep 25, 2021 · 7 comments
Open

Need prettyprinter exports to convert between String and Text #207

newhoggy opened this issue Sep 25, 2021 · 7 comments

Comments

@newhoggy
Copy link

newhoggy commented Sep 25, 2021

I'm trying to write a patch to optparse-applicative to use prettyprinter instead.

One of the requirements is to not create a dependency on the text package.

The SText constructor uses Text which forces me to use Data.Text.pack and Data.Text.unpack which forces me to add a dependency on text.

If prettyprinter exports pack, unpack and Text, I believe it would solve my problem because I will be able to use it to convert to String do my work there and convert back.

@newhoggy
Copy link
Author

This is what I had in mind: #208

@sjakobi
Copy link
Collaborator

sjakobi commented Sep 27, 2021

If prettyprinter exports pack, unpack and Text, I believe it would solve my problem because I will be able to use it to convert to String do my work there and convert back.

I wonder whether adding these functions to, say, Prettyprinter.Util.TextCompat would be sufficient. If versions for lazy text etc are needed, they could be named stringToText, stringToLazyText etc.

@Bodigrim
Copy link
Contributor

Let it be clear that while I indeed wish for o-a do not depend on text, I am in no position to impose my wish onto maintainers of o-a or prettyprinter or anyone else. My understanding is that o-a can migrate to prettyprinter more or less as is, and an extended API is required only to enable colouring of o-a output. With all due respect, I do not quite understand why @newhoggy pushes both changes in a same PR (pcapriotti/optparse-applicative#428).

FWIW pack is just Data.String.fromString, and unpack is read . show, so this does not warrant any additional exports.

@newhoggy
Copy link
Author

newhoggy commented Sep 27, 2021

@Bodigrim fromString works well. read . show doesn't seem good because it seems to me to impose needless overhead of escaping and quotation only to strip it out again. It's still useful to export Text which may be either a type alias String or Text or else otherwise I cannot give explicit type signatures in my code.

@newhoggy
Copy link
Author

newhoggy commented Sep 27, 2021

@sjakobi I'd be happy to export the minimum types and functions on an as need basis in Prettyprinter.Util.TextCompat starting with just pack, unpack and Text and will push a PR shortly.

@Bodigrim
Copy link
Contributor

read . show doesn't seem good because it seems to me to impose needless overhead of escaping and quotation only to strip it out again.

I'm not quite convinced that this is a blocker for o-a: its workload is mostly short ASCII texts.

@newhoggy
Copy link
Author

newhoggy commented Sep 28, 2021

Turns out the situation is better than I feared in that I don't need unpack which would have fallen back to read . show.

My motivation for this is much weaker now. Thanks to @Bodigrim for leading me the right way in this.

As for the PR exporting Text, pack and unpack.

I'll leave it to the maintainers to decide its fate. If it is merged I will convert optparse-applicative to use unpack, otherwise I will continue to use fromString.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants