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

JDevlieghere closed this revision. JDevlieghere added a comment. 13dbc16b4d82 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149472/new/ https://reviews.llvm.org/D149472 __

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

bulbazord accepted this revision. bulbazord added a comment. LGTM! Thanks for cleaning this up. Comment at: lldb/source/Target/Thread.cpp:1762-1763 + frame_sc.line_entry.file, frame_sc.line_entry.line)) { +LLDB_LOG_ERROR(GetLog(LLDBLog::Host), std::

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

JDevlieghere updated this revision to Diff 518007. JDevlieghere marked 10 inline comments as done. JDevlieghere added a comment. - Return an `llvm::Erorr` and percolate errors up - Don't fall back to the default editor if the one specified can't be found - Limit roles to editors only CHANGES SIN

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

jingham added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:387-388 + 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 exte

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

bulbazord added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:337 + + LLDB_LOG(log, "Sending {0}:{1} to external editor", file_path, line_no); + JDevlieghere wrote: > bulbazord wrote: > > nit: Move log line below checking if the file_path i

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

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, "Lo

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

mib added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:420 + error = ::LSOpenURLsWithRole( + /*inURLs=*/cf_array.get(), /*inRole=*/kLSRolesAll, + /*inAEParam=*/&file_and_line_desc, I guess we could narrow down the `inRole` argume

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

mib added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:343-344 + CFCString file_cfstr(file_path.c_str(), kCFStringEncodingUTF8); + CFCReleaser file_URL(::CFURLCreateWithFileSystemPath( + NULL, file_cfstr.get(), kCFURLPOSIXPathStyle, false)); + --

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

JDevlieghere marked 5 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:335 + + const std::string file_path = file_spec.GetPath(); + mib wrote: > I guess this doesn't need to be a `const std::string`,

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

JDevlieghere marked 3 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:337 + + LLDB_LOG(log, "Sending {0}:{1} to external editor", file_path, line_no); + bulbazord wrote: > nit: Move log line below c

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

mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. LGTM with some comments Comment at: lldb/source/Host/macosx/objcxx/Host.mm:335 + + const std::string file_path = file_spec.GetPath(); + I guess this doesn't need

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

bulbazord added a comment. LGTM overall. A few nits but overall an excellent cleanup! :) Comment at: lldb/source/Host/macosx/objcxx/Host.mm:337 + + LLDB_LOG(log, "Sending {0}:{1} to external editor", file_path, line_no); + nit: Move log line below checking if

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

JDevlieghere created this revision. JDevlieghere added reviewers: aprantl, bulbazord, mib. Herald added a project: All. JDevlieghere requested review of this revision. This patch refactors the macOS implementation of `OpenFileInExternalEditor`: - Fix `AppleEvent` memory leak - Fix caching of `LLD