[Lldb-commits] [lldb] [lldb] Store the command in the CommandReturnObject (PR #125132)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Feb 3 09:50:32 PST 2025
jimingham wrote:
> This idea has bothered me from the beginning, but I didn't want to say anything, as it was mostly just a feeling. After seeing this patch, I think I can put some words behind it.
>
> The thing I don't like about this patch is the repetition/redundancy it creates. Like, you first create the CommandReturnObject, give it the command it's hold the result of. Then, you pass that object into HandleCommand, but that's not all you give it -- the function takes the string of the command, again.
>
> It's not the end of the world, but I think it's unfortunate, and while it works nice for your current use case, I don't think it makes that much sense for other usages. For example, if the `CommandReturnObject` had always contained the command part, would the signature of a python command still be `def cmd(debugger, command, result, dict):`, or would we have expected the command to retrieve it from the result object?
>
> It's hard to say, but what I think we can say is that if you make your current callback take the "command" and the "result of that command" as two separate arguments, then it will at least be consistent with the signature above.
One difference here is that what we're putting in the command is the command as the user typed it. That isn't what we pass to the command - the command gets the result after alias substitution. Also, though I was late providing it in the future when possible we really want command writers to use the parsed form for everything but raw commands. So that's even further from the raw string we're adding to the SBCommandReturnObject. The string we're putting in there is really only useful for logging.
https://github.com/llvm/llvm-project/pull/125132
More information about the lldb-commits
mailing list