DiscIO: Make use of fmt where applicable #9150
Conversation
| return fmt::format( | ||
| "{:12.12} {:#04x} {:#04x} {:#06x} {:#06x} {:#010x} {:#06x} {:#06x} {:#06x} {:#010x}", |
There was a problem hiding this comment.
This change in width seems a bit weird, at least I can't see any indication in the docs that the width used for the alternate form includes the prefix.
Then again, it probably doesn't matter too much since this is a debug string.
There was a problem hiding this comment.
I've tested this code, and it works correctly if I write it like this.
For what it's worth, https://fmt.dev/latest/syntax.html contains the following example (but other than that I indeed can't find anything relevant in the documentation):
fmt::format("{:#04x}", 0);
// Result: "0x00"
| StringFromFormat(Common::GetStringT("The %s partition is not properly aligned.").c_str(), | ||
| name.c_str())); | ||
| AddProblem(Severity::Medium, | ||
| fmt::format(Common::GetStringT("The {0} partition is not properly aligned."), name)); |
There was a problem hiding this comment.
Given this is used quite a bit, maybe this should be its own helper function in common, like:
template <typename... Args>
std::string GetStringTFmt(const char* string, Args&&... args)
{
return fmt::format(GetStringT(string), std::forward<Args>(args)...);
}So that fmt::format doesn't need to be typed all the time in conjunction with another function immediately one after another in several spots.
There was a problem hiding this comment.
Added. I wasn't sure whether it would fit best in StringUtil.h (because it deals with string formatting) or MsgHandler.h (because it deals with translation), but I put it in StringUtil.h.
There was a problem hiding this comment.
I moved it to MsgHandler.h because I figured we will need to include fmt/format.h in that header anyway if we migrate panic alerts over to using fmt.
|
Does anyone understand why the CMake build fails to find the fmt/format.h header now? Common does have fmt::fmt in its target_link_libraries. (I had the same problem before when I used fmt::format from DiscIO without adding fmt::fmt to DiscIO's target_link_libraries.) |
|
Those errors come from |
Fixing that seems to have made it work. |
|
Okay, I don't understand what's wrong with macOS now... |
|
The issue is that fmt checks whether |
Once nice benefit of fmt is that we can use positional arguments in localizable strings. This a feature which has been requested for the Korean translation of strings like "Errors were found in %zu blocks in the %s partition." and which will no doubt be useful for other languages too.
Once nice benefit of fmt is that we can use positional arguments in localizable strings. This a feature which has been requested for the Korean translation of strings like "Errors were found in %zu blocks in the %s partition." and which will no doubt be useful for other languages too.