[Konsole-devel] Review Request: Allow tab tearing

[Jekyll Wu]

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


Thanks for this one-liner patch. 

"I failed to implement the following: drag a tab from an instance of konsole A and drop to an instance of konsole B."

I guess 'instance' here means process instead of window, since konsole already supports dragging tab between windows as long as those windows belong to the same process. Dragging tab between different konsole processes is practically impossible. So not a issue.

There is a minor issue with that simple else clause. If there is only one tab in the current tabbar , dragging that tab out would trigger the detach, which feels unnecessary and unnatural. I think an extra check for the existing tab number would be better, like this :

    if( _tabBar.count() > 1 )
        emit detachTab(this, view);


- Jekyll Wu


On May 13, 2012, 8:25 p.m., makis marimpis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104933/
> -----------------------------------------------------------
> 
> (Updated May 13, 2012, 8:25 p.m.)
> 
> 
> Review request for Konsole.
> 
> 
> Description
> -------
> 
> An one-liner patch :}
> Allows the user to tear out the tabs (with drag and drop).
> I failed to implement the following: drag a tab from an instance of konsole A and drop to an instance of konsole B.
> 
> 
> This addresses bug 56749.
>     http://bugs.kde.org/show_bug.cgi?id=56749
> 
> 
> Diffs
> -----
> 
>   src/ViewContainer.cpp 1a0148e9e2e4185a2f8356b83b0bc971f4e8ce52 
> 
> Diff: http://git.reviewboard.kde.org/r/104933/diff/
> 
> 
> Testing
> -------
> 
> Yes, seems to work fine.
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20120513/b6f86d38/attachment.html>