Be explicit on what the ViewContainer accepts
ClosedPublic

Authored by tcanabrava on Dec 10 2018, 5:29 PM.

Details

Summary

This is needed to clean up a bit of the code later.
being really explicit here removes the need to handle
QWidget pointers, as that could in theory be anything

Handling only TerminalDisplays* we can remove the
SessionController map from the ViewContainer and
add it to the TerminalDisplay class in the future

Diff Detail

Repository
R319 Konsole
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
tcanabrava created this revision.Dec 10 2018, 5:29 PM
Restricted Application added a project: Konsole. · View Herald TranscriptDec 10 2018, 5:29 PM
Restricted Application added a subscriber: konsole-devel. · View Herald Transcript
tcanabrava requested review of this revision.Dec 10 2018, 5:29 PM
tcanabrava updated this revision to Diff 47305.Dec 10 2018, 6:06 PM
  • Simplify logic: SessionController already has a map

I'll see if I find anything after testing

src/ViewContainer.cpp
90

Why not use terminalAt() here?

tcanabrava updated this revision to Diff 47422.Dec 12 2018, 8:51 AM
  • use terminalAt instead of casting explicetly

If you can comment on the foreach removals, I don't see any other issues ATM

src/ViewContainer.cpp
471–474

The only think that looks odd to me is you removed a couple of these foreach widgetsForItems loops. Are you sure that's what you wanted?

471, 489, 493

Before 7a43e3b2cde05a7 the tabs where mirrors of sessions, so more
than one tab could belong to the same session controller, this is not
true anymore, a tab has it's own TerminalDisplay / SessionManager
pair.

hindenburg accepted this revision.Dec 12 2018, 4:14 PM

Ok that makes sense now - thanks

This revision is now accepted and ready to land.Dec 12 2018, 4:14 PM
This revision was automatically updated to reflect the committed changes.