Review Request 119877: Make it possible again to open an archive as folder with "dolphin archive.tar.gz"

[Frank Reininghaus]

> On Aug. 21, 2014, 7:09 p.m., Emmanuel Pescosta wrote:
> > dolphin/src/dolphinviewcontainer.cpp, line 463
> > <https://git.reviewboard.kde.org/r/119877/diff/1/?file=306796#file306796line463>
> >
> >     Nitpick: Maybe a more generic description than '... is an archive ...', because openItemAsFolderUrl can not only determine the folder url of archives but also of other files (maybe more in future)?

OK, I'll change it! Please don't worry about introducing the regression. It was far from obvious that your patch would introduce such problems, and I think that the real reason for the regression was that the code for opening archives via the command line was a bit fragile to begin with. Calling slotItemActivated() from slotUrlIsFolderError() is far from elegant for opening archives. We should only do this if we really have no clue what to do with the file.


- Frank


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


On Aug. 20, 2014, 11:03 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119877/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2014, 11:03 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Bugs: 333078
>     http://bugs.kde.org/show_bug.cgi?id=333078
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> This used to work some time ago (at least if "Open archives as folder" was enabled in the settings). I found out some time ago with git bisect that this was broken by https://git.reviewboard.kde.org/r/110487/ , but it was a mystery to me why that (apparently not really related) patch could be a problem.
> 
> I just spent some time debugging it, and it turned out that the crucial difference caused by the patch is that checking if the file is an archive is only done if the mime type is known already. This wasn't the case before that commit.
> 
> The reason why this change in DolphinViewContainer::slotItemActivated(const KFileItem& item) (and code called from there) matters at all in situations where we tell Dolphin to open a file from the command line, and nothing has actually been activated at all inside Dolphin, is the following: the function that gets called when the dir lister reports the error that it has been told to open a file (with an unsuitable protocol) calls slotItemActivated, which then tries to correct the mistake and open the file properly.
> 
> I think the best way to fix this is to handle the "file is archive" case directly in slotUrlIsFileError, and determine the mime type there. We should keep the "isMimeTypeKnown"-check in DolphinView::openItemAsFolderUrl(KFileItem, bool) to prevent GUI blocking if lots of files with unknown type are opened at the same time.
> 
> The additional benefit of this solution is that "dolphin archive.tar.gz" works even if "Open archives as folder" is disabled. So users who usually want to use Ark to handle files can use this method to open an archive inside Dolphin. (I think it's unlikely that anyone actually expects Dolphin to open Ark when entering "dolphin archive.tar.gz" on the command line.)
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphinviewcontainer.cpp 2a45152 
> 
> Diff: https://git.reviewboard.kde.org/r/119877/diff/
> 
> 
> Testing
> -------
> 
> Fixes the problem for me. No new problems found so far.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140831/d2b74433/attachment.htm>