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

modules missing pieces #3399

Merged
merged 1 commit into from
May 5, 2023
Merged

Conversation

DanielaE
Copy link
Contributor

  • don't export names from detail
  • put more headers into the global module fragment
  • support MSYS2 and Clang

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.

Thanks for the PR! Mostly looks good, just one comment inline.

Comment on lines 1042 to 1045
FMT_BEGIN_DETAIL_NAMESPACE
FMT_API bool write_console(std::FILE* f, string_view text);
FMT_API void print(std::FILE*, string_view);
} // namespace detail
FMT_END_DETAIL_NAMESPACE
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use FMT_{BEGIN,END}_EXPORT instead of "magically" folding exports into namespace macros.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, will do!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

- don't export names from `detail`
- put more headers into the global module fragment
- support MSYS2 and Clang
@vitaut vitaut merged commit 192df93 into fmtlib:master May 5, 2023
@vitaut
Copy link
Contributor

vitaut commented May 5, 2023

Thank you!

@DanielaE DanielaE deleted the feature/missing-pieces branch May 8, 2023 06:20
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