[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