Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Less than ideal diff results #44422

Closed
alexdima opened this issue Feb 26, 2018 · 1 comment
Closed

Less than ideal diff results #44422

alexdima opened this issue Feb 26, 2018 · 1 comment
Assignees
Labels
diff-editor Diff editor mode issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded

Comments

@alexdima
Copy link
Member

Logically, the diff has one multi-line insertion and one multi-line deletion, yet, the diff algorithm identifies 3 differences:

image

  • original:
export class MainThreadTextEditor {

	public revealRange(range: IRange, revealType: TextEditorRevealType): void {
		if (!this._codeEditor) {
			return;
		}
		switch (revealType) {
			case TextEditorRevealType.Default:
				this._codeEditor.revealRange(range, editorCommon.ScrollType.Smooth);
				break;
			case TextEditorRevealType.InCenter:
				this._codeEditor.revealRangeInCenter(range, editorCommon.ScrollType.Smooth);
				break;
			case TextEditorRevealType.InCenterIfOutsideViewport:
				this._codeEditor.revealRangeInCenterIfOutsideViewport(range, editorCommon.ScrollType.Smooth);
				break;
			case TextEditorRevealType.AtTop:
				this._codeEditor.revealRangeAtTop(range, editorCommon.ScrollType.Smooth);
				break;
			default:
				console.warn(`Unknown revealType: ${revealType}`);
				break;
		}
	}

	private _readConfiguration(model: ITextModel, codeEditor: ICodeEditor): IResolvedTextEditorConfiguration {
		if (model.isDisposed()) {
			// shutdown time
			return this._configuration;
		}
		let cursorStyle = this._configuration ? this._configuration.cursorStyle : TextEditorCursorStyle.Line;
		let lineNumbers: TextEditorLineNumbersStyle = this._configuration ? this._configuration.lineNumbers : TextEditorLineNumbersStyle.On;
		if (codeEditor) {
			let codeEditorOpts = codeEditor.getConfiguration();
			cursorStyle = codeEditorOpts.viewInfo.cursorStyle;

			switch (codeEditorOpts.viewInfo.renderLineNumbers) {
				case RenderLineNumbersType.Off:
					lineNumbers = TextEditorLineNumbersStyle.Off;
					break;
				case RenderLineNumbersType.Relative:
					lineNumbers = TextEditorLineNumbersStyle.Relative;
					break;
				default:
					lineNumbers = TextEditorLineNumbersStyle.On;
					break;
			}
		}

		let indent = model.getOptions();
		return {
			insertSpaces: indent.insertSpaces,
			tabSize: indent.tabSize,
			cursorStyle: cursorStyle,
			lineNumbers: lineNumbers
		};
	}

	private _setConfiguration(newConfiguration: IResolvedTextEditorConfiguration): void {
		if (configurationsEqual(this._configuration, newConfiguration)) {
			return;
		}
		this._configuration = newConfiguration;
		this._onConfigurationChanged.fire(this._configuration);
	}

	public isFocused(): boolean {
		if (this._codeEditor) {
			return this._codeEditor.isFocused();
		}
		return false;
	}

}
  • modified:
export class MainThreadTextEditor {









	constructor(
		modelService: IModelService
	) {









	}


	public revealRange(range: IRange, revealType: TextEditorRevealType): void {
		if (!this._codeEditor) {
			return;
		}
		switch (revealType) {
			case TextEditorRevealType.Default:
				this._codeEditor.revealRange(range, editorCommon.ScrollType.Smooth);
				break;
			case TextEditorRevealType.InCenter:
				this._codeEditor.revealRangeInCenter(range, editorCommon.ScrollType.Smooth);
				break;
			case TextEditorRevealType.InCenterIfOutsideViewport:
				this._codeEditor.revealRangeInCenterIfOutsideViewport(range, editorCommon.ScrollType.Smooth);
				break;
			case TextEditorRevealType.AtTop:
				this._codeEditor.revealRangeAtTop(range, editorCommon.ScrollType.Smooth);
				break;
			default:
				console.warn(`Unknown revealType: ${revealType}`);
				break;
		}
	}

	public isFocused(): boolean {
		if (this._codeEditor) {
			return this._codeEditor.isFocused();
		}
		return false;
	}

}
@alexdima alexdima added the diff-editor Diff editor mode issues label Feb 26, 2018
@alexdima alexdima self-assigned this Feb 26, 2018
@escape0707
Copy link

Recently came into this bug, too. And almost report a issue before found this is already under fixing!
I think this maybe due to the diff tool ignores essential formatting spaces.
Really appreciate you guys' work and looking forward to cheer after the fix!

@alexdima alexdima added the feature-request Request for new features or functionality label Apr 18, 2018
@alexdima alexdima added this to the On Deck milestone Apr 18, 2018
@alexdima alexdima modified the milestones: On Deck, September 2018 Sep 22, 2018
@bpasero bpasero added the verification-needed Verification of issue is requested label Sep 25, 2018
@sandy081 sandy081 added the verified Verification succeeded label Sep 25, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
diff-editor Diff editor mode issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants