jingham marked 8 inline comments as done.
jingham added inline comments.
================
Comment at: lldb/include/lldb/Interpreter/OptionValueFileColonLine.h:26
+
+ // Virtual subclass pure virtual overrides
+
----------------
JDevlieghere wrote:
> I think these comments add very little, if anything. The `override` keyword
> already conveys the same information. Same for the comment below.
I removed them here, but note they were just copied from OptionValueFileSpec.h.
We started with some header file template that had these way back in the day,
and they are all over. If we don't want them around then it would be better to
just do that as a patch and not piecemeal.
================
Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:67
+ if (value.size() > 0) {
+ // This is in the form filename:linenumber:column
+ // I wish we could use filename:linenumber.column, that would make the
----------------
JDevlieghere wrote:
> period
Ah, nuts. I forgot the period. I'll add it before checking this in.
================
Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:74
+
+ // First separate the file and the line specification:
+ llvm::StringRef right_of_last_colon;
----------------
JDevlieghere wrote:
> I think parsing this would be a lot easier with a regex:
> `([^:]+):(\d+)(:(\d+))?`. What do you think?
A regex seems overpowered for what I'm doing here, plus that might tempt people
to add more stuff to the specifier and I don't want to back into -y being the
gdb breakpoint specifier mini-language...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83975/new/
https://reviews.llvm.org/D83975
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
