Skip to content
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

Provide more overloads for the wide string flavour #867

Merged
merged 1 commit into from Sep 23, 2018

Conversation

DanielaE
Copy link
Contributor

Signed-off-by: Daniela Engert dani@ngrt.de

@vitaut
Copy link
Contributor

vitaut commented Sep 19, 2018

Thanks for the PR, but we are trying to move from overloads to function templates in order to support more character types and reduce copy-paste. Please see

format(const String &format_str, const Args & ... args) {
for an example.

@DanielaE
Copy link
Contributor Author

Great - I totally love to see this happening! I've already changed my code with respect to the format(rgb color, ...) family, now on to the vprint_rgb(rgb color, ...) family.

@DanielaE DanielaE force-pushed the feature/more-wide-overloads branch 3 times, most recently from 1d676f4 to fa9c80a Compare September 20, 2018 08:02
@DanielaE
Copy link
Contributor Author

@vitaut Victor, do you have an idea why gcc 4.8 is failing to compile the templated printf(rgb color, ...) function, while all other compilers have no problems with it?

@vitaut
Copy link
Contributor

vitaut commented Sep 21, 2018

gcc 4.8 is finicky. You need to explicitly construct basic_format_args to make it happy (see https://github.com/fmtlib/fmt/blob/master/include/fmt/core.h#L1447 for example).

void vprint_rgb(rgb fd, rgb bg, string_view format, format_args args);
namespace internal {
FMT_CONSTEXPR_DECL const char FOREGROUND_COLOR[] = "\x1b[38;2;000;000;000m";
FMT_CONSTEXPR_DECL const char BACKGROUND_COLOR[] = "\x1b[48;2;000;000;000m";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should go to internal::data. Also you don't need the 000;000;000m part because you construct it in ansi_color_escape.


template <typename Char>
struct ansi_color_escape {
FMT_CONSTEXPR ansi_color_escape(rgb color, const char esc[]) FMT_NOEXCEPT {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: here and elsewhere const char [] -> const char *

private:
Char buffer[sizeof(FOREGROUND_COLOR)];

FMT_CONSTEXPR void to_esc(uint8_t c, Char out[], int offset,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest getting rid of the offset parameter and passing buffer pointer adjusted by offset instead.

@DanielaE
Copy link
Contributor Author

@vitaut Thanks, Victor, for the tip regarding gcc 4.8 - it's happy now, too. I've incorporated the changes that you requested, as well.

…' on the type of the format string.

Signed-off-by: Daniela Engert <dani@ngrt.de>
@vitaut vitaut merged commit 73c53d7 into fmtlib:master Sep 23, 2018
@vitaut
Copy link
Contributor

vitaut commented Sep 23, 2018

Thanks!

iz

@DanielaE DanielaE deleted the feature/more-wide-overloads branch September 23, 2018 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants