Review Request 125339: Add an "Open" action

[Elvis Angelaccio]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125339/#review86430
-----------------------------------------------------------



part/part.h (line 84)
<https://git.reviewboard.kde.org/r/125339/#comment59548>

    Why `int` instead of `OpenFileMode`? Is it due to `QSignalMapper` non supporting enums?



part/part.cpp (line 324)
<https://git.reviewboard.kde.org/r/125339/#comment59550>

    Since now we can update entries, you must also check the value of `isWritable`.
    
    Please fix also the sytle, the `);` on a single line is not consistent with the other code here.



part/part.cpp (line 326)
<https://git.reviewboard.kde.org/r/125339/#comment59551>

    Same as the comment above.



part/part.cpp (line 617)
<https://git.reviewboard.kde.org/r/125339/#comment59552>

    The `isPreviewable()` function should be called only if the opening mode is `Preview`. Otherwise, if you want to call it regardless the openening mode, you should do so also in `updateActions()`.



part/part.cpp (line 661)
<https://git.reviewboard.kde.org/r/125339/#comment59549>

    Please update also this comment, since you have renamed `m_previewMode`.



part/part.cpp (line 704)
<https://git.reviewboard.kde.org/r/125339/#comment59555>

    This prints to the user the absolute path of the temporary file. It would be better to only show the filename.



part/part.cpp (line 710)
<https://git.reviewboard.kde.org/r/125339/#comment59554>

    This part is a bit confusing. Are you using a relative path for a file being updated and an absolute path for a file being added from scratch?



part/part.cpp (line 725)
<https://git.reviewboard.kde.org/r/125339/#comment59553>

    Isn't this called the next time the user will open this file?


- Elvis Angelaccio


On Oct. 5, 2015, 5:25 p.m., Ragnar Thomsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125339/
> -----------------------------------------------------------
> 
> (Updated Oct. 5, 2015, 5:25 p.m.)
> 
> 
> Review request for KDE Utils, KDE Usability, Elvis Angelaccio, and Raphael Kubo da Costa.
> 
> 
> Bugs: 201162 and 208330
>     http://bugs.kde.org/show_bug.cgi?id=201162
>     http://bugs.kde.org/show_bug.cgi?id=208330
> 
> 
> Repository: ark
> 
> 
> Description
> -------
> 
> Add an Open action, which opens an archive entry with the associated application. This was previously only possible through the "Open With" action, but this required several clicks. Currently, the Preview action is still the default, i.e. when the user clicks an archive entry the Preview action is called.
> 
> QSignalMapper is used to connect the signals from the Open, Open With and Preview actions to the same slot.
> 
> As for Preview, the file is extracted to a QTemporaryDir and then opened. The temporarily extracted file is monitored for changes using QFileSystemWatcher and on change a KMessageBox is opened to query the user if the archive should be updated with the modified file. If the user accepts slotAddFiles() is called.
> 
> The previously unimplemented path argument of slotAddFiles() is used to allow for updating a file in a subdirectory within the archive.
> 
> Things to consider:
> 1. Should we set some default keyboard shortcuts for "Open File" and "Open File With"? If yes, which?
> 
> 
> Diffs
> -----
> 
>   part/ark_part.rc da04d47 
>   part/part.h 1d733c0 
>   part/part.cpp 8854fdd 
> 
> Diff: https://git.reviewboard.kde.org/r/125339/diff/
> 
> 
> Testing
> -------
> 
> Tested with tar-based, zip, rar and 7z archives.
> 
> 
> File Attachments
> ----------------
> 
> Context menu
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/10/05/b0b7c44d-2b40-4772-b8d0-a81f6895518a__open-action-1.png
> Action menu
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/10/05/deab8bb1-722c-48b8-bce3-64190afc6a53__open-action-2.png
> 
> 
> Thanks,
> 
> Ragnar Thomsen
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20151006/089858c4/attachment.html>