[Differential] [Requested Changes To] D1364: Implement GUI for setting compression level

elvisangelaccio (Elvis Angelaccio) noreply at phabricator.kde.org
Sat Apr 9 10:24:17 UTC 2016


elvisangelaccio requested changes to this revision.
This revision now requires changes to proceed.

INLINE COMMENTS
  kerfuffle/archiveformat.cpp:63 Initialize this to `Archive::Unencrypted`, so that you can drop the `else` branch below.
  kerfuffle/archiveformat.h:41-70 What about using an enum here? Something like: 
  
  ```
  enum CompressionLevel
  {
      MIN,
      MAX,
      DEFAULT
  }
  ```
  
  Then you would only need one class member and one accessor method. 
  kerfuffle/cliinterface.cpp:255-258 Shorter version: `options.value(QStringLiteral("CompressionLevel"), -1).toInt()`
  kerfuffle/cliinterface.cpp:656 Please move this check within `compressionLevelSwitch()` (to be consistent with the other switch functions). If the level is -1, the function should return an empty string.
  kerfuffle/createdialog.ui:148 I don't think we should expand it by default. Otherwise the user might think that he's forced to choose a compression level.
  kerfuffle/createdialog.ui:154 Not sure about the "Less" and "More" labels. Maybe "Min" and "Max" would be better?
  plugins/clizipplugin/kerfuffle_clizip.json:61 Call this key `CompressionLevelDefault` ?
  plugins/libarchive/kerfuffle_libarchive.json:59-63 These are unnecessary, you can remove them. `QJsonValue::toInt()` already defaults to 0 if the key is not found.
  Same for the other formats that don't support compression levels.

REPOSITORY
  rARK Ark

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

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

To: rthomsen, elvisangelaccio
Cc: kde-utils-devel, tctara


More information about the Kde-utils-devel mailing list