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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
