From 93982ded1e09ccfcd741081f009b0ceec79715a7 Mon Sep 17 00:00:00 2001 From: Tomaz Canabrava Date: Mon, 16 Jul 2018 13:59:10 +0200 Subject: [PATCH] Re-Enable detaching actions (Detaching is still broken atm) Move out of the CMake the check for crashes in detaching It's better to check that in the code - I was quite lost for a while on why there was a DETACHING_ENABLED definition as it did not made sense --- src/CMakeLists.txt | 7 ------- src/ViewContainer.cpp | 25 +++++++++++++------------ src/ViewManager.cpp | 4 ++-- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 507bf63a6..1ba27e0d1 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -2,13 +2,6 @@ add_definitions(-DTRANSLATION_DOMAIN=\"konsole\") -### Too many crashes/issues with detaching on MacOSX -IF(${CMAKE_SYSTEM_NAME} MATCHES "Darwin") - set(ENABLE_DETACHING 0) -else() - set(ENABLE_DETACHING 1) -endif() - ### Handle DragonFlyBSD here instead of using __DragonFly__ IF(${CMAKE_SYSTEM_NAME} MATCHES "DragonFly") set(HAVE_OS_DRAGONFLYBSD 1) diff --git a/src/ViewContainer.cpp b/src/ViewContainer.cpp index 2cf50f17b..e98bb0f39 100644 --- a/src/ViewContainer.cpp +++ b/src/ViewContainer.cpp @@ -27,6 +27,7 @@ #include #include #include +#include // KDE #include @@ -90,12 +91,10 @@ TabbedViewContainer::TabbedViewContainer(ViewManager *connectedViewManager, QWid } }); - -#if defined(ENABLE_DETACHING) -// _contextPopupMenu->addAction(QIcon::fromTheme(QStringLiteral("tab-detach")), -// i18nc("@action:inmenu", "&Detach Tab"), this, -// SLOT(tabContextMenuDetachTab())); -#endif + auto detachAction = _contextPopupMenu->addAction(QIcon::fromTheme(QStringLiteral("tab-detach")), + i18nc("@action:inmenu", "&Detach Tab"), this, + SLOT(tabContextMenuDetachTab())); + detachAction->setObjectName(QStringLiteral("tab-detach")); auto editAction = _contextPopupMenu->addAction(QIcon::fromTheme(QStringLiteral("edit-rename")), i18nc("@action:inmenu", "&Rename Tab..."), this, @@ -285,12 +284,14 @@ void TabbedViewContainer::openTabContextMenu(const QPoint &point) return; } -#if defined(ENABLE_DETACHING) - // Enable 'Detach Tab' menu item only if there is more than 1 tab - // Note: the code is coupled with that action's position within the menu - QAction *detachAction = _contextPopupMenu->actions().at(0); - detachAction->setEnabled(count() > 1); -#endif + //TODO: add a countChanged signal so we can remove this for. + // Detaching in mac causes crashes. + const auto isDarwin = QOperatingSystemVersion::currentType() == QOperatingSystemVersion::MacOS; + for(auto action : _contextPopupMenu->actions()) { + if (action->objectName() == QStringLiteral("tab-detach")) { + action->setEnabled( !isDarwin && count() > 1); + } + } // Add the read-only action auto controller = _navigation[widget(contextMenuTabIndex)]; diff --git a/src/ViewManager.cpp b/src/ViewManager.cpp index ba1f49775..fecb135d8 100644 --- a/src/ViewManager.cpp +++ b/src/ViewManager.cpp @@ -196,8 +196,9 @@ void ViewManager::setupActions() multiViewOnlyActions << shrinkActiveAction; -#if defined(ENABLE_DETACHING) QAction *detachViewAction = collection->addAction(QStringLiteral("detach-view")); + // Crashes on Mac. + detachViewAction->setEnabled(QOperatingSystemVersion::currentType() != QOperatingSystemVersion::MacOS); detachViewAction->setIcon(QIcon::fromTheme(QStringLiteral("tab-detach"))); detachViewAction->setText(i18nc("@action:inmenu", "D&etach Current Tab")); // Ctrl+Shift+D is not used as a shortcut by default because it is too close @@ -207,7 +208,6 @@ void ViewManager::setupActions() connect(this, &Konsole::ViewManager::splitViewToggle, this, &Konsole::ViewManager::updateDetachViewState); connect(detachViewAction, &QAction::triggered, this, &Konsole::ViewManager::detachActiveView); -#endif // Next / Previous View , Next Container collection->addAction(QStringLiteral("next-view"), nextViewAction);