Index: lldb/include/lldb/Host/Host.h =================================================================== --- lldb/include/lldb/Host/Host.h +++ lldb/include/lldb/Host/Host.h @@ -236,8 +236,8 @@ bool run_in_shell = true, bool hide_stderr = false); - static bool OpenFileInExternalEditor(const FileSpec &file_spec, - uint32_t line_no); + static llvm::Error OpenFileInExternalEditor(const FileSpec &file_spec, + uint32_t line_no); /// Check if we're running in an interactive graphical session. /// Index: lldb/source/Host/common/Host.cpp =================================================================== --- lldb/source/Host/common/Host.cpp +++ lldb/source/Host/common/Host.cpp @@ -548,7 +548,7 @@ #if !defined(__APPLE__) bool Host::OpenFileInExternalEditor(const FileSpec &file_spec, uint32_t line_no) { - return false; + return llvm::formErrorCode(std::error_code(ENOTSUP, std::system_category())); } bool Host::IsInteractiveGraphicSession() { return false; } Index: lldb/source/Host/macosx/objcxx/Host.mm =================================================================== --- lldb/source/Host/macosx/objcxx/Host.mm +++ lldb/source/Host/macosx/objcxx/Host.mm @@ -325,12 +325,35 @@ #endif // TARGET_OS_OSX -bool Host::OpenFileInExternalEditor(const FileSpec &file_spec, - uint32_t line_no) { +llvm::Error Host::OpenFileInExternalEditor(const FileSpec &file_spec, + uint32_t line_no) { #if !TARGET_OS_OSX - return false; + return llvm::formErrorCode(std::error_code(ENOTSUP, std::system_category())); #else // !TARGET_OS_OSX - // We attach this to an 'odoc' event to specify a particular selection + Log *log = GetLog(LLDBLog::Host); + + const std::string file_path = file_spec.GetPath(); + + LLDB_LOG(log, "Sending {0}:{1} to external editor", + file_path.empty() ? "" : file_path, line_no); + + if (file_path.empty()) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "no file specified"); + + CFCString file_cfstr(file_path.c_str(), kCFStringEncodingUTF8); + CFCReleaser file_URL = ::CFURLCreateWithFileSystemPath( + /*allocator=*/NULL, + /*filePath*/ file_cfstr.get(), + /*pathStyle=*/kCFURLPOSIXPathStyle, + /*isDirectory=*/false); + + if (!file_URL.get()) + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + llvm::formatv("could not create CFURL from path \"{0}\"", file_path)); + + // Create a new Apple Event descriptor. typedef struct { int16_t reserved0; // must be zero int16_t fLineNumber; @@ -340,18 +363,7 @@ uint32_t reserved2; // must be zero } BabelAESelInfo; - Log *log = GetLog(LLDBLog::Host); - char file_path[PATH_MAX]; - file_spec.GetPath(file_path, PATH_MAX); - CFCString file_cfstr(file_path, kCFStringEncodingUTF8); - CFCReleaser file_URL(::CFURLCreateWithFileSystemPath( - NULL, file_cfstr.get(), kCFURLPOSIXPathStyle, false)); - - LLDB_LOGF(log, - "Sending source file: \"%s\" and line: %d to external editor.\n", - file_path, line_no); - - long error; + // We attach this to an 'odoc' event to specify a particular selection. BabelAESelInfo file_and_line_info = { 0, // reserved0 (int16_t)(line_no - 1), // fLineNumber (zero based line number) @@ -362,64 +374,73 @@ }; AEKeyDesc file_and_line_desc; - - error = ::AECreateDesc(typeUTF8Text, &file_and_line_info, - sizeof(file_and_line_info), - &(file_and_line_desc.descContent)); - - if (error != noErr) { - LLDB_LOGF(log, "Error creating AEDesc: %ld.\n", error); - return false; - } - file_and_line_desc.descKey = keyAEPosition; + long error = ::AECreateDesc(/*typeCode=*/typeUTF8Text, + /*dataPtr=*/&file_and_line_info, + /*dataSize=*/sizeof(file_and_line_info), + /*result=*/&(file_and_line_desc.descContent)); + + if (error != noErr) + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + llvm::formatv("creating Apple Event descriptor failed: error {0}", + error)); + + // Deallocate the descriptor on exit. + auto on_exit = llvm::make_scope_exit( + [&]() { AEDisposeDesc(&(file_and_line_desc.descContent)); }); + + static std::optional 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}\": error {1}", + external_editor, app_error) + .str(); + } + } + }); - static std::string g_app_name; - static FSRef g_app_fsref; + if (!g_app_error.empty()) + return llvm::createStringError(llvm::inconvertibleErrorCode(), g_app_error); + // Build app launch parameters. LSApplicationParameters app_params; ::memset(&app_params, 0, sizeof(app_params)); app_params.flags = 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()); ProcessSerialNumber psn; - CFCReleaser file_array( - CFArrayCreate(NULL, (const void **)file_URL.ptr_address(false), 1, NULL)); - error = ::LSOpenURLsWithRole(file_array.get(), kLSRolesAll, - &file_and_line_desc, &app_params, &psn, 1); - - AEDisposeDesc(&(file_and_line_desc.descContent)); - - if (error != noErr) { - LLDB_LOGF(log, "LSOpenURLsWithRole failed, error: %ld.\n", error); - - return false; - } - - return true; + std::array file_array = {file_URL.get()}; + CFCReleaser cf_array( + CFArrayCreate(/*allocator=*/NULL, /*values=*/(const void **)&file_array, + /*numValues*/ 1, /*callBacks=*/NULL)); + error = ::LSOpenURLsWithRole( + /*inURLs=*/cf_array.get(), /*inRole=*/kLSRolesEditor, + /*inAEParam=*/&file_and_line_desc, + /*inAppParams=*/&app_params, /*outPSNs=*/&psn, /*inMaxPSNCount=*/1); + + if (error != noErr) + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + llvm::formatv("LSOpenURLsWithRole failed: error {0}", error)); + + return llvm::Error::success(); #endif // TARGET_OS_OSX } Index: lldb/source/Interpreter/CommandInterpreter.cpp =================================================================== --- lldb/source/Interpreter/CommandInterpreter.cpp +++ lldb/source/Interpreter/CommandInterpreter.cpp @@ -3241,8 +3241,10 @@ if (GetOpenTranscriptInEditor() && Host::IsInteractiveGraphicSession()) { const FileSpec file_spec; error = file->GetFileSpec(const_cast(file_spec)); - if (error.Success()) - Host::OpenFileInExternalEditor(file_spec, 1); + if (error.Success()) { + if (llvm::Error e = Host::OpenFileInExternalEditor(file_spec, 1)) + result.AppendError(llvm::toString(std::move(e))); + } } return true; Index: lldb/source/Target/Thread.cpp =================================================================== --- lldb/source/Target/Thread.cpp +++ lldb/source/Target/Thread.cpp @@ -305,8 +305,13 @@ frame_sp->GetSymbolContext(eSymbolContextLineEntry)); if (GetProcess()->GetTarget().GetDebugger().GetUseExternalEditor() && frame_sc.line_entry.file && frame_sc.line_entry.line != 0) { - already_shown = Host::OpenFileInExternalEditor( - frame_sc.line_entry.file, frame_sc.line_entry.line); + if (llvm::Error e = Host::OpenFileInExternalEditor( + frame_sc.line_entry.file, frame_sc.line_entry.line)) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Host), std::move(e), + "OpenFileInExternalEditor failed: {0}"); + } else { + already_shown = true; + } } bool show_frame_info = true; @@ -1752,8 +1757,11 @@ SymbolContext frame_sc( frame_sp->GetSymbolContext(eSymbolContextLineEntry)); if (frame_sc.line_entry.line != 0 && frame_sc.line_entry.file) { - Host::OpenFileInExternalEditor(frame_sc.line_entry.file, - frame_sc.line_entry.line); + if (llvm::Error e = Host::OpenFileInExternalEditor( + frame_sc.line_entry.file, frame_sc.line_entry.line)) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Host), std::move(e), + "OpenFileInExternalEditor failed: {0}"); + } } } }