labath added inline comments.
================
Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:120-127
+ // The setting value may have whitespace, double-quotes, or single-quotes
+ // around the file path to indicate that internal spaces are not word
+ // breaks. Strip off any ws & quotes from the start and end of the file
+ // path - we aren't doing any word // breaking here so the quoting is
+ // unnecessary. NB this will cause a problem if someone tries to specify
+ // a file path that legitimately begins or ends with a " or ' character,
+ // or whitespace.
----------------
This seems excessive. The command interpreter should already handle the word
splitting and quotes, which means that things like `br set -y "f i l
e.cpp":12:23` will work without this. And it can handle escaping properly, so
`br set -y "\"file.cpp:12:23"` would work, if this code wouldn't get in the way.
================
Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:129
+ m_value_was_set = true;
+ m_file_spec.SetFile(file_name.str(), FileSpec::Style::native);
+ NotifyValueChanged();
----------------
`SetFile` already accepts a StringRef.
================
Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:74
+
+ // First separate the file and the line specification:
+ llvm::StringRef right_of_last_colon;
----------------
jingham wrote:
> 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...
I am not a fan of regex parsing, but I still can't escape the feeling that this
should be easier. Maybe a utility function would help:
```
bool chop_number(StringRef &str, uint32_t &num) {
auto parts = str.rsplit(':');
if (to_integer(parts.second, num)) {
str = parts.first;
return true;
}
return false;
}
...
if (!input.contains(':')
one_colon_expected();
if (!chop_number(input, col))
bad_line_number(); // This complains about line numbers because col will be
promoted to a line number if the second chop_number fails.
if (!chop_number(input, line)) {
line = col;
col = 0;
}
file = input;
```
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
