Skip to content
This repository has been archived by the owner on Sep 5, 2023. It is now read-only.

Compile with 1.1 stable #17

Merged
merged 2 commits into from
Jun 26, 2015
Merged

Compile with 1.1 stable #17

merged 2 commits into from
Jun 26, 2015

Conversation

mseri
Copy link
Contributor

@mseri mseri commented Jun 26, 2015

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
Copy link
Owner

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:

#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
#17.

@mseri
Copy link
Contributor Author

mseri commented Jun 26, 2015

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
Copy link
Owner

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
#17 (comment).

killercup added a commit that referenced this pull request Jun 26, 2015
@killercup killercup merged commit d6f6b51 into killercup:master Jun 26, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants