[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.


================
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);
----------------
bulbazord wrote:
> 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.
If both are set and not the same, maybe we should flag that.  Maybe we can check when the setting is set, and if the environment variable is also set and different, we can warn at that point.  

But the intention of the setting is to change what the current behavior is, no matter how we got to the current behavior, so the setting should take precedence.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149472/new/

https://reviews.llvm.org/D149472