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

Support compiling {fmt} as named module #2235

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

DanielaE
Copy link
Contributor

Start by tagging official API for module export

@DanielaE DanielaE force-pushed the feature/named-module branch 2 times, most recently from 8bc95b8 to 1948ae5 Compare April 16, 2021 12:26
Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR. LGTM, just two minor comments inline.

@@ -231,6 +235,7 @@ struct color_type {
} // namespace detail

/** A text style consisting of foreground and background colors and emphasis. */
FMT_MODULE_EXPORT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not include this class in the FMT_MODULE_EXPORT_BEGIN/FMT_MODULE_EXPORT_END block below and remove FMT_MODULE_EXPORT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, why not? I'll tell ya: staring at code for extended periods of time didn't prevent me from being silly.

template <typename Char>
class basic_printf_parse_context : public basic_format_parse_context<Char> {
using basic_format_parse_context<Char>::basic_format_parse_context;
};
// already exported through "ostream.h" above
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this comment, the one above can apply to both contexts.

 * functions
 * classes
 * UDLs
 * other declarations

Export everything in namespace 'fmt' from core.h and format.h
@DanielaE DanielaE force-pushed the feature/named-module branch from 1948ae5 to 16a4cc6 Compare April 16, 2021 17:16
@vitaut vitaut merged commit f4bbc54 into fmtlib:master Apr 16, 2021
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 this pull request may close these issues.

2 participants