Support carriage return ('\r') and beep ('\b') characters in the qtconsole #1089

Merged
merged 3 commits into from Dec 7, 2011

Projects

None yet

4 participants

@mdboom
Contributor
mdboom commented Dec 2, 2011

This addresses the bug in #629.

It extends AnsiCodeProcessor to understand the '\r' character and move the cursor back to the start of the line. It also understands the '\b' character and calls QTApplication::beep(). Neither are strictly speaking ANSI code sequences, of course, but they seem related enough and was simple enough to do it this way.

Here's a "poor man's scrollbar" function to help test this:

def scrollbar():
    import sys
    import time
    for i in range(80):
        print '\r' + ('=' * i),
        sys.stdout.flush() # required for regular console
        time.sleep(0.1)
    print '\b'
@epatters
Contributor
epatters commented Dec 3, 2011

Thanks for this! A few comments:

  1. Rather than changing the main regex and processing loop, just add '\r' and '\b' to SPECIAL_PATTERN = re.compile('([\f])'), then add a few lines here: https://github.com/ipython/ipython/blob/master/IPython/frontend/qt/console/ansi_code_processor.py#L235.
  2. Please unit test this functionality (see https://github.com/ipython/ipython/blob/master/IPython/frontend/qt/console/tests/test_ansi_code_processor.py). This is one of the few components of the Qt console that's easy to test, so let's not miss the low-hanging fruit.

Thanks again. I'm sure others will appreciate this.

@mdboom
Contributor
mdboom commented Dec 5, 2011
  1. I don't think this creates the correct semantics. SPECIAL_PATTERN is not used for splitting the string, only finding characters in it. But we need to insert the carriage-return action at exactly the point in the string where it appears. I would expect:
In [1]: print 'foo\rbar'
bar

but by putting everything in SPECIAL_PATTERN, one gets:

In [1]: print 'foo\rbar'
foobar

Admittedly, for the \b character, I don't think it's as crucial.

  1. Very good point. I've added some tests that work against the current commits (using the '\r' and '\b' characters as split points).
@minrk
Member
minrk commented Dec 6, 2011

This seems good to go. @epatters - does the explanation satisfy you?

@epatters
Contributor
epatters commented Dec 7, 2011

Yes, thanks for the explanation @mdboom. This looks good to me.

@fperez
Member
fperez commented Dec 7, 2011

@epatters, many thanks for the review and feedback. This looks solid, merging now. Thanks @mdboom!!

@fperez fperez merged commit 21c436b into ipython:master Dec 7, 2011
@minrk minrk added a commit to minrk/ipython that referenced this pull request Dec 9, 2011
@minrk minrk [qtconsole] carriage-return action matches CR only, not CRLF
CarriageReturn action introduced in #1089 clears a line in the qtconsole, which means that CRLF line endings would replace whole lines with '\n'.

This changes the regex to only match `\r` not followed by `\n` preventing the CR action from firing on CRLF.

Test included

closes #1111
814d5b9
@mattvonrocketstein mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
@minrk minrk [qtconsole] carriage-return action matches CR only, not CRLF
CarriageReturn action introduced in #1089 clears a line in the qtconsole, which means that CRLF line endings would replace whole lines with '\n'.

This changes the regex to only match `\r` not followed by `\n` preventing the CR action from firing on CRLF.

Test included

closes #1111
c061f16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment