[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[Greg Clayton via Phabricator via lldb-commits]

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