respect scroll margins, tests

src/InputHandler.test.ts

@@ -1530,6 +1530,43 @@ describe('InputHandler', () => {
         assert.deepEqual(getLines(term, 2), ['    ', '  ']);
         assert.equal(term.buffer.x, 0);
       });
+      it('should not reverse outside of scroll margins', () => {
+        // prepare buffer content
+        term.writeSync('#####abcdefghijklmnopqrstuvwxy');
+        assert.deepEqual(getLines(term, 6), ['#####', 'abcde', 'fghij', 'klmno', 'pqrst', 'uvwxy']);
+        assert.equal(term.buffer.ydisp, 1);
+        assert.equal(term.buffer.x, 5);
+        assert.equal(term.buffer.y, 4);
+        term.writeSync(ttyBS.repeat(100));
+        assert.deepEqual(getLines(term, 6), ['#####', 'abcde', 'fghij', 'klmno', 'pqrst', '    y']);
+
+        term.writeSync('\x1b[?45h');
+        term.writeSync('uvwxy');
+
+        // set top/bottom to 1/3 (0-based)
+        term.writeSync('\x1b[2;4r');
+        // place cursor below scroll bottom
+        term.buffer.x = 5;
+        term.buffer.y = 4;
+        term.writeSync(ttyBS.repeat(100));
+        assert.deepEqual(getLines(term, 6), ['#####', 'abcde', 'fghij', 'klmno', 'pqrst', '     ']);
+
+        term.writeSync('uvwxy');
+        // place cursor within scroll margins
+        term.buffer.x = 5;
+        term.buffer.y = 3;
+        term.writeSync(ttyBS.repeat(100));
+        assert.deepEqual(getLines(term, 6), ['#####', 'abcde', '     ', '     ', '     ', 'uvwxy']);
+        assert.equal(term.buffer.x, 0);
+        assert.equal(term.buffer.y, term.buffer.scrollTop);  // stops at 0, scrollTop
+
+        term.writeSync('fghijklmnopqrst');
+        // place cursor above scroll top
+        term.buffer.x = 5;
+        term.buffer.y = 0;
+        term.writeSync(ttyBS.repeat(100));
+        assert.deepEqual(getLines(term, 6), ['#####', '     ', 'fghij', 'klmno', 'pqrst', 'uvwxy']);
+      });
     });
   });
 });

src/InputHandler.ts

@@ -719,32 +719,46 @@ export class InputHandler extends Disposable implements IInputHandler {
    * Backspace (Ctrl-H).
    *
    * @vt: #Y   C0    BS   "Backspace"  "\b, \x08"  "Move the cursor one position to the left."
+   * By default it is not possible to move the cursor past the leftmost position.
+   * If `reverse wrap-around` (`CSI ? 45 h`) is set, a previous soft line wrap (DECAWM)
+   * can be undone with BS within the scroll margins. In that case the cursor will wrap back
+   * to the end of the previous row. Note that it is not possible to peek back into the scrollbuffer
+   * with the cursor, thus at the home position (top-leftmost cell) this has no effect.
    */
   public backspace(): void {
-    // xterm allows deleting of the last col if reverseWraparound is set
-    // otherwise the last cell cannot be deleted by BS
+    const buffer = this._bufferService.buffer;
+
+    // by default (resverse wrap-around not set) we restrict the cursor to 0 .. rows/cols - 1
+    // thus the last cell of a row cannot be accessed by BS
     if (!this._coreService.decPrivateModes.reverseWraparound) {
       this._restrictCursor();
     } else {
-      // we still clamp the cursor to sane values here, but have to allow x=cols as well
-      this._bufferService.buffer.x = Math.min(this._bufferService.cols, Math.max(0, this._bufferService.buffer.x));
-      this._bufferService.buffer.y = this._coreService.decPrivateModes.origin
-        ? Math.min(this._bufferService.buffer.scrollBottom, Math.max(this._bufferService.buffer.scrollTop, this._bufferService.buffer.y))
-        : Math.min(this._bufferService.rows - 1, Math.max(0, this._bufferService.buffer.y));
-      this._dirtyRowService.markDirty(this._bufferService.buffer.y);
+      // copy of _restrictCursor(), but with x=cols as allowed max cursor position
+      // to make last cell accessible by BS
+      buffer.x = Math.min(this._bufferService.cols, Math.max(0, this._bufferService.buffer.x));
+      buffer.y = this._coreService.decPrivateModes.origin
+        ? Math.min(buffer.scrollBottom, Math.max(buffer.scrollTop, buffer.y))
+        : Math.min(this._bufferService.rows - 1, Math.max(0, buffer.y));
+      this._dirtyRowService.markDirty(buffer.y);
     }
-    const buffer = this._bufferService.buffer;
+
     if (buffer.x > 0) {
       buffer.x--;
     } else {
-      // reverse wraparound
-      // TODO: test in xterm - should we peek/unshift from scrollback if y == 0?
+      /**
+       * reverse wrap-around:
+       * Our implementation deviates from xterm on purpose. Details:
+       * - only previous soft NLs can be reversed (isWrapped=true)
+       * - only works within scrollborders (top/bottom, left/right not yet supported)
+       * - cannot peek into scrollbuffer
+       * - any cursor movement sequence keeps working as expected
+       */
       if (this._coreService.decPrivateModes.reverseWraparound
           && buffer.x === 0
-          && buffer.y > 0
+          && buffer.y > buffer.scrollTop
+          && buffer.y <= buffer.scrollBottom
           && buffer.lines.get(buffer.y + buffer.ybase).isWrapped)
       {
-        // unset isWrapped
         buffer.lines.get(buffer.y + buffer.ybase).isWrapped = false;
         buffer.y--;
         buffer.x = this._bufferService.cols - 1;
@@ -761,8 +775,7 @@ export class InputHandler extends Disposable implements IInputHandler {
         }
       }
     }
-    // should we use _restrictCursor() here? FIXME: how about origin mode / scroll margins?
-    buffer.x = Math.max(Math.min(buffer.x, this._bufferService.cols - 1), 0);
+    this._restrictCursor();
   }
 
   /**

src/Terminal2.test.ts

@@ -19,7 +19,7 @@ const TESTFILES = glob.sync('**/escape_sequence_files/*.in', { cwd: path.join(__
 const SKIP_FILES = [
   't0084-CBT.in',
   't0101-NLM.in',
-  't0103-reverse_wrap.in',
+  't0103-reverse_wrap.in',  // not comparable, we deviate from xterm reverse wrap on purpose
   't0504-vim.in'
 ];
 if (os.platform() === 'darwin') {