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

Use fmt::format for loggers #1026

Open
ken-matsui opened this issue Oct 12, 2024 · 0 comments · May be fixed by #1034
Open

Use fmt::format for loggers #1026

ken-matsui opened this issue Oct 12, 2024 · 0 comments · May be fixed by #1034

Comments

@ken-matsui
Copy link
Member

ken-matsui commented Oct 12, 2024

https://github.com/poac-dev/poac/blob/23471b6c352809dff0d0655cbb47586e1c8e3358/src/Logger.hpp

For error, warn, info, debug, and trace, we'd like to use fmt::format style formatting instead of std::ostream (internal implementation details). For example, the following usage is not that clear:

poac/src/main.cc

Lines 94 to 96 in 23471b6

logger::error(
"'poac ", *itr, "' failed with exit code `", exitCode, '`'
);

We may want to do like this instead:

logger::error(
    "'poac {}' failed with exit code `{}`", *itr, exitCode
 ); 

We can use fmt::format for arguments to these loggers and we do, but the new interface will be clearer:

poac/src/Cmd/Build.cc

Lines 52 to 58 in 23471b6

logger::info(
"Compiling",
fmt::format(
"{} v{} ({})", packageName, getPackageVersion().toString(),
getProjectBasePath().string()
)
);

logger::info can be:

logger::info( 
    "Compiling", "{} v{} ({})", packageName,
    getPackageVersion().toString(),
    getProjectBasePath().string()
);

Theoretically, error, warn, debug, and trace's function arguments would be like:

(fmt::format_string<T...> fmt, T&&... args)

And info has:

(std::string_view header, fmt::format_string<T...> fmt, T&&... args)

Note that the actual implementation may be different as to conform to the current implementation style.

@yaito3014 yaito3014 linked a pull request Nov 10, 2024 that will close this issue
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 a pull request may close this issue.

1 participant