lemo added inline comments.
================
Comment at: source/Interpreter/CommandInterpreter.cpp:2708-2710
+ const char *data = str.data();
+ size_t size = str.size();
+ while (size > 0 && !WasInterrupted()) {
----------------
lemo wrote:
> clayborg wrote:
> > Since we are using "llvm::StringRef" here, why not use its split
> > functionality? Something like:
> > ```
> > bool done = false;
> > while (!done) {
> > auto pair = str.split('\n');
> > auto len = pair.first.size();
> > done = pair.second.empty();
> > // Include newline if we are not done
> > if (!done)
> > ++len;
> > stream.Write(pair.first.data(),
> > }
> > ```
> >
> > This approach also avoids the issue amccarth mentioned below about not
> > ending with a newline. It is also quite a bit simpler to follow.
> I'll give it a try, thanks for the suggestion.
I forgot to reply to this: I tried using str.split() as suggested, with a small
tweak to avoid reading past the end of the first slice (++len to include the
'\n'):
while (!str.empty() && !WasInterrupted()) {
auto pair = str.split('\n');
stream.Write(pair.first.data(), pair.first.size());
if (str.size() != pair.first.size())
stream.PutChar('\n');
str = pair.second;
}
if (!str.empty()) {
stream.Printf("\n... Interrupted.\n");
}
While this version is shorter there I see a number of small problems with it:
1. It requires a close look at the StringRef::split() interface (ex. done =
pair.second.empty() is actually not the correct check since it will eat the
final \n)
2. While conceptually straightforward, the StringRef::split() interface is not
ideal in this case - see #1
3. This version assumes that stream.Write() actually writes everything (ie. not
handling the return size)
4. In order to avoid reading past the end of the first slice we need to do a
separate PutChar() for each \n
So in the end I prefer the original version, which although a bit more verbose,
tracks the invariants clearly, is potentially faster and handles strings
regardless if they are terminated with \n or not.
https://reviews.llvm.org/D37923
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
