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

Remove regex module from libunicode #24339

Merged
merged 1 commit into from
Apr 13, 2015
Merged

Remove regex module from libunicode #24339

merged 1 commit into from
Apr 13, 2015

Conversation

lambda-fairy
Copy link
Contributor

The regex crate keeps its own tables now (rust-lang/regex#41) so we
don't need them here.

[breaking-change]

@rust-highfive
Copy link
Collaborator

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@huonw
Copy link
Member

huonw commented Apr 12, 2015

@bors r+

@bors
Copy link
Contributor

bors commented Apr 12, 2015

📌 Commit 963f371 has been approved by huonw

@bors
Copy link
Contributor

bors commented Apr 12, 2015

⌛ Testing commit 963f371 with merge 0837f53...

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 12, 2015
The regex crate keeps its own tables now (rust-lang/regex#41) so we
don't need them here.

[breaking-change]
@bors
Copy link
Contributor

bors commented Apr 12, 2015

⛄ The build was interrupted to prioritize another pull request.

@bors
Copy link
Contributor

bors commented Apr 12, 2015

⌛ Testing commit 963f371 with merge ff97ebf...

@bors
Copy link
Contributor

bors commented Apr 12, 2015

⛄ The build was interrupted to prioritize another pull request.

@bors
Copy link
Contributor

bors commented Apr 12, 2015

⌛ Testing commit 963f371 with merge 8eb857f...

@bors
Copy link
Contributor

bors commented Apr 12, 2015

⛄ The build was interrupted to prioritize another pull request.

bors added a commit that referenced this pull request Apr 12, 2015
@bors
Copy link
Contributor

bors commented Apr 12, 2015

⌛ Testing commit 963f371 with merge 8de10fa...

@bors
Copy link
Contributor

bors commented Apr 12, 2015

💔 Test failed - auto-linux-64-nopt-t

@kwantam
Copy link
Contributor

kwantam commented Apr 12, 2015

@lfairy good catch!

You can remove the script module to fix many of the unused code errors; this module was only used by the UNICODE_CLASSES table in the regex module. Just get rid of the ("script", scripts, []) tuple in the argument to for (name, cat, pfuns) in unicode.py.

You also will want to get rid of the other_derived variable in unicode.py, which will make unicode.py not emit the Default_Ignorable_Code_Point table; again, this was only used in the UNICODE_CLASSES table.

general_category takes some more work, since it's used in the char module, but you only need to emit N and Cc. One very straightforward way of getting the result you want here is to modify the emit_property_module function to only emit tables for categories in the emit_fn argument. Right now the behavior is to emit all tables in the tbl argument, and only emit corresponding search functions for the emit_fn argument.

bors added a commit that referenced this pull request Apr 12, 2015
The regex crate keeps its own tables now (rust-lang/regex#41) so we
don't need them here.

[breaking-change]
@lambda-fairy
Copy link
Contributor Author

@kwantam Thanks for the tips. I've force pushed a fixed version with your suggestions.

@huonw re-r?

@alexcrichton
Copy link
Member

@bors: r+ 5308ac9

@bors
Copy link
Contributor

bors commented Apr 13, 2015

⌛ Testing commit 5308ac9 with merge 5245475...

bors added a commit that referenced this pull request Apr 13, 2015
The regex crate keeps its own tables now (rust-lang/regex#41) so we
don't need them here.

[breaking-change]
@bors bors merged commit 5308ac9 into rust-lang:master Apr 13, 2015
@kwantam kwantam mentioned this pull request Apr 14, 2015
@lambda-fairy lambda-fairy deleted the remove-regex-data branch August 30, 2015 10:17
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.

6 participants