Skip to content

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Apr 12, 2015

This changes the name to #[derive(NumFromPrimitive)] in order to not conflict with the libsyntax macro.

If the general direction of this num library will supplant std::num, then I'd instead prefer to do the same trick we did for rustc-serialize, and have the compiler add a #[derive(RustcFromPrimitive)] that uses the trait in this crate instead.

This changes the name to #[derive(NumFromPrimitive)]
in order to not conflict with the libsyntax macro.
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.


[dev-dependencies.num_macros]
path = "num_macros/"
version = "0.1.22"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine to start this at 0.1.0, and could this be formatted as:

[dev-dependencies]
num-macros = { path = "num-macros", version = "0.1.0" }

@alexcrichton
Copy link
Contributor

Nice! Could you also edit .travis.yml to run the tests for this new crate as well?

Also cc @aturon, how do you feel about this?

@aturon
Copy link
Contributor

aturon commented Apr 27, 2015

Sorry I missed this before. Someone wrote a macro to replace this derive mode, so my preference for the moment would be to remove this altogether.

The rust-lang num crate may eventually be a place to test ideas for std::num, but at the moment it's largely a place that deprecated items have been dropped into.

@nwin
Copy link
Contributor

nwin commented May 1, 2015

The macro solution is quite limited. It is difficult to impossible to document your enums (#24189). I would be very much in favor of #[derive(NumFromPrimitive)] as an intermediate solution.

@aturon
Copy link
Contributor

aturon commented May 1, 2015

OK, taking a closer look at this, I'm fine with this direction for the moment. (In the long run, I still hope we can provide better built-in support in Rust itself for this kind of conversion of enums).

@alexcrichton alexcrichton merged commit f4a69a6 into rust-num:master May 2, 2015
@alexcrichton
Copy link
Contributor

I renamed num_macros to num-macros and otherwise just tidied up the merge a bit, thanks again @erickt!

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.

5 participants