D18945: New tab should be placed after the current tab

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> dolphintabwidget.cpp:161-168
> +    switch (tabPlacement) {
> +    case TabPlacement_AfterCurrent:
> +        newTabIndex = currentIndex() + 1;
> +        break;
> +    case TabPlacement_Last:
> +        newTabIndex = -1;
> +        break;

How about a simpler if?

  int newTabIndex = -1;
  if (tabPlacement == AfterCurrent) {
      newTabIndex = currentIndex() + 1;
  }

(we can always make it a `switch` if we add new values to the enum, which is unlikely).

> dolphintabwidget.h:42
> +          */
> +        TabPlacement_AfterCurrent,
> +        /**

How about `AfterCurrentTab` and `AfterLastTab` ? The underscore is unusual in camelcase names.

REPOSITORY
  R318 Dolphin

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

To: hallas, #dolphin, ngraham, elvisangelaccio
Cc: elvisangelaccio, ngraham, kfm-devel, alexde, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190216/a36365c2/attachment.htm>