JDevlieghere added inline comments.
================
Comment at: lldb/include/lldb/Core/UserSettingsController.h:62-64
virtual void DumpAllPropertyValues(const ExecutionContext *exe_ctx,
- Stream &strm, uint32_t dump_mask);
+ Stream &strm, uint32_t dump_mask,
+ bool is_json = false);
----------------
A flag doesn't scale if we ever decide we need another format. An enum would
solve that and would eliminate the need for a comment `/*is_json=*/. I think
the two values could be `Plain` (or `Text`) and `JSON`.
================
Comment at: lldb/include/lldb/Interpreter/OptionValue.h:86-87
+ // TODO: make this function pure virtual after implementing it in all
+ // child classes.
+ virtual llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) {
----------------
Is this done?
================
Comment at: lldb/source/Core/UserSettingsController.cpp:63
+ if (is_json) {
+ llvm::json::Value json = properties_sp->ToJSON(exe_ctx);
+ strm.Printf("%s", llvm::formatv("{0:2}", json).str().c_str());
----------------
Earlier you said that `ToJSON` should never return NULL. We should enforce that
with an assert here.
================
Comment at: lldb/source/Interpreter/OptionValueArray.cpp:80-81
+ llvm::json::Array json_array;
+ const uint32_t size = m_values.size();
+ for (uint32_t i = 0; i < size; ++i)
+ json_array.emplace_back(m_values[i]->ToJSON(exe_ctx));
----------------
================
Comment at: lldb/source/Interpreter/OptionValueDictionary.cpp:89-91
+ for (const auto &value : m_values) {
+ dict.try_emplace(value.first.GetCString(), value.second->ToJSON(exe_ctx));
+ }
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133038/new/
https://reviews.llvm.org/D133038
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
