Skip to content

add Char.is_digit#13657

Closed
redianthus wants to merge 1 commit intoocaml:trunkfrom
redianthus:is_digit
Closed

add Char.is_digit#13657
redianthus wants to merge 1 commit intoocaml:trunkfrom
redianthus:is_digit

Conversation

@redianthus
Copy link
Copy Markdown
Member

It is easy to define but it's convenient to be able to use it directly. Plus it is quite often redefined.

@dbuenzli
Copy link
Copy Markdown
Contributor

dbuenzli commented Dec 4, 2024

This should at least have ascii mentioned in the name. However I'd suggest we do something else. I never got the time to upstream it but I think these things should be put in an Char.Ascii submodule.

If at least two dev team member tell me they would ok with the module I linked above, indicating if possible what they disagree1 with upfront, I could take the time before the next freeze2 to upstream it.

Note that all of these functions remain useful in an Unicode dominated world, because that world dominates in UTF-8 (notably in command line tool outputs) and all of these functions work with the purported ASCII semantics on UTF-8 aswell and remains widely useful.

Footnotes

  1. IIRC the definitions therein are mostly taken from the corresponding standard libc functions when they exist, at least I'll check they do.

  2. @Octachron the calendar has not been updated.

@nojb
Copy link
Copy Markdown
Contributor

nojb commented Dec 4, 2024

If at least two dev team member tell me they would ok with the module I linked above

Personally, I would be in favor.

@redianthus
Copy link
Copy Markdown
Member Author

I would indeed prefer to have such a Char.Ascii module ! I'm leaving this open for now until a new PR is opened.

@Octachron
Copy link
Copy Markdown
Member

I tend to agree that having a submodule for ascii property might be a good idea, since there are probably other "ascii-property" function that we might wish to add (is_letter? is_whitespace?).

@Octachron Octachron self-assigned this Dec 11, 2024
@Octachron
Copy link
Copy Markdown
Member

I read too fast @dbuenzli's proposal previously, but after reading the proposed Ascii submodule, upstreaming it does sound like a good idea to me.

@dbuenzli
Copy link
Copy Markdown
Contributor

Ok I'll look into that in the upcoming weeks. Thanks for the calendar update.

@dbuenzli dbuenzli mentioned this pull request Dec 23, 2024
@nojb nojb closed this in #13695 Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants