Review Request 110487: Bug 196035 - middle clicking on archive files in dolphin does not open them in a new tab

[Emmanuel Pescosta]

> On May 23, 2013, 11:40 a.m., Frank Reininghaus wrote:
> > Thanks for the patch! The approach looks good, but I think that a function DolphinView::itemCanBeOpenedAsFolder(const KFileItem& item, KUrl& url) which has side effects on the argument 'url' is not a perfect solution. I see though that there is no obvious more elegant alternative :-(
> > 
> > I'll think about this... Maybe we could make DolphinView::itemCanBeOpenedAsFolder(KFileItem) only check if opening the item is possible and adjust the 'protocol' of the URL somewhere else (if necessary), like in DolphinView's constructor and setUrl()? One could then also move the 'GeneralSettings::browseThroughArchives()' check out of that function - I see no reason why middle-clicking an archive should not open it even if that setting is disabled. What do you think?

> DolphinView::itemCanBeOpenedAsFolder(const KFileItem& item, KUrl& url) which has side effects on the argument 'url' is not a perfect solution
Yes I agree with you. 
My first idea was to return a empty url if the item could not be opened as a folder and return the adjusted url if the item could be opened as a folder, but with this solution we need a lot of url.isEmpty() checks, which is also far from perfect I think ;)

Your solution sounds great :) Moving the protocol adjustment code to DolphinView's setUrl() function makes sense.

But we need a new DolphinView::setUrl(const KFileItem& item) member function here, because we need to know the mimetype of the item to adjust the url (Look at DolphinView::itemCanBeOpenedAsFolder). 


- Emmanuel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110487/#review33030
-----------------------------------------------------------


On May 17, 2013, 9:17 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110487/
> -----------------------------------------------------------
> 
> (Updated May 17, 2013, 9:17 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> When 'browse through archives' is enabled, open archive files
> like folders on middle clicking, context menu -> new tab action
> and context menu -> new window action.
> 
> 
> This addresses bug 196035.
>     http://bugs.kde.org/show_bug.cgi?id=196035
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphincontextmenu.cpp af8b613 
>   dolphin/src/dolphinmainwindow.cpp 347489d 
>   dolphin/src/dolphinviewcontainer.cpp 0e413bc 
>   dolphin/src/views/dolphinview.h e50dffc 
>   dolphin/src/views/dolphinview.cpp abf572f 
> 
> Diff: http://git.reviewboard.kde.org/r/110487/diff/
> 
> 
> Testing
> -------
> 
> Middle clicking or context menu -> new tab action opens the archive in a new tab, if 'browse through archives' is active.
> 
> Context menu -> new window action opens the archive in a new window, if 'browse through archives' is active.
> 
> Works for me.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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