From ee40f195150f39a57dcdf7b5328c2bebe4c455c3 Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Tue, 31 Dec 2024 02:10:49 +0100 Subject: [PATCH 1/4] [contour] Improve tab switch handling and add `SwitchToPreviousTab` Signed-off-by: Christian Parpart --- metainfo.xml | 2 + src/contour/Actions.cpp | 1 + src/contour/Actions.h | 6 + src/contour/TerminalSession.cpp | 6 + src/contour/TerminalSession.h | 2 + src/contour/TerminalSessionManager.cpp | 158 +++++++++++++----------- src/contour/TerminalSessionManager.h | 30 +++-- src/contour/ui.template/Terminal.qml.in | 3 + 8 files changed, 127 insertions(+), 81 deletions(-) diff --git a/metainfo.xml b/metainfo.xml index c6a2b847dc..10a17a7e45 100644 --- a/metainfo.xml +++ b/metainfo.xml @@ -113,6 +113,8 @@
  • Fixes startup crash when window is not yet fully initialized
  • Fixes backtab (Shift+Tab) handling (#1685)
  • Fixes various spelling typos across the codebase (#1688)
    • +
  • Improve tab close handling to better select previously focused tab
    • +
  • Add action `SwitchToPreviousTab` to switch to the previously focused tab
    • diff --git a/src/contour/Actions.cpp b/src/contour/Actions.cpp index 5822f7397d..320403fa73 100644 --- a/src/contour/Actions.cpp +++ b/src/contour/Actions.cpp @@ -82,6 +82,7 @@ optional fromString(string const& name) mapAction("CreateNewTab"), mapAction("CloseTab"), mapAction("SwitchToTab"), + mapAction("SwitchToPreviousTab"), mapAction("SwitchToTabLeft"), mapAction("SwitchToTabRight"), }; diff --git a/src/contour/Actions.h b/src/contour/Actions.h index a50d1d0810..a6713cce1e 100644 --- a/src/contour/Actions.h +++ b/src/contour/Actions.h @@ -82,6 +82,7 @@ struct WriteScreen{ std::string chars; }; // "\033[2J\033[3J" struct CreateNewTab{}; struct CloseTab{}; struct SwitchToTab{ int position; }; +struct SwitchToPreviousTab{}; struct SwitchToTabLeft{}; struct SwitchToTabRight{}; // clang-format on @@ -140,6 +141,7 @@ using Action = std::variant; @@ -260,6 +262,7 @@ namespace documentation constexpr inline std::string_view SwitchToTab { "Switch to absolute tab position (starting at number 1)" }; + constexpr inline std::string_view SwitchToPreviousTab { "Switch to the previously focused tab" }; constexpr inline std::string_view SwitchToTabLeft { "Switch to tab to the left" }; constexpr inline std::string_view SwitchToTabRight { "Switch to tab to the right" }; } // namespace documentation @@ -321,6 +324,7 @@ inline auto getDocumentation() std::tuple { Action { CreateNewTab {} }, documentation::CreateNewTab }, std::tuple { Action { CloseTab {} }, documentation::CloseTab }, std::tuple { Action { SwitchToTab {} }, documentation::SwitchToTab }, + std::tuple { Action { SwitchToPreviousTab {} }, documentation::SwitchToPreviousTab }, std::tuple { Action { SwitchToTabLeft {} }, documentation::SwitchToTabLeft }, std::tuple { Action { SwitchToTabRight {} }, documentation::SwitchToTabRight }, }; @@ -393,6 +397,7 @@ DECLARE_ACTION_FMT(ViNormalMode) DECLARE_ACTION_FMT(WriteScreen) DECLARE_ACTION_FMT(CreateNewTab) DECLARE_ACTION_FMT(CloseTab) +DECLARE_ACTION_FMT(SwitchToPreviousTab) DECLARE_ACTION_FMT(SwitchToTabLeft) DECLARE_ACTION_FMT(SwitchToTabRight) // }}} @@ -471,6 +476,7 @@ struct std::formatter: std::formatter HANDLE_ACTION(ViNormalMode); HANDLE_ACTION(CreateNewTab); HANDLE_ACTION(CloseTab); + HANDLE_ACTION(SwitchToPreviousTab); HANDLE_ACTION(SwitchToTabLeft); HANDLE_ACTION(SwitchToTabRight); if (std::holds_alternative(_action)) diff --git a/src/contour/TerminalSession.cpp b/src/contour/TerminalSession.cpp index 88060444cc..b29ae8445e 100644 --- a/src/contour/TerminalSession.cpp +++ b/src/contour/TerminalSession.cpp @@ -1446,6 +1446,12 @@ bool TerminalSession::operator()(actions::SwitchToTab const& event) return true; } +bool TerminalSession::operator()(actions::SwitchToPreviousTab) +{ + emit switchToPreviousTab(); + return true; +} + bool TerminalSession::operator()(actions::SwitchToTabLeft) { emit switchToTabLeft(); diff --git a/src/contour/TerminalSession.h b/src/contour/TerminalSession.h index 281640f615..f54e0b3029 100644 --- a/src/contour/TerminalSession.h +++ b/src/contour/TerminalSession.h @@ -357,6 +357,7 @@ class TerminalSession: public QAbstractItemModel, public vtbackend::Terminal::Ev bool operator()(actions::CreateNewTab); bool operator()(actions::CloseTab); bool operator()(actions::SwitchToTab const& event); + bool operator()(actions::SwitchToPreviousTab); bool operator()(actions::SwitchToTabLeft); bool operator()(actions::SwitchToTabRight); @@ -406,6 +407,7 @@ class TerminalSession: public QAbstractItemModel, public vtbackend::Terminal::Ev // Tab handling signals void createNewTab(); void closeTab(); + void switchToPreviousTab(); void switchToTabLeft(); void switchToTabRight(); void switchToTab(int position); diff --git a/src/contour/TerminalSessionManager.cpp b/src/contour/TerminalSessionManager.cpp index 62e344a8e4..6444a96bfb 100644 --- a/src/contour/TerminalSessionManager.cpp +++ b/src/contour/TerminalSessionManager.cpp @@ -14,7 +14,6 @@ #include #include -#include #include using namespace std::string_literals; @@ -44,10 +43,16 @@ std::unique_ptr TerminalSessionManager::createPty(std::optional std::optional { @@ -72,7 +77,7 @@ TerminalSession* TerminalSessionManager::createSession() #endif auto* session = new TerminalSession(createPty(ptyPath), _app); - managerLog()("CREATE SESSION, new session: {}", (void*) session); + managerLog()("Create new session with ID {} at index {}", session->id(), _sessions.size()); _sessions.push_back(session); @@ -84,9 +89,6 @@ TerminalSession* TerminalSessionManager::createSession() // sessions. This will work around it, by explicitly claiming ownership of the object. QQmlEngine::setObjectOwnership(session, QQmlEngine::CppOwnership); - // we can close application right after session has been created - _lastTabChange = std::chrono::steady_clock::now() - std::chrono::seconds(1); - _activeSession = session; return session; } @@ -94,39 +96,67 @@ void TerminalSessionManager::setSession(size_t index) { Require(index <= _sessions.size()); managerLog()(std::format("SET SESSION: index: {}, _sessions.size(): {}", index, _sessions.size())); + if (!isAllowedToChangeTabs()) return; - Require(display != nullptr); - auto const pixels = display->pixelSize(); - auto const totalPageSize = display->calculatePageSize() + _activeSession->terminal().statusLineHeight(); + if (index < _sessions.size()) + activateSession(_sessions[index]); + else + activateSession(createSessionInBackground()); +} - auto* oldSession = _activeSession; +TerminalSession* TerminalSessionManager::activateSession(TerminalSession* session) +{ + managerLog()( + "Activating session ID {} at index {}", session->id(), getSessionIndexOf(session).value_or(-1)); - if (index < _sessions.size()) + if (_activeSession == session) + { + managerLog()("Session is already active. (index {}, ID {})", getCurrentSessionIndex(), session->id()); + return session; + } + + _previousActiveSession = _activeSession; + _activeSession = session; + _lastTabChange = std::chrono::steady_clock::now(); + updateStatusLine(); + + if (display) { - _activeSession = _sessions[index]; + managerLog()("Attaching display to session."); + auto const pixels = display->pixelSize(); + auto const totalPageSize = + display->calculatePageSize() + _activeSession->terminal().statusLineHeight(); + // Ensure that the existing session is resized to the display's size. _activeSession->terminal().resizeScreen(totalPageSize, pixels); - } - else - createSession(); - if (oldSession == _activeSession) - return; + display->setSession(_activeSession); - display->setSession(_activeSession); - // Resize active session after display is attached to it - // to return a lost line - _activeSession->terminal().resizeScreen(totalPageSize, pixels); - updateStatusLine(); + // Resize active session after display is attached to it + // to return a lost line + _activeSession->terminal().resizeScreen(totalPageSize, pixels); + } - _lastTabChange = std::chrono::steady_clock::now(); + return session; } void TerminalSessionManager::addSession() { - setSession(_sessions.size()); + activateSession(createSessionInBackground()); +} + +void TerminalSessionManager::switchToPreviousTab() +{ + managerLog()("SWITCH TO LAST TAB (current: {}, last: {})", + getSessionIndexOf(_activeSession).value_or(-1), + getSessionIndexOf(_previousActiveSession).value_or(-1)); + + if (!isAllowedToChangeTabs()) + return; + + activateSession(_previousActiveSession); } void TerminalSessionManager::switchToTabLeft() @@ -164,73 +194,59 @@ void TerminalSessionManager::switchToTabRight() void TerminalSessionManager::switchToTab(int position) { - managerLog()(std::format( - "switchToTab from {} to {} (out of {})", getCurrentSessionIndex(), position - 1, _sessions.size())); + managerLog()("switchToTab from index {} to {} (out of {})", + getSessionIndexOf(_activeSession).value_or(-1), + position - 1, + _sessions.size()); + + if (!isAllowedToChangeTabs()) + return; if (1 <= position && position <= static_cast(_sessions.size())) - { - setSession(position - 1); - } + activateSession(_sessions[position - 1]); } void TerminalSessionManager::closeTab() { - const auto currentSessionIndex = getCurrentSessionIndex(); - managerLog()(std::format( - "CLOSE TAB: currentSessionIndex: {}, _sessions.size(): {}", currentSessionIndex, _sessions.size())); - - // Session was removed outside of terminal session manager, we need to switch to another tab. - if (currentSessionIndex == -1 && !_sessions.empty()) - { - // We need to switch to another tab, so we permit consequent tab changes. - // TODO: This is a bit hacky. - _lastTabChange = std::chrono::steady_clock::now() - std::chrono::seconds(1); - setSession(0); - return; - } - - if (_sessions.size() > 1) - { - - if (!isAllowedToChangeTabs()) - return; + managerLog()("Close tab: current session ID {}, index {}", + getSessionIndexOf(_activeSession).value_or(-1), + _activeSession->id()); - removeSession(*_activeSession); - - // We need to switch to another tab, so we permit consequent tab changes. - // TODO: This is a bit hacky. - _lastTabChange = std::chrono::steady_clock::now() - std::chrono::seconds(1); - - if (std::cmp_less_equal(currentSessionIndex, _sessions.size() - 1)) - { - setSession(currentSessionIndex + 1); - } - else - { - setSession(currentSessionIndex - 1); - } - } + removeSession(*_activeSession); } void TerminalSessionManager::removeSession(TerminalSession& thatSession) { - managerLog()(std::format( - "REMOVE SESSION: session: {}, _sessions.size(): {}", (void*) &thatSession, _sessions.size())); + managerLog()("REMOVE SESSION: session: {}, _sessions.size(): {}", (void*) &thatSession, _sessions.size()); if (!isAllowedToChangeTabs()) return; - _app.onExit(thatSession); // TODO: the logic behind that impl could probably be moved here. + if (&thatSession == _activeSession && _previousActiveSession) + activateSession(_previousActiveSession); - auto i = std::ranges::find_if(_sessions, [&](auto p) { return p == &thatSession; }); - if (i != _sessions.end()) + auto i = std::ranges::find(_sessions, &thatSession); + if (i == _sessions.end()) { - _sessions.erase(i); + managerLog()("Session not found in session list."); + return; } + _sessions.erase(i); + _app.onExit(thatSession); // TODO: the logic behind that impl could probably be moved here. + + _previousActiveSession = [&]() -> TerminalSession* { + auto const currentIndex = getSessionIndexOf(_activeSession).value_or(0); + if (currentIndex + 1 < _sessions.size()) + return _sessions[currentIndex + 1]; + else if (currentIndex > 0) + return _sessions[currentIndex - 1]; + else + return nullptr; + }(); + managerLog()("Calculated next \"previous\" session index {}", + getSessionIndexOf(_previousActiveSession).value_or(-1)); updateStatusLine(); - _lastTabChange = std::chrono::steady_clock::now(); - // Notify app if all sessions have been killed to trigger app termination. } void TerminalSessionManager::updateColorPreference(vtbackend::ColorPreference const& preference) diff --git a/src/contour/TerminalSessionManager.h b/src/contour/TerminalSessionManager.h index 66cab609c2..4daa9d9146 100644 --- a/src/contour/TerminalSessionManager.h +++ b/src/contour/TerminalSessionManager.h @@ -27,14 +27,19 @@ class TerminalSessionManager: public QAbstractListModel public: TerminalSessionManager(ContourGuiApp& app); + contour::TerminalSession* createSessionInBackground(); + contour::TerminalSession* activateSession(TerminalSession* session); + Q_INVOKABLE contour::TerminalSession* createSession(); Q_INVOKABLE void addSession(); + Q_INVOKABLE void switchToPreviousTab(); Q_INVOKABLE void switchToTabLeft(); Q_INVOKABLE void switchToTabRight(); Q_INVOKABLE void switchToTab(int position); Q_INVOKABLE void closeTab(); - Q_INVOKABLE void setSession(size_t index); + + void setSession(size_t index); void removeSession(TerminalSession&); @@ -50,13 +55,17 @@ class TerminalSessionManager: public QAbstractListModel private: std::unique_ptr createPty(std::optional cwd); - [[nodiscard]] auto getCurrentSessionIndex() const + + [[nodiscard]] std::optional getSessionIndexOf(TerminalSession* session) const noexcept + { + if (auto const i = std::ranges::find(_sessions, session); i != _sessions.end()) + return static_cast(std::distance(_sessions.begin(), i)); + return std::nullopt; + } + + [[nodiscard]] auto getCurrentSessionIndex() const noexcept { - return [](auto const& sessions, auto const& activeSession) { - auto i = - std::find_if(sessions.begin(), sessions.end(), [&](auto p) { return p == activeSession; }); - return i != sessions.end() ? i - sessions.begin() : -1; - }(_sessions, _activeSession); + return getSessionIndexOf(_activeSession).value(); } void updateStatusLine() @@ -66,11 +75,11 @@ class TerminalSessionManager: public QAbstractListModel _activeSession->terminal().setGuiTabInfoForStatusLine(vtbackend::TabsInfo { .tabCount = _sessions.size(), - .activeTabPosition = _sessions.empty() ? 0 : static_cast(1 + getCurrentSessionIndex()), + .activeTabPosition = 1 + getSessionIndexOf(_activeSession).value_or(0), }); } - bool isAllowedToChangeTabs() + [[nodiscard]] bool isAllowedToChangeTabs() const { // QML for some reason sends multiple signals requests in a row, so we need to ignore them. auto now = std::chrono::steady_clock::now(); @@ -85,9 +94,10 @@ class TerminalSessionManager: public QAbstractListModel ContourGuiApp& _app; std::chrono::seconds _earlyExitThreshold; TerminalSession* _activeSession = nullptr; + TerminalSession* _previousActiveSession = nullptr; std::vector _sessions; std::chrono::time_point _lastTabChange; - std::chrono::milliseconds _timeBetweenTabSwitches { 10 }; + std::chrono::milliseconds _timeBetweenTabSwitches { 50 }; }; } // namespace contour diff --git a/src/contour/ui.template/Terminal.qml.in b/src/contour/ui.template/Terminal.qml.in index 465e7872b4..c5995e1be8 100644 --- a/src/contour/ui.template/Terminal.qml.in +++ b/src/contour/ui.template/Terminal.qml.in @@ -17,10 +17,12 @@ ContourTerminal session: terminalSessions.createSession() signal switchToTab(int index) + signal switchToPreviousTab signal switchToTabLeft signal switchToTabRight signal closeTab + onSwitchToPreviousTab: terminalSessions.switchToPreviousTab() onSwitchToTabLeft: terminalSessions.switchToTabLeft() onSwitchToTabRight: terminalSessions.switchToTabRight() onCloseTab: terminalSessions.closeTab() @@ -289,6 +291,7 @@ ContourTerminal vt.createNewTab.connect(onCreateNewTab); vt.closeTab.connect(closeTab); vt.switchToTab.connect(switchToTab); + vt.switchToPreviousTab.connect(switchToPreviousTab); vt.switchToTabLeft.connect(switchToTabLeft); vt.switchToTabRight.connect(switchToTabRight); } From 46cadcf94bd7851bfe4cb31b72ccffa4e3f1c647 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Tue, 31 Dec 2024 09:28:44 +0100 Subject: [PATCH 2/4] Preserve existent logic --- src/contour/TerminalSessionManager.cpp | 13 +++++++------ src/contour/TerminalSessionManager.h | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/contour/TerminalSessionManager.cpp b/src/contour/TerminalSessionManager.cpp index 6444a96bfb..b283d3edc5 100644 --- a/src/contour/TerminalSessionManager.cpp +++ b/src/contour/TerminalSessionManager.cpp @@ -106,7 +106,7 @@ void TerminalSessionManager::setSession(size_t index) activateSession(createSessionInBackground()); } -TerminalSession* TerminalSessionManager::activateSession(TerminalSession* session) +TerminalSession* TerminalSessionManager::activateSession(TerminalSession* session, bool isNewSession) { managerLog()( "Activating session ID {} at index {}", session->id(), getSessionIndexOf(session).value_or(-1)); @@ -127,10 +127,11 @@ TerminalSession* TerminalSessionManager::activateSession(TerminalSession* sessio managerLog()("Attaching display to session."); auto const pixels = display->pixelSize(); auto const totalPageSize = - display->calculatePageSize() + _activeSession->terminal().statusLineHeight(); + display->calculatePageSize() + _previousActiveSession->terminal().statusLineHeight(); // Ensure that the existing session is resized to the display's size. - _activeSession->terminal().resizeScreen(totalPageSize, pixels); + if(!isNewSession) + _activeSession->terminal().resizeScreen(totalPageSize, pixels); display->setSession(_activeSession); @@ -144,12 +145,12 @@ TerminalSession* TerminalSessionManager::activateSession(TerminalSession* sessio void TerminalSessionManager::addSession() { - activateSession(createSessionInBackground()); + activateSession(createSessionInBackground(), true); } void TerminalSessionManager::switchToPreviousTab() { - managerLog()("SWITCH TO LAST TAB (current: {}, last: {})", + managerLog()("switch to previous tab (current: {}, previous: {})", getSessionIndexOf(_activeSession).value_or(-1), getSessionIndexOf(_previousActiveSession).value_or(-1)); @@ -162,7 +163,7 @@ void TerminalSessionManager::switchToPreviousTab() void TerminalSessionManager::switchToTabLeft() { const auto currentSessionIndex = getCurrentSessionIndex(); - managerLog()(std::format("PREVIOUS TAB: currentSessionIndex: {}, _sessions.size(): {}", + managerLog()(std::format("previous tab: currentSessionIndex: {}, _sessions.size(): {}", currentSessionIndex, _sessions.size())); diff --git a/src/contour/TerminalSessionManager.h b/src/contour/TerminalSessionManager.h index 4daa9d9146..288e325b0c 100644 --- a/src/contour/TerminalSessionManager.h +++ b/src/contour/TerminalSessionManager.h @@ -28,7 +28,7 @@ class TerminalSessionManager: public QAbstractListModel TerminalSessionManager(ContourGuiApp& app); contour::TerminalSession* createSessionInBackground(); - contour::TerminalSession* activateSession(TerminalSession* session); + contour::TerminalSession* activateSession(TerminalSession* session, bool isNewSession = false); Q_INVOKABLE contour::TerminalSession* createSession(); Q_INVOKABLE void addSession(); From ef5c21472b162d5248b872cceecbefb77159b41e Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Tue, 31 Dec 2024 14:36:13 +0100 Subject: [PATCH 3/4] add comment on what this `true` is about Signed-off-by: Christian Parpart --- src/contour/TerminalSessionManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/contour/TerminalSessionManager.cpp b/src/contour/TerminalSessionManager.cpp index b283d3edc5..937cef1c86 100644 --- a/src/contour/TerminalSessionManager.cpp +++ b/src/contour/TerminalSessionManager.cpp @@ -145,7 +145,7 @@ TerminalSession* TerminalSessionManager::activateSession(TerminalSession* sessio void TerminalSessionManager::addSession() { - activateSession(createSessionInBackground(), true); + activateSession(createSessionInBackground(), true /*force resize on before display-attach*/); } void TerminalSessionManager::switchToPreviousTab() From 0b8597925212edc1e6cfc3717700cfab521ae85d Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Tue, 31 Dec 2024 16:28:13 +0100 Subject: [PATCH 4/4] style-fix Signed-off-by: Christian Parpart --- src/contour/TerminalSessionManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/contour/TerminalSessionManager.cpp b/src/contour/TerminalSessionManager.cpp index 937cef1c86..c427f542b6 100644 --- a/src/contour/TerminalSessionManager.cpp +++ b/src/contour/TerminalSessionManager.cpp @@ -130,7 +130,7 @@ TerminalSession* TerminalSessionManager::activateSession(TerminalSession* sessio display->calculatePageSize() + _previousActiveSession->terminal().statusLineHeight(); // Ensure that the existing session is resized to the display's size. - if(!isNewSession) + if (!isNewSession) _activeSession->terminal().resizeScreen(totalPageSize, pixels); display->setSession(_activeSession);