Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement scroll up and down #103

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Implement scroll up and down #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a2dcb2
Add SU and SD constants to pyte.escape
srstevenson Nov 21, 2017
1da5b4a
Add scroll_down and scroll_up entries to csi dict
srstevenson Nov 21, 2017
f15ea30
Override copy method so that StaticDefaultDict does not return a regu…
zblz Nov 21, 2017
97843a0
Implement Screen.scroll_down and Screen.scroll_up
srstevenson Nov 21, 2017
2e86d5d
Test Screen.scroll_down and Screen.scroll_up
srstevenson Nov 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pyte/escape.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,9 @@

#: *Horizontal position adjust*: Same as :data:`CHA`.
HPA = "'"

#: *Scroll up*
SU = "S"

#: *Scroll down*
SD = "T"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the xterm documentation states that 'T' is the correct code for Scroll Down (SD), the xterm code is implemented to handle the incorrect code '^' and treats 'T' as either mouse tracking or SD (depending on the arguments).
I'm not sure how many (or if any) applications still use the incorrect code.

25 changes: 25 additions & 0 deletions pyte/screens.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ def __init__(self, default):
def __missing__(self, key):
return self.default

def copy(self):
new_static_default_dict = type(self)(self.default)
new_static_default_dict.update(self)
return new_static_default_dict


class Screen(object):
"""
Expand Down Expand Up @@ -1043,6 +1048,26 @@ def debug(self, *args, **kwargs):
By default is a noop.
"""

def scroll_up(self, lines):
"""Scroll up `lines` lines."""
Comment on lines +1051 to +1052
Copy link

@altoidbox altoidbox Jul 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation states that a scroll with no parameters scrolls 1 line.

Suggested change
def scroll_up(self, lines):
"""Scroll up `lines` lines."""
def scroll_up(self, lines=None):
"""Scroll up `lines` lines.
:param lines: number of lines to scroll up. defaults to 1 if unspecified.
"""
lines = lines or 1

lines_to_scroll = range(self.margins.top, self.margins.bottom + 1)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should handle when margins is None.

Suggested change
lines_to_scroll = range(self.margins.top, self.margins.bottom + 1)
margins = self.margins or Margins(0, self.lines - 1)
lines_to_scroll = range(margins.top, margins.bottom + 1)

for y in lines_to_scroll:
if y + lines in set(lines_to_scroll):
self.buffer[y] = self.buffer[y + lines].copy()
else:
self.buffer[y].clear()
Comment on lines +1055 to +1058
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scrolling is similar to indexing multiple times, is it not? If we follow its example (and it may be more efficient) we can simply move the lines we want to move and pop the lines we want to erase, rather than copying and clearing the dictionaries. We can also improve efficiency by checking the bottom margin rather than creating a set to test for set inclusion.

Suggested change
if y + lines in set(lines_to_scroll):
self.buffer[y] = self.buffer[y + lines].copy()
else:
self.buffer[y].clear()
if y + lines <= margins.bottom:
self.buffer[y] = self.buffer[y + lines]
else:
self.buffer.pop(y, None)

self.dirty = set(lines_to_scroll)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should update the dirty set rather than replace it.

Suggested change
self.dirty = set(lines_to_scroll)
self.dirty.update(lines_to_scroll)


def scroll_down(self, lines):
"""Scroll down `lines` lines."""
lines_to_scroll = range(self.margins.bottom, self.margins.top - 1, -1)
for y in lines_to_scroll:
if y - lines in set(lines_to_scroll):
self.buffer[y] = self.buffer[y - lines].copy()
else:
self.buffer[y].clear()
self.dirty = set(lines_to_scroll)
Comment on lines +1061 to +1069
Copy link

@altoidbox altoidbox Jul 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to scroll_up, this should also default to 1 line if the argument is unspecified. However, an argument of 0 is used to reset mouse tracking (may be xterm specific). In any case, if we get an argument of 0, we don't want to scroll at all. And if we don't get an argument we do want to scroll exactly 1 line. However, looking at the current implementation of _parser_fsm, we may be unable to differentiate between receiving no arguments and receiving a 0 as the first argument. Reading vt100.net, they seem to indicate that for their purposes there is no need to distinguish between the value 0 and an unspecified argument (i.e., an explicit 0 still implies the default). Perhaps this is an xterm specific issue in this case.

Any other changes made to scroll_up should be appropriately mirrored here in scroll_down.



class DiffScreen(Screen):
"""
Expand Down
4 changes: 3 additions & 1 deletion pyte/streams.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ class Stream(object):
esc.SGR: "select_graphic_rendition",
esc.DSR: "report_device_status",
esc.DECSTBM: "set_margins",
esc.HPA: "cursor_to_column"
esc.HPA: "cursor_to_column",
esc.SD: "scroll_down",
esc.SU: "scroll_up"
}

#: A set of all events dispatched by the stream.
Expand Down
51 changes: 51 additions & 0 deletions tests/test_screen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1505,3 +1505,54 @@ def test_screen_set_icon_name_title():

screen.set_title(text)
assert screen.title == text


def test_scroll_down():
screen = pyte.Screen(1, 5)
screen.set_mode(mo.LNM)
screen.linefeed()
for i in "abcd":
screen.draw(i)
screen.linefeed()
screen.draw("e")

screen.set_margins(top=2, bottom=4)

assert screen.display == ["a",
"b",
"c",
"d",
"e"]

screen.scroll_down(2)

assert screen.display == ["a",
" ",
" ",
"b",
"e"]

def test_scroll_up():
screen = pyte.Screen(1, 5)
screen.set_mode(mo.LNM)
screen.linefeed()
for i in "abcd":
screen.draw(i)
screen.linefeed()
screen.draw("e")

screen.set_margins(top=2, bottom=4)

assert screen.display == ["a",
"b",
"c",
"d",
"e"]

screen.scroll_up(2)

assert screen.display == ["a",
"d",
" ",
" ",
"e"]