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

moji char predicates #8233

Merged
merged 5 commits into from
Sep 13, 2014
Merged

moji char predicates #8233

merged 5 commits into from
Sep 13, 2014

Conversation

catawbasam
Copy link
Contributor

This is a rebased version of PR #8110, addressing issue #5939.
cc @stevengj @ivarne

@@ -30,6 +30,11 @@ Library improvements

* Efficient `mean` and `median` for ranges ([#8089]).

* Character predicates such as `islower()`, `isspace()`, etc. use `utf8proc`\`libmojibake`
Copy link
Member

Choose a reason for hiding this comment

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

forward slash, not backslash, in English. Also, no backquotes around utf8proc/libmojibake since these aren't code per se.

@stevengj
Copy link
Member

stevengj commented Sep 4, 2014

You also need to update the documentation in julia/doc/stdlib/base.rst. We need to document precisely what each of these functions does (no one should have to read the source code), remove the documentation for isblank, and also document the new function isnumber.

@catawbasam
Copy link
Contributor Author

Docs added. Not sure what's up with the timestamps on the latest commits; perhaps related to the fact that I had to move Julia over to a different computer this morning.

@stevengj
Copy link
Member

stevengj commented Sep 8, 2014

Something's wrong with the documentation patch, because the diff shows a lot of changes unrelated to the character predicates.

Oh ... it's apparently just that helpdb hadn't been updated recently, since that file is generated.

@stevengj
Copy link
Member

stevengj commented Sep 8, 2014

Looks good to me. @jiahao?

@@ -1483,32 +1483,39 @@ Strings
.. function:: isgraph(c::Union(Char,String)) -> Bool

Tests whether a character is printable, and not a space, or whether this
is true for all elements of a string.
is true for all elements of a string. Any character that would cause a printer
to use ink should be classified with isgraphic(c)==true.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be isgraph(c)==true, not isgraphic.

Copy link
Contributor

Choose a reason for hiding this comment

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

OTOH, I think isgraph should be isgraphical... more clear.

Copy link
Member

Choose a reason for hiding this comment

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

We're using the standard libc names for everything else, though, right? Would be weird to have just one of them different.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I didn't know that was a libc thing. It probably shouldn't change then.

@catawbasam catawbasam mentioned this pull request Sep 9, 2014
@catawbasam
Copy link
Contributor Author

Travis passed on Linux but failed on OS X. It looks like #7939, so I think this PR is ok.

@catawbasam
Copy link
Contributor Author

Passed Travis this time.

stevengj added a commit that referenced this pull request Sep 13, 2014
@stevengj stevengj merged commit 86ac298 into JuliaLang:master Sep 13, 2014
@stevengj
Copy link
Member

I'm going to go ahead and merge. Thanks for all your efforts on this!

@catawbasam
Copy link
Contributor Author

Great. Thanks for your patience in walking me through the process.

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.

4 participants