Fix compilation issues with clang

daantimmer commented 9 months ago

🤔 What's changed?

modified cmake files in order to be used with FetchContent. No more assumption that the project will always be the root project
reworked template code for is_container_v, does not work with clang-cl//MSVC. Made more explicit and more standard compliant
removed demangle.hpp/cpp, dead code and does not work with clang-cl/MSVC
fixed the full_match regex function as there was a mismatch between template parameters and actual parameters

⚡️ What's your motivation?

Fixes #191
Fixes #192

🏷️ What kind of change is this?

:bank: Refactoring/debt/DX (improvement to code design, tooling, documentation etc. without changing behaviour)
:bug: Bug fix (non-breaking change which fixes a defect)

♻️ Anything particular you want feedback on?

I tried to adhere to the already existing coding standard, but not sure if I did it right (there is, sadly, no accompanying clang-format file. Might be a nice todo item?)
not sure if the template magic for detecing container type is to your liking

📋 Checklist:

[x] I agree to respect and uphold the Cucumber Community Code of Conduct
[x] Users should know about my change
- [ ] I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

chybz commented 8 months ago

Hey there, been working on those fixes for some time now. That might not be aligned with your schedule but those issues are being resolved real soon now, albeit sometimes differently.

daantimmer commented 8 months ago

No problem. Take your time. In the mean time we will just use our fork with the required fixes.

daantimmer commented 8 months ago

My colleague, who just pushed an update to our fork, uses MSVC and visual studio btw.

chybz commented 8 months ago

@daantimmer @richardapeters Could you please test with main branch and report any problem back to me ? Thank you.

daantimmer commented 8 months ago

@daantimmer @richardapeters Could you please test with main branch and report any problem back to me ? Thank you.

-> https://github.com/philips-software/amp-cucumber-cpp-runner/pull/22

~~I seem to have to update some header locations. Will do later :-)~~ done, results: initial tests seem OK. I'll have to ask @richardapeters to test the branch with MSVC.

richardapeters commented 8 months ago

@chybz

I have also tested; it compiles with MSVC and runs perfectly! Thanks!

chybz commented 8 months ago

@daantimmer @richardapeters Thank you very much for reporting the issues and help me resolve them, greatly appreciated ! Had to make other adjustments beyond just compile fixes, hope you understand why I've not merged this PR. Unless there are other topics, do we all agree to close this PR and the other one on cucumber/messages ?

daantimmer commented 8 months ago

@chybz the PRs and issues may be closed as resolved.

Thank you for your time as well. This library is hugely appreciated and have given us the opportunity to create the open source runner in such a short time.

cucumber / gherkin