I like the removal of duplicated code; however something still not correct. If you have konsole debug turned on and run konsole from another term you can see "org.kde.konsole: Attempted to re-run an already running session ( 28261 )" which should not be there. Briefly looking at the code, ViewManager::newSession has session->run() which is the removed code does not.

Ok, haven't found any regressions. The below item should be fixed later as I didn't realize it before.

While testing this, I notice Konsole on desktop session restore and demo_konsolepart, KONSOLE_DBUS_WINDOW is not set.

In D12968#277762, @hindenburg wrote:

Ok, haven't found any regressions. The below item should be fixed later as I didn't realize it before.

While testing this, I notice Konsole on desktop session restore and demo_konsolepart, KONSOLE_DBUS_WINDOW is not set.

While looking for this I'v found a 4rth time the same code was duplicated in the Konsole codebase, but on the parts it didn't set the KONSOLE_DBUS_WINDOW, I'v fixed by calling the SessionManager method and removing duplication.
For the restore window part - how do I test this?

In D12968#277814, @tcanabrava wrote:

For the restore window part - how do I test this?

Having a Konsole open and then logging off your desktop; when you log back in, the Konsole should be restored

> Having a Konsole open and then logging off your desktop; when you log back in, the Konsole should be restored

I found the code that deals with that:

void restoreSession(Application &app)
{
    int n = 1;
    while (KMainWindow::canBeRestored(n)) {
        app.newMainWindow()->restore(n++);
    }
}

and I'v tried to debug it but I failed to find the place that created the session while restoring. It's probabely calling SessionManager::createSession directly instead of requesting a new session from the ViewManager.
if you have time to help me find the incorrect call I'll be grateful as I'v spend quite a lot of time trying to find it

In D12968#277887, @tcanabrava wrote:

> Having a Konsole open and then logging off your desktop; when you log back in, the Konsole should be restored

I found the code that deals with that:

void restoreSession(Application &app)
{
    int n = 1;
    while (KMainWindow::canBeRestored(n)) {
        app.newMainWindow()->restore(n++);
    }
}

Application::newMainWindow, it calls this

connect(window, &Konsole::MainWindow::newWindowRequest, this,
        &Konsole::Application::createWindow);

createWindow:

MainWindow *window = newMainWindow();
window->createSession(profile, directory);
finalizeNewMainWindow(window);

In D12968#278225, @hindenburg wrote:

In D12968#277887, @tcanabrava wrote:

> Having a Konsole open and then logging off your desktop; when you log back in, the Konsole should be restored

I found the code that deals with that:

void restoreSession(Application &app)
{
    int n = 1;
    while (KMainWindow::canBeRestored(n)) {
        app.newMainWindow()->restore(n++);
    }
}

Application::newMainWindow, it calls this

connect(window, &Konsole::MainWindow::newWindowRequest, this,
        &Konsole::Application::createWindow);

createWindow:

MainWindow *window = newMainWindow();
window->createSession(profile, directory);
finalizeNewMainWindow(window);

and window->createSession calls ViewManager::newSession that calls
session->addEnvironmentEntry(QStringLiteral("KONSOLE_DBUS_WINDOW=/Windows/%1").arg(managerId()));

that's the reason I said it's strange that the KONSOLE_DBUS_WINDOW is not set, as the only codepoint that I can follow always sets it.

In D12968#278262, @tcanabrava wrote:

and window->createSession calls ViewManager::newSession that calls
session->addEnvironmentEntry(QStringLiteral("KONSOLE_DBUS_WINDOW=/Windows/%1").arg(managerId()));

that's the reason I said it's strange that the KONSOLE_DBUS_WINDOW is not set, as the only codepoint that I can follow always sets it.

Yes, I'd have to debug this more to see why the addEnvironmentEntry doesn't appear be saved. We don't need to hold up this patch IMHO

Be my guest to accept and commit. I'm debugging this right now so I'd expect a patch today / tomorrow.