kastiglione added inline comments.
================ Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:82 Log *m_log; + std::unique_ptr<Progress> m_current_progress_up; + std::vector<std::string> m_module_build_stack; ---------------- aprantl wrote: > kastiglione wrote: > > mib wrote: > > > `Progress` makes use of RAII to complete the progress report event. > > > > > > I think a local variable would be more suitable. > > Since this is a callback, and the work being done doesn't happen in this > > function, I reasoned that that a local wouldn't match the semantics. Since > > this function is so short lived, the Progress would be immediately > > destructed and completed, but the work of building the module is not yet > > completed. Also, when an event is completed, the console text is cleared. > > As a result, the user doesn't see which module is currently building. > Might be good to comment this? by extending the life of the Progress, this code, which clears the console of the current progress message, is deferred. https://github.com/apple/llvm-project/blob/d0fbbd7e14e13b0f915eefdb20b57a60eb177039/lldb/source/Core/Debugger.cpp#L1945-L1949 ``` if (data->GetCompleted() == data->GetTotal()) { // Clear the current line. output->Printf("\x1B[2K"); output->Flush(); return; ``` This screen clearing deferred until either a new remark_module_build is emitted, or a remark_module_build_done is emitted. This allows the current message to stay on for screen, showing the user what's being built and giving them a sense for how long it's taking. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140056/new/ https://reviews.llvm.org/D140056 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
