[Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.

[Jim Ingham via lldb-commits]

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

See inlined comments.  Otherwise this is great.


================
Comment at: include/lldb/Core/SourceManager.h:34-35
@@ -33,3 +33,4 @@
   public:
-    File(const FileSpec &file_spec, Target *target);
+    File(const FileSpec &file_spec, Target *target,
+         lldb::DebuggerSP debugger_sp);
     ~File();
----------------
A target always has a debugger, so given a target you can always get to the debugger.  If you have another use case where you need to make this File, but only have a DebuggerSP, can you make a constructor that takes a FileSpec & a DebuggerSP, but not a target?  That seems clearer to me.

================
Comment at: source/Commands/CommandObjectSource.cpp:1155-1158
@@ -1152,2 +1154,6 @@
 
+          const uint32_t column =
+              m_interpreter.GetDebugger().GetStopShowColumn()
+                  ? sc.line_entry.column
+                  : 0;
           target->GetSourceManager().DisplaySourceLinesWithLineNumbers(
----------------
I think I will occasionally want to see the source listing w/o the column stop info (e.g. to cut & paste it somewhere) so it might be convenient to have a command option that overrides the setting.  It's always annoying to have to go change & reset a setting to do this, plus then I can't make aliases for the different behaviors.


https://reviews.llvm.org/D20835