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
Common: Migrate logging to fmt #9187
Conversation
Continues the migration of our code over to the fmt logger.
| @@ -221,8 +221,8 @@ HttpRequest::Response HttpRequest::Impl::Fetch(const std::string& url, Method me | |||
| curl_easy_getinfo(m_curl.get(), CURLINFO_RESPONSE_CODE, &response_code); | |||
| if (response_code != 200) | |||
| { | |||
| ERROR_LOG(COMMON, "Failed to %s %s: server replied with code %li and body\n\x1b[0m%.*s", type, | |||
| url.c_str(), response_code, static_cast<int>(buffer.size()), buffer.data()); | |||
| ERROR_LOG_FMT(COMMON, "Failed to {} {}: server replied with code {} and body\n\x1b[0m{:.{}}", | |||
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.
Just wondering, why does this one have an ANSI reset code in it?
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 legitimately don't know
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.
For the record:
[23:44:56] <@Bh44L> leoetlino: do you remember why that log message in 9187 has an ANSI reset code in it?
[23:45:16] <@Bh44L> came from 5599
[23:47:05] <@leoetlino> uh, I think it was to make the response body stand out more in stderr output
[23:48:01] <@leoetlino> it's not strictly required, but it makes it easier to see where the body begins and ends when you have several ERROR_LOGs in a row
[23:56:40] <@Bh44L> oh, right, log goes to console as well
[23:56:42] <@Bh44L> ¯\_(ツ)_/¯
| #ifdef _WIN32 | ||
| if (CopyFile(UTF8ToTStr(source_path).c_str(), UTF8ToTStr(destination_path).c_str(), FALSE)) | ||
| return true; | ||
|
|
||
| ERROR_LOG(COMMON, "Copy: failed %s --> %s: %s", source_path.c_str(), destination_path.c_str(), | ||
| GetLastErrorString().c_str()); | ||
| ERROR_LOG_FMT(COMMON, "Copy: failed %s --> %s: %s", source_path, destination_path, |
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.
This looks like the problem.
Fixes a recent regression: dolphin-emu#9187 (comment)
Fixes a recent regression: dolphin-emu#9187 (comment)
Continues the migration of our code over to the fmt logger.