feat: static_path macro for creating paths#239
Conversation
tewaro
force-pushed
the
tewaro/static_path
branch
2 times, most recently
from
2be2ed5 to
8b02a77
Compare
Creates a path with a static lifetime so it can be used in ad-hoc oqueues for tracing.
tewaro
force-pushed
the
tewaro/static_path
branch
from
8b02a77 to
0ed98a3
Compare
| #[ktest] | ||
| fn test_static_path_display() { | ||
| let path: &'static Path = static_path!(a.b[3].j); | ||
| assert_eq!(path.to_string(), "a.b[3].j"); | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't feel like copying all the tests is useful when the macro is a thin wrapper over another tested macro. Just a couple of tests that compare the two are enough. If there are any tests you think are specifically useful, that's fine. I just don't want to blindly copy them all. It just makes everything harder to follow and the goal of the tests less clear.
There was a problem hiding this comment.
The tests are useful for future code iterations.
There was a problem hiding this comment.
I'm not totally sure what you mean. The only difference is construction .to_string() will always call the exact same function regardless of if you use static_path! or path!. If you do want to run all the tests with both kinds of construction, maybe pull asserts out into another function and call it on the different paths. That way we can also be sure the semantics of the two kinds of path are always the same. (This is definitely reasonable since as you say we maybe changing the internal representation a bit to support both static and dynamic strings.)
arthurp
left a comment
arthurp
left a comment
There was a problem hiding this comment.
This is great.
Not merging this until we get the big merge in would make things marginally easier for me. But since it shouldn't conflict, the machinations required to move the merge up one on main isn't a big problem.
| }}; | ||
| } | ||
|
|
||
| /// Similar to path! Creates a new [`Path`] from `.` delimited literal using static storage. |
There was a problem hiding this comment.
Please document that interpolation is not allowed. It's impossible to implement, but people may not intuit that.
There was a problem hiding this comment.
what is interpolation sorry?
|
|
||
| /// A path whose components are stored in owned heap memory. | ||
| #[derive(Debug, Clone)] | ||
| pub struct OwnedPath { |
There was a problem hiding this comment.
I kinda think this shouldn't be pub. No one should write code that REQUIRES the use of an owned path. What's your take?
There was a problem hiding this comment.
I think that's fair.
tewaro
force-pushed
the
tewaro/static_path
branch
2 times, most recently
from
fc14b76 to
531876e
Compare
tewaro
force-pushed
the
tewaro/static_path
branch
from
531876e to
a675fba
Compare
|
I just mean this https://en.wikipedia.org/wiki/String_interpolation
|
2 participants
Creates a path with a static lifetime so it can be used in ad-hoc oqueues for tracing.