BUG: qtconsole -- non-standard handling of \a and \b. [Fixes #1561] #1569
Conversation
|
This PR also fixes an invisible bug in #1089 -- the actions list was not cleared after processing bell and carriage return, which caused these to be processed twice. This was inconsequential (beep+beep sounds like beep, return+return looks like return), but not so for backspace. The former tests did not catch this because they only tested one element of the actions list; this PR also corrects and expands those tests. |
|
Actually in an odd corner case, backspace handling in this PR is still not compatible with terminal. EDIT: replacing spaces with "=" for clarity. |
| def test_combined(self): | ||
| """ Are return and backspace characters processed correctly in combination? | ||
| """ | ||
| string = 'abc\rdef\b' # CR and backspace |
There was a problem hiding this comment.
A comment should explain that for compatibility with IPython terminal, a BS at EOL is effectively ignored because it is treated as a change in print position rather than a backwards character deletion.
|
My stuff doesn't appear to rely on the broken functionality (it works just as well with this PR), so +1. |
|
@jdmarch Thanks for reviewing and testing the patch, and the elaborate comments! I force pushed a change, to handle the corner case you pointed out. |
|
This looks good now, thanks! Tests are much stronger. The new module to test actual output can provide a basis for other such tests in the future. This is admittedly very 20th Century but still useful for some beginner training in escape sequences, and terminal progress bars. OK to merge? |
| groups = filter(lambda x: x is not None, match.groups()) | ||
| if groups[0] == '\r': | ||
| if groups[0] == '\a': |
There was a problem hiding this comment.
Minor nitpick: since groups[0] is now used multiple times for this dispatch, it might be a good idea to create a local g0 = groups[0] and do the whole if/elif/elif... dispatch on g0 instead, to avoid the cost of multiple lookups on the groups object...
|
@fperez -- Thanks for the comments. Fixed the issues and pushed changes. |
|
Great, these changes look good now. I concur with @jdmarch that it's best to have I'll go ahead and merge this one so it's out of the way; on-list we discussed @jdmarch will take care of shepherding these Qt-related PRs, which is fantastic. We'll have a quicker-moving pipeline for all of them. |
…quences BUG: qtconsole -- non-standard handling of \a and \b. This makes the behavior of the Qt console match that of an ANSI terminal with regards to \a and \b. Fixes #1561.
…beep-sequences BUG: qtconsole -- non-standard handling of \a and \b. This makes the behavior of the Qt console match that of an ANSI terminal with regards to \a and \b. Fixes ipython#1561.
Fixes #1561 and adds many tests