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.

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

Fix animation of long lines #247

Merged
merged 19 commits into from
Oct 29, 2019
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a26b2bb
Truncate long lines with animation, to prevent multi-line.
itsayellow Oct 20, 2019
0fefbf2
Remove unused TERM_ROWS.
itsayellow Oct 20, 2019
4bcce89
Tailor truncation depending on animate_at_beginning_of_line.
itsayellow Oct 20, 2019
6bfe12c
Make if test correspond to string format precision.
itsayellow Oct 21, 2019
c7ef9d8
Move get_terminal_size() inside of animate().
itsayellow Oct 21, 2019
4f31d46
Fix bug from last commit.
itsayellow Oct 21, 2019
58cbefb
Make max message length method a bit clearer.
itsayellow Oct 22, 2019
9fdf83f
Add tests for message truncation.
itsayellow Oct 24, 2019
aec4c98
Deleted unused import sys.
itsayellow Oct 24, 2019
9e1eb97
Add HIDE_CURSOR, SHOW_CURSOR, CLEAR_LINE global strings to animate.py.
itsayellow Oct 27, 2019
29a058a
Add EMOJI_ANIMATION_FRAMES, NONEMOJI_ANIMATION_FRAMES globals to anim…
itsayellow Oct 27, 2019
f3aa77a
Add EMOJI_ANIMATION_PERIOD, NONEMOJI_ANIMATION_PERIOD globals to anim…
itsayellow Oct 27, 2019
c9be0a7
Remove python shebang.
itsayellow Oct 27, 2019
73b861f
Implement teardown to re-do original terminal state.
itsayellow Oct 28, 2019
49228e5
Use monkeypatch instead of terminal_state fixture.
itsayellow Oct 28, 2019
9f60dc5
Refactor and remove unused import pytest.
itsayellow Oct 28, 2019
443b621
Increase sleep time to ensure all frames print.
itsayellow Oct 28, 2019
f60e310
Merge remote-tracking branch 'upstream/master' into animate_long_lines
itsayellow Oct 28, 2019
8f89f97
Merge branch 'animate_long_lines' of github.com:itsayellow/pipx into …
itsayellow Oct 28, 2019
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
35 changes: 25 additions & 10 deletions pipx/animate.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,22 @@
from contextlib import contextmanager
from threading import Event, Thread
from typing import Generator, List
import shutil

from pipx.constants import emoji_support

stderr_is_tty = sys.stderr.isatty()


HIDE_CURSOR = "\033[?25l"
SHOW_CURSOR = "\033[?25h"
CLEAR_LINE = "\033[K"
EMOJI_ANIMATION_FRAMES = ["⣷", "⣯", "⣟", "⡿", "⢿", "⣻", "⣽", "⣾"]
NONEMOJI_ANIMATION_FRAMES = ["", ".", "..", "..."]
Copy link
Member

Choose a reason for hiding this comment

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

nice

EMOJI_FRAME_PERIOD = 0.1
NONEMOJI_FRAME_PERIOD = 1


@contextmanager
def animate(message: str, do_animation: bool) -> Generator[None, None, None]:

Expand All @@ -20,12 +30,12 @@ def animate(message: str, do_animation: bool) -> Generator[None, None, None]:

if emoji_support:
animate_at_beginning_of_line = True
symbols = ["⣷", "⣯", "⣟", "⡿", "⢿", "⣻", "⣽", "⣾"]
period = 0.1
symbols = EMOJI_ANIMATION_FRAMES
period = EMOJI_FRAME_PERIOD
else:
animate_at_beginning_of_line = False
symbols = ["", ".", "..", "..."]
period = 1
symbols = NONEMOJI_ANIMATION_FRAMES
period = NONEMOJI_FRAME_PERIOD

thread_kwargs = {
"message": message,
Expand Down Expand Up @@ -59,12 +69,17 @@ def print_animation(
period: float,
animate_at_beginning_of_line: bool,
):
(term_cols, _) = shutil.get_terminal_size(fallback=(9999, 24))
while not event.wait(0):
for s in symbols:
if animate_at_beginning_of_line:
cur_line = f"{s} {message}"
max_message_len = term_cols - len(f"{s} ... ")
cur_line = f"{s} {message:.{max_message_len}}"
if len(message) > max_message_len:
cur_line += "..."
gaborbernat marked this conversation as resolved.
Show resolved Hide resolved
else:
cur_line = f"{message}{s}"
max_message_len = term_cols - len("... ")
cur_line = f"{message:.{max_message_len}}{s}"

clear_line()
sys.stderr.write("\r")
Expand All @@ -74,13 +89,13 @@ def print_animation(


def hide_cursor():
sys.stderr.write("\033[?25l")
sys.stderr.write(f"{HIDE_CURSOR}")


def show_cursor():
sys.stderr.write("\033[?25h")
sys.stderr.write(f"{SHOW_CURSOR}")


def clear_line():
sys.stderr.write("\033[K")
sys.stdout.write("\033[K")
sys.stderr.write(f"{CLEAR_LINE}")
sys.stdout.write(f"{CLEAR_LINE}")
94 changes: 94 additions & 0 deletions tests/test_animate.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import time
Copy link
Member

Choose a reason for hiding this comment

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

This is great! Thanks for adding these tests.


import pipx.animate
from pipx.animate import (
HIDE_CURSOR,
CLEAR_LINE,
EMOJI_ANIMATION_FRAMES,
NONEMOJI_ANIMATION_FRAMES,
EMOJI_FRAME_PERIOD,
NONEMOJI_FRAME_PERIOD,
)


def check_animate_output(
capsys, test_string, frame_strings, frame_period, frames_to_test
):
expected_string = f"{HIDE_CURSOR}" + "".join(frame_strings)

chars_to_test = 1 + len("".join(frame_strings[:frames_to_test]))

with pipx.animate.animate(test_string, do_animation=True):
time.sleep(frame_period * (frames_to_test - 1) + 0.1)
captured = capsys.readouterr()

# print for debug help if fail
print("expected_string:")
Copy link
Contributor

Choose a reason for hiding this comment

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

adding '-rvva --showlocals`` achieves the same, not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, didn't know about that.

print(repr(expected_string[:chars_to_test]))
print("capsys sterr:")
print(repr(captured.err[:chars_to_test]))

assert captured.err[:chars_to_test] == expected_string[:chars_to_test]


def test_line_lengths_emoji(capsys, monkeypatch):
# emoji_support and stderr_is_tty is set only at import animate.py
# since we are already after that, we must override both here
pipx.animate.stderr_is_tty = True
pipx.animate.emoji_support = True
Copy link
Member

@cs01 cs01 Oct 27, 2019

Choose a reason for hiding this comment

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

Recommend defining a fixture to ensure pipx.animate.stderr_is_tty, pipx.animate.emoji_support values are stored at entry to the fixture, then yield to let the test run, then reset the values to their orignal value in the fixture. We should not assume what they were at the time they were computed on first pass.

https://docs.pytest.org/en/latest/fixture.html#fixture-finalization-executing-teardown-code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, for the hint.
Done.

Copy link
Contributor Author

@itsayellow itsayellow Oct 28, 2019

Choose a reason for hiding this comment

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

Used @gaborbernat 's tip about monkeypatch instead, but if things get more complicated in the future I am happy to know how to do teardown.


frames_to_test = 4
# matches animate.py
frame_period = EMOJI_FRAME_PERIOD

# 40-char test_string counts columns e.g.: "0204060810 ... 363840"
test_string = "".join([f"{x:02}" for x in range(2, 41, 2)])

columns_to_test = [45, 46, 47]
expected_message = [f"{test_string:.{45-6}}...", f"{test_string}", f"{test_string}"]

for i, columns in enumerate(columns_to_test):
monkeypatch.setenv("COLUMNS", str(columns))

frame_strings = [
f"{CLEAR_LINE}\r{x} {expected_message[i]}" for x in EMOJI_ANIMATION_FRAMES
]
check_animate_output(
capsys, test_string, frame_strings, frame_period, frames_to_test
)

# set back to test-normal
pipx.animate.stderr_is_tty = False
pipx.animate.emoji_support = True


def test_line_lengths_no_emoji(capsys, monkeypatch):
# emoji_support and stderr_is_tty is set only at import animate.py
# since we are already after that, we must override both here
pipx.animate.stderr_is_tty = True
Copy link
Contributor

Choose a reason for hiding this comment

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

use monkeypatch instead of direct set to ensure their correctly reset in case tests fails for the next test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tip!

pipx.animate.emoji_support = False

frames_to_test = 2
# matches animate.py
frame_period = NONEMOJI_FRAME_PERIOD

# 40-char test_string counts columns e.g.: "0204060810 ... 363840"
test_string = "".join([f"{x:02}" for x in range(2, 41, 2)])

columns_to_test = [43, 44, 45]
expected_message = [f"{test_string:.{43-4}}", f"{test_string}", f"{test_string}"]

for i, columns in enumerate(columns_to_test):
monkeypatch.setenv("COLUMNS", str(columns))

frame_strings = [
f"{CLEAR_LINE}\r{expected_message[i]}{x}" for x in NONEMOJI_ANIMATION_FRAMES
]

check_animate_output(
capsys, test_string, frame_strings, frame_period, frames_to_test
)

# set back to test-normal
pipx.animate.stderr_is_tty = False
pipx.animate.emoji_support = True