D5183: Reopen accidentally closed tab

[Sven Brauch]

brauch added a comment.


  Looks good except what is noted below, thanks!

INLINE COMMENTS

> container.cpp:309
>      connect(d->tabBar, &ContainerTabBar::tabCloseRequested, this, static_cast<void(Container::*)(int)>(&Container::requestClose));
> +    connect(d->tabBar, &ContainerTabBar::tabCloseRequested, this, static_cast<void(Container::*)(int)>(&Container::addToReopenQueue));
>      connect(d->tabBar, &ContainerTabBar::newTabRequested, this, &Container::newTabRequested);

why do you need the cast here?

> container.cpp:365
> +    if (auto view = viewForWidget(widget(idx))) {
> +        auto urlDocument = qobject_cast<UrlDocument*>(view->document());
> +        QUrl url = urlDocument->url();

there is no point in using qobject_cast over static_cast if you don't check the result; not sure if you need to?

> mainwindow_p.cpp:136
> +    action = new QAction(i18n("Reopen Closed File"), this);
> +    ac->setDefaultShortcut(action, Qt::META | Qt::Key_T);
> +    action->setIcon(QIcon::fromTheme(QStringLiteral("document-open")));

I suggest Ctrl+Shift+T, like in Firefox for example. I think meta is weird for shortcuts, it is not used anywhere else.

REPOSITORY
  R33 KDevPlatform

REVISION DETAIL
  https://phabricator.kde.org/D5183

To: coopht, #kdevelop
Cc: brauch, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170326/f4b53a08/attachment-0001.html>