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: > 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. Thoughts? https://reviews.llvm.org/D37923 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits