-
Notifications
You must be signed in to change notification settings - Fork 483
feat: add name de-mangling for C++ symbols in the Test Explorer #4340
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
feat: add name de-mangling for C++ symbols in the Test Explorer #4340
Conversation
@gcampbell-msft this is the PR that adds de-mangling. I have added a package to do the actual de-mangling, and I think I should not have pushed the yarn.lock file. I will revert the changes in that file and hope you and your team can do the magic to get the added package onboard. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I don't have any concerns necessarily, but being somewhat unfamiliar with the packages available in node for demangling, I wanted to ask, why did you pick this specific one?
I see that there are a couple of npm packages for it: https://www.npmjs.com/search?q=demangler, and there are a couple that have more downloads (not that more downloads = better quality), and I was curious if there was any reason specifically?
Thanks
I picked the package that:
That left the demangler-js package as most viable option in my opinion. |
@gcampbell-msft is there any further action required from my side to drive this home? |
@rjaegers No, nothing from your side, I need to get around to updating the package-lock.json on our side. Thanks for the ping. |
@rjaegers Update, I've updated the yarn.lock file and kicked off the builds, we will review and hopefully merge this PR today or next week. |
1b51bb0
This changes visible behavior
The following changes are proposed:
The purpose of this change
Currently when coverage is measured during test execution the mangled name is shown in case of C++ symbols. This PR adds name de-mangling for C++ symbols. When the symbol is not a mangled C++ name, the input is left un-touched.