D24932: [WIP]: Add Button to open the folder in filelight for more details

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kpropertiesdialog.cpp:444
>  {
> -    for (KPropertiesDialogPlugin *it : qAsConst(d->m_pageList)) {
> +    foreach (KPropertiesDialogPlugin *it, d->m_pageList) {
>          if (auto *filePropsPlugin = qobject_cast<KFilePropsPlugin *>(it)) {

We're trying to port our software //away// from using `foreach` and `Q_FOREACH`; not back to them! :) Don't change these.

> kpropertiesdialog.cpp:745
>  
> +static bool isFilelightInstalled()
> +{

All of this logic is unnecessary; instead use https://doc.qt.io/qt-5/qstandardpaths.html#findExecutable

> kpropertiesdialog.cpp:1118
> +        if (isFilelightInstalled()) {
> +            d->m_sizeDetailsButton = new QPushButton(i18n("Open in Filelight"), d->m_frame);
> +            d->m_sizeDetailsButton->setIcon(QIcon::fromTheme(QStringLiteral("filelight")));

Maybe "Explore in Filelight?" or "See usage with Filelight" If the user isn't familiar with what Filelight is, it might be unclear what you would want to open a folder in it.

> kpropertiesdialog.cpp:1409
> +    QProcess *m_filelight = nullptr;
> +    m_filelight->start(QStringLiteral("filelight"), QStringList() << directory);
> +    m_filelight->waitForFinished(); 

Use `KRun` to launch it, not `QProcess`

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks
Cc: kde-frameworks-devel, #frameworks, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191024/976f7428/attachment.html>