clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed.
The description confused me a bit as I thought we were going to be doing some "CommandObjectSource::DumpLinesInSymbolContexts()" stuff somewhere. But this path is really just "return the same source path from the setBreakpoints" request in the response and I am all for that. So a few nits and this will be good to go. See my inline comments. ================ Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:335-336 + llvm::json::Object source; + lldb::SBFileSpec file(sourcePath.str().c_str()); + const char *name = file.GetFilename(); + EmplaceSafeString(source, "name", name); ---------------- There might be a cheaper llvm::sys::path operation that can extract the basename from a StringRef instead of creating a SBFileSpec? ================ Comment at: lldb/tools/lldb-vscode/JSONUtils.h:207 +/// \param[in] sourcePath +/// The full path to the source to store in the JSON value. +/// ---------------- Maybe reword a bit to make sure people understand this is the sourcePath that was pass in the setBreakpoint request? ``` The path that was specified in the setBreakpoint request. ``` ================ Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1375 lldb::SBError status; + g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status)); ---------------- remove white only change ================ Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1377 g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status)); + if (status.Fail()) { ---------------- remove white only change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76968/new/ https://reviews.llvm.org/D76968 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits