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

Jim Ingham via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri Apr 28 11:14:44 PDT 2023


jingham added inline comments.


================
Comment at: lldb/source/Host/macosx/objcxx/Host.mm:411-412
       kLSLaunchDefaults | kLSLaunchDontAddToRecents | kLSLaunchDontSwitch;
-
-  char *external_editor = ::getenv("LLDB_EXTERNAL_EDITOR");
-
-  if (external_editor) {
-    LLDB_LOGF(log, "Looking for external editor \"%s\".\n", external_editor);
-
-    if (g_app_name.empty() ||
-        strcmp(g_app_name.c_str(), external_editor) != 0) {
-      CFCString editor_name(external_editor, kCFStringEncodingUTF8);
-      error = ::LSFindApplicationForInfo(kLSUnknownCreator, NULL,
-                                         editor_name.get(), &g_app_fsref, NULL);
-
-      // If we found the app, then store away the name so we don't have to
-      // re-look it up.
-      if (error != noErr) {
-        LLDB_LOGF(log,
-                  "Could not find External Editor application, error: %ld.\n",
-                  error);
-        return false;
-      }
-    }
-    app_params.application = &g_app_fsref;
-  }
+  if (g_app_fsref)
+    app_params.application = &(g_app_fsref.value());
 
----------------
JDevlieghere wrote:
> bulbazord wrote:
> > Should we exit early if we don't have `g_app_fsref` filled in? The `application` field of `app_params` won't be filled in so what will `LSOpenURLsWithRole` do?
> No, the application is optional. If none is specified we use the default. You can't pass a default constructor one though, it has to be NULL, which is why we have the check here.
LSOpenURLsWithRole will use the default application for the extension that the file path we pass it has, however, that's likely to be a different app than the one the user dialed up with LLDB_EXTERNAL_EDITOR.  

If the user told us to use an external editor at some path, but we can't actually find that application, then letting the default application open seems wrong.  Ideally, we would raise an error saying "External Editor: ... not found" but it doesn't look like there's any way to report errors from this code.


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

https://reviews.llvm.org/D149472



More information about the lldb-commits mailing list