search

killercup / trpl-ebook

UNMAINTAINED
http://killercup.github.io/trpl-ebook/
478 stars 56 forks source link

Compile with 1.1 stable #17

Closed mseri closed 9 years ago

mseri commented 9 years ago

Remove the dependency on regex_macros to make it compatible (and compilable) with rust stable.

I have tested with 1.1 and all the tests are passing and the book is properly generated.

It should be fine even performance-wise, at least according to the discussion here: http://www.reddit.com/r/rust/comments/3b2i0f/psa_regex_is_now_slower_than_regexnew/

killercup commented 9 years ago

Thank you! It's awesome that this works on stable!

Do you think the code would be cleaner if we define the regex! macro ourselves in main.rs (as Regex::new($x).unwrap())?

mseri notifications@github.com schrieb am Fr., 26. Juni 2015 um 18:23:

Remove the dependency on regex_macros to make it compatible (and compilable) with rust stable.

I have tested with 1.1 and all the tests are passing and the book is properly generated.

It should be fine even performance-wise, at least according to the discussion here:

http://www.reddit.com/r/rust/comments/3b2i0f/psa_regex_is_now_slower_than_regexnew/

You can view, comment on, or merge this pull request online at:

https://github.com/killercup/trpl-ebook/pull/17 Commit Summary

  • Move to regex only to compile with stable
  • Revert to 'dev' feature name

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/killercup/trpl-ebook/pull/17.

mseri commented 9 years ago

I don't know. At first I thought about it too, as a way to revert to regex_macros as soon as they become stable. Then I've read that discussion on reddit and decided to go simply with Regex::new.

In addition, even if it's not a problem for a small codebase, it could be confusing to see regex! (that sould be on the stack) and have it dynamical on the heap.

killercup commented 9 years ago

Good point. (The stack-expectation… stackspectation?) Changing it back to the macro form is pretty easy anyway.

I'll merge this later when I get to a computer.

Thanks again! mseri notifications@github.com schrieb am Fr., 26. Juni 2015 um 21:13:

I don't know. At first I thought about it too, as a way to revert to regex_macros as soon as they become stable. Then I've read that discussion on reddit and decided to go simply with Regex::new.

In addition, even if it's not a problem for a small codebase, it could be confusing to see regex! (that sould be on the stack) and have it dynamical on the heap.

— Reply to this email directly or view it on GitHub https://github.com/killercup/trpl-ebook/pull/17#issuecomment-115843237.