[Lldb-commits] [PATCH] D125347: Add "indexedVariables" to variables with lots of children.

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan added inline comments.


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:1035
+  // or if we have a synthetic child provider. We don't want to call
+  // "v.GetNumChildren()" on all objects as class, struct and union types don't
+  // need to be completed if they are never expanded. So we want to avoid
----------------
It sucks that `indexedVariables` has be to be filled at container variable creation time to determine the children number. 
A better DAP design would be VSCode asking for children count at container expansion time. Then we can always call `GetNumChildren()` since all children will be enumerated anyway.


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:1055
+      const char *first_child_name = v.GetChildAtIndex(0).GetName();
+      if (first_child_name && strcmp(first_child_name, "[0]") == 0)
+        object.try_emplace("indexedVariables", num_children);
----------------
Do you think if we can handle the map cases? Assuming there is a formatter for unordered_map<>, if we can detect it and provide paging as well.


================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:46
           [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }) {
-  const char *log_file_path = getenv("LLDBVSCODE_LOG");
+  const char *log_file_path = "/tmp/vscode.txt"; // getenv("LLDBVSCODE_LOG");
 #if defined(_WIN32)
----------------
I assume this is your test change? Revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125347