Move isPrimaryScreen() to Session
ClosedPublic

Authored by ahmadsamir on May 30 2018, 4:22 PM.

Details

Summary

Previously the code enabled sending emulated up/down key press events to
the terminal only if the current screen is the alternate buffer. The
problem with this approach is that when detaching a tab/view, a new
TerminalDisplay is constructed and _isPrimaryScreen is initialized to
true even though the current screen in the detached tab is the alternate
one.

Since the Session is preserved when detaching a tab, move that bit of
code to Session, and query the Session about the currently used screen
buffer.

Test Plan

Before:

  • Open a window with two tabs, and in one of them execute man man
  • Make sure "Enable Alternate screen buffer scrolling" is enabled in the

current profile, and that you can can scroll in the alternate buffer

  • Detach the tab showing the manual page, now scrolling doesn't work in

the alternate buffer

With the patch applied, scrolling should work after detaching the tab.

Diff Detail

Repository
R319 Konsole
Branch
alternate-buffer-scrolling (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
ahmadsamir created this revision.May 30 2018, 4:22 PM
Restricted Application added a project: Konsole. · View Herald TranscriptMay 30 2018, 4:22 PM
Restricted Application added a subscriber: konsole-devel. · View Herald Transcript
ahmadsamir requested review of this revision.May 30 2018, 4:22 PM
ahmadsamir updated this revision to Diff 35197.May 30 2018, 4:25 PM
ahmadsamir edited the test plan for this revision. (Show Details)

Used --verbatim before adding a Test Plan:

hindenburg edited the summary of this revision. (Show Details)May 31 2018, 2:55 AM
hindenburg added a subscriber: hindenburg.

Thanks- I'll test it more later

src/TerminalDisplay.cpp
2838 ↗(On Diff #35197)

Do we need to check sessionController()->session() != nullptr?

ahmadsamir updated this revision to Diff 35267.May 31 2018, 4:36 PM
ahmadsamir retitled this revision from Move isPrimaryScreen() to the Session class to Move isPrimaryScreen() to the Session.
ahmadsamir edited the summary of this revision. (Show Details)
ahmadsamir removed a subscriber: hindenburg.

Change the code to check that session* is valid before accessing it

ahmadsamir marked an inline comment as done.May 31 2018, 4:37 PM
ahmadsamir added inline comments.
src/TerminalDisplay.cpp
2838 ↗(On Diff #35197)

Better safe than sorry; IIRC, there are multiple places throughout the code where the session* is checked to prevent crashes.

ahmadsamir updated this revision to Diff 35268.May 31 2018, 4:45 PM
ahmadsamir marked an inline comment as done.
ahmadsamir retitled this revision from Move isPrimaryScreen() to the Session to Move isPrimaryScreen() to Session.

Tweak commit message

OK thanks, using 'man man' did not work for me but yes vim file did work in the testing.

hindenburg accepted this revision.Jun 1 2018, 12:42 PM
This revision is now accepted and ready to land.Jun 1 2018, 12:42 PM
This revision was automatically updated to reflect the committed changes.
ahmadsamir marked an inline comment as done.Jun 1 2018, 1:23 PM

OK thanks, using 'man man' did not work for me but yes vim file did work in the testing.

Assuming the default pager on your setup is less, then I guess the behaviour well depend on whatever $LESS is set to, for example "-X" in LESS will make it use the alternate buffer is some weird way.