I’m ok with it.

• More guards removed.
• Remove unused include

• Fix compilation

already merged and a followup was also merged fixing the issues related by @broulik

@hindenburg ping?

• Fix code style
• re-add assert
• Fix code style

In D18575#401418, @ngraham wrote:

No other KDE app does this

That's not true. Kate and Gwenview do this too, and for good reason: without this warning, there's no way to know how to get the menu bar back again if you remove it by accident and/or don't know about the ctrl+m shortcut.

Konsole doesn't show a warning because it's a power user app; presumably the user knows what he/she is doing. Dolphin doesn't have this warning because it automatically creates a menu-like toolbar button when the menubar is hidden, which is an acceptable alternative. But for apps that don't do this or something else (e.g. putting the menubar in the titlebar instead) we need to show an ugly warning like this.

There are two different patches for that, one from myself that adds a dim
effect (that Kurt asked to remove from this change set) and another one
that implements a header, also in a different review (but not from myself
and I don’t have the review number on my head).

Be my guest

• don't copy the map

• Call directly, lambda is uneeded here.

• Remove unused function - a findChildren is what it's needed

• Properly count the number of Terminals

• Fix creation of splits in the middle of the grid
• Remove Dim code: I'll add that to another review

• Remove deprecated code
• Refactor to reduce the number of boilerplate
• Fix updating the detach menus
• Dim unfocused terminals
• Rebase on master

While I'm reopening this I have zero time to tackle this right now because of real work.

They aren’t, just forgot to remove all of them

seems pretty straigthforward.

I tough about that but it’s not that easy unless we also refactor the
Profile class, because everything in the profile class is stored inside a
map. It would be quite expensive to always access it.

geh, then let's keep this one and remove the other one.

I'll close this one, let's use yours.

please rebase on top of master, there's a conflict.

@ngraham is this accepted? I can't tel because of the talk after the last ok.

I have also a patch for Yakuake to fix them too, they are using one kpart
per view instead of using the tabbar that konsole part provides. :/
This week it will be hard for me to touch this code but I’ll see what I can
do.

No; both are unrelated

In D18578#401842, @apol wrote:

Looks okay from a theoretical point of view, I can expect the practical impact of this be about 0~ :P

ping?

I agree with the change, specially that now there's a warning on implicit falltrougth

I still don't like the shared ptr but let's go with that for now.

Let me interfeer a bit.

landed.

• Accept commas for PID

@apol can you also check here?

Ok. I’ll extract from here.