Skip to content

Commit

Permalink
bug - don't close peek when changing editors, #83752
Browse files Browse the repository at this point in the history
  • Loading branch information
jrieken committed Nov 6, 2019
1 parent d76c6d6 commit 0f9bbdc
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 32 deletions.
15 changes: 3 additions & 12 deletions src/vs/editor/contrib/gotoSymbol/goToCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { ITextModel, IWordAtPosition } from 'vs/editor/common/model';
import { LocationLink, Location, isLocationLink } from 'vs/editor/common/modes';
import { MessageController } from 'vs/editor/contrib/message/messageController';
import { PeekContext } from 'vs/editor/contrib/peekView/peekView';
import { ReferencesController, RequestOptions } from 'vs/editor/contrib/gotoSymbol/peek/referencesController';
import { ReferencesController } from 'vs/editor/contrib/gotoSymbol/peek/referencesController';
import { ReferencesModel } from 'vs/editor/contrib/gotoSymbol/referencesModel';
import * as nls from 'vs/nls';
import { MenuId } from 'vs/platform/actions/common/actions';
Expand Down Expand Up @@ -157,9 +157,6 @@ abstract class SymbolNavigationAction extends EditorAction {
let controller = ReferencesController.get(target);
if (controller && target.hasModel()) {
controller.toggleWidget(target.getSelection(), createCancelablePromise(_ => Promise.resolve(model)), {
getMetaTitle: (model) => {
return this._getMetaTitle(model);
},
onGoto: (reference) => {
controller.closeWidget();
return this._openReference(target, editorService, reference, false);
Expand Down Expand Up @@ -650,12 +647,6 @@ registerEditorAction(class PeekReferencesAction extends ReferencesAction {

//#region --- REFERENCE search special commands

const defaultReferenceSearchOptions: RequestOptions = {
getMetaTitle(model) {
return model.references.length > 1 ? nls.localize('meta.titleReference', " – {0} references", model.references.length) : '';
}
};

CommandsRegistry.registerCommand({
id: 'editor.action.findReferences',
handler: (accessor: ServicesAccessor, resource: URI, position: corePosition.IPosition) => {
Expand All @@ -679,7 +670,7 @@ CommandsRegistry.registerCommand({

const references = createCancelablePromise(token => getReferencesAtPosition(control.getModel(), corePosition.Position.lift(position), token).then(references => new ReferencesModel(references)));
const range = new Range(position.lineNumber, position.column, position.lineNumber, position.column);
return Promise.resolve(controller.toggleWidget(range, references, defaultReferenceSearchOptions));
return Promise.resolve(controller.toggleWidget(range, references, {}));
});
}
});
Expand Down Expand Up @@ -718,7 +709,7 @@ CommandsRegistry.registerCommand({
return controller.toggleWidget(
new Range(position.lineNumber, position.column, position.lineNumber, position.column),
createCancelablePromise(_ => Promise.resolve(new ReferencesModel(references))),
defaultReferenceSearchOptions
{}
);
});
},
Expand Down
52 changes: 32 additions & 20 deletions src/vs/editor/contrib/gotoSymbol/peek/referencesController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { Range } from 'vs/editor/common/core/range';
import { Position } from 'vs/editor/common/core/position';
import { Location } from 'vs/editor/common/modes';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { CancelablePromise } from 'vs/base/common/async';
import { CancelablePromise, createCancelablePromise } from 'vs/base/common/async';
import { getOuterEditor, PeekContext } from 'vs/editor/contrib/peekView/peekView';
import { IListService } from 'vs/platform/list/browser/listService';
import { KeybindingsRegistry, KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
Expand All @@ -28,7 +28,6 @@ import { KeyCode, KeyMod } from 'vs/base/common/keyCodes';
export const ctxReferenceSearchVisible = new RawContextKey<boolean>('referenceSearchVisible', false);

export interface RequestOptions {
getMetaTitle(model: ReferencesModel): string;
onGoto?: (reference: Location) => Promise<any>;
}

Expand Down Expand Up @@ -154,8 +153,15 @@ export abstract class ReferencesController implements editorCommon.IEditorContri
// show widget
return this._widget.setModel(this._model).then(() => {
if (this._widget && this._model && this._editor.hasModel()) { // might have been closed

// set title
this._widget.setMetaTitle(options.getMetaTitle(this._model));
if (this._model.references.length === 1) {
this._widget.setMetaTitle(nls.localize('metaTitle.1', "1 result"));
} else if (!this._model.isEmpty) {
this._widget.setMetaTitle(nls.localize('metaTitle.N', "{0} results", this._model.references.length));
} else {
this._widget.setMetaTitle('');
}

// set 'best' selection
let uri = this._editor.getModel().uri;
Expand Down Expand Up @@ -196,16 +202,12 @@ export abstract class ReferencesController implements editorCommon.IEditorContri
}

closeWidget(): void {
if (this._widget) {
dispose(this._widget);
this._widget = undefined;
}
this._referenceSearchVisible.reset();
this._disposables.clear();
if (this._model) {
dispose(this._model);
this._model = undefined;
}
dispose(this._widget);
dispose(this._model);
this._widget = undefined;
this._model = undefined;
this._editor.focus();
this._requestIdPool += 1; // Cancel pending requests
}
Expand All @@ -224,21 +226,31 @@ export abstract class ReferencesController implements editorCommon.IEditorContri
}, this._editor).then(openedEditor => {
this._ignoreModelChangeEvent = false;

if (!openedEditor || openedEditor !== this._editor) {
// TODO@Alex TODO@Joh
// when opening the current reference we might end up
// in a different editor instance. that means we also have
// a different instance of this reference search controller
// and cannot hold onto the widget (which likely doesn't
// exist). Instead of bailing out we should find the
// 'sister' action and pass our current model on to it.
if (!openedEditor || !this._widget) {
// something went wrong...
this.closeWidget();
return;
}

if (this._widget) {
if (this._editor === openedEditor) {
//
this._widget.show(range);
this._widget.focus();

} else {
// we opened a different editor instance which means a different controller instance.
// therefore we stop with this controller and continue with the other
const other = ReferencesController.get(openedEditor);
const model = this._model!.clone();

this.closeWidget();
openedEditor.focus();

other.toggleWidget(
range,
createCancelablePromise(_ => Promise.resolve(model)),
{}
);
}

}, (err) => {
Expand Down
7 changes: 7 additions & 0 deletions src/vs/editor/contrib/gotoSymbol/referencesModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,16 @@ export class FileReferences implements IDisposable {
export class ReferencesModel implements IDisposable {

private readonly _disposables = new DisposableStore();
private readonly _links: LocationLink[];

readonly groups: FileReferences[] = [];
readonly references: OneReference[] = [];

readonly _onDidChangeReferenceRange = new Emitter<OneReference>();
readonly onDidChangeReferenceRange: Event<OneReference> = this._onDidChangeReferenceRange.event;

constructor(links: LocationLink[]) {
this._links = links;

// grouping and sorting
const [providersFirst] = links;
Expand Down Expand Up @@ -188,6 +191,10 @@ export class ReferencesModel implements IDisposable {
this.groups.length = 0;
}

clone(): ReferencesModel {
return new ReferencesModel(this._links);
}

get isEmpty(): boolean {
return this.groups.length === 0;
}
Expand Down
5 changes: 5 additions & 0 deletions src/vs/editor/contrib/peekView/media/peekViewWidget.css
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
margin-left: 0.5em;
}

.monaco-editor .peekview-widget .head .peekview-title .meta::before {
content: '-';
padding: 0 0.3em;
}

.monaco-editor .peekview-widget .head .peekview-actions {
flex: 1;
text-align: right;
Expand Down

0 comments on commit 0f9bbdc

Please sign in to comment.