Review Request 125339: Add an "Open" action

[Ragnar Thomsen]

> On Oct. 6, 2015, 5:06 p.m., Elvis Angelaccio wrote:
> > part/part.cpp, line 716
> > <https://git.reviewboard.kde.org/r/125339/diff/2/?file=409801#file409801line716>
> >
> >     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?

This is rather complicated, but I tried to explain it in the comment. slotAddFiles() takes two arguments. A QStringList with the files to be added with full absolute path, and a relative path. The relative path argument represents the path of the file within the archive, and enables addition of files to subfolders in the archive.

See also new comments in Part::slotAddFiles().


> On Oct. 6, 2015, 5:06 p.m., Elvis Angelaccio wrote:
> > part/part.h, line 84
> > <https://git.reviewboard.kde.org/r/125339/diff/2/?file=409800#file409800line84>
> >
> >     Why `int` instead of `OpenFileMode`? Is it due to `QSignalMapper` non supporting enums?

Yes, QSignalMapper does not support enums.


> On Oct. 6, 2015, 5:06 p.m., Elvis Angelaccio wrote:
> > part/part.cpp, line 731
> > <https://git.reviewboard.kde.org/r/125339/diff/2/?file=409801#file409801line731>
> >
> >     Isn't this called the next time the user will open this file?

If the user opens the file again, it is in a different QTemporaryDir so is not the same file. This code is needed to continue monitoring the file after e.g. Kate saves it.


- Ragnar


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


On Oct. 7, 2015, 10:18 p.m., Ragnar Thomsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125339/
> -----------------------------------------------------------
> 
> (Updated Oct. 7, 2015, 10:18 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/20151007/deb4f736/attachment.html>