Skip to content

add TerminalWriter.chars_on_current_line read-only property #155

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

Merged
merged 1 commit into from
Nov 10, 2017
Merged

add TerminalWriter.chars_on_current_line read-only property #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
- turn iniconfig and apipkg into vendored packages and ease de-vendoring for distributions
- fix #68 remove invalid py.test.ensuretemp references
- fix #25 - deprecate path.listdir(sort=callable)
- add ``TerminalWriter.chars_on_current_line`` read-only property that tracks how many characters
have been written to the current line.

1.4.34
====================================================================
Expand Down
27 changes: 27 additions & 0 deletions py/_io/terminalwriter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def __init__(self, file=None, stringio=False, encoding=None):
self._file = file
self.hasmarkup = should_do_markup(file)
self._lastlen = 0
self._chars_on_current_line = 0

@property
def fullwidth(self):
Expand All @@ -150,6 +151,19 @@ def fullwidth(self):
def fullwidth(self, value):
self._terminal_width = value

@property
def chars_on_current_line(self):
"""Return the number of characters written so far in the current line.

Please note that this count does not produce correct results after a reline() call,
see #164.

.. versionadded:: 1.5.0

:rtype: int
"""
return self._chars_on_current_line

def _escaped(self, text, esc):
if esc and self.hasmarkup:
text = (''.join(['\x1b[%sm' % cod for cod in esc]) +
Expand Down Expand Up @@ -200,12 +214,22 @@ def write(self, msg, **kw):
if msg:
if not isinstance(msg, (bytes, text)):
msg = text(msg)

self._update_chars_on_current_line(msg)

if self.hasmarkup and kw:
markupmsg = self.markup(msg, **kw)
else:
markupmsg = msg
write_out(self._file, markupmsg)

def _update_chars_on_current_line(self, text):
fields = text.rsplit('\n', 1)
if '\n' in text:
self._chars_on_current_line = len(fields[-1])
Copy link
Member

Choose a reason for hiding this comment

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

this breaks in combination with reline and _lastlen

Copy link
Member Author

Choose a reason for hiding this comment

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

True, thanks

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm not sure what to do here. Here's reline implementation:

    def reline(self, line, **kw):
        if not self.hasmarkup:
            raise ValueError("cannot use rewrite-line without terminal")
        self.write(line, **kw)
        self._checkfill(line)
        self.write('\r')
        self._lastlen = len(line)

It always ends up with a \r, so the cursor is at the start of the line; anything you write afterwards will begin from there, but it seems there are more characters on the line. What should chars_on_current_line return in this case?

And notice that reline always expects that it will be called multiple times for the same line only. IOW, if we want to write a "collecting" message using reline, we can only call reline() and never write() otherwise things will get out of sync.

As an exercise, what should be the value of chars_on_current_line at the end of this code:

w = TerminalWriter()
w.reline('Hello World')
w.reline('pytest')
assert w.chars_on_current_line == ??

0 or 6?

Note that the "cursor" is at the beginning of the line because of the \r added by reline() at this point, so if we now write:

w.write('foo')

We end up with fooest on the terminal (this is why I mention that the only write() which produces correct result after reline() is write('\n')).

Hmm perhaps the property should be named line_cursor_position or something to avoid ambiguity?

Copy link
Member

Choose a reason for hiding this comment

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

if its hard to exlain it might be a bad idea

im under the impression we are trying something at the wrong abstraction level and i'd like to avoid sinking too much time into terminalwriter

in addition we are currently spagettifying multiple concepts - we should take a step back and make what we need and what boundaries those need to have

Copy link
Member Author

Choose a reason for hiding this comment

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

i'd like to avoid sinking too much time into terminalwriter

Perhaps we should just drop the idea of messing with TerminalWriter and leave the hack in pytest-dev/pytest#2858?

Alternatively, we might just leave reline as it is. reline() is not really used in pytest or xdist, so this patch as it stands is enough for my needs in pytest-dev/pytest#2858.

Copy link
Member

Choose a reason for hiding this comment

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

we can leave it as is heren but then
please document it as broken interaction and open a help wanted ticket to fix the interaction later on

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, done: #164

Copy link
Member

Choose a reason for hiding this comment

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

thanks for the BTS preps, for pedantic-ness, please add a todo comment with a short description and a link to that issue

else:
self._chars_on_current_line += len(fields[-1])

def line(self, s='', **kw):
self.write(s, **kw)
self._checkfill(s)
Expand All @@ -229,6 +253,9 @@ def write(self, msg, **kw):
if msg:
if not isinstance(msg, (bytes, text)):
msg = text(msg)

self._update_chars_on_current_line(msg)

oldcolors = None
if self.hasmarkup and kw:
handle = GetStdHandle(STD_OUTPUT_HANDLE)
Expand Down
21 changes: 21 additions & 0 deletions testing/io_/test_terminalwriter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,27 @@ class fil:
assert l == set(["2"])


def test_chars_on_current_line():
tw = py.io.TerminalWriter(stringio=True)

written = []

def write_and_check(s, expected):
tw.write(s, bold=True)
written.append(s)
assert tw.chars_on_current_line == expected
assert tw.stringio.getvalue() == ''.join(written)

write_and_check('foo', 3)
write_and_check('bar', 6)
write_and_check('\n', 0)
write_and_check('\n', 0)
write_and_check('\n\n\n', 0)
write_and_check('\nfoo', 3)
write_and_check('\nfbar\nhello', 5)
write_and_check('10', 7)


@pytest.mark.skipif(sys.platform == "win32", reason="win32 has no native ansi")
def test_attr_hasmarkup():
tw = py.io.TerminalWriter(stringio=True)
Expand Down