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
