• Allow to move a tab between splitters
• Simplify Check
• Fix tab management
• currentTabChanged was never connected
• Fix the redraw after focus
• Create the default container on constructor
• Return the current tabWidget from the view
• Be explicit in where we are creating the new session
• Always pass the tab that the session will be created
• Export the ViewContainer
• Expand the signals / slots to use the tabWidget
• After cretaing the view, focus it
• If a side of the split is close, focus another tab
• Fix size of the new Tab

As usual thanks for the really good review.
I’ll address the Points tomorrow as today I’m traveling.

The last fix (opening a tab by double clicking on the empty space on the tabbar) was a bit more intrusive than what I wanted (as the signal must be forwarded to the MainWindow / KPart, to be forwarded back and the pointer to the TabWidget that was clicked needed to be in the callchain. To make it more safe I removed the signal-casts in the connect changing the names of the signals, keeping compile-time correctness, so I'm pretty sure I didn't broke code by doing that. The calls that could use the ~default~ tab now just ask for activeContainer() as a parameter. in the end the code is a bit safer and more explicit on what's happening.

• Allow to move a tab between splitters
• Simplify Check
• Fix tab management
• currentTabChanged was never connected
• Fix the redraw after focus
• Create the default container on constructor
• Return the current tabWidget from the view
• Be explicit in where we are creating the new session
• Always pass the tab that the session will be created
• Export the ViewContainer
• Expand the signals / slots to use the tabWidget

Ok, I'll take a look.
Seems easy to fix.

considering that this is a behavioral change I'll wait for @hindenburg ok to merge this.

• currentTabChanged was never connected
• Fix the redraw after focus

That makes sense if you are processing the line and doing something with it
and discarding, but not appending to a string doing nothing with it until
the result is the whole file content. If what you want is the same as the
readAll, this one is faster as there’s no allocations for the temporary.

untill you click on it. I'v realized that but I belive this is not a
blocker. The TerminalDisplay has *too many* issues not redrawing correctly
after a move untill someone clicks on it or it gains focus.

Ready for review, I belive I killed all the bugs related to this.

• Fix tab management

There's still one bug when I try to drag a tab from a window *back* into another window when I just habe one tab. I need to fix that before having an accept.

• Simplify Check

• Allow to move a tab between splitters

I plan to remove the hability to have tabs per split in the future, I
believe an implementation like terminator makes more sense. One Tabbar,
splits inside of the tabs. But konsole code is really tied and I’m doing
many cleanup commits just to be able to change that in the future.

arc diff created a new revision instead of updating this one, sorry for the noise. https://phabricator.kde.org/D17267

I'm a bit lost on what cause it so I accept help if you have a bit of time.
The crash is inside the eventFilter of the mainwindow with the following
backtrace:

sorry for the crash, I'm researching what caused it.

before:

repaint will trigger a reppaint forcefully, if you use update() are we skipping frames?

Just a nitpick, an empty QString should be QString(), not QString("")

Well, the code that got in qApp->applicationName() == "konsole" shouldn't be part of a class, it's a perfectly valid function. this would fix everything. I'll prepare a patch for this later.

I wouldn't add this. it's just more clutter to the code and to the settings.
mathematically speaking, if a number is not prepended with a sign, it's positive.
I would say to drop the '+' instead of adding a configuration for that.

I don't understand why patrick wanted all regular expressions to be static const (I would create them as non-static, but that's a non issue)

Waiting for the whitespace clenaup before merging.

In D16863#358962, @apol wrote:

Note you have changed the indentation of the modified lines. Make sure indentation isn't changed (unless it's required specificly).

too many +1 to not be accepted.

arc diff created a new revision, sorry for that. https://phabricator.kde.org/D16706

I didn't, I looked for isPart, isKPart but my grep didn't try
isKonsokePart. I'll update the code.

In D16138#352911, @hindenburg wrote:

well I'm a bit confused why I don't see your results under the current master branch - I've tried 2 different VMs and verified there are no konsole files anywhere and get the results I mentioned above.

While this seems correct the code feels like it should be part of a model. I'm +1 this but can you add a TODO: to create a proper model later?

Just this small thing and it can go in.

We can always revert when this is working again

Can you post a screenshoot of the before / after?

ping?

You have to rebase, and probably fix the conflict because I moved the code
to a real model.

now I need a re-acceptance.

There, no more std::get nor cast from enum ;)

• Implement the QuickOpen as a TableModel
• Code Cleanup
• Fix displayign in bold the open files
• Add License
• Use struct instead of std::tuple

If the user has a encrypted disk but the disk is mounted, then this is
userfull to not allow other mallicious app in the same disk to peek into
the history file, no?

Strings with spaces should be inside quotes., just like the Helvetica Neue is.

I don't like that. What happens if the person is not int he internet? can you remove that url completely and see if it works?
*if* it works, I'd rather remove the need to be on the internet.

In D16134#342047, @langbeck wrote:

I asked @frederico to run in an old notebook that he has with a Celeron u2300 1.2GHz running Debian Buster.
He used find in its home folder to test. After some time I asked him to scroll back and forth and tell if he noticed any difference.
Apparently he did not found any noticeable difference.

In D16129#342351, @ervin wrote:

Yes, I went for my own properties due to more precise naming, but I admit I don't mind either way on the technical level. So what should I do? Change it or stay as is? You guys seem to disagree a bit. :-)