Skip to content

added function codePointFromChar #92

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

Merged
merged 4 commits into from
Dec 28, 2017

Conversation

csicar
Copy link
Contributor

@csicar csicar commented Dec 21, 2017

while writing documentation for Data.String.CodePoints I realized that a function converting a char to a codepoint might be useful.
Correct me if I am wrong but the charcode should always be smaller than 0x10FFFF

@hdgarrood
Copy link
Contributor

Looks good! Yes, the code point value of a Char is always less than 0xFFFF; see https://pursuit.purescript.org/builtins/docs/Prim#t:Char. I'll wait until #93 is resolved to merge this.

@hdgarrood
Copy link
Contributor

Looks great, but I think the example doesn't quite work because we don't export the CodePoint constructor. Perhaps singleton (codePointFromChar 'B') == "B" would be better? Or you think of a better example we could use?

@csicar
Copy link
Contributor Author

csicar commented Dec 24, 2017

the new commit uses the representation, that show produces. What do you think?

@MonoidMusician
Copy link
Contributor

Hi, looks good! Would you consider adding a conversion from Char (which has to be partial) to your PR? like this: MonoidMusician@7819fb4#diff-73034b1d5857b2ac02dfa778bea9ef06R63

Thanks!

@hdgarrood
Copy link
Contributor

The problem with writing it this way is that it suggests that you can write codePointFromChar 'B' == CodePoint 0x42 into e.g. a repl and get true out; of course, you'll actually get "Unknown data constructor CodePoint.

I'd prefer something like this instead:

>>> codePointFromChar 'B'
CodePoint 0x42

@hdgarrood
Copy link
Contributor

Thanks for putting up with my nitpicking! :)

@hdgarrood hdgarrood merged commit 7a2a33c into purescript:master Dec 28, 2017
@csicar
Copy link
Contributor Author

csicar commented Dec 28, 2017

I don't mind it. Thanks for merging!

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