#1322 tuple join #1330
#1322 tuple join #1330
Conversation
|
Things I'm aware of that need to be done:
Note that I opted to use simple ADL instead of template specialization |
|
One thing that wasn't immediately obvious to me was how to do the concatenation using a fold on C++17 enabled compilers (aka without recursion). I think this should be possible but I'm not sure whether it's worth losing the clarity of this approach (which is simpler) for some indeterminate compile time gains. I'm open to suggestions here though. |
jeremyong
changed the title
|
I think recursive solution is fine. It can even be ported to pre-C++17. AFACS the only C++17 construct is |
|
OK does the FMT_CONSTEXPR macro expand to constexpr if available? |
|
|
jeremyong
force-pushed
the
tuple_join
branch
from
e065c79
to
adf26d1
Compare
jeremyong
changed the title
jeremyong
force-pushed
the
tuple_join
branch
3 times, most recently
from
151ee37
to
b282d6b
Compare
|
@vitaut I believe all feedback is now addressed and I have finished my own TODOs. (ignore force-pushes to my branch. that was for cleanup of the history) |
jeremyong
force-pushed
the
tuple_join
branch
from
b282d6b
to
80fb380
Compare
|
Last push addresses a caught unused-parameter warning on GCC and fixes a documentation build issue I noticed. |
|
Looks great! Just one minor comment regarding RST and please run |
|
I use the clang-format extension which runs on save so we should be good there |
Address enhancement request fmtlib#1322. The overload is provided in `ranges` (original `fmt::join` exists currently in `format.h` for historical reasons. Tests for prvalue and lvalue tuple arguments as well as the empty tuple are provided in `ranges-test.cc`.
jeremyong
force-pushed
the
tuple_join
branch
from
80fb380
to
8358a5e
Compare
|
OK done. The change was squashed into the single commit as before. |
|
Merged with minor tweaks in b4f1988. Thanks! |
|
Thanks! |
I agree that my contributions are licensed under the {fmt} license, and agree to future changes to the licensing.
This addresses the enhancement proposed in #1322 .
This is my initial 10 minute pass or so for early review (needs cleanup, documentation, more tests, etc). Note that this code only works with C++17... it's been a while since I've needed to code in the earlier standards. Feel free to comment or leave feedback, thanks.