clayborg wrote:

> @clayborg , I actually insist on aborting on failure for this feature. You 
> mention:
> 
> > It would be nice if the RunLLDBCommands function could take an extra bool 
> > &fatal_error that clients could check after running when needed to decide 
> > what to do if this is set to true due to any ! prefixed commands failing.
> 
> That would imply that some commands would have a specific behavior upon a 
> failed ! command, and other commands would have a different behavior. That 
> makes this feature a bit hard to use by actual users, as it wouldn't have a 
> consistent behavior, unlike ? commands.
> 
> What I wanted to achieve with ! was to have a way for users to tell the 
> lldb-dap: hey, this command is critical for my flow, abort if it fails. 
> Having a mild behavior instead of aborting kind of defeats the purpose of 
> this feature.

Yeah but does aborting tell the client anything useful? Lets say we are in the 
middle of doing an `attach` packet and an attach command fails, what do we see 
in the IDE? Probably something like "DAP crashed" and the user is going to need 
to know that they should check the debug console? I would much rather have the 
`attach` packet return an erorr that says "error: one of your required 
'attachCommands' failed, check the debug console for details", or during 
`launch` seeing "error: one of your required 'launchCommands' failed, check the 
debug console for details".

If you look at how `request_attach` and `request_lauch` are configured, they do 
most of the `g_dap.RunXXXCommands` and the both packets allow us to return an 
error that would make it much more clear what went wrong.

There are a few other areas that run commands like `SendTerminatedEvent()` that 
calls `g_dap.RunTerminateCommands();` and `SendThreadStoppedEvent()` that can 
call `g_dap.RunStopCommands();`, but other than that they are mostly in 
`launch` or `attach`. 

So I don't see how just crashing the DAP helps in any way when we can easily 
return a very clear error to either `launch` or `attach` and the user will know 
exactly what went wrong. 

https://github.com/llvm/llvm-project/pull/74808
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to