Parameterize core functions on the type of the format string. #885
Conversation
include/fmt/core.h
Outdated
| } | ||
|
|
||
| template <typename Container, typename... Args> | ||
| #define FMT_CHAR(Str) typename internal::format_string_traits<Str>::char_type |
There was a problem hiding this comment.
Please add a blank line to separate the macro from the function definition.
There was a problem hiding this comment.
Ok, I've moved it up just below the traits block.
BTW: this FMT_CHAR(Str) macro really bothers me. What about replacing it with a type alias like so:
template <typename S>
using char_t = typename format_string_traits<S>::char_type;
Is there a reason not to use type aliases (like ancient gcc or so)? Even msvc from Visual Studio 2013 compiles them just fine.
There was a problem hiding this comment.
The macro is there to support gcc versions older than 4.7, e.g. gcc 4.6.4 released in 2013. I don't like it either and eventually it will be replace with the type alias you wrote or similar.
It looks like FMT_CHAR is defined twice now.
There was a problem hiding this comment.
Oops, too much git history editing without coffee. Fixed now.
include/fmt/core.h
Outdated
| /** | ||
| Prints formatted data to the file *f* which should be in wide-oriented mode | ||
|
|
||
| **Hint**:: |
There was a problem hiding this comment.
I don't think :: is correct here. It's probably better to just have a normal sentence:
Prints formatted data to the file *f*. For wide format strings, *f* should be in wide-oriented mode ...
There was a problem hiding this comment.
Well, if you say so 😄 I have no clue about rst and just copied the syntax from three lines above. Thanks for the correction and the improved wording.
DanielaE
force-pushed
the
feature/parameterize-core
branch
from
6206649
to
de947c2
Compare
Take #1 of n Signed-off-by: Daniela Engert <dani@ngrt.de>
DanielaE
force-pushed
the
feature/parameterize-core
branch
from
de947c2
to
58f8346
Compare
|
Merged in 267fdc7. Thanks! |
Take #1 of n
Signed-off-by: Daniela Engert dani@ngrt.de