| elvisangelaccio |
This diff enables readwrite support for lzipped archives in the libarchive plugin. Lzip is a file compressor and is usually used in combination with tar.
One issue:
lzip does not have a "tar.lz"-mimetype but only a "lz" mimetype (application/x-lzip). This means that when creating a new archive, the dropdown menu only shows the extention .lz, but libarchive actually creates a tar.lz archive. Do we want to add an exception and force a tar.lz filter in CreateDialog?
| Lint Skipped |
| Unit Tests Skipped |
Changed CreateDialog to be able to modify the comment and glob pattern for lzip archives. We now use KFileWidget::setFilter() instead of KFileWidget::setMimeFilter() to populate the filterlist. This allows more flexibility in adding archive types to the list.
It is now possible to modify the filterlist, so we can support archives with no mimetype in shared-mime-info. Also, now the primary glob pattern is visible in the dropdown menu of CreateDialog.
| autotests/kerfuffle/archivetest.cpp | ||
|---|---|---|
| 286 | QString() instead of QStringLiteral(""), same for the other occurrences below. | |
| kerfuffle/createdialog.cpp | ||
| 254 | Opening brace after the new line, same for CreateDialog::currentFilterMimeType() | |
| kerfuffle/createdialog.h | ||
| 53 | Since these are public, better not use the m_ prefix scheme. | |
| 81 | Missing const. | |
| 82 | If this function accepted a QString with the name() of the mimetype, the patch would be a bit simpler:
| |
Sorry, didn't notice this earlier. We're almost done :)
| autotests/kerfuffle/createdialogtest.cpp | ||
|---|---|---|
| 71 ↗ | (On Diff #2684) | This slot is now basically useless outside of CreateDialog. It makes more sense to have it called from setCurrentFilterFromMimeType() instead. |
| autotests/kerfuffle/createdialogtest.cpp | ||
|---|---|---|
| 71 ↗ | (On Diff #2684) | Yeah, there is something strange about this. When I run Ark normally and open the CreateDialog, calling setCurrentFilterFromMimeType() will emit the filterChanged() signal and hence call slotFilterChanged(). This does not happen when calling setCurrentFilterFromMimeType() from the tests and therefore I added the direct calls to slotFilterChanged() in the tests. So making setCurrentFilterFromMimeType() call slotFilterChanged() will result in slotFilterChanged() being called twice normally. |
| autotests/kerfuffle/createdialogtest.cpp | ||
|---|---|---|
| 71 ↗ | (On Diff #2684) | I think we should try to connect to the currentIndexChanged signal instead. That's the one emitted when setting the index within setCurrentFilterFromMimeType(). |
| kerfuffle/createdialog.cpp | ||
| 311 ↗ | (On Diff #2684) | Would be better to use QStringLiteral("%1 (%2)\n").arg(...) here. |
Implement Elvis' suggestions.
We now connect the KFileFilterCombo::currentIndexChanged() signal instead of KFileWidget::currentFilterChanged(). The currentIndexChanged() signal is overloaded and can send both an int and QString, so the conect syntax is slightly more complex. OTOH we can get rid of the direct slotCurrentFilterChanged() calls in the CreateDialogTest and also we pass an int now with the signal so we can actually use it and save a currentFilterMimeType() call in slotFilterChanged(). slotFilterChanged() was changed from public to protected.
Also renamed updateDefaultMimeType() to slotUdateDefaultMimeType() to be consistent.
Thanks! Now is really good to go :)
| kerfuffle/createdialog.cpp | ||
|---|---|---|
| 309 | Is the | character a typo? (btw it does not show up in the UI...) | |
| kerfuffle/createdialog.cpp | ||
|---|---|---|
| 309 | No, the combined filterstring needs to be constructed like this: | |