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

[Nathaniel Graham]

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


  We're getting there, but this code now causes a crash that must be fixed. Please make sure you are testing your changes for all conditions (i.e. don't just make sure that it works when Filelight is installed, also test the case where it's not installed).

INLINE COMMENTS

> kpropertiesdialog.cpp:782
>      QLabel *m_sizeLabel;
> +    
>      QPushButton *m_sizeDetermineButton;

When you add a newline, make sure it doesn't have any spaces in it

> kpropertiesdialog.cpp:786
> +    QPushButton *m_sizeDetailsButton;
> +    
>      KLineEdit *m_linkTargetLineEdit;

ditto

> kpropertiesdialog.cpp:1099
>          d->m_sizeStopButton = new QPushButton(i18n("Stop"), d->m_frame);
> +        
> +        if (!QStandardPaths::findExecutable(QStringLiteral("filelight")).isEmpty()) {

ditto

> kpropertiesdialog.cpp:1101
> +        if (!QStandardPaths::findExecutable(QStringLiteral("filelight")).isEmpty()) {
> +            d->m_sizeDetailsButton = new QPushButton(i18n("Explore in Filelight?"), d->m_frame);
> +            d->m_sizeDetailsButton->setIcon(QIcon::fromTheme(QStringLiteral("filelight")));

Heh, I think I confused you, my bad. I wasn't trying to imply that the label should actually have a question mark in it. Button labels don't typically end with punctuation like that. Just "Explore in Filelight" is enough.

> kpropertiesdialog.cpp:1105
> +        }
> +        
>          d->m_sizeDetermineButton->setIcon(QIcon::fromTheme(QStringLiteral("view-refresh")));

ditto

> kpropertiesdialog.cpp:1108
>          d->m_sizeStopButton->setIcon(QIcon::fromTheme(QStringLiteral("dialog-cancel")));
> +        
>          connect(d->m_sizeDetermineButton, &QAbstractButton::clicked, this, &KFilePropsPlugin::slotSizeDetermine);

ditto

> kpropertiesdialog.cpp:1114
>          sizelay->addWidget(d->m_sizeStopButton, 0);
> +        sizelay->addWidget(d->m_sizeDetailsButton, 0);
> +

This causes a crash when Filelight isn't installed, because in that case, `d->m_sizeDetailsButton` is a `nullptr`.

> kpropertiesdialog.cpp:1115
> +        sizelay->addWidget(d->m_sizeDetailsButton, 0);
> +
>          sizelay->addStretch(10); // so that the buttons don't grow horizontally

ditto

> kpropertiesdialog.cpp:1453
> +    const QUrl url = properties->url();
> +        
> +    // Open the current folder in filelight

ditto

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/20191227/a2817726/attachment-0001.html>