[Lldb-commits] [PATCH] D149565: [lldb] Add debugger.external-editor setting

[Alex Langford via Phabricator via lldb-commits]

bulbazord requested changes to this revision.
bulbazord added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/source/Host/macosx/objcxx/Host.mm:395-396
 
-  static std::optional<FSRef> g_app_fsref;
-  static std::string g_app_error;
-  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);
-
-      FSRef app_fsref;
-      CFCString editor_name(external_editor, kCFStringEncodingUTF8);
-      long app_error = ::LSFindApplicationForInfo(
-          /*inCreator=*/kLSUnknownCreator, /*inBundleID=*/NULL,
-          /*inName=*/editor_name.get(), /*outAppRef=*/&app_fsref,
-          /*outAppURL=*/NULL);
-      if (app_error == noErr) {
-        g_app_fsref = app_fsref;
-      } else {
-        g_app_error =
-            llvm::formatv("could not find external editor \"{0}\": "
-                          "LSFindApplicationForInfo returned error {1}",
-                          external_editor, app_error)
-                .str();
+  if (const char *lldb_external_editor = ::getenv("LLDB_EXTERNAL_EDITOR")) {
+    if (!editor.empty()) {
+      if (editor != llvm::StringRef(lldb_external_editor)) {
----------------
JDevlieghere wrote:
> mib wrote:
> > Should we even check for the env variable is the user set the editor setting ?
> I asked that question in D149472  and this was Jim's suggestion: https://reviews.llvm.org/D149472#inline-1443754
This is a hard error and not a warning, which I don't think is a good idea. I think the point of a setting here is to be able to change behavior during a debugging session instead of needing to start over. If these 2 values conflict, I would have to restart my session to unset LLDB_EXTERNAL_EDITOR to actually get the desired behavior.


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

https://reviews.llvm.org/D149565