bulbazord added inline comments.
================
Comment at: lldb/source/Host/macosx/objcxx/Host.mm:337
+
+ LLDB_LOG(log, "Sending {0}:{1} to external editor", file_path, line_no);
+
----------------
JDevlieghere wrote:
> bulbazord wrote:
> > nit: Move log line below checking if the file_path is empty? If the
> > file_spec is empty we may see strange log lines like "Sending :0 to
> > external editor" which is just noise.
> I actually did that on purpose, so we could tell from the logs that it's
> empty. It didn't seem worth adding a whole separate log line for, but if you
> think `:10` looks weird, I'm happy to do it.
I think a separate log line would be easier to read for somebody not familiar
with this codepath. It would be confusing otherwise. Something like
`"OpenFileInExternalEditor called with empty path"`? Or maybe you can keep the
log line but change it to:
```
LLDB_LOG(log, "Sending {0}:{1} to external editor", file_path.empty() ?
"<invalid>" : file_path, line_no);
================
Comment at: lldb/source/Host/macosx/objcxx/Host.mm:387-388
+ static std::once_flag g_once_flag;
+ std::call_once(g_once_flag, [&]() {
+ if (const char *external_editor = ::getenv("LLDB_EXTERNAL_EDITOR")) {
+ LLDB_LOG(log, "Looking for external editor: {0}", external_editor);
----------------
JDevlieghere wrote:
> bulbazord wrote:
> > I know you're preserving existing behavior here (or rather, fixing its
> > faulty implementation), but I think it would be nice if you didn't have to
> > restart your session and add an environment variable to get this working
> > with an existing debug session. Or maybe make it a setting?
> I think we're trying to mimic the `EDITOR` and `VISUAL` environment variables
> but I agree a setting would be better. I can add that in a follow-up. Which
> one should take preference if you have both?
I would say a setting should probably take priority? That way you can change it
during your session if desired.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149472/new/
https://reviews.llvm.org/D149472
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
