Addressing last review comments

Author: zedinosaur

src/terminal/terminaldisplay.cc

@@ -388,9 +388,7 @@ bool Display::put_row( bool initialized,
       frame.append_silent_move( frame_y, frame_x - clear_count );
       frame.update_rendition( blank_renditions );
       frame.update_hyperlink( blank_hyperlink );
-      bool can_use_erase
-        = has_bce || ( frame.current_rendition == initial_rendition() && frame.current_hyperlink.empty() );
-      if ( can_use_erase && has_ech && clear_count > 4 ) {
+      if ( can_use_erase( frame ) && has_ech && clear_count > 4 ) {
         snprintf( tmp, 64, "\033[%dX", clear_count );
         frame.append( tmp );
       } else {
@@ -442,9 +440,7 @@ bool Display::put_row( bool initialized,
     frame.update_rendition( blank_renditions );
     frame.update_hyperlink( blank_hyperlink );
 
-    bool can_use_erase
-      = has_bce || ( frame.current_rendition == initial_rendition() && frame.current_hyperlink.empty() );
-    if ( can_use_erase && !wrap_this ) {
+    if ( can_use_erase( frame ) && !wrap_this ) {
       frame.append( "\033[K" );
     } else {
       frame.append( clear_count, ' ' );
@@ -472,6 +468,11 @@ bool Display::put_row( bool initialized,
   return false;
 }
 
+bool Display::can_use_erase( const FrameState& frame ) const
+{
+  return has_bce || ( frame.current_rendition == initial_rendition() && frame.current_hyperlink.empty() );
+}
+
 FrameState::FrameState( const Framebuffer& s_last )
   : str(), cursor_x( 0 ), cursor_y( 0 ), current_rendition( 0 ), current_hyperlink(),
     cursor_visible( s_last.ds.cursor_visible ), last_frame( s_last )

src/terminal/terminaldisplay.h

@@ -83,6 +83,8 @@ class Display
                 const Row& old_row,
                 bool wrap ) const;
 
+  bool can_use_erase( const FrameState& frame ) const;
+
 public:
   std::string open() const;
   std::string close() const;

src/terminal/terminalframebuffer.cc

@@ -606,7 +606,7 @@ std::string Renditions::sgr( void ) const
 
 bool Hyperlink::operator==( const Hyperlink& x ) const
 {
-  if ( rep == nullptr && x.rep == nullptr ) {
+  if ( rep == x.rep ) {
     return true;
   }
   if ( rep == nullptr || x.rep == nullptr ) {
@@ -621,14 +621,12 @@ std::string Hyperlink::osc8() const
   std::string ret;
 
   ret.append( "\033]8;" );
-  if ( empty() ) {
-    ret.append( ";\033\\" );
-    return ret;
-  }
 
-  ret.append( rep->params );
+  if ( *this )
+    ret.append( rep->params );
   ret.append( ";" );
-  ret.append( rep->url );
+  if ( *this )
+    ret.append( rep->url );
 
   ret.append( "\033\\" );
   return ret;

src/terminal/terminalframebuffer.h

@@ -110,6 +110,7 @@ class Hyperlink
   std::string osc8() const;
 
   bool empty() const { return rep == nullptr; }
+  operator bool() const { return !empty(); }
 
   bool operator==( const Hyperlink& x ) const;
 

src/terminal/terminalfunctions.cc

@@ -619,8 +619,8 @@ static void OSC_8( const std::string& OSC_string, Framebuffer* fb )
     return;
   }
 
-  fb->ds.set_hyperlink(
-    Hyperlink( OSC_string.substr( 2, second_semicolon - 2 ), OSC_string.substr( second_semicolon + 1 ) ) );
+  std::string url = OSC_string.substr( second_semicolon + 1 );
+  fb->ds.set_hyperlink( Hyperlink( OSC_string.substr( 2, second_semicolon - 2 ), std::move( url ) ) );
 }
 
 /* xterm uses an Operating System Command to set the window title */