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

Access to flag_to_string function #133

Closed
D00Movenok opened this issue Aug 21, 2024 · 2 comments
Closed

Access to flag_to_string function #133

D00Movenok opened this issue Aug 21, 2024 · 2 comments

Comments

@D00Movenok
Copy link
Contributor

D00Movenok commented Aug 21, 2024

Hi there!

I'm using your lib with external logging. For nicer logs representation flag_to_string function needs to be public.

To solve this problem I found two ways:

  1. Make it VM::flag_to_string, not VM::util::flag_to_string and make it public;
  2. Make VM::util structure public.

Because it is your project, going to ask you, which one do you prefer? The first one seems better to me. Will PR according to your answer :)

Also it seems that macros from boost/preprocessor.hpp (https://stackoverflow.com/a/5094430) may be used to automate the headache of creating such a big switch-case. Is it ok for you, if I'll try to rewrite the func?

@kernelwernel
Copy link
Owner

kernelwernel commented Aug 22, 2024

the thing is, that function was meant only for debug purposes to convert the uint8_t enum values into the corresponding technique as a string because of how annoying it was for me to do that manually.

I also don't really want the util struct to be public because it was never meant to be public in the first place, it's mostly meant to provide shared functionalities for the techniques. So that's a no go for me.

I'm also not really keen with the idea of adding boost, because that's going to require people to download it, making the library not ready out of the box anymore, which I highly value. And plus I'd say it's not worth sacrificing the default compatibility of the lib for all systems by adding boost as a requirement just for a debug feature. So that's also a no-go for me again.

I can definitely make VM::flag_to_string() in a public context, but I sort of have a strict guideline for myself to carefully pick what should be made public, which I'll add in probably under an hour.

Thanks for the recommendation though!

@kernelwernel
Copy link
Owner

kernelwernel commented Aug 22, 2024

@D00Movenok done: #134

I removed the "VM::" parts in the output because I assumed that was a bit redundant. Let me know if anything else is needed :)

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

No branches or pull requests

2 participants