From e4aed30d30dde6c58e8548f86fe7d67e8dd98ae4 Mon Sep 17 00:00:00 2001 From: John Hood Date: Mon, 14 Nov 2016 23:03:29 -0500 Subject: [PATCH] Fix broken wrap behavior causing broken copy/paste. The terminal framebuffer was not resetting the wrap state of a row when a previously-wrapping line was overwritten by a non-wrapping line. Restore previous, subtle behavior of line wrap. Fix wrap verification bug now exposed by emulation-wrap-across-frames.test. Also hoist some getters, mostly for clarity. Fixes #820. --- src/terminal/terminaldisplay.cc | 25 +++++++++++++++++++------ src/terminal/terminalframebuffer.cc | 17 +++++++++++++---- src/terminal/terminalframebuffer.h | 13 ++++++++----- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/terminal/terminaldisplay.cc b/src/terminal/terminaldisplay.cc index 0298f2a..2993f85 100644 --- a/src/terminal/terminaldisplay.cc +++ b/src/terminal/terminaldisplay.cc @@ -348,12 +348,14 @@ bool Display::put_row( bool initialized, FrameState &frame, const Framebuffer &f return false; } + const bool wrap_this = row.get_wrap(); + const int row_width = f.ds.get_width(); int clear_count = 0; bool wrote_last_cell = false; Renditions blank_renditions = initial_rendition(); /* iterate for every cell */ - while ( frame_x < f.ds.get_width() ) { + while ( frame_x < row_width ) { const Cell &cell = cells.at( frame_x ); @@ -405,12 +407,23 @@ bool Display::put_row( bool initialized, FrameState &frame, const Framebuffer &f /* Now draw a character cell. */ /* Move to the right position. */ + const int cell_width = cell.get_width(); + /* If we are about to print the last character in a wrapping row, + trash the cursor position to force explicit positioning. We do + this because our input terminal state may have the cursor on + the autowrap column ("column 81"), but our output terminal + states always snap the cursor to the true last column ("column + 80"), and we want to be able to apply the diff to either, for + verification. */ + if ( wrap_this && frame_x + cell_width >= row_width ) { + frame.cursor_x = frame.cursor_y = -1; + } frame.append_silent_move( frame_y, frame_x ); frame.update_rendition( cell.get_renditions() ); frame.append_cell( cell ); - frame_x += cell.get_width(); - frame.cursor_x += cell.get_width(); - if ( frame_x >= f.ds.get_width() ) { + frame_x += cell_width; + frame.cursor_x += cell_width; + if ( frame_x >= row_width ) { wrote_last_cell = true; } } @@ -423,7 +436,7 @@ bool Display::put_row( bool initialized, FrameState &frame, const Framebuffer &f frame.append_silent_move( frame_y, frame_x - clear_count ); frame.update_rendition( blank_renditions ); - bool can_use_erase = !row.get_wrap() && ( has_bce || ( frame.current_rendition == initial_rendition() ) ); + bool can_use_erase = !wrap_this && ( has_bce || ( frame.current_rendition == initial_rendition() ) ); if ( can_use_erase ) { frame.append( "\033[K" ); } else { @@ -438,7 +451,7 @@ bool Display::put_row( bool initialized, FrameState &frame, const Framebuffer &f /* To hint that a word-select should group the end of one line with the beginning of the next, we let the real cursor actually wrap around in cases where it wrapped around for us. */ - if ( row.get_wrap() ) { + if ( wrap_this ) { /* Update our cursor, and ask for wrap on the next row. */ frame.cursor_x = 0; frame.cursor_y++; diff --git a/src/terminal/terminalframebuffer.cc b/src/terminal/terminalframebuffer.cc index 59a7a70..7132893 100644 --- a/src/terminal/terminalframebuffer.cc +++ b/src/terminal/terminalframebuffer.cc @@ -42,13 +42,15 @@ Cell::Cell( color_type background_color ) : contents(), renditions( background_color ), width( 1 ), - fallback( false ) + fallback( false ), + wrap( false ) {} Cell::Cell() /* default constructor required by C++11 STL */ : contents(), renditions( 0 ), width( 1 ), - fallback( false ) + fallback( false ), + wrap( false ) { assert( false ); } @@ -59,6 +61,7 @@ void Cell::reset( color_type background_color ) renditions = Renditions( background_color ); width = 1; fallback = false; + wrap = false; } void DrawState::reinitialize_tabs( unsigned int start ) @@ -350,11 +353,11 @@ void Framebuffer::delete_line( int row, int count ) } Row::Row( const size_t s_width, const color_type background_color ) - : cells( s_width, Cell( background_color ) ), wrap( false ), gen( get_gen() ) + : cells( s_width, Cell( background_color ) ), gen( get_gen() ) {} Row::Row() /* default constructor required by C++11 STL */ - : cells( 1, Cell() ), wrap( false ), gen( get_gen() ) + : cells( 1, Cell() ), gen( get_gen() ) { assert( false ); } @@ -650,5 +653,11 @@ bool Cell::compare( const Cell &other ) const fprintf( stderr, "renditions differ\n" ); } + if ( wrap != other.wrap ) { + ret = true; + fprintf( stderr, "wrap: %d vs. %d\n", + wrap, other.wrap ); + } + return ret; } diff --git a/src/terminal/terminalframebuffer.h b/src/terminal/terminalframebuffer.h index beb00d2..1d98e50 100644 --- a/src/terminal/terminalframebuffer.h +++ b/src/terminal/terminalframebuffer.h @@ -91,6 +91,7 @@ namespace Terminal { Renditions renditions; uint8_t width; bool fallback; /* first character is combining character */ + bool wrap; public: Cell( color_type background_color ); @@ -103,7 +104,8 @@ namespace Terminal { return ( (contents == x.contents) && (fallback == x.fallback) && (width == x.width) - && (renditions == x.renditions) ); + && (renditions == x.renditions) + && (wrap == x.wrap) ); } bool operator!=( const Cell &x ) const { return !operator==( x ); } @@ -192,13 +194,14 @@ namespace Terminal { void set_width( unsigned int w ) { width = w; } bool get_fallback( void ) const { return fallback; } void set_fallback( bool f ) { fallback = f; } + bool get_wrap( void ) const { return wrap; } + void set_wrap( bool f ) { wrap = f; } }; class Row { public: typedef std::vector cells_type; cells_type cells; - bool wrap; /* if last cell, wrap to next line */ // gen is a generation counter. It can be used to quickly rule // out the possibility of two rows being identical; this is useful // in scrolling. @@ -214,11 +217,11 @@ namespace Terminal { bool operator==( const Row &x ) const { - return ( gen == x.gen && cells == x.cells && wrap == x.wrap ); + return ( gen == x.gen && cells == x.cells ); } - bool get_wrap( void ) const { return wrap; } - void set_wrap( bool w ) { wrap = w; } + bool get_wrap( void ) const { return cells.back().get_wrap(); } + void set_wrap( bool w ) { cells.back().set_wrap( w ); } uint64_t get_gen() const; };