[Lldb-commits] [PATCH] D22286: [LLDB] Help text overhaul

[Jim Ingham via lldb-commits]

jingham added a subscriber: jingham.
jingham requested changes to this revision.
jingham added a reviewer: jingham.
jingham added a comment.
This revision now requires changes to proceed.

This is great!  Thanks for slogging through these.  There are a few comments inline.


================
Comment at: source/Commands/CommandObjectBreakpoint.cpp:1318
@@ -1317,3 +1317,3 @@
 
-)" "The first command disables all the locations of breakpoint 1, \
+)" "The first command disables all locations matching breakpoint 1, \
 the second re-enables the first location."
----------------
Not sure what it means for locations to match a breakpoint.  Maybe "all the locations generated by breakpoint 1"?

================
Comment at: source/Commands/CommandObjectExpression.cpp:225-226
@@ -224,4 +224,4 @@
                      "expression",
-                     "Evaluate an expression in the current program context, using user defined variables and variables currently in scope.",
+                     "Evaluate an expression on the current thread.  Displays any returned value with LLDB's default formatting.",
                      nullptr,
                      eCommandProcessMustBePaused | eCommandTryTargetAPILock),
----------------
"the current thread" seems a little terse.  It is important that that the expression is run ON the current thread - because you need to know that that thread isn't going to make progress during the expression evaluation, though other threads might.  But it is equally important to know that the context for lookups is the current frame.

================
Comment at: source/Commands/CommandObjectTarget.cpp:4342-4343
@@ -4341,4 +4341,4 @@
                             "target modules search-paths",
-                            "A set of commands for operating on debugger target image search paths.",
+                            "Commands for operating on debugger target image search paths.",
                             "target modules search-paths <subcommand> [<subcommand-options>]")
     {
----------------
You were pretty consistent about using "modules" not "images".  Do you want to do that here?

================
Comment at: source/Commands/CommandObjectThread.cpp:1107-1108
@@ -1106,4 +1106,4 @@
                             "thread until",
-                            "Run the current or specified thread until it reaches a given line number or address or leaves the current function.",
+                            "Continue until a line number or address is reached by the current or specified thread.  Stops when returning from the current function as a safety measure.",
                             nullptr,
                             eCommandRequiresThread        |
----------------
I think "when returning" in this context makes it sound like it stops before the function returns.  That would be better, but pretty tricky to do.  The current command stops AFTER returning from the current function.

================
Comment at: source/Interpreter/CommandObject.cpp:690-691
@@ -689,4 +689,4 @@
 {
-    return "Breakpoint ID's consist major and minor numbers;  the major number "
+    return "Breakpoint IDs consist major and minor numbers;  the major number "
     "corresponds to the single entity that was created with a 'breakpoint set' "
     "command; the minor numbers correspond to all the locations that were actually "
----------------
Breakpoint IDs consist major and minor numbers

isn't a sentence.  Maybe "are specified by"

================
Comment at: source/Interpreter/CommandObject.cpp:695
@@ -694,2 +694,3 @@
     "3.14, meaning the 14th location set for the 3rd breakpoint.  You can specify "
     "all the locations of a breakpoint by just indicating the major breakpoint "
+    "number. A valid breakpoint ID consists either of just the major number, "
----------------
The "just" on this line doesn't add anything, I'd omit it.

================
Comment at: source/Interpreter/CommandObject.cpp:708
@@ -707,3 +707,3 @@
     "breakpoint locations under a major breakpoint, you can use the major "
     "breakpoint number followed by '.*', eg. '5.*' means all the locations under "
     "breakpoint 5.  You can also indicate a range of breakpoints by using "
----------------
Why do we support this, when "5.*" and "5" mean exactly the same thing???


http://reviews.llvm.org/D22286