-
Notifications
You must be signed in to change notification settings - Fork 202
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
wkt macro to create geo-types #1063
Conversation
|
||
let multi_x = MultiPoint::new(vec![point![x: 0., y: 0.], point![x: 10.+delta, y: 10.]]); | ||
let mut multi_x = multi.clone(); | ||
*multi_x.0[0].x_mut() += delta; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's an example of where it's less nice. Unlike the serde_json::json!
macro, you can't embed expressions. The wkt!
macro only accepts literals.
I like this, I think this is better that what I wrote. A lot more options. |
b5a0780
to
77341a1
Compare
This is great! |
77341a1
to
9710db7
Compare
(Replying to @culebron #1061 (comment) here since I think it's about the changes in this PR)
re: enum vs inner type I initially thought it should return a re: naming I think it's important to have 'wkt' in the name since it's a macro that takes WKT. I'm not too concerned with the naming collision of But, I'd also be OK with something like |
@@ -262,15 +260,11 @@ macro_rules! polygon { | |||
$crate::line_string![ | |||
$($exterior_coord), * | |||
], | |||
<[_]>::into_vec( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the nostd build, we need to give a fully qualified path to the vec macro. That's the primary change in this commit.
But also I noticed these into_vec calls. I don't think we were gaining anything by building an array and then boxing it into a Vec vs. building the vec!
directly, so I've changed it.
be2160c
to
fb3d0f8
Compare
fb3d0f8
to
818004a
Compare
I'd like to merge this. Anyone else want to chime in? Speak now (or any time before tomorrow morning) |
now what |
CHANGES.md
if knowledge of this change could be valuable to users.Inspired by #1061. I don't think it necessarily replaces #1061, but it might obviate it. It's arguable we might want both (or maybe neither 😢)... how do people feel?
I've applied the new macro to a sampling of tests to give people an idea of where it's nice / less nice.