yelite commented on code in PR #12344:
URL: https://github.com/apache/tvm/pull/12344#discussion_r941674874


##########
src/script/printer/base_doc_printer.cc:
##########
@@ -23,19 +23,256 @@ namespace tvm {
 namespace script {
 namespace printer {
 
-DocPrinter::DocPrinter(int indent_spaces) : indent_spaces_(indent_spaces) {}
+namespace {
 
-void DocPrinter::Append(const Doc& doc) { PrintDoc(doc); }
+void SortAndMergeSpans(std::vector<ByteSpan>* spans) {
+  if (spans->empty()) {
+    return;
+  }
+  std::sort(spans->begin(), spans->end());
+  auto last = spans->begin();
+  for (auto cur = spans->begin() + 1; cur != spans->end(); ++cur) {
+    if (cur->first > last->second) {
+      *++last = *cur;
+    } else if (cur->second > last->second) {
+      last->second = cur->second;
+    }
+  }
+  spans->erase(++last, spans->end());
+}
+
+size_t GetTextWidth(const std::string& text, const ByteSpan& span) {
+  // FIXME: this only works for ASCII characters.
+  // To do this "correctly", we need to parse UTF-8 into codepoints
+  // and call wcwidth() or equivalent for every codepoint.
+  size_t ret = 0;
+  for (size_t i = span.first; i != span.second; ++i) {
+    if (isprint(text[i])) {
+      ret += 1;
+    }
+  }
+  return ret;
+}
+
+size_t MoveBack(size_t pos, size_t distance) { return distance > pos ? 0 : pos 
- distance; }
+
+size_t MoveForward(size_t pos, size_t distance, size_t max) {
+  return distance > max - pos ? max : pos + distance;
+}
+
+size_t GetLineIndex(size_t byte_pos, const std::vector<size_t>& line_starts) {
+  auto it = std::upper_bound(line_starts.begin(), line_starts.end(), byte_pos);
+  return (it - line_starts.begin()) - 1;
+}
+
+using UnderlineIter = typename std::vector<ByteSpan>::const_iterator;
+
+ByteSpan PopNextUnderline(UnderlineIter* next_underline, UnderlineIter 
end_underline) {
+  if (*next_underline == end_underline) {
+    return {std::numeric_limits<size_t>::max(), 
std::numeric_limits<size_t>::max()};
+  } else {
+    return *(*next_underline)++;
+  }
+}
+
+void PrintChunk(const std::pair<size_t, size_t>& lines,

Review Comment:
   Maybe something like `lines_range` will be a more appropriate name for this 
variable?



##########
src/script/printer/base_doc_printer.cc:
##########
@@ -23,19 +23,256 @@ namespace tvm {
 namespace script {
 namespace printer {
 
-DocPrinter::DocPrinter(int indent_spaces) : indent_spaces_(indent_spaces) {}
+namespace {
 
-void DocPrinter::Append(const Doc& doc) { PrintDoc(doc); }
+void SortAndMergeSpans(std::vector<ByteSpan>* spans) {
+  if (spans->empty()) {
+    return;
+  }
+  std::sort(spans->begin(), spans->end());
+  auto last = spans->begin();
+  for (auto cur = spans->begin() + 1; cur != spans->end(); ++cur) {
+    if (cur->first > last->second) {
+      *++last = *cur;
+    } else if (cur->second > last->second) {
+      last->second = cur->second;
+    }
+  }
+  spans->erase(++last, spans->end());
+}
+
+size_t GetTextWidth(const std::string& text, const ByteSpan& span) {
+  // FIXME: this only works for ASCII characters.
+  // To do this "correctly", we need to parse UTF-8 into codepoints
+  // and call wcwidth() or equivalent for every codepoint.
+  size_t ret = 0;
+  for (size_t i = span.first; i != span.second; ++i) {
+    if (isprint(text[i])) {
+      ret += 1;
+    }
+  }
+  return ret;
+}
+
+size_t MoveBack(size_t pos, size_t distance) { return distance > pos ? 0 : pos 
- distance; }
+
+size_t MoveForward(size_t pos, size_t distance, size_t max) {
+  return distance > max - pos ? max : pos + distance;
+}
+
+size_t GetLineIndex(size_t byte_pos, const std::vector<size_t>& line_starts) {
+  auto it = std::upper_bound(line_starts.begin(), line_starts.end(), byte_pos);
+  return (it - line_starts.begin()) - 1;
+}
+
+using UnderlineIter = typename std::vector<ByteSpan>::const_iterator;
+
+ByteSpan PopNextUnderline(UnderlineIter* next_underline, UnderlineIter 
end_underline) {
+  if (*next_underline == end_underline) {
+    return {std::numeric_limits<size_t>::max(), 
std::numeric_limits<size_t>::max()};
+  } else {
+    return *(*next_underline)++;
+  }
+}
+
+void PrintChunk(const std::pair<size_t, size_t>& lines,
+                const std::pair<UnderlineIter, UnderlineIter>& underlines, 
const std::string& text,
+                const std::vector<size_t>& line_starts, const 
DocPrinterOptions& options,
+                size_t line_number_width, std::string* out) {
+  UnderlineIter next_underline = underlines.first;
+  ByteSpan current_underline = PopNextUnderline(&next_underline, 
underlines.second);
+
+  for (size_t line_idx = lines.first; line_idx < lines.second; ++line_idx) {
+    if (options.print_line_numbers) {
+      std::string line_num_str = std::to_string(line_idx + 1);
+      line_num_str.push_back(' ');
+      for (size_t i = line_num_str.size(); i < line_number_width; ++i) {
+        out->push_back(' ');
+      }
+      *out += line_num_str;
+    }
+
+    size_t line_start = line_starts.at(line_idx);
+    size_t line_end =
+        line_idx + 1 == line_starts.size() ? text.size() : 
line_starts.at(line_idx + 1);
+    out->append(text.begin() + line_start, text.begin() + line_end);
+
+    bool printed_underline = false;
+    size_t line_pos = line_start;
+    bool printed_extra_caret = 0;
+    while (current_underline.first < line_end) {
+      if (!printed_underline) {
+        *out += std::string(line_number_width, ' ');
+        printed_underline = true;
+      }
+
+      size_t underline_end_for_line = std::min(line_end, 
current_underline.second);
+      size_t num_spaces = GetTextWidth(text, {line_pos, 
current_underline.first});
+      if (num_spaces > 0 && printed_extra_caret) {
+        num_spaces -= 1;
+        printed_extra_caret = false;
+      }
+      *out += std::string(num_spaces, ' ');

Review Comment:
   Should it skip printing carets under the indentation spaces for multi-line 
underline?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to