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

Reference string-conversions in README. #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fisx
Copy link

@fisx fisx commented Jun 29, 2018

See also: soenkehahn/string-conversions#14

If you disagree with the "matter of taste" thing I'd be very interested in your opinion. :-)

For what it's worth I think I am going to trade string-conversions with text-conversions. It's a little more explicit, means more work to write code and less work to read it. Always a good thing!

Thanks for this, and for your great blog that led me here!

@fisx fisx changed the title Reference text-conversions in README. Reference string-conversions in README. Jun 29, 2018
@aztecrex
Copy link
Contributor

Thanks.

I don't think the README's scope should include enumerating alternatives. That can quickly become out of date and puts an extra burden on the maintainers.

@lexi-lambda
Copy link
Collaborator

I also think this PR is wrong. text-conversions exists specifically because string-conversions throws exceptions on a lot of its conversions. The README already mentions this difference:

Unlike some other libraries that attempt to solve the same problem, text-conversions is:

  • Safe. This library treats binary data (aka ByteString) like binary data, and it does not assume a particular encoding, nor does it ever throw exceptions when failing to decode. It does, however, provide failable conversions between binary data and textual data.

Emphasis mine.

@fisx
Copy link
Author

fisx commented Jul 1, 2018

I disagree with you @aztecrex. At least in this instance the reference will not get out of date any time soon, and even if it would its value would still be enormous. I've been happily using and recommending string-conversions for years, and now I'm considering refactoring a lot of code because of that gap in my knowledge. People gather information differently, and I personally like it if things link to similar things.

string-conversions does not throw errors, but replaces bad bytes with \xFFFD:

I may still be wrong and explicitly returning errors in the type may be objectively better.

Anyway, feel free to keep this closed, I'm just trying to help. :-)

Thanks again!

@lexi-lambda
Copy link
Collaborator

Oh, I only just noticed I pressed the “close and comment” button instead of the “comment” button when I left my comment earlier. It was not intentional; I apologize for coming off as unnecessarily aggressive!

It looks like you’re right that string-conversions replaces bad sequences with U+FFFD; I was thinking of string-conv, which gives the option of either throwing or replacing bad sequences with U+FFFD. I think the lenient behavior is normally not what you want in practice, but it could be helpful to include a lenient newtype in this library if it’s something people found valuable.

In any case, I’ll reopen this and let the current maintainers decide what to do with it; I do not work at CJ anymore, and I don’t have the spare cycles to be a good maintainer right now. :)

@lexi-lambda lexi-lambda reopened this Jul 1, 2018
@fisx
Copy link
Author

fisx commented Jul 2, 2018

no offense taken, and i don't mind this PR getting rejected, even though i would prefer to add string-conv to the list. :-)

yes, maintainers, please make a call.

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.

3 participants