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 multiple arguments #94

Merged
merged 4 commits into from
May 26, 2020
Merged

Support multiple arguments #94

merged 4 commits into from
May 26, 2020

Conversation

ShikChen
Copy link
Contributor

Fixes #2.

@sharkdp
Copy link
Owner

sharkdp commented Apr 26, 2020

Thank you very much for your contribution. This looks great!

I haven't reviewed it in detail, as there seem to be some issues:

  • The tests on Windows currently fail with:
    [..macro\tests\demo.cpp:66 (main)] ====== multiple arguments
    [..macro\tests\demo.cpp:68 (main)] The number of arguments mismatch, please check unprotected comma
    [..macro\tests\demo.cpp:68 (main)] test_int, (std::vector<int>{2, 3, 4}), test_string = 42 (int&)
    [..macro\tests\demo.cpp:68 (main)] Y�����U��j�h�m� = {2, 3, 4} (std::vector<int>)
    
  • The displayed types seem to have an additional reference attached as compared to before:
    before:
    [..macro/tests/demo.cpp:34 (main)] test_int = 42 (int)
    [..macro/tests/demo.cpp:35 (main)] test_float = 3.14 (const float)
    [..macro/tests/demo.cpp:36 (main)] test_bool = false (const bool)
    
    on this branch:
    [..macro/tests/demo.cpp:34 (main)] test_int = 42 (int&)
    [..macro/tests/demo.cpp:35 (main)] test_float = 3.14 (const float&)
    [..macro/tests/demo.cpp:36 (main)] test_bool = false (const bool&)
    

@ShikChen
Copy link
Contributor Author

Thanks, I will try to fix those issues :)

@ShikChen ShikChen changed the title Support multiple arguments. Support multiple arguments Apr 26, 2020
@ShikChen
Copy link
Contributor Author

Both issues should be fixed now. Please help review the PR, thanks :)

@sharkdp
Copy link
Owner

sharkdp commented May 1, 2020

Thank you very much for the updates!

It might take some weeks until I find the time to properly review this.

@ShikChen
Copy link
Contributor Author

Friendly ping :)

Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

This is fantastic! Thank you very much!

@sharkdp sharkdp merged commit 7efa2e3 into sharkdp:master May 26, 2020
@ShikChen
Copy link
Contributor Author

Thanks for the review!

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.

Support for multiple comma-separated arguments?
2 participants