D19201: New tab placed after current tab when middle-clicking

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> hallas wrote in dolphinmainwindow.h:50
> We could, I was just trying to minimize the amounts of includes in the header file, I guess that is also the reason for the other forward declarations? One of the arguments for using enum class is specifically that you can forward declare it, it is technically also possible with an unscoped enumeration but you can't do it in all situations.

> We could, I was just trying to minimize the amounts of includes in the header file, I guess that is also the reason for the other forward declarations?

Those are fine because you don't need to include the whole header if you just need a pointer to the object.
But `DolphinTabPlacement` is part of the API of `DolphinTabWidget`, so it's ok to include its header file. Otherwise you have to duplicate a pieace of the API here. As you had to do it in `dolphinview.h`. Imho this makes the code (and the patch) more complex without gaining anything useful.

> hallas wrote in dolphintabwidget.h:33
> Forward declaration is one argument, but also that you get a scope so that the individual enumerators doesn't name clash with anything else. Does KDE have any general guidelines for using/not using this? Should I change it?

The clash protection is a good argument, but then it's an unrelated change that should go in another commit (and there are many other old-style enums in dolphin which should be ported).

I don't think we have guidelines about enums in KDE, but using `enum class` for new code makes sense of course.

REPOSITORY
  R318 Dolphin

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

To: hallas, #dolphin, ngraham, elvisangelaccio
Cc: 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/20190224/11bf5648/attachment.htm>