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.

Sign up for GitHub

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

Jump to bottom

modules missing pieces #3399

Merged
merged 1 commit into from
May 5, 2023
Merged

modules missing pieces #3399

merged 1 commit into from
May 5, 2023

Conversation

DanielaE
Copy link
Contributor

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

@DanielaE DanielaE force-pushed the feature/missing-pieces branch from b19310b to ed407d8 Compare April 23, 2023 13:47
vitaut
vitaut reviewed Apr 29, 2023
View reviewed changes
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.

include/fmt/format.h Outdated
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!

@DanielaE
modules missing pieces
728333c 
- don't export names from `detail`
- put more headers into the global module fragment
- support MSYS2 and Clang
@DanielaE DanielaE force-pushed the feature/missing-pieces branch from ed407d8 to 728333c Compare May 2, 2023 08:19
@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
Reviewers

@vitaut vitaut vitaut left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DanielaE @vitaut