[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)

[via lldb-commits]

jimingham wrote:

This is a good direction.  The API for creating the error message seems fine, a known part and a "site-specific error" is a workable division w/o being over designed.

The one thing that bugs me about this whole setup is that we know the argument type for each option, so even more of the stuff we're doing by hand here should be automated.  We should know the `--auto-continue` is a bool because we said so when we defined the option, so it's awkward we have to intervene at all.

That's a bigger problem than you are addressing now.  

The reason it comes up at all with this patch is that you also shouldn't need to add the g_bool_parsing_error_message context by hand, we should know that from the option type.  I thought about that when I saw the code:

`  if (!additional_context.empty())
    stream << ": " << additional_context;
`

I was first thinking "we should put an assert there, it shouldn't be possible to have an option parse fail and just say:

`Invalid value for -f (--foo)
`
That's really unhelpful."  But at the same time I was thinking "wait, we know the argument types, so in a lot of cases we could make a more helpful conversion error."  But that's currently fairly far from being plumbed through to the point where you could use it.

In the end I think this is fine.  As you work more on this it might be nice to add some kind of trap to catch people who don't say why a value is wrong.  And far down the road, the Option error reporting could provide a useful fallback for common option kinds like bool, int, etc.

But that can all be added.  This is a fine way to get started.

https://github.com/llvm/llvm-project/pull/82273