[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

LGTM with minor comments, this is a big set of changes, I would prefer if one of the other reviewers also gave it a second look.



================
Comment at: lldb/source/DataFormatters/StringPrinter.cpp:49
+
+  const uint8_t *GetBytes() const { return &m_data[0]; }
+
----------------
just `m_data` it will decay to a pointer. 


================
Comment at: lldb/source/DataFormatters/StringPrinter.cpp:268
   case GetPrintableElementType::UTF8:
-    return [](uint8_t *buffer, uint8_t *buffer_end,
-              uint8_t *&next) -> StringPrinter::StringPrinterBufferPointer {
-      return GetPrintable(StringPrinter::StringElementType::UTF8, buffer,
-                          buffer_end, next);
+    return [escape_style](uint8_t *buffer, uint8_t *buffer_end,
+                          uint8_t *&next) -> DecodedCharBuffer {
----------------
It feels like the two lambdas here could be one by also capturing `elem_type`, it would make the lambda larger but is that a concern? Not a big deal just feels like needlessly duplicate code. 


================
Comment at: lldb/source/DataFormatters/StringPrinter.cpp:554
+template <>
+bool StringPrinter::ReadBufferAndDumpToStream<StringElementType::UTF8>(
+    const ReadBufferAndDumpToStreamOptions &options) {
----------------
Super nitpick, can we switch order of the `UTF8` and the `ASCII` specialization so that they appear in the same order as the `ReadStringAndDumpToStream` above.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77843/new/

https://reviews.llvm.org/D77843