-
Notifications
You must be signed in to change notification settings - Fork 288
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
Apply the normal "to_string" pattern in more places. #1521
Conversation
Co-authored-by: Thomas1664 <46387399+Thomas1664@users.noreply.github.com>
@@ -18,3 +18,11 @@ VCPKG_MSVC_WARNING(disable : 6239 4702) | |||
#include <fmt/format.h> | |||
#include <fmt/ranges.h> | |||
VCPKG_MSVC_WARNING(pop) | |||
|
|||
template<class T> | |||
std::string adapt_to_string(const T& val) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a constexpr functor object?
constexpr struct AdaptToString
{
template<class T>
std::string operator()(const T& val) {
std::string result;
val.to_string(result);
return result;
}
} adapt_to_string;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe anything else would want the name adapt_to_string to require such ADL defenses?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, to put this another way: Maybe. To accomplish what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make it "better" when used in functional style like .map(adapt_to_string)
@@ -9,11 +9,13 @@ namespace vcpkg | |||
|
|||
void serialize(const StatusParagraph& pgh, std::string& out_str) | |||
{ | |||
auto want_literal = to_string_literal(pgh.want); | |||
auto state_literal = to_string_literal(pgh.state); | |||
serialize(pgh.package, out_str); | |||
out_str.append("Status: ") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out_str.append("Status: ") | |
Strings::append(out_str, "Status: ", want_literal, " ok ", state_literal, "\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to do this because in #1514 I replaced all of this with the normal 'the way we write paragraph fields' function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
append_paragraph_field(ParagraphIdStatus, pgh.status.to_string(), out);
Extracted from #1514