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

Add support for formatting via variadic templates where possible #25

Closed
jdale88 opened this issue Feb 27, 2014 · 6 comments
Closed

Add support for formatting via variadic templates where possible #25

jdale88 opened this issue Feb 27, 2014 · 6 comments

Comments

@jdale88
Copy link
Contributor

jdale88 commented Feb 27, 2014

I feel that C++11 variadic templates would provide a nice alternative to the streaming syntax that format uses by default.

I've implemented a version of this for my own project, and have had a go at rolling it back into format itself in a more generic form.

The change can be found here: https://github.com/jdale88/format/commit/5a2eb12a783246f87c1a0d1b5355fe8df22ec6d2

At present, the syntax for using that change is as follows (it will accept any number of parameters after the initial format string):

std::string str = fmt::VariadicFormat<fmt::Formatter<>>("Hello {0}", "World");
std::wstring wstr = fmt::VariadicFormat<fmt::Formatter<fmt::NoAction, wchar_t>>(L"Hello {0}", L"World");

I felt it best to discuss this feature before submitting a pull request, as it might be completely unsuitable, or you might want to approach it differently. This bug is really just to open up a discussion about it.

Thanks.

EDIT:
I improved upon that initial commit a bit.

std::string str = fmt::Format("Hello {0}", "World");
std::wstring wstr = fmt::Format(L"Hello {0}", L"World");
@vitaut
Copy link
Contributor

vitaut commented Mar 6, 2014

This is a very good idea. The implementation can be substantially simplified though: see https://github.com/vitaut/format/blob/master/format-test.cc#L1490 - it only needs to be parameterized on character type. What do you think?
Again, sorry that it took me so long to reply.

@jdale88
Copy link
Contributor Author

jdale88 commented Mar 6, 2014

Ah, I didn't realise a formatter could already take an initialisation list (I'm not sure that was available in the version I was using).

Yes, I agree, that is much simpler :)

So this would basically only need to add the code to work out if variadic templates were supported, and if so, add some extra Format functions like so?

#if FMT_USE_VARIADIC_TEMPLATES
template<typename... Args>
std::string Format(const StringRef &format, const Args & ... args) {
  Writer w;
  BasicFormatter<char> f(w, format.c_str(), { args... });
  return fmt::str(f);
}

template<typename... Args>
std::wstring Format(const WStringRef &format, const Args & ... args) {
  WWriter w;
  BasicFormatter<wchar_t> f(w, format.c_str(), { args... });
  return fmt::str(f);
}
#endif

Would you like me to roll that into a pull-request?

EDIT:
To be on the extra-safe side, it should probably also test for FMT_USE_INITIALIZER_LIST as well.

@vitaut
Copy link
Contributor

vitaut commented Mar 6, 2014

Exactly, it could be even simplified a little bit further by parameterizing on character type:

template<typename Char, typename... Args>
std::basic_string<Char> Format(const BasicStringRef<Char> ...);

or something like that.

Sure, feel free to submit a pull request and thanks for working on this.

@jdale88
Copy link
Contributor Author

jdale88 commented Mar 6, 2014

I had thought about templating it further like that, but it saves little in the way of code duplication and adds extra syntax to the calling code to specify the char type:

template<typename Char, typename... Args>
std::basic_string<Char> Format(const BasicStringRef<Char> &format, const Args & ... args) {
  BasicWriter<Char> w;
  BasicFormatter<Char> f(w, format.c_str(), { args... });
  return fmt::str(f);
}

// ...

std::string str2 = fmt::Format<char>("Hello {0}", "World");
std::wstring wstr2 = fmt::Format<wchar_t>(L"Hello {0}", L"World");

Based on that, I feel having the two separate functions is cleaner.

I'll try and take a look at this shortly.

@vitaut
Copy link
Contributor

vitaut commented Mar 6, 2014

Making the calling code more complicated is not worth it of course, but isn't the Char type deduced from the first argument?

@vitaut
Copy link
Contributor

vitaut commented Mar 6, 2014

Ah, it doesn't. Two overloads (one for char and another for wchar_t) is fine then.

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

No branches or pull requests

2 participants