[Differential] [Requested Changes To] D3303: Implement support for setting encryption method

[elvisangelaccio (Elvis Angelaccio)]

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

INLINE COMMENTS

> archive_kerfuffle.cpp:182
> +
> +    if (!property("compressionMethods").toStringList().contains(method) &&
> +        method != QLatin1String("Store")) {

Double lookup, we can just use `methods` here.

> archive_kerfuffle.cpp:196
> +
> +    if (!property("encryptionMethods").toStringList().contains(method)) {
> +        methods.append(method);

Same here, use `methods`.

> archiveformat.cpp:48-49
> +                             const QString& defaultCompressionMethod,
> +                             QVariantMap encryptionMethods,
> +                             QString defaultEncryptionMethod) :
>      m_mimeType(mimeType),

Pass by reference

> archiveformat.h:50-51
> +                           const QString& defaultCompressionMethod,
> +                           QVariantMap encryptionMethods,
> +                           QString defaultEncryptionMethod);
>  

Pass by reference.

> cliproperties.cpp:264
> +
> +    QString cliMethod = ArchiveFormat::fromMetadata(m_mimeType, m_metaData).encryptionMethods().value(method).toString();
> +

Double lookup, let's use a variable to hold the archive format object.

> compressionoptionswidget.cpp:277
> +        if (value == QLatin1String("RAR4")) {
> +            encMethodComboBox->clear();
> +            encMethodComboBox->insertItem(0, QStringLiteral("AES128"));

This can be moved before the if/else

> propertiesdialog.cpp:87
>      case Archive::Encrypted:
> -        m_ui->lblPasswordProtected->setText(i18n("yes (excluding the list of files)"));
> +        m_ui->lblPasswordProtected->setText(i18n("yes (%1 encryption)", archive->property("encryptionMethods").toStringList().join(QStringLiteral(", "))));
>          break;

I would keep the information about simple VS header-encryption. How about something like this:

Password-protected: No | Only files content (<enc. method>) | Yes (<enc. method>)

The last case is header encryption, but we don't really need to be that specific. After all it's our default in the CreateDialog, so a simple Yes could be enough here.

REPOSITORY
  rARK Ark

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: rthomsen, elvisangelaccio
Cc: kde-utils-devel, tctara
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20161108/75b2d835/attachment.html>