Skip to content

Conversation

@barcharcraz
Copy link
Contributor

adds simple tests for replacement fields and arg-id based indexing.

some fun facts:

  • it looks like make_format_args("f") does not work because format_args_store doesn't understand char (&)[N]. This is a bug that's probably a very simple fix in format_arg_store (a decay_t around line 845 is probably all that's needed). I want to talk to miscco first to see if there's anything I'm missing but I'll have a PR that does that tonight if possible.
  • commented tests are for writer functions that are to be implemented in an upcoming PR from Elnar, He'll uncomment them in his PR.

@barcharcraz barcharcraz requested a review from a team as a code owner February 22, 2021 21:27
@barcharcraz barcharcraz added cxx20 C++20 feature format C++20/23 format test Related to test code labels Feb 22, 2021
@barcharcraz barcharcraz self-assigned this Feb 22, 2021
assert(output_string == "}x");

output_string.clear();
vformat_to(back_insert_iterator(output_string), locale::classic(), "{}", make_format_args((const char*) "f"));
Copy link
Member

Choose a reason for hiding this comment

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

We conventionally avoid C-style casts everywhere (because they can do so many things, and are impossible to search for). Occurs below.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also say, that we should fix the bug and not try to work around it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're going to merge and fix the bug in a PR, just because that unblocks Elnar and I have to get a foot X-Ray before fixing the overload set

@barcharcraz barcharcraz merged commit 62e9bbe into microsoft:feature/format Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cxx20 C++20 feature format C++20/23 format test Related to test code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants