Skip to content

Fix multiple windows and tabs support #1739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions metainfo.xml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
<li>Fixes `CancelSelection` default binding with escape (#1710)</li>
<li>Fixes `CreateTab` to sometimes spawn more than one tab (#1695)</li>
<li>Fixes crash using Chinese IME (#1707)</li>
<li>Fixes handling of multiple windows and tabs (#1725)</li>
<li>Ensure inserting new tabs happens right next to the currently active tab (#1695)</li>
<li>Allow glyphs to underflow if they are not bigger than the cell size (#1603)</li>
<li>Adds `MoveTabToLeft` and `MoveTabToRight` actions to move tabs around (#1695)</li>
Expand Down
1 change: 0 additions & 1 deletion src/contour/ContourGuiApp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,6 @@ void ContourGuiApp::ensureTermInfoFile()
void ContourGuiApp::newWindow()
{
_qmlEngine->load(resolveResource("ui/main.qml"));
_sessionManager.display = _sessionManager.getSession()->display();
}

void ContourGuiApp::showNotification(std::string_view title, std::string_view content)
Expand Down
5 changes: 2 additions & 3 deletions src/contour/ContourGuiApp.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,9 @@ class ContourGuiApp: public QObject, public ContourApp
[[nodiscard]] config::Config const& config() const noexcept { return _config; }
[[nodiscard]] config::TerminalProfile const& profile() const noexcept
{
if (const auto* const profile = config().profile(profileName()))
return *profile;
const auto* const profile = config().profile(profileName());
return *profile;
displayLog()("Failed to access config profile.");
Require(false);
}

[[nodiscard]] bool liveConfig() const noexcept { return _config.live.value(); }
Expand Down
10 changes: 6 additions & 4 deletions src/contour/TerminalSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -606,8 +606,9 @@ void TerminalSession::inspect()
// Deferred termination? Then close display now.
if (_terminal.device().isClosed() && !_app.dumpStateAtExit().has_value())
{
sessionLog()("Terminal device is closed. Closing display.");
_display->closeDisplay();
sessionLog()("Terminal device is closed. Notify session manager.");
_manager->currentSessionIsTerminated();
//_display->closeDisplay(); // TODO MOVE LOGIC
}
}

Expand Down Expand Up @@ -680,8 +681,9 @@ void TerminalSession::onClosed()
inspect();
else if (_display)
{
sessionLog()("Terminal device is closed. Closing display.");
_display->closeDisplay();
sessionLog()("Terminal device is closed. Notify manager.");
_manager->currentSessionIsTerminated();
// _display->closeDisplay(); // TODO MOVE LOGIC
}
else
sessionLog()("Terminal device is closed. But no display available (yet).");
Expand Down
2 changes: 2 additions & 0 deletions src/contour/TerminalSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@ class TerminalSession: public QAbstractItemModel, public vtbackend::Terminal::Ev
void attachDisplay(display::TerminalDisplay& display);
void detachDisplay(display::TerminalDisplay& display);

TerminalSessionManager* getTerminalManager() const noexcept { return _manager; }

Q_INVOKABLE void applyPendingFontChange(bool allow, bool remember);
Q_INVOKABLE void applyPendingPaste(bool allow, bool remember);
Q_INVOKABLE void executePendingBufferCapture(bool allow, bool remember);
Expand Down
173 changes: 120 additions & 53 deletions src/contour/TerminalSessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,18 @@ TerminalSession* TerminalSessionManager::createSessionInBackground()
// TODO: Remove dependency on app-knowledge and pass shell / terminal-size instead.
// The GuiApp *or* (Global)Config could be made a global to be accessable from within QML.

_previousActiveSession = _activeSession;
if (!activeDisplay)
{
managerLog()("No active display found. something went wrong.");
}

#if !defined(_WIN32)
auto ptyPath = [this]() -> std::optional<std::string> {
if (_activeSession)
{
auto& terminal = _activeSession->terminal();
if (auto const* ptyProcess = dynamic_cast<vtpty::Process const*>(&terminal.device()))
return ptyProcess->workingDirectory();
}
if (_sessions.empty())
return std::nullopt;
auto& terminal = _sessions[0]->terminal();
if (auto const* ptyProcess = dynamic_cast<vtpty::Process const*>(&terminal.device()))
return ptyProcess->workingDirectory();
return std::nullopt;
}();
#else
Expand All @@ -72,13 +74,10 @@ TerminalSession* TerminalSessionManager::createSessionInBackground()
#endif

auto* session = new TerminalSession(this, createPty(ptyPath), _app);
managerLog()("Create new session with ID {} at index {}", session->id(), _sessions.size());

auto const currentSessionIterator = std::ranges::find(_sessions, _activeSession);
auto const insertPoint = currentSessionIterator != _sessions.end() ? std::next(currentSessionIterator)
: currentSessionIterator;
managerLog()(
"Create new session with ID {}({}) at index {}", session->id(), (void*) session, _sessions.size());

_sessions.insert(insertPoint, session);
_sessions.insert(_sessions.end(), session);

connect(session, &TerminalSession::sessionClosed, [this, session]() { removeSession(*session); });

Expand Down Expand Up @@ -107,52 +106,101 @@ TerminalSession* TerminalSessionManager::activateSession(TerminalSession* sessio
if (!session)
return nullptr;

managerLog()(
"Activating session ID {} at index {}", session->id(), getSessionIndexOf(session).value_or(-1));
// debug for displayStates
for (auto& [display, state]: _displayStates)
{
managerLog()("display: {}, session: {}\n", (void*) display, (void*) state);
}

managerLog()("Activating session ID {} {} at index {}",
session->id(),
(void*) session,
getSessionIndexOf(session).value_or(-1));

// iterate over _displayStates to see if this session is already active
for (auto& [display, state]: _displayStates)
{
if (state == session && (nullptr != display))
{
managerLog()("Session is already active : (display {}, ID {} {})",
(void*) display,
session->id(),
(void*) session);
return session;
}
}

if (_activeSession == session)
if (!activeDisplay)
{
managerLog()("Session is already active. (index {}, ID {})", getCurrentSessionIndex(), session->id());
return session;
managerLog()("No active display fond. something went wrong.");
Copy link
Preview

Copilot AI Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in the error message: 'fond' should be 'found'.

Suggested change
managerLog()("No active display fond. something went wrong.");
managerLog()("No active display found. something went wrong.");

Copilot uses AI. Check for mistakes.

}

_previousActiveSession = _activeSession;
_activeSession = session;
auto& displayState = _displayStates[activeDisplay];
displayState = session;
updateStatusLine();

if (display)
if (activeDisplay)
{
managerLog()("Attaching display to session.");
auto const pixels = display->pixelSize();

auto const pixels = activeDisplay->pixelSize();
auto const totalPageSize =
display->calculatePageSize() + _previousActiveSession->terminal().statusLineHeight();
activeDisplay->calculatePageSize() + displayState->terminal().statusLineHeight();

// Ensure that the existing session is resized to the display's size.
if (!isNewSession)
_activeSession->terminal().resizeScreen(totalPageSize, pixels);
{
managerLog()("Resize existing session to display size: {}x{}.",
activeDisplay->width(),
activeDisplay->height());
displayState->terminal().resizeScreen(totalPageSize, pixels);
}

display->setSession(_activeSession);
managerLog()(
"Set display {} to session: {}({}).", (void*) activeDisplay, session->id(), (void*) session);
// resize terminal session before display is attached to it
activeDisplay->setSession(displayState);

// Resize active session after display is attached to it
// to return a lost line
_activeSession->terminal().resizeScreen(totalPageSize, pixels);
displayState->terminal().resizeScreen(totalPageSize, pixels);
}

return session;
}

void TerminalSessionManager::FocusOnDisplay(display::TerminalDisplay* display)
{
managerLog()("Setting active display to {}", (void*) display);
activeDisplay = display;

// if we have a session in nullptr display, set it to this one
if (_displayStates[nullptr] != nullptr)
{
_displayStates[activeDisplay] = _displayStates[nullptr];
_displayStates[nullptr] = nullptr;
activateSession(_displayStates[activeDisplay]);
}

// if this is new display, find a session to attach to
if (_displayStates[activeDisplay] == nullptr)
{
tryFindSessionForDisplayOrClose();
}
}

TerminalSession* TerminalSessionManager::createSession()
{
return activateSession(createSessionInBackground(), true /*force resize on before display-attach*/);
}

void TerminalSessionManager::switchToPreviousTab()
{
managerLog()("switch to previous tab (current: {}, previous: {})",
getSessionIndexOf(_activeSession).value_or(-1),
getSessionIndexOf(_previousActiveSession).value_or(-1));
return;
// managerLog()("switch to previous tab (current: {}, previous: {})",
// getSessionIndexOf(_displayStates[activeDisplay].activeSession).value_or(-1),
// getSessionIndexOf(_displayStates[activeDisplay].previousActiveSession).value_or(-1));

activateSession(_previousActiveSession);
// activateSession(_displayStates[activeDisplay].previousActiveSession);
}

void TerminalSessionManager::switchToTabLeft()
Expand Down Expand Up @@ -191,7 +239,7 @@ void TerminalSessionManager::switchToTabRight()
void TerminalSessionManager::switchToTab(int position)
{
managerLog()("switchToTab from index {} to {} (out of {})",
getSessionIndexOf(_activeSession).value_or(-1),
getSessionIndexOf(_displayStates[activeDisplay]).value_or(-1),
position - 1,
_sessions.size());

Expand All @@ -202,15 +250,15 @@ void TerminalSessionManager::switchToTab(int position)
void TerminalSessionManager::closeTab()
{
managerLog()("Close tab: current session ID {}, index {}",
getSessionIndexOf(_activeSession).value_or(-1),
_activeSession->id());
getSessionIndexOf(_displayStates[activeDisplay]).value_or(-1),
_displayStates[activeDisplay]->id());

removeSession(*_activeSession);
removeSession(*_displayStates[activeDisplay]);
}

void TerminalSessionManager::moveTabTo(int position)
{
auto const currentIndexOpt = getSessionIndexOf(_activeSession);
auto const currentIndexOpt = getSessionIndexOf(_displayStates[activeDisplay]);
if (!currentIndexOpt)
return;

Expand Down Expand Up @@ -253,12 +301,16 @@ void TerminalSessionManager::moveTabToRight(TerminalSession* session)
}
}

void TerminalSessionManager::removeSession(TerminalSession& thatSession)
void TerminalSessionManager::currentSessionIsTerminated()
{
managerLog()("REMOVE SESSION: session: {}, _sessions.size(): {}", (void*) &thatSession, _sessions.size());
managerLog()("got notified that session is terminated, number of existing sessions: _sessions.size(): {}",
_sessions.size());
return;
}

if (&thatSession == _activeSession && _previousActiveSession)
activateSession(_previousActiveSession);
void TerminalSessionManager::removeSession(TerminalSession& thatSession)
{
managerLog()("remove session: session: {}, _sessions.size(): {}", (void*) &thatSession, _sessions.size());

auto i = std::ranges::find(_sessions, &thatSession);
if (i == _sessions.end())
Expand All @@ -267,21 +319,36 @@ void TerminalSessionManager::removeSession(TerminalSession& thatSession)
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));
tryFindSessionForDisplayOrClose();
//_app.onExit(thatSession); // TODO: the logic behind that impl could probably be moved here.
}

void TerminalSessionManager::tryFindSessionForDisplayOrClose()
{
for (auto& session: _sessions)
{
bool saveToSwitch { true };
// check if session is not used by any display and then switch
for (auto& [display, state]: _displayStates)
{
if ((state == session) && (display != nullptr))
{
saveToSwitch = false;
break;
}
}

if (saveToSwitch)
{
managerLog()("Switching to session: {}", (void*) session);
activateSession(session);
return;
}
}

updateStatusLine();
_displayStates.erase(activeDisplay);
activeDisplay->closeDisplay();
}

void TerminalSessionManager::updateColorPreference(vtbackend::ColorPreference const& preference)
Expand Down
Loading
Loading