This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG13dbc16b4d82: [lldb] Refactor host::OpenFileInExternalEditor
(authored by JDevlieghere).
Changed prior to commit:
https://reviews.llvm.org/D149482?vs=518041&id=518109#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149482/new/
https://reviews.llvm.org/D149482
Files:
lldb/include/lldb/Host/Host.h
lldb/source/Host/common/Host.cpp
lldb/source/Host/macosx/objcxx/Host.mm
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Target/Thread.cpp
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;
@@ -1725,8 +1730,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}");
+ }
}
}
}
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3271,8 +3271,10 @@
if (GetOpenTranscriptInEditor() && Host::IsInteractiveGraphicSession()) {
const FileSpec file_spec;
error = file->GetFileSpec(const_cast<FileSpec &>(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/Host/macosx/objcxx/Host.mm
===================================================================
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -325,12 +325,36 @@
#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::errorCodeToError(
+ 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() ? "<invalid>" : 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<CFURLRef> 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 +364,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<CFURLRef> 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 +375,74 @@
};
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<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();
+ }
+ }
+ });
- 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<CFArrayRef> 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<CFURLRef, 1> file_array = {file_URL.get()};
+ CFCReleaser<CFArrayRef> 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/Host/common/Host.cpp
===================================================================
--- lldb/source/Host/common/Host.cpp
+++ lldb/source/Host/common/Host.cpp
@@ -546,9 +546,10 @@
#endif
#if !defined(__APPLE__)
-bool Host::OpenFileInExternalEditor(const FileSpec &file_spec,
- uint32_t line_no) {
- return false;
+llvm::Error Host::OpenFileInExternalEditor(const FileSpec &file_spec,
+ uint32_t line_no) {
+ return llvm::errorCodeToError(
+ std::error_code(ENOTSUP, std::system_category()));
}
bool Host::IsInteractiveGraphicSession() { return false; }
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.
///
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
