clayborg added a comment.

The changes look fine from a quick read. See inline comments



================
Comment at: lldb/source/Commands/CommandObjectCommands.cpp:1817
+      : CommandObjectParsed(
+            interpreter, "command multiword add",
+            "Add a multiword command to lldb.  Adding to built-"
----------------
can't this just be "command add" instead of "command multiword add"? 


================
Comment at: lldb/source/Commands/CommandObjectCommands.cpp:1959
+      : CommandObjectParsed(
+            interpreter, "command multiword delete",
+            "Delete a multiword command previously added to "
----------------
Could we just modify the existing "command delete" instead of adding a new 
command here?


================
Comment at: lldb/source/Commands/CommandObjectCommands.cpp:2091-2092
                  CommandObjectSP(new 
CommandObjectCommandsDelete(interpreter)));
+  LoadSubCommand("multiword", CommandObjectSP(new 
CommandObjectCommandMultiword(
+                                  interpreter)));
   LoadSubCommand(
----------------
"multiword" makes sense to us LLDB software folks, not sure it will resonate 
with general public. See comments above about maybe doing "command add" and 
then modifying "command delete" to also handle multiword duties


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110298/new/

https://reviews.llvm.org/D110298

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to