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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to