D21783: [WIP]Show more details in warning dialog shown before starting a privileged operation

[David Faure]

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

INLINE COMMENTS

> slavebase.cpp:1525
> +
> +#ifndef KIOCORE_NO_DEPRECATED
> +PrivilegeOperationStatus SlaveBase::requestPrivilegeOperation()

`#if KIOCORE_BUILD_DEPRECATED_SINCE(5, 65) `

(notice BUILD, not ENABLE)

> slavebase.h:960
> +
> +#ifndef KIOCORE_NO_DEPRECATED
> +    KIOCORE_DEPRECATED PrivilegeOperationStatus requestPrivilegeOperation();

The modern way is `#if KIOCORE_ENABLE_DEPRECATED_SINCE(5, 65)`

> slavebase.h:961
> +#ifndef KIOCORE_NO_DEPRECATED
> +    KIOCORE_DEPRECATED PrivilegeOperationStatus requestPrivilegeOperation();
> +#endif

`KIOCORE_DEPRECATED_VERSION(5, 65, "Pass QString action to requestPrivilegeOperation")`

> slaveinterface.h:89
>      MSG_PRIVILEGE_EXEC,
> -    MSG_SLAVE_STATUS_V2
> +    MSG_SLAVE_STATUS_V2,
>      // add new ones here once a release is done, to avoid breaking binary compatibility

unrelated/unneeded anymore

> slaveinterface_p.h:56
> +    // Since 5.65 this is used for sending privilege operation details.
> +    // KF6 TODO remove this hack.
> +    MetaData privilegeConfMetaData;

Please explain how, for when the time comes (in case you're not around to do it).

> file_unix.cpp:84
> +    QString action, detail;
> +    switch(actionType) {
> +    case CHMOD:

missing space after `switch` (same rule as `if`, `for` etc.)

> dfaure wrote in file_unix.cpp:81
> `const QVariantList &`

marked as done, but not done

> jobuidelegate.cpp:375
> +        const QString details = metaData.value(QStringLiteral("privilege_conf_details"));
> +        result =  KMessageBox::warningContinueCancelDetailed(
> +                      window(), text, caption, KStandardGuiItem::cont(), KStandardGuiItem::cancel(), dontAskAgainName,

two spaces after `=`, remove one

> jobuidelegate.cpp:377
> +                      window(), text, caption, KStandardGuiItem::cont(), KStandardGuiItem::cancel(), dontAskAgainName,
> +                      KMessageBox::Options(KMessageBox::Dangerous | KMessageBox::WindowModal), details);
> +        break;

I don't think you need the `KMessageBox::Options(` around the value?

> jobuidelegate.h:155
>                            const QString &dontAskAgainName = QString(),
> -                          const KIO::MetaData &sslMetaData = KIO::MetaData()) override;
> +                          const KIO::MetaData &metaData = KIO::MetaData()) override; // KF6 TODO Add proper API
>  

what would be the proper API you have in mind?

REPOSITORY
  R241 KIO

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

To: chinmoyr, #vdg, #frameworks, dfaure
Cc: mreeves, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191206/95ec0c91/attachment-0001.html>