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.

Sign up for GitHub

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

Jump to bottom

char predicates using libmojibake #8110

Closed
wants to merge 10 commits into from
Closed

char predicates using libmojibake #8110

wants to merge 10 commits into from

Conversation

catawbasam
Copy link
Contributor

see issue #5939

Please note from string.jl the following:

include("utf8proc.jl")
importall .UTF8proc

The include was needed to resolve a bootstrap related error. Is there a preferred means of dealing with this type of issue?

@catawbasam
Copy link
Contributor Author

Fair enough. Looks like my first attempt at dealing with the bootstrapping was also hosed.
This update eliminate the 2-letter names and moves the functions over to utf8proc, which helps with the bootstrapping.

@catawbasam
Copy link
Contributor Author

@stevengj -- ok to move these functions into utf8proc?

@stevengj
Copy link
Member

Seems reasonable to put them into the UTF8Proc module and then import the to Base from there.

stevengj
stevengj reviewed Aug 25, 2014
View reviewed changes
base/utf8proc.jl
## libc character class predicates ##


islower(c::Char) = (_catcode(c)==UTF8PROC_CATEGORY_LL)
Copy link
Member

Choose a reason for hiding this comment

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

This indentation is non-typical for Base.

@catawbasam
Copy link
Contributor Author

OK, all comments to date addressed.

@stevengj
Copy link
Member

You don't need to export isblank; @deprecate will do that for you. Be sure to update exports.jl too.

[pao: quotes]

@catawbasam
Copy link
Contributor Author

Done.

@stevengj
Copy link
Member

Also needs a NEWS item.

@pao
Copy link
Member

pao commented Aug 26, 2014

@stevengj reminder: quote macro names (@deprecate is actually an active user for once)

@catawbasam
Copy link
Contributor Author

Ok, ran julia doc/update-NEWS.jl. It added "+[#5939]: #5939".

@catawbasam
Copy link
Contributor Author

I did a local rebase to check on the Travis failure, and afterwards the local make also yielded a segfault.
make clean all resolved the issue and make clean testall subsequently passed.
Is there a way to ask Travis to do a make clean all?

@catawbasam
Copy link
Contributor Author

Closing due to dirty rebase. I will create a new PR.

@catawbasam catawbasam closed this Sep 2, 2014
@catawbasam
Copy link
Contributor Author

Currently stymied by #8200. I'll re-open once that is resolved.

@ivarne ivarne mentioned this pull request Sep 3, 2014
Closed
@catawbasam catawbasam mentioned this pull request Sep 4, 2014
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@catawbasam @stevengj @pao @nalimilan @quinnj