[Lldb-commits] [lldb] [lldb] Use AppendMessageWithFormatv in ComandObjectWatchpoint (PR #184128)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)

<details>
<summary>Changes</summary>

Most of the AppendMessage... methods of CommandReturnObject automatically add a newline, but AppendMessageWithFormat does not. AppendMessageWithFormatv does.

This gets pretty confusing when reviewing changes to commands.

While there are use cases for building a message as you go, controlling when the newline is emitted, a lot of calls to AppendMessageWithFormat add a newline right away anyway.

Such as in the watchpoint commands. So I've converted them to equivalent AppendMessageWithFormatv calls so that:
* They have the less surprising behaviour re. newlines.
* They are in many cases more readable than the printf style notation.

---
Full diff: https://github.com/llvm/llvm-project/pull/184128.diff


1 Files Affected:

- (modified) lldb/source/Commands/CommandObjectWatchpoint.cpp (+19-19) 


``````````diff
diff --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp b/lldb/source/Commands/CommandObjectWatchpoint.cpp
index 12effed12a3cf..9d2095b0bf35d 100644
--- a/lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -209,8 +209,8 @@ class CommandObjectWatchpointList : public CommandObjectParsed {
             process_sp->GetWatchpointSlotCount();
 
         if (num_supported_hardware_watchpoints)
-          result.AppendMessageWithFormat(
-              "Number of supported hardware watchpoints: %u\n",
+          result.AppendMessageWithFormatv(
+              "Number of supported hardware watchpoints: {0}",
               *num_supported_hardware_watchpoints);
       }
     }
@@ -305,9 +305,9 @@ class CommandObjectWatchpointEnable : public CommandObjectParsed {
     if (command.GetArgumentCount() == 0) {
       // No watchpoint selected; enable all currently set watchpoints.
       target.EnableAllWatchpoints();
-      result.AppendMessageWithFormat("All watchpoints enabled. (%" PRIu64
-                                     " watchpoints)\n",
-                                     (uint64_t)num_watchpoints);
+      result.AppendMessageWithFormatv(
+          "All watchpoints enabled. ({0} watchpoints)",
+          (uint64_t)num_watchpoints);
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
     } else {
       // Particular watchpoints selected; enable them.
@@ -323,7 +323,7 @@ class CommandObjectWatchpointEnable : public CommandObjectParsed {
       for (size_t i = 0; i < size; ++i)
         if (target.EnableWatchpointByID(wp_ids[i]))
           ++count;
-      result.AppendMessageWithFormat("%d watchpoints enabled.\n", count);
+      result.AppendMessageWithFormatv("{0} watchpoints enabled.", count);
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
     }
   }
@@ -373,9 +373,9 @@ class CommandObjectWatchpointDisable : public CommandObjectParsed {
     if (command.GetArgumentCount() == 0) {
       // No watchpoint selected; disable all currently set watchpoints.
       if (target.DisableAllWatchpoints()) {
-        result.AppendMessageWithFormat("All watchpoints disabled. (%" PRIu64
-                                       " watchpoints)\n",
-                                       (uint64_t)num_watchpoints);
+        result.AppendMessageWithFormatv(
+            "All watchpoints disabled. ({0} watchpoints)",
+            (uint64_t)num_watchpoints);
         result.SetStatus(eReturnStatusSuccessFinishNoResult);
       } else {
         result.AppendError("Disable all watchpoints failed\n");
@@ -394,7 +394,7 @@ class CommandObjectWatchpointDisable : public CommandObjectParsed {
       for (size_t i = 0; i < size; ++i)
         if (target.DisableWatchpointByID(wp_ids[i]))
           ++count;
-      result.AppendMessageWithFormat("%d watchpoints disabled.\n", count);
+      result.AppendMessageWithFormatv("{0} watchpoints disabled.\n", count);
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
     }
   }
@@ -488,9 +488,9 @@ class CommandObjectWatchpointDelete : public CommandObjectParsed {
         result.AppendMessage("Operation cancelled...");
       } else {
         target.RemoveAllWatchpoints();
-        result.AppendMessageWithFormat("All watchpoints removed. (%" PRIu64
-                                       " watchpoints)\n",
-                                       (uint64_t)num_watchpoints);
+        result.AppendMessageWithFormatv(
+            "All watchpoints removed. ({0} watchpoints)",
+            (uint64_t)num_watchpoints);
       }
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
       return;
@@ -509,7 +509,7 @@ class CommandObjectWatchpointDelete : public CommandObjectParsed {
     for (size_t i = 0; i < size; ++i)
       if (target.RemoveWatchpointByID(wp_ids[i]))
         ++count;
-    result.AppendMessageWithFormat("%d watchpoints deleted.\n", count);
+    result.AppendMessageWithFormatv("{0} watchpoints deleted.", count);
     result.SetStatus(eReturnStatusSuccessFinishNoResult);
   }
 
@@ -602,9 +602,9 @@ class CommandObjectWatchpointIgnore : public CommandObjectParsed {
 
     if (command.GetArgumentCount() == 0) {
       target.IgnoreAllWatchpoints(m_options.m_ignore_count);
-      result.AppendMessageWithFormat("All watchpoints ignored. (%" PRIu64
-                                     " watchpoints)\n",
-                                     (uint64_t)num_watchpoints);
+      result.AppendMessageWithFormatv(
+          "All watchpoints ignored. ({0} watchpoints)",
+          (uint64_t)num_watchpoints);
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
     } else {
       // Particular watchpoints selected; ignore them.
@@ -620,7 +620,7 @@ class CommandObjectWatchpointIgnore : public CommandObjectParsed {
       for (size_t i = 0; i < size; ++i)
         if (target.IgnoreWatchpointByID(wp_ids[i], m_options.m_ignore_count))
           ++count;
-      result.AppendMessageWithFormat("%d watchpoints ignored.\n", count);
+      result.AppendMessageWithFormatv("{0} watchpoints ignored.", count);
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
     }
   }
@@ -741,7 +741,7 @@ class CommandObjectWatchpointModify : public CommandObjectParsed {
           ++count;
         }
       }
-      result.AppendMessageWithFormat("%d watchpoints modified.\n", count);
+      result.AppendMessageWithFormatv("{0} watchpoints modified.", count);
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
     }
   }

``````````

</details>


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