bulbazord wrote: > This version is still a little awkward because there's an ambiguity about > what the incoming string being empty means. In the VerifyCommandOptionValue, > it means "No Value" and the return is not distinguishable from "error" - but > in a bunch of the places where you use VerifyCommandOptionValue an empty > string just means a value wasn't provided, so you end up with constructs > where you set the optional (checking the incoming string for `empty`) then > check the input string again so you can tell empty apart from error, THEN > make up the error on the outside w/o knowing what actually was wrong with the > string.
I did this intentionally so that `VerifyCommandOptionValue` would not attempt to interpret the result. The caller knows whether or not its input is empty, so it's able to interpret the results of `VerifyCommandOptionValue` however it wants. Maybe `VerifyCommandOptionValue` is not a good name for this? It seems to just be trying to take a string and trying to get a bool out of it. Thinking about it further, I'm starting to question the value of `VerifyCommandOptionValue`... Maybe we should be using `OptionArgParser::ToBoolean`? That seems to be the end goal anyway. > I wonder if it would be better to add a Status to the call. Then empty input > -> "Empty Optional but no error"; error in parsing -> "Empty Optional but > error" and result means success. Then you wouldn't have to do two checks on > the input string in different places, or make up the error string. https://github.com/llvm/llvm-project/pull/79901 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
