Add TinyAsciiStr::concat #5772
Add TinyAsciiStr::concat #5772
Conversation
|
I considered a few other signatures. I picked this one because:
|
| pub const fn concat<const M: usize, const Q: usize>( | ||
| self, | ||
| other: TinyAsciiStr<M>, | ||
| ) -> TinyAsciiStr<Q> { |
There was a problem hiding this comment.
nit: I think you should debug assert that Q = N+M.
There was a problem hiding this comment.
Did you see my comment above about why I didn't add the assertion? It's fine if you disagree and want me to add the assertion, but I wanted to verify that you read my reasoning and are making this comment despite it
There was a problem hiding this comment.
Please also note that the only use case for this function (merging two subtags) will involve .concat().resize() if I force Q to be N + M. Seems better to do it in a single function call. There's not a good reason not to do so.
There was a problem hiding this comment.
I don't think I understand your logic. My interpretation of your code is that I can now do:
let t: TinyAsciiStr<12> = tinystr!(4, "abcd").concat(tinystr!(4, "efgh"));
|
@zbraniecki You can submit this (to unblock your PR) if you agree with my logic about not adding the assertion. |
|
I'm fine merging it as is. No blockers. |
#5729