- use strings for view zone ids

- make it unlikely that a new view instance would accept a view zone id from a previous view instance - remove that the find widget removes a view zone id from another view (fixes #71745)
microsoft · Aug 14, 2019 · 16051c7 · 16051c7
1 parent c3fcaef
commit 16051c7
Show file tree

Hide file tree

Showing 18 changed files with 88 additions and 99 deletions.
diff --git a/src/vs/base/common/strings.ts b/src/vs/base/common/strings.ts
@@ -729,3 +729,18 @@ export function getNLines(str: string, n = 1): string {
 		str.substr(0, idx) :
 		str;
 }
+
+/**
+ * Produces 'a'-'z', followed by 'A'-'Z'... followed by 'a'-'z', etc.
+ */
+export function singleLetterHash(n: number): string {
+	const LETTERS_CNT = (CharCode.Z - CharCode.A + 1);
+
+	n = n % (2 * LETTERS_CNT);
+
+	if (n < LETTERS_CNT) {
+		return String.fromCharCode(CharCode.a + n);
+	}
+
+	return String.fromCharCode(CharCode.A + n - LETTERS_CNT);
+}
diff --git a/src/vs/editor/browser/controller/mouseHandler.ts b/src/vs/editor/browser/controller/mouseHandler.ts
@@ -49,7 +49,7 @@ export interface IPointerHandlerHelper {
 	 */
 	getLastViewCursorsRenderData(): IViewCursorRenderData[];
 
-	shouldSuppressMouseDownOnViewZone(viewZoneId: number): boolean;
+	shouldSuppressMouseDownOnViewZone(viewZoneId: string): boolean;
 	shouldSuppressMouseDownOnWidget(widgetId: string): boolean;
 
 	/**

diff --git a/src/vs/editor/browser/controller/mouseTarget.ts b/src/vs/editor/browser/controller/mouseTarget.ts
@@ -19,7 +19,7 @@ import { IViewModel } from 'vs/editor/common/viewModel/viewModel';
 import { CursorColumns } from 'vs/editor/common/controller/cursorCommon';
 
 export interface IViewZoneData {
-	viewZoneId: number;
+	viewZoneId: string;
 	positionBefore: Position | null;
 	positionAfter: Position | null;
 	position: Position;

diff --git a/src/vs/editor/browser/editorBrowser.ts b/src/vs/editor/browser/editorBrowser.ts
@@ -83,17 +83,17 @@ export interface IViewZoneChangeAccessor {
 	 * @param zone Zone to create
 	 * @return A unique identifier to the view zone.
 	 */
-	addZone(zone: IViewZone): number;
+	addZone(zone: IViewZone): string;
 	/**
 	 * Remove a zone
 	 * @param id A unique identifier to the view zone, as returned by the `addZone` call.
 	 */
-	removeZone(id: number): void;
+	removeZone(id: string): void;
 	/**
 	 * Change a zone's position.
 	 * The editor will rescan the `afterLineNumber` and `afterColumn` properties of a view zone.
 	 */
-	layoutZone(id: number): void;
+	layoutZone(id: string): void;
 }
 
 /**

diff --git a/src/vs/editor/browser/view/viewImpl.ts b/src/vs/editor/browser/view/viewImpl.ts
@@ -248,7 +248,7 @@ export class View extends ViewEventHandler {
 			getLastViewCursorsRenderData: () => {
 				return this.viewCursors.getLastRenderData() || [];
 			},
-			shouldSuppressMouseDownOnViewZone: (viewZoneId: number) => {
+			shouldSuppressMouseDownOnViewZone: (viewZoneId: string) => {
 				return this.viewZones.shouldSuppressMouseDownOnViewZone(viewZoneId);
 			},
 			shouldSuppressMouseDownOnWidget: (widgetId: string) => {
@@ -473,17 +473,17 @@ export class View extends ViewEventHandler {
 
 		this._renderOnce(() => {
 			const changeAccessor: editorBrowser.IViewZoneChangeAccessor = {
-				addZone: (zone: editorBrowser.IViewZone): number => {
+				addZone: (zone: editorBrowser.IViewZone): string => {
 					zonesHaveChanged = true;
 					return this.viewZones.addZone(zone);
 				},
-				removeZone: (id: number): void => {
+				removeZone: (id: string): void => {
 					if (!id) {
 						return;
 					}
 					zonesHaveChanged = this.viewZones.removeZone(id) || zonesHaveChanged;
 				},
-				layoutZone: (id: number): void => {
+				layoutZone: (id: string): void => {
 					if (!id) {
 						return;
 					}

diff --git a/src/vs/editor/browser/viewParts/viewZones/viewZones.ts b/src/vs/editor/browser/viewParts/viewZones/viewZones.ts
@@ -14,7 +14,7 @@ import * as viewEvents from 'vs/editor/common/view/viewEvents';
 import { IViewWhitespaceViewportData } from 'vs/editor/common/viewModel/viewModel';
 
 export interface IMyViewZone {
-	whitespaceId: number;
+	whitespaceId: string;
 	delegate: IViewZone;
 	isVisible: boolean;
 	domNode: FastDomNode<HTMLElement>;
@@ -74,7 +74,7 @@ export class ViewZones extends ViewPart {
 			const id = keys[i];
 			const zone = this._zones[id];
 			const props = this._computeWhitespaceProps(zone.delegate);
-			if (this._context.viewLayout.changeWhitespace(parseInt(id, 10), props.afterViewLineNumber, props.heightInPx)) {
+			if (this._context.viewLayout.changeWhitespace(id, props.afterViewLineNumber, props.heightInPx)) {
 				this._safeCallOnComputedHeight(zone.delegate, props.heightInPx);
 				hadAChange = true;
 			}
@@ -183,7 +183,7 @@ export class ViewZones extends ViewPart {
 		};
 	}
 
-	public addZone(zone: IViewZone): number {
+	public addZone(zone: IViewZone): string {
 		const props = this._computeWhitespaceProps(zone);
 		const whitespaceId = this._context.viewLayout.addWhitespace(props.afterViewLineNumber, this._getZoneOrdinal(zone), props.heightInPx, props.minWidthInPx);
 
@@ -200,29 +200,29 @@ export class ViewZones extends ViewPart {
 		myZone.domNode.setPosition('absolute');
 		myZone.domNode.domNode.style.width = '100%';
 		myZone.domNode.setDisplay('none');
-		myZone.domNode.setAttribute('monaco-view-zone', myZone.whitespaceId.toString());
+		myZone.domNode.setAttribute('monaco-view-zone', myZone.whitespaceId);
 		this.domNode.appendChild(myZone.domNode);
 
 		if (myZone.marginDomNode) {
 			myZone.marginDomNode.setPosition('absolute');
 			myZone.marginDomNode.domNode.style.width = '100%';
 			myZone.marginDomNode.setDisplay('none');
-			myZone.marginDomNode.setAttribute('monaco-view-zone', myZone.whitespaceId.toString());
+			myZone.marginDomNode.setAttribute('monaco-view-zone', myZone.whitespaceId);
 			this.marginDomNode.appendChild(myZone.marginDomNode);
 		}
 
-		this._zones[myZone.whitespaceId.toString()] = myZone;
+		this._zones[myZone.whitespaceId] = myZone;
 
 
 		this.setShouldRender();
 
 		return myZone.whitespaceId;
 	}
 
-	public removeZone(id: number): boolean {
-		if (this._zones.hasOwnProperty(id.toString())) {
-			const zone = this._zones[id.toString()];
-			delete this._zones[id.toString()];
+	public removeZone(id: string): boolean {
+		if (this._zones.hasOwnProperty(id)) {
+			const zone = this._zones[id];
+			delete this._zones[id];
 			this._context.viewLayout.removeWhitespace(zone.whitespaceId);
 
 			zone.domNode.removeAttribute('monaco-visible-view-zone');
@@ -242,10 +242,10 @@ export class ViewZones extends ViewPart {
 		return false;
 	}
 
-	public layoutZone(id: number): boolean {
+	public layoutZone(id: string): boolean {
 		let changed = false;
-		if (this._zones.hasOwnProperty(id.toString())) {
-			const zone = this._zones[id.toString()];
+		if (this._zones.hasOwnProperty(id)) {
+			const zone = this._zones[id];
 			const props = this._computeWhitespaceProps(zone.delegate);
 			// const newOrdinal = this._getZoneOrdinal(zone.delegate);
 			changed = this._context.viewLayout.changeWhitespace(zone.whitespaceId, props.afterViewLineNumber, props.heightInPx) || changed;
@@ -259,9 +259,9 @@ export class ViewZones extends ViewPart {
 		return changed;
 	}
 
-	public shouldSuppressMouseDownOnViewZone(id: number): boolean {
-		if (this._zones.hasOwnProperty(id.toString())) {
-			const zone = this._zones[id.toString()];
+	public shouldSuppressMouseDownOnViewZone(id: string): boolean {
+		if (this._zones.hasOwnProperty(id)) {
+			const zone = this._zones[id];
 			return Boolean(zone.delegate.suppressMouseDown);
 		}
 		return false;
@@ -314,7 +314,7 @@ export class ViewZones extends ViewPart {
 
 		let hasVisibleZone = false;
 		for (let i = 0, len = visibleWhitespaces.length; i < len; i++) {
-			visibleZones[visibleWhitespaces[i].id.toString()] = visibleWhitespaces[i];
+			visibleZones[visibleWhitespaces[i].id] = visibleWhitespaces[i];
 			hasVisibleZone = true;
 		}
 

diff --git a/src/vs/editor/browser/widget/diffEditorWidget.ts b/src/vs/editor/browser/widget/diffEditorWidget.ts
@@ -69,7 +69,7 @@ interface IDiffEditorWidgetStyle {
 }
 
 class VisualEditorState {
-	private _zones: number[];
+	private _zones: string[];
 	private _zonesMap: { [zoneId: string]: boolean; };
 	private _decorations: string[];
 

diff --git a/src/vs/editor/common/model/textModel.ts b/src/vs/editor/common/model/textModel.ts
@@ -110,21 +110,6 @@ export function createTextBuffer(value: string | model.ITextBufferFactory, defau
 
 let MODEL_ID = 0;
 
-/**
- * Produces 'a'-'z', followed by 'A'-'Z'... followed by 'a'-'z', etc.
- */
-function singleLetter(result: number): string {
-	const LETTERS_CNT = (CharCode.Z - CharCode.A + 1);
-
-	result = result % (2 * LETTERS_CNT);
-
-	if (result < LETTERS_CNT) {
-		return String.fromCharCode(CharCode.a + result);
-	}
-
-	return String.fromCharCode(CharCode.A + result - LETTERS_CNT);
-}
-
 const LIMIT_FIND_COUNT = 999;
 export const LONG_LINE_BOUNDARY = 10000;
 
@@ -343,7 +328,7 @@ export class TextModel extends Disposable implements model.ITextModel {
 			}
 		});
 
-		this._instanceId = singleLetter(MODEL_ID);
+		this._instanceId = strings.singleLetterHash(MODEL_ID);
 		this._lastDecorationId = 0;
 		this._decorations = Object.create(null);
 		this._decorationsTree = new DecorationsTrees();

diff --git a/src/vs/editor/common/viewLayout/linesLayout.ts b/src/vs/editor/common/viewLayout/linesLayout.ts
@@ -63,14 +63,14 @@ export class LinesLayout {
 	 * @param heightInPx The height of the whitespace, in pixels.
 	 * @return An id that can be used later to mutate or delete the whitespace
 	 */
-	public insertWhitespace(afterLineNumber: number, ordinal: number, heightInPx: number, minWidth: number): number {
+	public insertWhitespace(afterLineNumber: number, ordinal: number, heightInPx: number, minWidth: number): string {
 		return this._whitespaces.insertWhitespace(afterLineNumber, ordinal, heightInPx, minWidth);
 	}
 
 	/**
 	 * Change properties associated with a certain whitespace.
 	 */
-	public changeWhitespace(id: number, newAfterLineNumber: number, newHeight: number): boolean {
+	public changeWhitespace(id: string, newAfterLineNumber: number, newHeight: number): boolean {
 		return this._whitespaces.changeWhitespace(id, newAfterLineNumber, newHeight);
 	}
 
@@ -80,7 +80,7 @@ export class LinesLayout {
 	 * @param id The whitespace to remove
 	 * @return Returns true if the whitespace is found and it is removed.
 	 */
-	public removeWhitespace(id: number): boolean {
+	public removeWhitespace(id: string): boolean {
 		return this._whitespaces.removeWhitespace(id);
 	}
 

diff --git a/src/vs/editor/common/viewLayout/viewLayout.ts b/src/vs/editor/common/viewLayout/viewLayout.ts
@@ -173,13 +173,13 @@ export class ViewLayout extends Disposable implements IViewLayout {
 
 	// ---- IVerticalLayoutProvider
 
-	public addWhitespace(afterLineNumber: number, ordinal: number, height: number, minWidth: number): number {
+	public addWhitespace(afterLineNumber: number, ordinal: number, height: number, minWidth: number): string {
 		return this._linesLayout.insertWhitespace(afterLineNumber, ordinal, height, minWidth);
 	}
-	public changeWhitespace(id: number, newAfterLineNumber: number, newHeight: number): boolean {
+	public changeWhitespace(id: string, newAfterLineNumber: number, newHeight: number): boolean {
 		return this._linesLayout.changeWhitespace(id, newAfterLineNumber, newHeight);
 	}
-	public removeWhitespace(id: number): boolean {
+	public removeWhitespace(id: string): boolean {
 		return this._linesLayout.removeWhitespace(id);
 	}
 	public getVerticalOffsetForLineNumber(lineNumber: number): number {