D11382: Add an option to show tabs from last time when Dolphin starts

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  In D11382#644972 <https://phabricator.kde.org/D11382#644972>, @ngraham wrote:
  
  > As for the mis-alignment in the settings window, I was not able to figure out how to improve that. Suggestions are welcome.
  
  
  Maybe there is some padding left in the layouts that needs to be zeroed out?

INLINE COMMENTS

> dolphinmainwindow.cpp:212-218
> +    if (GeneralSettings::rememberOpenedTabs())  {
> +        KConfigGui::setSessionConfig(QStringLiteral("dolphin"), QStringLiteral("dolphin"));
> +        KConfig *config = KConfigGui::sessionConfig();
> +        saveGlobalProperties(config);
> +        savePropertiesInternal(config, 1);
> +        config->sync();
> +    }

Please move this logic in `DolphinMainWindow::closeEvent()`.

> global.cpp:81
>  
> -    QVector<QPair<QSharedPointer<OrgKdeDolphinMainWindowInterface>, QStringList>> dolphinInterfaces;
> -    if (!preferredService.isEmpty()) {
> -        QSharedPointer<OrgKdeDolphinMainWindowInterface> preferredInterface(
> -            new OrgKdeDolphinMainWindowInterface(preferredService,
> -                QStringLiteral("/dolphin/Dolphin_1"),
> -                QDBusConnection::sessionBus()));
> -        if (preferredInterface->isValid() && !preferredInterface->lastError().isValid()) {
> -            dolphinInterfaces.append(qMakePair(preferredInterface, QStringList()));
> -        }
> -    }
> -
> -    // Look for dolphin instances among all available dbus services.
> -    const QStringList dbusServices = QDBusConnection::sessionBus().interface()->registeredServiceNames().value();
> -    // Don't match the service without trailing "-" (unique instance)
> -    const QString pattern = QStringLiteral("org.kde.dolphin-");
> -    // Don't match the pid without leading "-"
> -    const QString myPid = QLatin1Char('-') + QString::number(QCoreApplication::applicationPid());
> -    for (const QString& service : dbusServices) {
> -        if (service.startsWith(pattern) && !service.endsWith(myPid)) {
> -            // Check if instance can handle our URLs
> -            QSharedPointer<OrgKdeDolphinMainWindowInterface> interface(
> -                        new OrgKdeDolphinMainWindowInterface(service,
> -                            QStringLiteral("/dolphin/Dolphin_1"),
> -                            QDBusConnection::sessionBus()));
> -            if (interface->isValid() && !interface->lastError().isValid()) {
> -                dolphinInterfaces.append(qMakePair(interface, QStringList()));
> -            }
> -        }
> -    }
> -
> +     QVector<QPair<QSharedPointer<OrgKdeDolphinMainWindowInterface>, QStringList>> dolphinInterfaces = dolphinInstanceData(preferredService);
>      if (dolphinInterfaces.isEmpty()) {

Please use `auto`.

> main.cpp:144
> +    // We later mutate urls, so we need to store if it was empty originally
> +    bool urlCommandLineArguments = !urls.isEmpty();
> +

Please give it a descriptive name, e.g. `startedWithUrls`, and make it `const`.

> main.cpp:187
> +    // 1. Dolphin was not started with command line args
> +    // 2. There is a session availablr to restore
> +    // 3. The "remember state" setting is enabled

Typo: available

> main.cpp:192
> +        const QString serviceName = QStringLiteral("org.kde.dolphin-%1").arg(QCoreApplication::applicationPid());
> +        if (Dolphin::dolphinInstanceData(serviceName).size() > 0) {
> +            const QString className = KXmlGuiWindow::classNameOfToplevel(1);

What is this check needed for?

> startupsettingspage.cpp:131-132
> +    connect(m_rememberOpenedTabsRadioButton, &QRadioButton::toggled, this, &StartupSettingsPage::slotSettingsChanged);
> +    connect(m_homeUrlRadioButton, &QRadioButton::toggled, this,
> +    &StartupSettingsPage::slotSettingsChanged);
> +

Please use a single line.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D11382

To: ngraham, #dolphin, #vdg, feverfew, meven, elvisangelaccio, ndavis
Cc: davidedmundson, ndavis, intika, feverfew, kfm-devel, ngraham, broulik, #dolphin, azyx, nikolaik, pberestov, iasensio, fprice, fbampaloukas, alexde, Codezela, meven, spoorun, navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200412/891ea1bf/attachment.htm>