Implement AddAndRegisterDefaultEnumOption and propagate to codebase.#3903
Implement AddAndRegisterDefaultEnumOption and propagate to codebase.#3903B1ueber2y merged 8 commits intocolmap:mainfrom
Conversation
| std::string_view (*to_string_fn)(EnumT), | ||
| EnumT (*from_string_fn)(std::string_view), |
There was a problem hiding this comment.
Why not std::function<EnumT(std::string_view)>?
There was a problem hiding this comment.
In enum_utils.h we have overloaded the EnumTypeToString function with both int and EnumType, and because of this using std::function always throws.
To make it work we need to do static_cast<std::string_view(*)(FeatureExtractorType)>(FeatureExtractorTypeToString) at each call, which is very inconvenient.
There was a problem hiding this comment.
Maybe we should not overload but use a different name? E.g. FeatureExtractorTypeToString and FeatureExtractorIntTypeToString ?
There was a problem hiding this comment.
I think the overloading is quite conveient in practice. Lets keep it as it is now?
| }; | ||
|
|
||
| // Register as a string option pointing to our storage | ||
| AddDefaultOption(name, &info->value, help_text); |
There was a problem hiding this comment.
Wondering whether we can use the existing enum macros to automatically compile a help text with all the available enum values as "{VALUE_A, VALUE_B, ...}"?
There was a problem hiding this comment.
This is possible by adding another macro (e.g., ``xxxTypeValuesString) in enum_utils. However, it s a bit tricky what we should do for UNDEFINED and INVALID. It s unclear to me whether just filtering them is the right approach because they may be useful.
This feature was added in #3465 but never implemented.