Merge branch 'master' into options3

src/browser/Terminal.test.ts

@@ -11,7 +11,7 @@ import { IBufferService, IUnicodeService } from 'common/services/Services';
 import { Linkifier } from 'browser/Linkifier';
 import { MockLogService, MockUnicodeService } from 'common/TestUtils.test';
 import { IRegisteredLinkMatcher, IMouseZoneManager, IMouseZone } from 'browser/Types';
-import { IMarker } from 'common/Types';
+import { IMarker, ITerminalOptions } from 'common/Types';
 
 const INIT_COLS = 80;
 const INIT_ROWS = 24;
@@ -1501,6 +1501,22 @@ describe('Terminal', () => {
       assert.deepEqual(markers.map(el => el.line), [-1, -1, 0, 1, 2]);
     });
   });
+
+  describe('options', () => {
+    beforeEach(async () => {
+      term = new TestTerminal({});
+    });
+    it('get options', () => {
+      assert.equal(term.options.cols, 80);
+      assert.equal(term.options.rows, 24);
+    });
+    it('set options', async () => {
+      term.options.cols = 40;
+      assert.equal(term.options.cols, 40);
+      term.options.rows = 20;
+      assert.equal(term.options.rows, 20);
+    });
+  });
 });
 
 class TestLinkifier extends Linkifier {

src/browser/Terminal.ts

@@ -39,7 +39,7 @@ import { MouseZoneManager } from 'browser/MouseZoneManager';
 import { AccessibilityManager } from './AccessibilityManager';
 import { ITheme, IMarker, IDisposable, ISelectionPosition, ILinkProvider } from 'xterm';
 import { DomRenderer } from 'browser/renderer/dom/DomRenderer';
-import { IKeyboardEvent, KeyboardResultType, CoreMouseEventType, CoreMouseButton, CoreMouseAction, ITerminalOptions, ScrollSource, IAnsiColorChangeEvent } from 'common/Types';
+import { IKeyboardEvent, KeyboardResultType, CoreMouseEventType, CoreMouseButton, CoreMouseAction, ScrollSource, IAnsiColorChangeEvent } from 'common/Types';
 import { evaluateKeyboardEvent } from 'common/input/Keyboard';
 import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter';
 import { DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine';
@@ -52,9 +52,9 @@ import { MouseService } from 'browser/services/MouseService';
 import { Linkifier2 } from 'browser/Linkifier2';
 import { CoreBrowserService } from 'browser/services/CoreBrowserService';
 import { CoreTerminal } from 'common/CoreTerminal';
-import { ITerminalOptions as IInitializedTerminalOptions } from 'common/services/Services';
 import { rgba } from 'browser/Color';
 import { CharacterJoinerService } from 'browser/services/CharacterJoinerService';
+import { ITerminalOptions } from 'common/services/Services';
 
 // Let it work inside Node.js for automated testing purposes.
 const document: Document = (typeof window !== 'undefined') ? window.document : null as any;
@@ -74,9 +74,6 @@ export class Terminal extends CoreTerminal implements ITerminal {
 
   public browser: IBrowser = Browser as any;
 
-  // TODO: We should remove options once components adopt optionsService
-  public get options(): IInitializedTerminalOptions { return this.optionsService.options; }
-
   private _customKeyEventHandler: CustomKeyEventHandler | undefined;
 
   // browser services
@@ -537,7 +534,7 @@ export class Terminal extends CoreTerminal implements ITerminal {
     this.register(this._mouseZoneManager);
     this.register(this.onScroll(() => this._mouseZoneManager!.clearAll()));
     this.linkifier.attachToDom(this.element, this._mouseZoneManager);
-    this.linkifier2.attachToDom(this.element, this._mouseService, this._renderService);
+    this.linkifier2.attachToDom(this.screenElement, this._mouseService, this._renderService);
 
     // This event listener must be registered aftre MouseZoneManager is created
     this.register(addDisposableDomListener(this.element, 'mousedown', (e: MouseEvent) => this._selectionService!.onMouseDown(e)));

src/browser/Types.d.ts

@@ -16,7 +16,6 @@ export interface ITerminal extends IPublicTerminal, ICoreTerminal {
   browser: IBrowser;
   buffer: IBuffer;
   viewport: IViewport | undefined;
-  // TODO: We should remove options once components adopt optionsService
   options: ITerminalOptions;
   linkifier: ILinkifier;
   linkifier2: ILinkifier2;

src/browser/public/Terminal.ts

@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { Terminal as ITerminalApi, ITerminalOptions, IMarker, IDisposable, ILinkMatcherOptions, ITheme, ILocalizableStrings, ITerminalAddon, ISelectionPosition, IBufferNamespace as IBufferNamespaceApi, IParser, ILinkProvider, IUnicodeHandling, FontWeight, IModes } from 'xterm';
+import { Terminal as ITerminalApi, IMarker, IDisposable, ILinkMatcherOptions, ITheme, ILocalizableStrings, ITerminalAddon, ISelectionPosition, IBufferNamespace as IBufferNamespaceApi, IParser, ILinkProvider, IUnicodeHandling, FontWeight, IModes } from 'xterm';
 import { ITerminal } from 'browser/Types';
 import { Terminal as TerminalCore } from 'browser/Terminal';
 import * as Strings from 'browser/LocalizableStrings';
@@ -12,16 +12,42 @@ import { ParserApi } from 'common/public/ParserApi';
 import { UnicodeApi } from 'common/public/UnicodeApi';
 import { AddonManager } from 'common/public/AddonManager';
 import { BufferNamespaceApi } from 'common/public/BufferNamespaceApi';
+import { ITerminalOptions } from 'common/Types';
+
+/**
+ * The set of options that only have an effect when set in the Terminal constructor.
+ */
+const CONSTRUCTOR_ONLY_OPTIONS = ['cols', 'rows'];
 
 export class Terminal implements ITerminalApi {
   private _core: ITerminal;
   private _addonManager: AddonManager;
   private _parser: IParser | undefined;
   private _buffer: BufferNamespaceApi | undefined;
+  private _publicOptions: ITerminalOptions;
 
   constructor(options?: ITerminalOptions) {
     this._core = new TerminalCore(options);
     this._addonManager = new AddonManager();
+
+    this._publicOptions = new Proxy(this._core.options, {
+      get: (target, propName: string): any => {
+        return target[propName];
+      },
+      set: (target, propName: string, value): any => {
+        this._checkReadonlyOptions(propName);
+        target[propName] = value;
+      }
+    });
+  }
+
+  private _checkReadonlyOptions(propName: string): void {
+    // Throw an error if any constructor only option is modified
+    // from terminal.options
+    // Modifications from anywhere else are allowed
+    if (CONSTRUCTOR_ONLY_OPTIONS.includes(propName)) {
+      throw new Error(`Option "${propName}" can only be set in the constructor`);
+    }
   }
 
   private _checkProposedApi(): void {
@@ -90,7 +116,12 @@ export class Terminal implements ITerminalApi {
     };
   }
   public get options(): ITerminalOptions {
-    return this._core.options;
+    return this._publicOptions;
+  }
+  public set options(options: ITerminalOptions) {
+    for (const propName in options) {
+      this._publicOptions[propName] = options[propName];
+    }
   }
   public blur(): void {
     this._core.blur();
@@ -216,6 +247,7 @@ export class Terminal implements ITerminalApi {
   public setOption(key: 'cols' | 'rows', value: number): void;
   public setOption(key: string, value: any): void;
   public setOption(key: any, value: any): void {
+    this._checkReadonlyOptions(key);
     this._core.optionsService.setOption(key, value);
   }
   public refresh(start: number, end: number): void {

src/browser/selection/SelectionModel.test.ts

@@ -122,5 +122,13 @@ describe('SelectionModel', () => {
       model.selectionEnd = [5, 2];
       assert.deepEqual(model.finalSelectionEnd, [5, 2]);
     });
+    it('should not include a trailing EOL when the selection ends at the end of a line', () => {
+      model.selectionStart = [0, 0];
+      model.selectionStartLength = 80;
+      assert.deepEqual(model.finalSelectionEnd, [80, 0]);
+      model.selectionStart = [0, 0];
+      model.selectionStartLength = 160;
+      assert.deepEqual(model.finalSelectionEnd, [80, 1]);
+    });
   });
 });

src/browser/selection/SelectionModel.ts

@@ -79,6 +79,10 @@ export class SelectionModel {
     if (!this.selectionEnd || this.areSelectionValuesReversed()) {
       const startPlusLength = this.selectionStart[0] + this.selectionStartLength;
       if (startPlusLength > this._bufferService.cols) {
+        // Ensure the trailing EOL isn't included when the selection ends on the right edge
+        if (startPlusLength % this._bufferService.cols === 0) {
+          return [this._bufferService.cols, this.selectionStart[1] + Math.floor(startPlusLength / this._bufferService.cols) - 1];
+        }
         return [startPlusLength % this._bufferService.cols, this.selectionStart[1] + Math.floor(startPlusLength / this._bufferService.cols)];
       }
       return [startPlusLength, this.selectionStart[1]];