diff --git a/src/vs/editor/contrib/hover/browser/contentHover.ts b/src/vs/editor/contrib/hover/browser/contentHover.ts index 30de80e0952..e0f2246f5a6 100644 --- a/src/vs/editor/contrib/hover/browser/contentHover.ts +++ b/src/vs/editor/contrib/hover/browser/contentHover.ts @@ -24,6 +24,7 @@ import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { Context as SuggestContext } from 'vs/editor/contrib/suggest/browser/suggest'; import { AsyncIterableObject } from 'vs/base/common/async'; import { EditorContextKeys } from 'vs/editor/common/editorContextKeys'; +import { Constants } from 'vs/base/common/uint'; const $ = dom.$; @@ -240,7 +241,7 @@ export class ContentHoverController extends Disposable { } private _renderMessages(anchor: HoverAnchor, messages: IHoverPart[]): void { - const { showAtPosition, showAtRange, highlightRange } = ContentHoverController.computeHoverRanges(anchor.range, messages); + const { showAtPosition, showAtRange, highlightRange } = ContentHoverController.computeHoverRanges(this._editor, anchor.range, messages); const disposables = new DisposableStore(); const statusBar = disposables.add(new EditorHoverStatusBar(this._keybindingService)); @@ -301,7 +302,19 @@ export class ContentHoverController extends Disposable { className: 'hoverHighlight' }); - public static computeHoverRanges(anchorRange: Range, messages: IHoverPart[]) { + public static computeHoverRanges(editor: ICodeEditor, anchorRange: Range, messages: IHoverPart[]) { + let startColumnBoundary = 1; + let endColumnBoundary = Constants.MAX_SAFE_SMALL_INTEGER; + if (editor.hasModel()) { + // Ensure the range is on the current view line + const viewModel = editor._getViewModel(); + const coordinatesConverter = viewModel.coordinatesConverter; + const anchorViewRange = coordinatesConverter.convertModelRangeToViewRange(anchorRange); + const anchorViewRangeStart = new Position(anchorViewRange.startLineNumber, viewModel.getLineMinColumn(anchorViewRange.startLineNumber)); + const anchorViewRangeEnd = new Position(anchorViewRange.endLineNumber, viewModel.getLineMaxColumn(anchorViewRange.endLineNumber)); + startColumnBoundary = coordinatesConverter.convertViewPositionToModelPosition(anchorViewRangeStart).column; + endColumnBoundary = coordinatesConverter.convertViewPositionToModelPosition(anchorViewRangeEnd).column; + } // The anchor range is always on a single line const anchorLineNumber = anchorRange.startLineNumber; let renderStartColumn = anchorRange.startColumn; @@ -313,8 +326,8 @@ export class ContentHoverController extends Disposable { highlightRange = Range.plusRange(highlightRange, msg.range); if (msg.range.startLineNumber === anchorLineNumber && msg.range.endLineNumber === anchorLineNumber) { // this message has a range that is completely sitting on the line of the anchor - renderStartColumn = Math.min(renderStartColumn, msg.range.startColumn); - renderEndColumn = Math.max(renderEndColumn, msg.range.endColumn); + renderStartColumn = Math.max(Math.min(renderStartColumn, msg.range.startColumn), startColumnBoundary); + renderEndColumn = Math.min(Math.max(renderEndColumn, msg.range.endColumn), endColumnBoundary); } if (msg.forceShowAtRange) { forceShowAtRange = msg.range; diff --git a/src/vs/editor/contrib/hover/test/browser/contentHover.test.ts b/src/vs/editor/contrib/hover/test/browser/contentHover.test.ts index 7e35cea943b..1dacdd5a203 100644 --- a/src/vs/editor/contrib/hover/test/browser/contentHover.test.ts +++ b/src/vs/editor/contrib/hover/test/browser/contentHover.test.ts @@ -8,20 +8,45 @@ import { ContentHoverController } from 'vs/editor/contrib/hover/browser/contentH import { Range } from 'vs/editor/common/core/range'; import { Position } from 'vs/editor/common/core/position'; import { IHoverPart } from 'vs/editor/contrib/hover/browser/hoverTypes'; +import { TestCodeEditorInstantiationOptions, withTestCodeEditor } from 'vs/editor/test/browser/testCodeEditor'; suite('Content Hover', () => { test('issue #151235: Gitlens hover shows up in the wrong place', () => { - const actual = ContentHoverController.computeHoverRanges( - new Range(5, 5, 5, 5), - [{ range: new Range(4, 1, 5, 6) }] - ); - assert.deepStrictEqual( - actual, - { - showAtPosition: new Position(5, 5), - showAtRange: new Range(5, 5, 5, 5), - highlightRange: new Range(4, 1, 5, 6) - } - ); + const text = 'just some text'; + withTestCodeEditor(text, {}, (editor) => { + const actual = ContentHoverController.computeHoverRanges( + editor, + new Range(5, 5, 5, 5), + [{ range: new Range(4, 1, 5, 6) }] + ); + assert.deepStrictEqual( + actual, + { + showAtPosition: new Position(5, 5), + showAtRange: new Range(5, 5, 5, 5), + highlightRange: new Range(4, 1, 5, 6) + } + ); + }); + }); + + test('issue #95328: Hover placement with word-wrap', () => { + const text = 'just some text'; + const opts: TestCodeEditorInstantiationOptions = { wordWrap: 'wordWrapColumn', wordWrapColumn: 6 }; + withTestCodeEditor(text, opts, (editor) => { + const actual = ContentHoverController.computeHoverRanges( + editor, + new Range(1, 8, 1, 8), + [{ range: new Range(1, 1, 1, 15) }] + ); + assert.deepStrictEqual( + actual, + { + showAtPosition: new Position(1, 6), + showAtRange: new Range(1, 6, 1, 11), + highlightRange: new Range(1, 1, 1, 15) + } + ); + }); }); }); diff --git a/src/vs/editor/test/browser/testCodeEditor.ts b/src/vs/editor/test/browser/testCodeEditor.ts index 9fb9e854b35..9278f28c685 100644 --- a/src/vs/editor/test/browser/testCodeEditor.ts +++ b/src/vs/editor/test/browser/testCodeEditor.ts @@ -133,7 +133,7 @@ function isTextModel(arg: ITextModel | string | string[] | ITextBufferFactory): function _withTestCodeEditor(arg: ITextModel | string | string[] | ITextBufferFactory, options: TestCodeEditorInstantiationOptions, callback: (editor: ITestCodeEditor, viewModel: ViewModel, instantiationService: TestInstantiationService) => void): void; function _withTestCodeEditor(arg: ITextModel | string | string[] | ITextBufferFactory, options: TestCodeEditorInstantiationOptions, callback: (editor: ITestCodeEditor, viewModel: ViewModel, instantiationService: TestInstantiationService) => Promise): Promise; -async function _withTestCodeEditor(arg: ITextModel | string | string[] | ITextBufferFactory, options: TestCodeEditorInstantiationOptions, callback: (editor: ITestCodeEditor, viewModel: ViewModel, instantiationService: TestInstantiationService) => Promise | void): Promise | void> { +function _withTestCodeEditor(arg: ITextModel | string | string[] | ITextBufferFactory, options: TestCodeEditorInstantiationOptions, callback: (editor: ITestCodeEditor, viewModel: ViewModel, instantiationService: TestInstantiationService) => Promise | void): Promise | void { const disposables = new DisposableStore(); const instantiationService = createCodeEditorServices(disposables, options.serviceCollection); delete options.serviceCollection; @@ -149,12 +149,12 @@ async function _withTestCodeEditor(arg: ITextModel | string | string[] | ITextBu const editor = disposables.add(instantiateTestCodeEditor(instantiationService, model, options)); const viewModel = editor.getViewModel()!; viewModel.setHasFocus(true); - try { - const result = callback(editor, editor.getViewModel()!, instantiationService); - await result; - } finally { - disposables.dispose(); + const result = callback(editor, editor.getViewModel()!, instantiationService); + if (result) { + return result.then(() => disposables.dispose()); } + + disposables.dispose(); } export function createCodeEditorServices(disposables: DisposableStore, services: ServiceCollection = new ServiceCollection()): TestInstantiationService {