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

Reply via email to