From 763ae0943dc1639cfc540cdcd0f0ad47dcc1560b Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 6 Mar 2019 15:36:56 +0100 Subject: [PATCH 1/9] eng - some extHost strict-null work --- src/vs/workbench/api/electron-browser/mainThreadEditors.ts | 2 +- src/vs/workbench/api/node/extHost.api.impl.ts | 2 +- src/vs/workbench/api/node/extHost.protocol.ts | 2 +- src/vs/workbench/api/node/extHostTextEditors.ts | 2 +- src/vs/workbench/api/node/extHostWorkspace.ts | 6 +++--- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/api/electron-browser/mainThreadEditors.ts b/src/vs/workbench/api/electron-browser/mainThreadEditors.ts index d1d7e1cd812..97ff3f6f4a6 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadEditors.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadEditors.ts @@ -114,7 +114,7 @@ export class MainThreadTextEditors implements MainThreadTextEditorsShape { // --- from extension host process - $tryShowTextDocument(resource: UriComponents, options: ITextDocumentShowOptions): Promise { + $tryShowTextDocument(resource: UriComponents, options: ITextDocumentShowOptions): Promise { const uri = URI.revive(resource); const editorOptions: ITextEditorOptions = { diff --git a/src/vs/workbench/api/node/extHost.api.impl.ts b/src/vs/workbench/api/node/extHost.api.impl.ts index 4d2e7c01c30..652f64d8ca2 100644 --- a/src/vs/workbench/api/node/extHost.api.impl.ts +++ b/src/vs/workbench/api/node/extHost.api.impl.ts @@ -527,7 +527,7 @@ export function createApiFactory( onDidChangeWorkspaceFolders: function (listener, thisArgs?, disposables?) { return extHostWorkspace.onDidChangeWorkspace(listener, thisArgs, disposables); }, - asRelativePath: (pathOrUri, includeWorkspace) => { + asRelativePath: (pathOrUri, includeWorkspace?) => { return extHostWorkspace.getRelativePath(pathOrUri, includeWorkspace); }, findFiles: (include, exclude, maxResults?, token?) => { diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 0d20d99f8e1..6296303e877 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -224,7 +224,7 @@ export interface ITextDocumentShowOptions { } export interface MainThreadTextEditorsShape extends IDisposable { - $tryShowTextDocument(resource: UriComponents, options: ITextDocumentShowOptions): Promise; + $tryShowTextDocument(resource: UriComponents, options: ITextDocumentShowOptions): Promise; $registerTextEditorDecorationType(key: string, options: editorCommon.IDecorationRenderOptions): void; $removeTextEditorDecorationType(key: string): void; $tryShowEditor(id: string, position: EditorViewColumn): Promise; diff --git a/src/vs/workbench/api/node/extHostTextEditors.ts b/src/vs/workbench/api/node/extHostTextEditors.ts index a42ec9c245f..ce6b21dcc14 100644 --- a/src/vs/workbench/api/node/extHostTextEditors.ts +++ b/src/vs/workbench/api/node/extHostTextEditors.ts @@ -75,7 +75,7 @@ export class ExtHostEditors implements ExtHostEditorsShape { } return this._proxy.$tryShowTextDocument(document.uri, options).then(id => { - const editor = this._extHostDocumentsAndEditors.getEditor(id); + const editor = id && this._extHostDocumentsAndEditors.getEditor(id); if (editor) { return editor; } else { diff --git a/src/vs/workbench/api/node/extHostWorkspace.ts b/src/vs/workbench/api/node/extHostWorkspace.ts index bcd01710615..ad7f3b3097a 100644 --- a/src/vs/workbench/api/node/extHostWorkspace.ts +++ b/src/vs/workbench/api/node/extHostWorkspace.ts @@ -321,10 +321,10 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape, IExtHostWorkspac return folders[0].uri.fsPath; } - getRelativePath(pathOrUri: string | vscode.Uri, includeWorkspace?: boolean): string | undefined { + getRelativePath(pathOrUri: string | vscode.Uri, includeWorkspace?: boolean): string { let resource: URI | undefined; - let path: string | undefined; + let path: string = ''; if (typeof pathOrUri === 'string') { resource = URI.file(pathOrUri); path = pathOrUri; @@ -354,7 +354,7 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape, IExtHostWorkspac if (includeWorkspace && folder.name) { result = `${folder.name}/${result}`; } - return result; + return result!; } private trySetWorkspaceFolders(folders: vscode.WorkspaceFolder[]): void { From 672b272fa3e350e9191b444e351e325742b97364 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 6 Mar 2019 17:19:10 +0100 Subject: [PATCH 2/9] dont use super and async/wait... --- .../services/files/node/remoteFileService.ts | 61 ++++++++++--------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/src/vs/workbench/services/files/node/remoteFileService.ts b/src/vs/workbench/services/files/node/remoteFileService.ts index 36687a83c68..c01f0c214ac 100644 --- a/src/vs/workbench/services/files/node/remoteFileService.ts +++ b/src/vs/workbench/services/files/node/remoteFileService.ts @@ -549,13 +549,13 @@ export class RemoteFileService extends FileService { } } - private async _doMoveWithInScheme(source: URI, target: URI, overwrite: boolean = false): Promise { + private _doMoveWithInScheme(source: URI, target: URI, overwrite: boolean = false): Promise { - if (overwrite) { - await this.del(target, { recursive: true }).catch(_err => { /*ignore*/ }); - } + const prepare = overwrite + ? Promise.resolve(this.del(target, { recursive: true }).catch(_err => { /*ignore*/ })) + : Promise.resolve(); - return this._withProvider(source).then(RemoteFileService._throwIfFileSystemIsReadonly).then(provider => { + return prepare.then(() => this._withProvider(source)).then(RemoteFileService._throwIfFileSystemIsReadonly).then(provider => { return RemoteFileService._mkdirp(provider, resources.dirname(target)).then(() => { return provider.rename(source, target, { overwrite }).then(() => { return this.resolveFile(target); @@ -589,7 +589,7 @@ export class RemoteFileService extends FileService { return super.copyFile(source, target, overwrite); } - return this._withProvider(target).then(RemoteFileService._throwIfFileSystemIsReadonly).then(async provider => { + return this._withProvider(target).then(RemoteFileService._throwIfFileSystemIsReadonly).then(provider => { if (source.scheme === target.scheme && (provider.capabilities & FileSystemProviderCapabilities.FileFolderCopy)) { // good: provider supports copy withing scheme @@ -607,36 +607,37 @@ export class RemoteFileService extends FileService { }); } - if (overwrite) { - await this.del(target, { recursive: true }).catch(_err => { /*ignore*/ }); - } + const prepare = overwrite + ? Promise.resolve(this.del(target, { recursive: true }).catch(_err => { /*ignore*/ })) + : Promise.resolve(); // todo@ben, can only copy text files // https://github.com/Microsoft/vscode/issues/41543 - return this.resolveContent(source, { acceptTextOnly: true }).then(content => { - return this._withProvider(target).then(provider => { - return this._writeFile( - provider, target, - new StringSnapshot(content.value), - content.encoding, - { create: true, overwrite: !!overwrite } - ).then(fileStat => { - this._onAfterOperation.fire(new FileOperationEvent(source, FileOperation.COPY, fileStat)); - return fileStat; + return prepare.then(() => { + return this.resolveContent(source, { acceptTextOnly: true }).then(content => { + return this._withProvider(target).then(provider => { + return this._writeFile( + provider, target, + new StringSnapshot(content.value), + content.encoding, + { create: true, overwrite: !!overwrite } + ).then(fileStat => { + this._onAfterOperation.fire(new FileOperationEvent(source, FileOperation.COPY, fileStat)); + return fileStat; + }); + }, err => { + const result = this._tryParseFileOperationResult(err); + if (result === FileOperationResult.FILE_MOVE_CONFLICT) { + throw new FileOperationError(localize('fileMoveConflict', "Unable to move/copy. File already exists at destination."), result); + } else if (err instanceof Error && err.name === 'ENOPRO') { + // file scheme + return super.updateContent(target, content.value, { encoding: content.encoding }); + } else { + return Promise.reject(err); + } }); - }, err => { - const result = this._tryParseFileOperationResult(err); - if (result === FileOperationResult.FILE_MOVE_CONFLICT) { - throw new FileOperationError(localize('fileMoveConflict', "Unable to move/copy. File already exists at destination."), result); - } else if (err instanceof Error && err.name === 'ENOPRO') { - // file scheme - return super.updateContent(target, content.value, { encoding: content.encoding }); - } else { - return Promise.reject(err); - } }); }); - }); } From 7866a0a97409149880316c16c1c6b8e8a1a40800 Mon Sep 17 00:00:00 2001 From: isidor Date: Wed, 6 Mar 2019 17:18:03 +0100 Subject: [PATCH 3/9] null checking in debug/common --- src/tsconfig.strictNullChecks.json | 3 +- .../workbench/contrib/debug/common/debug.ts | 18 +-- .../contrib/debug/common/debugModel.ts | 113 ++++++++++-------- .../contrib/debug/common/debugSource.ts | 6 +- .../contrib/debug/common/replModel.ts | 2 +- .../debug/electron-browser/debugSession.ts | 2 +- 6 files changed, 77 insertions(+), 67 deletions(-) diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index 9ececdd3e50..4ac3a4bf562 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -30,7 +30,8 @@ "./vs/workbench/services/progress/**/*.ts", "./vs/workbench/services/preferences/**/*.ts", "./vs/workbench/services/timer/**/*.ts", - "./vs/workbench/contrib/webview/**/*.ts" + "./vs/workbench/contrib/webview/**/*.ts", + "./vs/workbench/contrib/debug/common/**/*.ts", ], "files": [ "./vs/monaco.d.ts", diff --git a/src/vs/workbench/contrib/debug/common/debug.ts b/src/vs/workbench/contrib/debug/common/debug.ts index e303532fc05..39fe368818e 100644 --- a/src/vs/workbench/contrib/debug/common/debug.ts +++ b/src/vs/workbench/contrib/debug/common/debug.ts @@ -152,7 +152,7 @@ export interface IDebugSession extends ITreeElement { getLabel(): string; getSourceForUri(modelUri: uri): Source; - getSource(raw: DebugProtocol.Source): Source; + getSource(raw?: DebugProtocol.Source): Source; setConfiguration(configuration: { resolved: IConfig, unresolved: IConfig }): void; rawUpdate(data: IRawModelUpdate): void; @@ -199,7 +199,7 @@ export interface IDebugSession extends ITreeElement { stackTrace(threadId: number, startFrame: number, levels: number): Promise; exceptionInfo(threadId: number): Promise; scopes(frameId: number): Promise; - variables(variablesReference: number, filter: 'indexed' | 'named', start: number, count: number): Promise; + variables(variablesReference: number | undefined, filter: 'indexed' | 'named' | undefined, start: number | undefined, count: number | undefined): Promise; evaluate(expression: string, frameId?: number, context?: string): Promise; customRequest(request: string, args: any): Promise; @@ -214,7 +214,7 @@ export interface IDebugSession extends ITreeElement { terminateThreads(threadIds: number[]): Promise; completions(frameId: number, text: string, position: Position, overwriteBefore: number): Promise; - setVariable(variablesReference: number, name: string, value: string): Promise; + setVariable(variablesReference: number | undefined, name: string, value: string): Promise; loadSource(resource: uri): Promise; getLoadedSources(): Promise; } @@ -284,7 +284,7 @@ export interface IScope extends IExpressionContainer { export interface IStackFrame extends ITreeElement { readonly thread: IThread; readonly name: string; - readonly presentationHint: string; + readonly presentationHint: string | undefined; readonly frameId: number; readonly range: IRange; readonly source: Source; @@ -319,9 +319,9 @@ export interface IBreakpointUpdateData { } export interface IBaseBreakpoint extends IEnablement { - readonly condition: string; - readonly hitCondition: string; - readonly logMessage: string; + readonly condition?: string; + readonly hitCondition?: string; + readonly logMessage?: string; readonly verified: boolean; readonly idFromAdapter: number | undefined; } @@ -330,7 +330,7 @@ export interface IBreakpoint extends IBaseBreakpoint { readonly uri: uri; readonly lineNumber: number; readonly endLineNumber?: number; - readonly column: number; + readonly column?: number; readonly endColumn?: number; readonly message?: string; readonly adapterData: any; @@ -394,7 +394,7 @@ export interface IDebugModel extends ITreeElement { getExceptionBreakpoints(): ReadonlyArray; getWatchExpressions(): ReadonlyArray; - onDidChangeBreakpoints: Event; + onDidChangeBreakpoints: Event; onDidChangeCallStack: Event; onDidChangeWatchExpressions: Event; } diff --git a/src/vs/workbench/contrib/debug/common/debugModel.ts b/src/vs/workbench/contrib/debug/common/debugModel.ts index 91422072397..0f833db72af 100644 --- a/src/vs/workbench/contrib/debug/common/debugModel.ts +++ b/src/vs/workbench/contrib/debug/common/debugModel.ts @@ -29,7 +29,7 @@ export class SimpleReplElement implements IReplElement { private id: string, public value: string, public severity: severity, - public sourceData: IReplElementSource, + public sourceData?: IReplElementSource, ) { } toString(): string { @@ -98,19 +98,19 @@ export class ExpressionContainer implements IExpressionContainer { protected children?: Promise; constructor( - protected session: IDebugSession, - private _reference: number, + protected session: IDebugSession | undefined, + private _reference: number | undefined, private id: string, - public namedVariables = 0, - public indexedVariables = 0, - private startOfVariables = 0 + public namedVariables: number | undefined = 0, + public indexedVariables: number | undefined = 0, + private startOfVariables: number | undefined = 0 ) { } - get reference(): number { + get reference(): number | undefined { return this._reference; } - set reference(value: number) { + set reference(value: number | undefined) { this._reference = value; this.children = undefined; // invalidate children cache } @@ -137,17 +137,17 @@ export class ExpressionContainer implements IExpressionContainer { return childrenThenable.then(childrenArray => { // Use a dynamic chunk size based on the number of elements #9774 let chunkSize = ExpressionContainer.BASE_CHUNK_SIZE; - while (this.indexedVariables > chunkSize * ExpressionContainer.BASE_CHUNK_SIZE) { + while (!!this.indexedVariables && this.indexedVariables > chunkSize * ExpressionContainer.BASE_CHUNK_SIZE) { chunkSize *= ExpressionContainer.BASE_CHUNK_SIZE; } - if (this.indexedVariables > chunkSize) { + if (!!this.indexedVariables && this.indexedVariables > chunkSize) { // There are a lot of children, create fake intermediate values that represent chunks #9537 const numberOfChunks = Math.ceil(this.indexedVariables / chunkSize); for (let i = 0; i < numberOfChunks; i++) { - const start = this.startOfVariables + i * chunkSize; + const start = (this.startOfVariables || 0) + i * chunkSize; const count = Math.min(chunkSize, this.indexedVariables - i * chunkSize); - childrenArray.push(new Variable(this.session, this, this.reference, `[${start}..${start + count - 1}]`, '', '', null, count, { kind: 'virtual' }, null, true, start)); + childrenArray.push(new Variable(this.session, this, this.reference, `[${start}..${start + count - 1}]`, '', '', undefined, count, { kind: 'virtual' }, undefined, true, start)); } return childrenArray; @@ -168,16 +168,16 @@ export class ExpressionContainer implements IExpressionContainer { get hasChildren(): boolean { // only variables with reference > 0 have children. - return this.reference > 0; + return !!this.reference && this.reference > 0; } - private fetchVariables(start: number, count: number, filter: 'indexed' | 'named'): Promise { - return this.session.variables(this.reference, filter, start, count).then(response => { + private fetchVariables(start: number | undefined, count: number | undefined, filter: 'indexed' | 'named' | undefined): Promise { + return this.session!.variables(this.reference, filter, start, count).then(response => { return response && response.body && response.body.variables - ? distinct(response.body.variables.filter(v => !!v && isString(v.name)), v => v.name).map( - v => new Variable(this.session, this, v.variablesReference, v.name, v.evaluateName, v.value, v.namedVariables, v.indexedVariables, v.presentationHint, v.type)) + ? distinct(response.body.variables.filter(v => !!v && isString(v.name)), (v: DebugProtocol.Variable) => v.name).map((v: DebugProtocol.Variable) => + new Variable(this.session, this, v.variablesReference, v.name, v.evaluateName, v.value, v.namedVariables, v.indexedVariables, v.presentationHint, v.type)) : []; - }, (e: Error) => [new Variable(this.session, this, 0, e.message, e.message, '', 0, 0, { kind: 'virtual' }, null, false)]); + }, (e: Error) => [new Variable(this.session, this, 0, e.message, e.message, '', 0, 0, { kind: 'virtual' }, undefined, false)]); } // The adapter explicitly sents the children count of an expression only if there are lots of children which should be chunked. @@ -204,7 +204,7 @@ export class Expression extends ExpressionContainer implements IExpression { public type: string; constructor(public name: string, id = generateUuid()) { - super(null, 0, id); + super(undefined, 0, id); this.available = false; // name is not set if the expression is just being added // in that case do not set default value to prevent flashing #14499 @@ -230,7 +230,7 @@ export class Expression extends ExpressionContainer implements IExpression { this.reference = response.body.variablesReference; this.namedVariables = response.body.namedVariables; this.indexedVariables = response.body.indexedVariables; - this.type = response.body.type; + this.type = response.body.type || this.type; } }, err => { this.value = err.message; @@ -250,15 +250,15 @@ export class Variable extends ExpressionContainer implements IExpression { public errorMessage: string; constructor( - session: IDebugSession, + session: IDebugSession | undefined, public parent: IExpressionContainer, - reference: number, + reference: number | undefined, public name: string, - public evaluateName: string, + public evaluateName: string | undefined, value: string, - namedVariables: number, - indexedVariables: number, - public presentationHint: DebugProtocol.VariablePresentationHint, + namedVariables: number | undefined, + indexedVariables: number | undefined, + public presentationHint: DebugProtocol.VariablePresentationHint | undefined, public type: string | undefined = undefined, public available = true, startOfVariables = 0 @@ -268,6 +268,10 @@ export class Variable extends ExpressionContainer implements IExpression { } setVariable(value: string): Promise { + if (!this.session) { + return Promise.resolve(undefined); + } + return this.session.setVariable((this.parent).reference, this.name, value).then(response => { if (response && response.body) { this.value = response.body.value; @@ -294,8 +298,8 @@ export class Scope extends ExpressionContainer implements IScope { public name: string, reference: number, public expensive: boolean, - namedVariables: number, - indexedVariables: number, + namedVariables?: number, + indexedVariables?: number, public range?: IRange ) { super(stackFrame.thread.session, reference, `scope:${stackFrame.getId()}:${name}:${index}`, namedVariables, indexedVariables); @@ -315,7 +319,7 @@ export class StackFrame implements IStackFrame { public frameId: number, public source: Source, public name: string, - public presentationHint: string, + public presentationHint: string | undefined, public range: IRange, private index: number ) { @@ -331,7 +335,7 @@ export class StackFrame implements IStackFrame { this.scopes = this.thread.session.scopes(this.frameId).then(response => { return response && response.body && response.body.scopes ? response.body.scopes.map((rs, index) => new Scope(this, index, rs.name, rs.variablesReference, rs.expensive, rs.namedVariables, rs.indexedVariables, - rs.line && rs.column && rs.endLine && rs.endColumn ? new Range(rs.line, rs.column, rs.endLine, rs.endColumn) : null)) : []; + rs.line && rs.column && rs.endLine && rs.endColumn ? new Range(rs.line, rs.column, rs.endLine, rs.endColumn) : undefined)) : []; }, err => []); } @@ -393,7 +397,6 @@ export class Thread implements IThread { public stopped: boolean; constructor(public session: IDebugSession, public name: string, public threadId: number) { - this.stoppedDetails = null; this.callStack = []; this.staleCallStack = []; this.stopped = false; @@ -465,8 +468,8 @@ export class Thread implements IThread { return new StackFrame(this, rsf.id, source, rsf.name, rsf.presentationHint, new Range( rsf.line, rsf.column, - rsf.endLine, - rsf.endColumn + rsf.endLine || rsf.line, + rsf.endColumn || rsf.column ), startFrame + index); }); }, (err: Error) => { @@ -545,9 +548,9 @@ export class BaseBreakpoint extends Enablement implements IBaseBreakpoint { constructor( enabled: boolean, - public hitCondition: string, - public condition: string, - public logMessage: string, + public hitCondition: string | undefined, + public condition: string | undefined, + public logMessage: string | undefined, id: string ) { super(enabled, id); @@ -594,11 +597,11 @@ export class Breakpoint extends BaseBreakpoint implements IBreakpoint { constructor( public uri: uri, private _lineNumber: number, - private _column: number, + private _column: number | undefined, enabled: boolean, - condition: string, - hitCondition: string, - logMessage: string, + condition: string | undefined, + hitCondition: string | undefined, + logMessage: string | undefined, private _adapterData: any, private textFileService: ITextFileService, id = generateUuid() @@ -620,7 +623,7 @@ export class Breakpoint extends BaseBreakpoint implements IBreakpoint { return true; } - get column(): number { + get column(): number | undefined { const data = this.getSessionData(); // Only respect the column if the user explictly set the column to have an inline breakpoint return data && typeof data.column === 'number' && typeof this._column === 'number' ? data.column : this._column; @@ -698,9 +701,9 @@ export class FunctionBreakpoint extends BaseBreakpoint implements IFunctionBreak constructor( public name: string, enabled: boolean, - hitCondition: string, - condition: string, - logMessage: string, + hitCondition: string | undefined, + condition: string | undefined, + logMessage: string | undefined, id = generateUuid() ) { super(enabled, hitCondition, condition, logMessage, id); @@ -798,7 +801,7 @@ export class DebugModel implements IDebugModel { this._onDidChangeCallStack.fire(undefined); } - get onDidChangeBreakpoints(): Event { + get onDidChangeBreakpoints(): Event { return this._onDidChangeBreakpoints.event; } @@ -832,7 +835,7 @@ export class DebugModel implements IDebugModel { fetchCallStack(thread: Thread): { topCallStack: Promise, wholeCallStack: Promise } { if (thread.session.capabilities.supportsDelayedStackTraceLoading) { // For improved performance load the first stack frame and then load the rest async. - let topCallStack: Promise; + let topCallStack = Promise.resolve(); const wholeCallStack = new Promise((c, e) => { topCallStack = thread.fetchCallStack(1).then(() => { if (!this.schedulers.has(thread.getId())) { @@ -913,7 +916,7 @@ export class DebugModel implements IDebugModel { } addBreakpoints(uri: uri, rawData: IBreakpointData[], fireEvent = true): IBreakpoint[] { - const newBreakpoints = rawData.map(rawBp => new Breakpoint(uri, rawBp.lineNumber, rawBp.column, rawBp.enabled, rawBp.condition, rawBp.hitCondition, rawBp.logMessage, undefined, this.textFileService, rawBp.id)); + const newBreakpoints = rawData.map(rawBp => new Breakpoint(uri, rawBp.lineNumber, rawBp.column, !!rawBp.enabled, rawBp.condition, rawBp.hitCondition, rawBp.logMessage, undefined, this.textFileService, rawBp.id)); newBreakpoints.forEach(bp => bp.setSessionId(this.breakpointsSessionId)); this.breakpoints = this.breakpoints.concat(newBreakpoints); this.breakpointsActivated = true; @@ -979,7 +982,10 @@ export class DebugModel implements IDebugModel { return resources.basenameOrAuthority(first.uri).localeCompare(resources.basenameOrAuthority(second.uri)); } if (first.lineNumber === second.lineNumber) { - return first.column - second.column; + if (first.column && second.column) { + return first.column - second.column; + } + return -1; } return first.lineNumber - second.lineNumber; @@ -1037,7 +1043,7 @@ export class DebugModel implements IDebugModel { removeFunctionBreakpoints(id?: string): void { - let removed: IFunctionBreakpoint[]; + let removed: FunctionBreakpoint[]; if (id) { removed = this.functionBreakpoints.filter(fbp => fbp.getId() === id); this.functionBreakpoints = this.functionBreakpoints.filter(fbp => fbp.getId() !== id); @@ -1075,10 +1081,11 @@ export class DebugModel implements IDebugModel { moveWatchExpression(id: string, position: number): void { const we = this.watchExpressions.filter(we => we.getId() === id).pop(); - this.watchExpressions = this.watchExpressions.filter(we => we.getId() !== id); - this.watchExpressions = this.watchExpressions.slice(0, position).concat(we, this.watchExpressions.slice(position)); - - this._onDidChangeWatchExpressions.fire(undefined); + if (we) { + this.watchExpressions = this.watchExpressions.filter(we => we.getId() !== id); + this.watchExpressions = this.watchExpressions.slice(0, position).concat(we, this.watchExpressions.slice(position)); + this._onDidChangeWatchExpressions.fire(undefined); + } } sourceIsNotAvailable(uri: uri): void { diff --git a/src/vs/workbench/contrib/debug/common/debugSource.ts b/src/vs/workbench/contrib/debug/common/debugSource.ts index 8efc84301ea..d9b9e1d1a6d 100644 --- a/src/vs/workbench/contrib/debug/common/debugSource.ts +++ b/src/vs/workbench/contrib/debug/common/debugSource.ts @@ -33,10 +33,12 @@ export class Source { public readonly uri: uri; public available: boolean; + public raw: DebugProtocol.Source; - constructor(public raw: DebugProtocol.Source, sessionId: string) { + constructor(raw_: DebugProtocol.Source | undefined, sessionId: string) { let path: string; - if (raw) { + if (raw_) { + this.raw = raw_; path = this.raw.path || this.raw.name || ''; this.available = true; } else { diff --git a/src/vs/workbench/contrib/debug/common/replModel.ts b/src/vs/workbench/contrib/debug/common/replModel.ts index c4556ec3eca..e8c1cd2580f 100644 --- a/src/vs/workbench/contrib/debug/common/replModel.ts +++ b/src/vs/workbench/contrib/debug/common/replModel.ts @@ -46,7 +46,7 @@ export class ReplModel { // remove potential empty lines between different repl types this.replElements.pop(); } else if (previousElement instanceof SimpleReplElement && sev === previousElement.severity && toAdd.length && toAdd[0].sourceData === previousElement.sourceData) { - previousElement.value += toAdd.shift().value; + previousElement.value += toAdd.shift()!.value; } this.addReplElements(toAdd); } else { diff --git a/src/vs/workbench/contrib/debug/electron-browser/debugSession.ts b/src/vs/workbench/contrib/debug/electron-browser/debugSession.ts index 3582ba6a3f1..cd82b55507d 100644 --- a/src/vs/workbench/contrib/debug/electron-browser/debugSession.ts +++ b/src/vs/workbench/contrib/debug/electron-browser/debugSession.ts @@ -785,7 +785,7 @@ export class DebugSession implements IDebugSession { return this.sources.get(this.getUriKey(uri)); } - getSource(raw: DebugProtocol.Source): Source { + getSource(raw?: DebugProtocol.Source): Source { let source = new Source(raw, this.getId()); const uriKey = this.getUriKey(source.uri); const found = this.sources.get(uriKey); From 2f62d285480cd40d66462d8c290c08de73ed5f73 Mon Sep 17 00:00:00 2001 From: isidor Date: Wed, 6 Mar 2019 17:30:38 +0100 Subject: [PATCH 4/9] debug/terminals.ts strict null checks --- src/tsconfig.strictNullChecks.json | 3 ++- .../workbench/contrib/debug/node/terminals.ts | 19 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index 4ac3a4bf562..9ae7f712530 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -517,7 +517,8 @@ "./vs/workbench/test/electron-browser/api/mainThreadDiagnostics.test.ts", "./vs/workbench/test/electron-browser/api/mainThreadDocumentContentProviders.test.ts", "./vs/workbench/test/electron-browser/api/mock.ts", - "./vs/workbench/test/electron-browser/api/testRPCProtocol.ts" + "./vs/workbench/test/electron-browser/api/testRPCProtocol.ts", + "./vs/workbench/contrib/debug/node/terminals.ts" ], "exclude": [ "./typings/require-monaco.d.ts", diff --git a/src/vs/workbench/contrib/debug/node/terminals.ts b/src/vs/workbench/contrib/debug/node/terminals.ts index f399b6bb8d9..3bb371f4b5b 100644 --- a/src/vs/workbench/contrib/debug/node/terminals.ts +++ b/src/vs/workbench/contrib/debug/node/terminals.ts @@ -33,7 +33,7 @@ export function getDefaultTerminalLinuxReady(): Promise { if (!_DEFAULT_TERMINAL_LINUX_READY) { _DEFAULT_TERMINAL_LINUX_READY = new Promise(c => { if (env.isLinux) { - Promise.all([pfs.exists('/etc/debian_version'), process.lazyEnv]).then(([isDebian]) => { + Promise.all([pfs.exists('/etc/debian_version'), process.lazyEnv]).then(([isDebian]) => { if (isDebian) { c('x-terminal-emulator'); } else if (process.env.DESKTOP_SESSION === 'gnome' || process.env.DESKTOP_SESSION === 'gnome-classic') { @@ -67,19 +67,18 @@ export function getDefaultTerminalWindows(): string { } abstract class TerminalLauncher implements ITerminalLauncher { - public runInTerminal(args: DebugProtocol.RunInTerminalRequestArguments, config: ITerminalSettings): Promise { - return this.runInTerminal0(args.title, args.cwd, args.args, args.env || {}, config); - } - runInTerminal0(title: string, dir: string, args: string[], envVars: env.IProcessEnvironment, config): Promise { - return undefined; + runInTerminal(args: DebugProtocol.RunInTerminalRequestArguments, config: ITerminalSettings): Promise { + return this.runInTerminal0(args.title!, args.cwd, args.args, args.env || {}, config); } + + abstract runInTerminal0(title: string, dir: string, args: string[], envVars: env.IProcessEnvironment | {}, config): Promise; } class WinTerminalService extends TerminalLauncher { private static readonly CMD = 'cmd.exe'; - public runInTerminal0(title: string, dir: string, args: string[], envVars: env.IProcessEnvironment, configuration: ITerminalSettings): Promise { + runInTerminal0(title: string, dir: string, args: string[], envVars: env.IProcessEnvironment, configuration: ITerminalSettings): Promise { const exec = configuration.external.windowsExec || getDefaultTerminalWindows(); @@ -117,7 +116,7 @@ class MacTerminalService extends TerminalLauncher { private static readonly DEFAULT_TERMINAL_OSX = 'Terminal.app'; private static readonly OSASCRIPT = '/usr/bin/osascript'; // osascript is the AppleScript interpreter on OS X - public runInTerminal0(title: string, dir: string, args: string[], envVars: env.IProcessEnvironment, configuration: ITerminalSettings): Promise { + runInTerminal0(title: string, dir: string, args: string[], envVars: env.IProcessEnvironment, configuration: ITerminalSettings): Promise { const terminalApp = configuration.external.osxExec || MacTerminalService.DEFAULT_TERMINAL_OSX; @@ -184,7 +183,7 @@ class LinuxTerminalService extends TerminalLauncher { private static readonly WAIT_MESSAGE = nls.localize('press.any.key', "Press any key to continue..."); - public runInTerminal0(title: string, dir: string, args: string[], envVars: env.IProcessEnvironment, configuration: ITerminalSettings): Promise { + runInTerminal0(title: string, dir: string, args: string[], envVars: env.IProcessEnvironment, configuration: ITerminalSettings): Promise { const terminalConfig = configuration.external; const execThenable: Promise = terminalConfig.linuxExec ? Promise.resolve(terminalConfig.linuxExec) : getDefaultTerminalLinuxReady(); @@ -348,7 +347,7 @@ export function prepareCommand(args: DebugProtocol.RunInTerminalRequestArguments } } if (args.args && args.args.length > 0) { - const cmd = quote(args.args.shift()); + const cmd = quote(args.args.shift()!); command += (cmd[0] === '\'') ? `& ${cmd} ` : `${cmd} `; for (let a of args.args) { command += `${quote(a)} `; From 84b046ce10eb3fc3e1afcf4a227f53c79a3549d5 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 6 Mar 2019 18:33:11 +0100 Subject: [PATCH 5/9] debt - introduce ILayoutService This allows to declare more services declaratively. --- .../standalone/browser/simpleServices.ts | 16 ++++ .../standalone/browser/standaloneServices.ts | 7 +- .../contextview/browser/contextViewService.ts | 7 +- .../platform/layout/browser/layoutService.ts | 30 ++++++ src/vs/workbench/browser/legacyLayout.ts | 18 ---- .../browser/parts/quickinput/quickInput.ts | 14 ++- .../parts/quickopen/quickOpenController.ts | 16 ++-- .../workbench/electron-browser/workbench.ts | 94 +++++++------------ .../services/history/browser/history.ts | 3 + src/vs/workbench/workbench.main.ts | 3 + 10 files changed, 113 insertions(+), 95 deletions(-) create mode 100644 src/vs/platform/layout/browser/layoutService.ts diff --git a/src/vs/editor/standalone/browser/simpleServices.ts b/src/vs/editor/standalone/browser/simpleServices.ts index c76b38b22df..ab55ea98971 100644 --- a/src/vs/editor/standalone/browser/simpleServices.ts +++ b/src/vs/editor/standalone/browser/simpleServices.ts @@ -43,6 +43,7 @@ import { ITelemetryInfo, ITelemetryService } from 'vs/platform/telemetry/common/ import { IWorkspace, IWorkspaceContextService, IWorkspaceFolder, IWorkspaceFoldersChangeEvent, WorkbenchState, WorkspaceFolder } from 'vs/platform/workspace/common/workspace'; import { ISingleFolderWorkspaceIdentifier, IWorkspaceIdentifier } from 'vs/platform/workspaces/common/workspaces'; import { IAccessibilityService, AccessibilitySupport } from 'vs/platform/accessibility/common/accessibility'; +import { ILayoutService, IDimension } from 'vs/platform/layout/browser/layoutService'; export class SimpleModel implements IResolvedTextEditorModel { @@ -671,3 +672,18 @@ export class SimpleUriLabelService implements ILabelService { return ''; } } + +export class SimpleLayoutService implements ILayoutService { + _serviceBrand: any; + + public onLayout = Event.None; + + private _dimension: IDimension; + get dimension(): IDimension { + if (!this._dimension) { + this._dimension = dom.getClientArea(window.document.body); + } + + return this._dimension; + } +} diff --git a/src/vs/editor/standalone/browser/standaloneServices.ts b/src/vs/editor/standalone/browser/standaloneServices.ts index df278c89b82..1d0568208d0 100644 --- a/src/vs/editor/standalone/browser/standaloneServices.ts +++ b/src/vs/editor/standalone/browser/standaloneServices.ts @@ -13,7 +13,7 @@ import { ModeServiceImpl } from 'vs/editor/common/services/modeServiceImpl'; import { IModelService } from 'vs/editor/common/services/modelService'; import { ModelServiceImpl } from 'vs/editor/common/services/modelServiceImpl'; import { ITextResourceConfigurationService, ITextResourcePropertiesService } from 'vs/editor/common/services/resourceConfiguration'; -import { SimpleBulkEditService, SimpleConfigurationService, SimpleDialogService, SimpleNotificationService, SimpleProgressService, SimpleResourceConfigurationService, SimpleResourcePropertiesService, SimpleUriLabelService, SimpleWorkspaceContextService, StandaloneCommandService, StandaloneKeybindingService, StandaloneTelemetryService, BrowserAccessibilityService } from 'vs/editor/standalone/browser/simpleServices'; +import { SimpleBulkEditService, SimpleConfigurationService, SimpleDialogService, SimpleNotificationService, SimpleProgressService, SimpleResourceConfigurationService, SimpleResourcePropertiesService, SimpleUriLabelService, SimpleWorkspaceContextService, StandaloneCommandService, StandaloneKeybindingService, StandaloneTelemetryService, BrowserAccessibilityService, SimpleLayoutService } from 'vs/editor/standalone/browser/simpleServices'; import { StandaloneCodeEditorServiceImpl } from 'vs/editor/standalone/browser/standaloneCodeServiceImpl'; import { StandaloneThemeServiceImpl } from 'vs/editor/standalone/browser/standaloneThemeServiceImpl'; import { IStandaloneThemeService } from 'vs/editor/standalone/common/standaloneThemeService'; @@ -46,6 +46,7 @@ import { IMarkerDecorationsService } from 'vs/editor/common/services/markersDeco import { MarkerDecorationsService } from 'vs/editor/common/services/markerDecorationsServiceImpl'; import { ISuggestMemoryService, SuggestMemoryService } from 'vs/editor/contrib/suggest/suggestMemory'; import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility'; +import { ILayoutService } from 'vs/platform/layout/browser/layoutService'; export interface IEditorOverrideServices { [index: string]: any; @@ -196,7 +197,9 @@ export class DynamicStandaloneServices extends Disposable { let keybindingService = ensure(IKeybindingService, () => this._register(new StandaloneKeybindingService(contextKeyService, commandService, telemetryService, notificationService, domElement))); - let contextViewService = ensure(IContextViewService, () => this._register(new ContextViewService(domElement, telemetryService, new NullLogService()))); + let layoutService = ensure(ILayoutService, () => new SimpleLayoutService()); + + let contextViewService = ensure(IContextViewService, () => this._register(new ContextViewService(domElement, telemetryService, new NullLogService(), layoutService))); ensure(IContextMenuService, () => this._register(new ContextMenuService(domElement, telemetryService, notificationService, contextViewService, keybindingService, themeService))); diff --git a/src/vs/platform/contextview/browser/contextViewService.ts b/src/vs/platform/contextview/browser/contextViewService.ts index 48b0f51eb86..0bcada24ea8 100644 --- a/src/vs/platform/contextview/browser/contextViewService.ts +++ b/src/vs/platform/contextview/browser/contextViewService.ts @@ -8,6 +8,7 @@ import { ContextView } from 'vs/base/browser/ui/contextview/contextview'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { ILogService } from 'vs/platform/log/common/log'; import { Disposable } from 'vs/base/common/lifecycle'; +import { ILayoutService } from 'vs/platform/layout/browser/layoutService'; export class ContextViewService extends Disposable implements IContextViewService { _serviceBrand: any; @@ -17,11 +18,15 @@ export class ContextViewService extends Disposable implements IContextViewServic constructor( container: HTMLElement, @ITelemetryService telemetryService: ITelemetryService, - @ILogService private readonly logService: ILogService + @ILogService private readonly logService: ILogService, + @ILayoutService readonly layoutService: ILayoutService ) { super(); this.contextView = this._register(new ContextView(container)); + this.layout(); + + this._register(layoutService.onLayout(() => this.layout())); } // ContextView diff --git a/src/vs/platform/layout/browser/layoutService.ts b/src/vs/platform/layout/browser/layoutService.ts new file mode 100644 index 00000000000..d9cc51f1a2d --- /dev/null +++ b/src/vs/platform/layout/browser/layoutService.ts @@ -0,0 +1,30 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Event } from 'vs/base/common/event'; +import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; + +export const ILayoutService = createDecorator('layoutService'); + +export interface IDimension { + width: number; + height: number; +} + +export interface ILayoutService { + + _serviceBrand: any; + + /** + * The dimensions of the container. + */ + readonly dimension: IDimension; + + /** + * An event that is emitted when the container is layed out. The + * event carries the dimensions of the container as part of it. + */ + readonly onLayout: Event; +} \ No newline at end of file diff --git a/src/vs/workbench/browser/legacyLayout.ts b/src/vs/workbench/browser/legacyLayout.ts index e47480d7e9f..24b179fe2ad 100644 --- a/src/vs/workbench/browser/legacyLayout.ts +++ b/src/vs/workbench/browser/legacyLayout.ts @@ -3,8 +3,6 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { QuickOpenController } from 'vs/workbench/browser/parts/quickopen/quickOpenController'; -import { QuickInputService } from 'vs/workbench/browser/parts/quickinput/quickInput'; import { Sash, ISashEvent, IVerticalSashLayoutProvider, IHorizontalSashLayoutProvider, Orientation } from 'vs/base/browser/ui/sash/sash'; import { IPartService, Position, ILayoutOptions, Parts } from 'vs/workbench/services/part/common/partService'; import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet'; @@ -14,8 +12,6 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { IThemeService } from 'vs/platform/theme/common/themeService'; import { isMacintosh } from 'vs/base/common/platform'; import { memoize } from 'vs/base/common/decorators'; -import { NotificationsCenter } from 'vs/workbench/browser/parts/notifications/notificationsCenter'; -import { NotificationsToasts } from 'vs/workbench/browser/parts/notifications/notificationsToasts'; import { Dimension, getClientArea, size, position, hide, show } from 'vs/base/browser/dom'; import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; import { EditorPart } from 'vs/workbench/browser/parts/editor/editorPart'; @@ -78,10 +74,6 @@ export class WorkbenchLegacyLayout extends Disposable implements IVerticalSashLa panel: PanelPart, statusbar: StatusbarPart }, - private quickopen: QuickOpenController, - private quickInput: QuickInputService, - private notificationsCenter: NotificationsCenter, - private notificationsToasts: NotificationsToasts, @IStorageService private readonly storageService: IStorageService, @IContextViewService private readonly contextViewService: IContextViewService, @IPartService private readonly partService: IPartService, @@ -626,16 +618,6 @@ export class WorkbenchLegacyLayout extends Disposable implements IVerticalSashLa show(statusbarContainer); } - // Quick open - this.quickopen.layout(this.workbenchSize); - - // Quick input - this.quickInput.layout(this.workbenchSize); - - // Notifications - this.notificationsCenter.layout(this.workbenchSize); - this.notificationsToasts.layout(this.workbenchSize); - // Sashes this.sashXOne.layout(); if (panelPosition === Position.BOTTOM) { diff --git a/src/vs/workbench/browser/parts/quickinput/quickInput.ts b/src/vs/workbench/browser/parts/quickinput/quickInput.ts index 8860f8046b7..7fa1be9a216 100644 --- a/src/vs/workbench/browser/parts/quickinput/quickInput.ts +++ b/src/vs/workbench/browser/parts/quickinput/quickInput.ts @@ -43,6 +43,8 @@ import { getIconClass } from 'vs/workbench/browser/parts/quickinput/quickInputUt import { IEditorOptions } from 'vs/editor/common/config/editorOptions'; import { IStorageService } from 'vs/platform/storage/common/storage'; import { IAccessibilityService, AccessibilitySupport } from 'vs/platform/accessibility/common/accessibility'; +import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; +import { ILayoutService } from 'vs/platform/layout/browser/layoutService'; const $ = dom.$; @@ -816,7 +818,6 @@ export class QuickInputService extends Component implements IQuickInputService { private static readonly MAX_WIDTH = 600; // Max total width of quick open widget private idPrefix = 'quickInput_'; // Constant since there is still only one. - private layoutDimensions: dom.Dimension; private titleBar: HTMLElement; private filterContainer: HTMLElement; private visibleCountContainer: HTMLElement; @@ -846,12 +847,14 @@ export class QuickInputService extends Component implements IQuickInputService { @IContextKeyService private readonly contextKeyService: IContextKeyService, @IThemeService themeService: IThemeService, @IStorageService storageService: IStorageService, - @IAccessibilityService private readonly accessibilityService: IAccessibilityService + @IAccessibilityService private readonly accessibilityService: IAccessibilityService, + @ILayoutService private readonly layoutService: ILayoutService ) { super(QuickInputService.ID, themeService, storageService); this.inQuickOpenContext = InQuickOpenContextKey.bindTo(contextKeyService); this._register(this.quickOpenService.onShow(() => this.inQuickOpen('quickOpen', true))); this._register(this.quickOpenService.onHide(() => this.inQuickOpen('quickOpen', false))); + this._register(this.layoutService.onLayout(dimension => this.layout(dimension))); this.registerKeyModsListeners(); } @@ -1400,17 +1403,16 @@ export class QuickInputService extends Component implements IQuickInputService { } layout(dimension: dom.Dimension): void { - this.layoutDimensions = dimension; this.updateLayout(); } private updateLayout() { - if (this.layoutDimensions && this.ui) { + if (this.ui) { const titlebarOffset = this.partService.getTitleBarOffset(); this.ui.container.style.top = `${titlebarOffset}px`; const style = this.ui.container.style; - const width = Math.min(this.layoutDimensions.width * 0.62 /* golden cut */, QuickInputService.MAX_WIDTH); + const width = Math.min(this.layoutService.dimension.width * 0.62 /* golden cut */, QuickInputService.MAX_WIDTH); style.width = width + 'px'; style.marginLeft = '-' + (width / 2) + 'px'; @@ -1467,3 +1469,5 @@ export class BackAction extends Action { return Promise.resolve(); } } + +registerSingleton(IQuickInputService, QuickInputService, true); \ No newline at end of file diff --git a/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts b/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts index 3eb74651206..0e4c31da45f 100644 --- a/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts +++ b/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts @@ -50,6 +50,8 @@ import { timeout } from 'vs/base/common/async'; import { IQuickInputService, IQuickPickItem } from 'vs/platform/quickinput/common/quickInput'; import { CancellationTokenSource, CancellationToken } from 'vs/base/common/cancellation'; import { IStorageService } from 'vs/platform/storage/common/storage'; +import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; +import { ILayoutService } from 'vs/platform/layout/browser/layoutService'; const HELP_PREFIX = '?'; @@ -73,7 +75,6 @@ export class QuickOpenController extends Component implements IQuickOpenService private lastInputValue: string; private lastSubmittedInputValue: string; private quickOpenWidget: QuickOpenWidget; - private dimension: Dimension; private mapResolvedHandlersToPrefix: { [prefix: string]: Promise; } = Object.create(null); private mapContextKeyToContext: { [id: string]: IContextKey; } = Object.create(null); private handlerOnOpenCalled: { [prefix: string]: boolean; } = Object.create(null); @@ -94,7 +95,8 @@ export class QuickOpenController extends Component implements IQuickOpenService @IPartService private readonly partService: IPartService, @IEnvironmentService private readonly environmentService: IEnvironmentService, @IThemeService themeService: IThemeService, - @IStorageService storageService: IStorageService + @IStorageService storageService: IStorageService, + @ILayoutService private readonly layoutService: ILayoutService ) { super(QuickOpenController.ID, themeService, storageService); @@ -109,6 +111,7 @@ export class QuickOpenController extends Component implements IQuickOpenService this._register(this.configurationService.onDidChangeConfiguration(() => this.updateConfiguration())); this._register(this.partService.onTitleBarVisibilityChange(() => this.positionQuickOpenWidget())); this._register(browser.onDidChangeZoomLevel(() => this.positionQuickOpenWidget())); + this._register(this.layoutService.onLayout(dimension => this.layout(dimension))); } private updateConfiguration(): void { @@ -195,9 +198,7 @@ export class QuickOpenController extends Component implements IQuickOpenService } // Layout - if (this.dimension) { - this.quickOpenWidget.layout(this.dimension); - } + this.quickOpenWidget.layout(this.layoutService.dimension); // Show quick open with prefix or editor history if (!this.quickOpenWidget.isVisible() || quickNavigateConfiguration) { @@ -624,9 +625,8 @@ export class QuickOpenController extends Component implements IQuickOpenService } layout(dimension: Dimension): void { - this.dimension = dimension; if (this.quickOpenWidget) { - this.quickOpenWidget.layout(this.dimension); + this.quickOpenWidget.layout(dimension); } } } @@ -863,3 +863,5 @@ export class RemoveFromEditorHistoryAction extends Action { }); } } + +registerSingleton(IQuickOpenService, QuickOpenController, true); \ No newline at end of file diff --git a/src/vs/workbench/electron-browser/workbench.ts b/src/vs/workbench/electron-browser/workbench.ts index 923b1bd1575..a28f951ca61 100644 --- a/src/vs/workbench/electron-browser/workbench.ts +++ b/src/vs/workbench/electron-browser/workbench.ts @@ -29,9 +29,6 @@ import { EditorPart } from 'vs/workbench/browser/parts/editor/editorPart'; import { IActionBarRegistry, Extensions as ActionBarExtensions } from 'vs/workbench/browser/actions'; import { PanelRegistry, Extensions as PanelExtensions } from 'vs/workbench/browser/panel'; import { ViewletRegistry, Extensions as ViewletExtensions } from 'vs/workbench/browser/viewlet'; -import { QuickOpenController } from 'vs/workbench/browser/parts/quickopen/quickOpenController'; -import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput'; -import { QuickInputService } from 'vs/workbench/browser/parts/quickinput/quickInput'; import { getServices } from 'vs/platform/instantiation/common/extensions'; import { Position, Parts, IPartService, ILayoutOptions } from 'vs/workbench/services/part/common/partService'; import { IWorkspaceContextService, WorkbenchState } from 'vs/platform/workspace/common/workspace'; @@ -46,9 +43,7 @@ import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet'; import { IFileService } from 'vs/platform/files/common/files'; import { IPanelService } from 'vs/workbench/services/panel/common/panelService'; import { ITitleService } from 'vs/workbench/services/title/common/titleService'; -import { IQuickOpenService } from 'vs/platform/quickOpen/common/quickOpen'; -import { IHistoryService } from 'vs/workbench/services/history/common/history'; -import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors'; import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; import { LifecyclePhase, StartupKind, ILifecycleService, WillShutdownEvent } from 'vs/platform/lifecycle/common/lifecycle'; @@ -97,12 +92,11 @@ import { IModelService } from 'vs/editor/common/services/modelService'; import { ModelServiceImpl } from 'vs/editor/common/services/modelServiceImpl'; import { IUntitledEditorService, UntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService'; import { ILocalizationsService } from 'vs/platform/localizations/common/localizations'; -import { HistoryService } from 'vs/workbench/services/history/browser/history'; import { WorkbenchThemeService } from 'vs/workbench/services/themes/browser/workbenchThemeService'; import { IProductService } from 'vs/platform/product/common/product'; import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility'; import { WorkbenchContextKeysHandler } from 'vs/workbench/browser/contextkeys'; -import { ServicesAccessor } from 'vs/editor/browser/editorExtensions'; +import { ILayoutService, IDimension } from 'vs/platform/layout/browser/layoutService'; // import@node import { getDelayedChannel } from 'vs/base/parts/ipc/node/ipc'; @@ -203,12 +197,6 @@ export class Workbench extends Disposable implements IPartService { private editorPart: EditorPart; private statusbarPart: StatusbarPart; - private quickOpen: QuickOpenController; - private quickInput: QuickInputService; - - private notificationsCenter: NotificationsCenter; - private notificationsToasts: NotificationsToasts; - constructor( private container: HTMLElement, private configuration: IWindowConfiguration, @@ -358,6 +346,7 @@ export class Workbench extends Disposable implements IPartService { // Parts serviceCollection.set(IPartService, this); // TODO@Ben use SyncDescriptor + serviceCollection.set(ILayoutService, this); // TODO@Ben use SyncDescriptor // Labels serviceCollection.set(ILabelService, new SyncDescriptor(LabelService, undefined, true)); @@ -502,17 +491,6 @@ export class Workbench extends Disposable implements IPartService { this.titlebarPart = this.instantiationService.createInstance(TitlebarPart, Identifiers.TITLEBAR_PART); serviceCollection.set(ITitleService, this.titlebarPart); // TODO@Ben use SyncDescriptor - // History - serviceCollection.set(IHistoryService, new SyncDescriptor(HistoryService)); - - // Quick open service (quick open controller) - this.quickOpen = this.instantiationService.createInstance(QuickOpenController); - serviceCollection.set(IQuickOpenService, this.quickOpen); // TODO@Ben use SyncDescriptor - - // Quick input service - this.quickInput = this.instantiationService.createInstance(QuickInputService); - serviceCollection.set(IQuickInputService, this.quickInput); // TODO@Ben use SyncDescriptor - // Contributed services const contributedServices = getServices(); for (let contributedService of contributedServices) { @@ -692,30 +670,26 @@ export class Workbench extends Disposable implements IPartService { private createNotificationsHandlers(accessor: ServicesAccessor): void { const notificationService = accessor.get(INotificationService) as NotificationService; - // Notifications Center - this.notificationsCenter = this._register(this.instantiationService.createInstance(NotificationsCenter, this.workbench, notificationService.model)); - - // Notifications Toasts - this.notificationsToasts = this._register(this.instantiationService.createInstance(NotificationsToasts, this.workbench, notificationService.model)); - - // Notifications Alerts + // Instantiate Notification components + const notificationsCenter = this._register(this.instantiationService.createInstance(NotificationsCenter, this.workbench, notificationService.model)); + const notificationsToasts = this._register(this.instantiationService.createInstance(NotificationsToasts, this.workbench, notificationService.model)); this._register(this.instantiationService.createInstance(NotificationsAlerts, notificationService.model)); - - // Notifications Status const notificationsStatus = this.instantiationService.createInstance(NotificationsStatus, notificationService.model); - // Eventing - this._register(this.notificationsCenter.onDidChangeVisibility(() => { + // Visibility + this._register(notificationsCenter.onDidChangeVisibility(() => { + notificationsStatus.update(notificationsCenter.isVisible); + notificationsToasts.update(notificationsCenter.isVisible); + })); - // Update status - notificationsStatus.update(this.notificationsCenter.isVisible); - - // Update toasts - this.notificationsToasts.update(this.notificationsCenter.isVisible); + // Layout + this._register(this.onLayout(dimension => { + notificationsCenter.layout(dimension); + notificationsToasts.layout(dimension); })); // Register Commands - registerNotificationCommands(this.notificationsCenter, this.notificationsToasts); + registerNotificationCommands(notificationsCenter, notificationsToasts); } private restoreWorkbench(): Promise { @@ -828,6 +802,12 @@ export class Workbench extends Disposable implements IPartService { private readonly _onZenMode: Emitter = this._register(new Emitter()); get onZenModeChange(): Event { return this._onZenMode.event; } + private readonly _onLayout = this._register(new Emitter()); + get onLayout(): Event { return this._onLayout.event; } + + private _dimension: IDimension; + get dimension(): IDimension { return this._dimension; } + private workbenchGrid: Grid | WorkbenchLegacyLayout; private titleBarPartView: View; @@ -837,7 +817,6 @@ export class Workbench extends Disposable implements IPartService { private editorPartView: View; private statusBarPartView: View; - private contextViewService: IContextViewService; private windowService: IWindowService; private readonly state = { @@ -1035,7 +1014,6 @@ export class Workbench extends Disposable implements IPartService { const environmentService = accessor.get(IEnvironmentService); this.windowService = accessor.get(IWindowService); - this.contextViewService = accessor.get(IContextViewService); // Fullscreen this.state.fullscreen = isFullscreen(); @@ -1406,38 +1384,30 @@ export class Workbench extends Disposable implements IPartService { sidebar: this.sidebarPart, panel: this.panelPart, statusbar: this.statusbarPart, - }, - this.quickOpen, - this.quickInput, - this.notificationsCenter, - this.notificationsToasts + } ); } } layout(options?: ILayoutOptions): void { - this.contextViewService.layout(); - if (!this.disposed) { + this._dimension = getClientArea(this.container); + if (this.workbenchGrid instanceof Grid) { - const dimensions = getClientArea(this.container); position(this.workbench, 0, 0, 0, 0, 'relative'); - size(this.workbench, dimensions.width, dimensions.height); + size(this.workbench, this._dimension.width, this._dimension.height); - // Layout the grid - this.workbenchGrid.layout(dimensions.width, dimensions.height); + // Layout the grid widget + this.workbenchGrid.layout(this._dimension.width, this._dimension.height); - // Layout non-view ui components - this.quickInput.layout(dimensions); - this.quickOpen.layout(dimensions); - this.notificationsCenter.layout(dimensions); - this.notificationsToasts.layout(dimensions); - - // Layout Grid + // Layout grid views this.layoutGrid(); } else { this.workbenchGrid.layout(options); } + + // Emit as event + this._onLayout.fire(this._dimension); } } diff --git a/src/vs/workbench/services/history/browser/history.ts b/src/vs/workbench/services/history/browser/history.ts index 7f52cda2121..11c83492a74 100644 --- a/src/vs/workbench/services/history/browser/history.ts +++ b/src/vs/workbench/services/history/browser/history.ts @@ -30,6 +30,7 @@ import { EditorServiceImpl } from 'vs/workbench/browser/parts/editor/editor'; import { IPartService } from 'vs/workbench/services/part/common/partService'; import { IContextKeyService, RawContextKey, IContextKey } from 'vs/platform/contextkey/common/contextkey'; import { coalesce } from 'vs/base/common/arrays'; +import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; /** * Stores the selection & view state of an editor and allows to compare it to other selection states. @@ -971,3 +972,5 @@ export class HistoryService extends Disposable implements IHistoryService { return undefined; } } + +registerSingleton(IHistoryService, HistoryService); \ No newline at end of file diff --git a/src/vs/workbench/workbench.main.ts b/src/vs/workbench/workbench.main.ts index e06ec70c0b0..d8bf039ff5d 100644 --- a/src/vs/workbench/workbench.main.ts +++ b/src/vs/workbench/workbench.main.ts @@ -74,6 +74,9 @@ import 'vs/workbench/services/textmodelResolver/common/textModelResolverService' import 'vs/workbench/services/textfile/common/textFileService'; import 'vs/workbench/services/dialogs/electron-browser/dialogService'; import 'vs/workbench/services/backup/node/backupFileService'; +import 'vs/workbench/services/history/browser/history'; +import 'vs/workbench/browser/parts/quickinput/quickInput'; +import 'vs/workbench/browser/parts/quickopen/quickOpenController'; registerSingleton(IMenuService, MenuService, true); registerSingleton(IListService, ListService, true); From b91adad1cfd8a75ebce9752878da177d1ac50cdc Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Wed, 6 Mar 2019 18:44:24 +0100 Subject: [PATCH 6/9] Fix file ordering issue in simple file picker --- .../services/dialogs/electron-browser/remoteFileDialog.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/services/dialogs/electron-browser/remoteFileDialog.ts b/src/vs/workbench/services/dialogs/electron-browser/remoteFileDialog.ts index 08f9ea471b8..1861f1c0bdf 100644 --- a/src/vs/workbench/services/dialogs/electron-browser/remoteFileDialog.ts +++ b/src/vs/workbench/services/dialogs/electron-browser/remoteFileDialog.ts @@ -468,7 +468,9 @@ export class RemoteFileDialog { if (i1.isFolder !== i2.isFolder) { return i1.isFolder ? -1 : 1; } - return i1.label.localeCompare(i2.label); + const trimmed1 = this.endsWithSlash(i1.label) ? i1.label.substr(0, i1.label.length - 1) : i1.label; + const trimmed2 = this.endsWithSlash(i2.label) ? i2.label.substr(0, i2.label.length - 1) : i2.label; + return trimmed1.localeCompare(trimmed2); }); if (backDir) { From 9a72cf08e47ec97abc1b9b5eae6f359888c73c44 Mon Sep 17 00:00:00 2001 From: Andre Weinand Date: Wed, 6 Mar 2019 18:45:22 +0100 Subject: [PATCH 7/9] null checking in debugger.ts --- src/tsconfig.strictNullChecks.json | 1 + .../workbench/contrib/debug/common/debug.ts | 4 +- .../contrib/debug/node/debugAdapter.ts | 32 +++++++------ .../workbench/contrib/debug/node/debugger.ts | 48 +++++++++++-------- 4 files changed, 48 insertions(+), 37 deletions(-) diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index 9ae7f712530..47698340041 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -243,6 +243,7 @@ "./vs/workbench/contrib/debug/common/debugViewModel.ts", "./vs/workbench/contrib/debug/electron-browser/rawDebugSession.ts", "./vs/workbench/contrib/debug/node/debugAdapter.ts", + "./vs/workbench/contrib/debug/node/debugger.ts", "./vs/workbench/contrib/debug/node/telemetryApp.ts", "./vs/workbench/contrib/debug/test/common/debugSource.test.ts", "./vs/workbench/contrib/debug/test/common/debugUtils.test.ts", diff --git a/src/vs/workbench/contrib/debug/common/debug.ts b/src/vs/workbench/contrib/debug/common/debug.ts index 39fe368818e..000458a22fc 100644 --- a/src/vs/workbench/contrib/debug/common/debug.ts +++ b/src/vs/workbench/contrib/debug/common/debug.ts @@ -112,7 +112,7 @@ export interface IExpression extends IReplElement, IExpressionContainer { export interface IDebugger { createDebugAdapter(session: IDebugSession, outputService: IOutputService): Promise; runInTerminal(args: DebugProtocol.RunInTerminalRequestArguments): Promise; - getCustomTelemetryService(): Promise; + getCustomTelemetryService(): Promise; } export const enum State { @@ -516,7 +516,7 @@ export interface IPlatformSpecificAdapterContribution { } export interface IDebuggerContribution extends IPlatformSpecificAdapterContribution { - type?: string; + type: string; label?: string; // debug adapter executable adapterExecutableCommand?: string; diff --git a/src/vs/workbench/contrib/debug/node/debugAdapter.ts b/src/vs/workbench/contrib/debug/node/debugAdapter.ts index 1f0a6b7e6dc..2f3845662d4 100644 --- a/src/vs/workbench/contrib/debug/node/debugAdapter.ts +++ b/src/vs/workbench/contrib/debug/node/debugAdapter.ts @@ -435,33 +435,35 @@ export class ExecutableDebugAdapter extends StreamDebugAdapter { } } - private static extract(contribution: IDebuggerContribution, extensionFolderPath: string): IDebuggerContribution | undefined { - if (!contribution) { + private static extract(platformContribution: IPlatformSpecificAdapterContribution, extensionFolderPath: string): IDebuggerContribution | undefined { + if (!platformContribution) { return undefined; } const result: IDebuggerContribution = Object.create(null); - if (contribution.runtime) { - if (contribution.runtime.indexOf('./') === 0) { // TODO - result.runtime = path.join(extensionFolderPath, contribution.runtime); + if (platformContribution.runtime) { + if (platformContribution.runtime.indexOf('./') === 0) { // TODO + result.runtime = path.join(extensionFolderPath, platformContribution.runtime); } else { - result.runtime = contribution.runtime; + result.runtime = platformContribution.runtime; } } - if (contribution.runtimeArgs) { - result.runtimeArgs = contribution.runtimeArgs; + if (platformContribution.runtimeArgs) { + result.runtimeArgs = platformContribution.runtimeArgs; } - if (contribution.program) { - if (!path.isAbsolute(contribution.program)) { - result.program = path.join(extensionFolderPath, contribution.program); + if (platformContribution.program) { + if (!path.isAbsolute(platformContribution.program)) { + result.program = path.join(extensionFolderPath, platformContribution.program); } else { - result.program = contribution.program; + result.program = platformContribution.program; } } - if (contribution.args) { - result.args = contribution.args; + if (platformContribution.args) { + result.args = platformContribution.args; } + const contribution = platformContribution as IDebuggerContribution; + if (contribution.win) { result.win = ExecutableDebugAdapter.extract(contribution.win, extensionFolderPath); } @@ -490,7 +492,7 @@ export class ExecutableDebugAdapter extends StreamDebugAdapter { const debuggers = ed.contributes['debuggers']; if (debuggers && debuggers.length > 0) { debuggers.filter(dbg => typeof dbg.type === 'string' && strings.equalsIgnoreCase(dbg.type, debugType)).forEach(dbg => { - // extract relevant attributes and make then absolute where needed + // extract relevant attributes and make them absolute where needed const extractedDbg = ExecutableDebugAdapter.extract(dbg, ed.extensionLocation.fsPath); // merge diff --git a/src/vs/workbench/contrib/debug/node/debugger.ts b/src/vs/workbench/contrib/debug/node/debugger.ts index a8072f51aab..0b858507e55 100644 --- a/src/vs/workbench/contrib/debug/node/debugger.ts +++ b/src/vs/workbench/contrib/debug/node/debugger.ts @@ -31,7 +31,7 @@ import { isDebuggerMainContribution } from 'vs/workbench/contrib/debug/common/de export class Debugger implements IDebugger { - private debuggerContribution: IDebuggerContribution = {}; + private debuggerContribution: IDebuggerContribution; private mergedExtensionDescriptions: IExtensionDescription[] = []; private mainExtensionDescription: IExtensionDescription | undefined; @@ -42,6 +42,7 @@ export class Debugger implements IDebugger { @IConfigurationResolverService private readonly configurationResolverService: IConfigurationResolverService, @ITelemetryService private readonly telemetryService: ITelemetryService, ) { + this.debuggerContribution = { type: dbgContribution.type }; this.merge(dbgContribution, extensionDescription); } @@ -149,12 +150,15 @@ export class Debugger implements IDebugger { if (this.debuggerContribution.adapterExecutableCommand) { console.info('debugAdapterExecutable attribute in package.json is deprecated and support for it will be removed soon; please use DebugAdapterDescriptorFactory.createDebugAdapterDescriptor instead.'); const rootFolder = session.root ? session.root.uri.toString() : undefined; - return this.commandService.executeCommand(this.debuggerContribution.adapterExecutableCommand, rootFolder).then((ae: { command: string, args: string[] }) => { - return { - type: 'executable', - command: ae.command, - args: ae.args || [] - }; + return this.commandService.executeCommand(this.debuggerContribution.adapterExecutableCommand, rootFolder).then(ae => { + if (ae) { + return { + type: 'executable', + command: ae.command, + args: ae.args || [] + }; + } + throw new Error('command adapterExecutableCommand did not return proper command.'); }); } @@ -197,15 +201,15 @@ export class Debugger implements IDebugger { return this.debuggerContribution.type; } - get variables(): { [key: string]: string } { + get variables(): { [key: string]: string } | undefined { return this.debuggerContribution.variables; } - get configurationSnippets(): IJSONSchemaSnippet[] { + get configurationSnippets(): IJSONSchemaSnippet[] | undefined { return this.debuggerContribution.configurationSnippets; } - get languages(): string[] { + get languages(): string[] | undefined { return this.debuggerContribution.languages; } @@ -254,8 +258,11 @@ export class Debugger implements IDebugger { } @memoize - getCustomTelemetryService(): Promise { - if (!this.debuggerContribution.aiKey) { + getCustomTelemetryService(): Promise { + + const aiKey = this.debuggerContribution.aiKey; + + if (!aiKey) { return Promise.resolve(undefined); } @@ -270,7 +277,7 @@ export class Debugger implements IDebugger { { serverName: 'Debug Telemetry', timeout: 1000 * 60 * 5, - args: [`${this.getMainExtensionDescriptor().publisher}.${this.type}`, JSON.stringify(data), this.debuggerContribution.aiKey], + args: [`${this.getMainExtensionDescriptor().publisher}.${this.type}`, JSON.stringify(data), aiKey], env: { ELECTRON_RUN_AS_NODE: 1, PIPE_LOGGING: 'true', @@ -286,10 +293,12 @@ export class Debugger implements IDebugger { }); } - getSchemaAttributes(): IJSONSchema[] { + getSchemaAttributes(): IJSONSchema[] | null { + if (!this.debuggerContribution.configurationAttributes) { return null; } + // fill in the default configuration attributes shared by all adapters. const taskSchema = TaskDefinitionRegistry.getJsonSchema(); return Object.keys(this.debuggerContribution.configurationAttributes).map(request => { @@ -339,9 +348,9 @@ export class Debugger implements IDebugger { }; properties['internalConsoleOptions'] = INTERNAL_CONSOLE_OPTIONS_SCHEMA; // Clear out windows, linux and osx fields to not have cycles inside the properties object - properties['windows'] = undefined; - properties['osx'] = undefined; - properties['linux'] = undefined; + delete properties['windows']; + delete properties['osx']; + delete properties['linux']; const osProperties = objects.deepClone(properties); properties['windows'] = { @@ -359,11 +368,10 @@ export class Debugger implements IDebugger { description: nls.localize('debugLinuxConfiguration', "Linux specific launch configuration attributes."), properties: osProperties }; - Object.keys(attributes.properties).forEach(name => { + Object.keys(properties).forEach(name => { // Use schema allOf property to get independent error reporting #21113 - ConfigurationResolverUtils.applyDeprecatedVariableMessage(attributes.properties[name]); + ConfigurationResolverUtils.applyDeprecatedVariableMessage(properties[name]); }); - return attributes; }); } From 6784a0bd27f8982b3bd539585f997206dbf6f7c1 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Wed, 6 Mar 2019 10:15:31 -0800 Subject: [PATCH 8/9] Strict null check settings files --- src/tsconfig.strictNullChecks.json | 2 + .../preferences/browser/settingsTree.ts | 55 ++++----- .../preferences/browser/settingsTreeModels.ts | 24 ++-- .../contrib/preferences/browser/tocTree.ts | 10 +- .../electron-browser/settingsEditor2.ts | 109 +++++++++++------- 5 files changed, 115 insertions(+), 85 deletions(-) diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index 47698340041..6ac1d410616 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -32,6 +32,8 @@ "./vs/workbench/services/timer/**/*.ts", "./vs/workbench/contrib/webview/**/*.ts", "./vs/workbench/contrib/debug/common/**/*.ts", + "./vs/workbench/contrib/preferences/common/**/*.ts", + "./vs/workbench/contrib/preferences/**/settings*.ts", ], "files": [ "./vs/monaco.d.ts", diff --git a/src/vs/workbench/contrib/preferences/browser/settingsTree.ts b/src/vs/workbench/contrib/preferences/browser/settingsTree.ts index 8f1fabdf359..8211f695bbc 100644 --- a/src/vs/workbench/contrib/preferences/browser/settingsTree.ts +++ b/src/vs/workbench/contrib/preferences/browser/settingsTree.ts @@ -112,7 +112,7 @@ function _resolveSettingsTree(tocData: ITOCEntry, allSettings: Set): I } if (!children && !settings) { - return null; + throw new Error(`TOC node has no child groups or settings: ${tocData.id}`); } return { @@ -439,7 +439,7 @@ export abstract class AbstractSettingRenderer implements ITreeRenderer this._onDidChangeSetting.fire({ key: element.setting.key, value, type: template.context.valueType }); + const onChange = value => this._onDidChangeSetting.fire({ key: element.setting.key, value, type: template.context!.valueType }); template.deprecationWarningElement.innerText = element.setting.deprecationMessage || ''; this.renderValue(element, template, onChange); @@ -488,7 +488,7 @@ export abstract class AbstractSettingRenderer implements ITreeRenderertemplate.controlElement.firstElementChild) { itemElement.setAttribute('role', 'combobox'); label += modifiedText; } @@ -587,7 +587,7 @@ export class SettingNewExtensionsRenderer implements ITreeRenderer { if (template.context) { @@ -621,9 +621,9 @@ export class SettingComplexRenderer extends AbstractSettingRenderer implements I renderTemplate(container: HTMLElement): ISettingComplexItemTemplate { const common = this.renderCommonTemplate(null, container, 'complex'); - const openSettingsButton = new Button(common.controlElement, { title: true, buttonBackground: null, buttonHoverBackground: null }); + const openSettingsButton = new Button(common.controlElement, { title: true, buttonBackground: undefined, buttonHoverBackground: undefined }); common.toDispose.push(openSettingsButton); - common.toDispose.push(openSettingsButton.onDidClick(() => template.onChange(null))); + common.toDispose.push(openSettingsButton.onDidClick(() => template.onChange!())); openSettingsButton.label = localize('editInSettingsJson', "Edit in settings.json"); openSettingsButton.element.classList.add('edit-in-settings-button'); @@ -770,7 +770,7 @@ export class SettingTextRenderer extends AbstractSettingRenderer implements ITre protected renderValue(dataElement: SettingsTreeSettingElement, template: ISettingTextItemTemplate, onChange: (value: string) => void): void { const label = this.setElementAriaLabels(dataElement, SETTINGS_TEXT_TEMPLATE_ID, template); - template.onChange = null; + template.onChange = undefined; template.inputBox.value = dataElement.value; template.onChange = value => { renderValidations(dataElement, template, false, label); onChange(value); }; @@ -784,7 +784,7 @@ export class SettingEnumRenderer extends AbstractSettingRenderer implements ITre renderTemplate(container: HTMLElement): ISettingEnumItemTemplate { const common = this.renderCommonTemplate(null, container, 'enum'); - const selectBox = new SelectBox([], undefined, this._contextViewService, undefined, { useCustomDrawn: true }); + const selectBox = new SelectBox([], 0, this._contextViewService, undefined, { useCustomDrawn: true }); common.toDispose.push(selectBox); common.toDispose.push(attachSelectBoxStyler(selectBox, this._themeService, { @@ -827,7 +827,7 @@ export class SettingEnumRenderer extends AbstractSettingRenderer implements ITre const enumDescriptions = dataElement.setting.enumDescriptions; const enumDescriptionsAreMarkdown = dataElement.setting.enumDescriptionsAreMarkdown; - const displayOptions = dataElement.setting.enum + const displayOptions = dataElement.setting.enum! .map(String) .map(escapeInvisibleChars) .map((data, index) => { @@ -842,10 +842,10 @@ export class SettingEnumRenderer extends AbstractSettingRenderer implements ITre const label = this.setElementAriaLabels(dataElement, SETTINGS_ENUM_TEMPLATE_ID, template); template.selectBox.setAriaLabel(label); - const idx = dataElement.setting.enum.indexOf(dataElement.value); - template.onChange = null; + const idx = dataElement.setting.enum!.indexOf(dataElement.value); + template.onChange = undefined; template.selectBox.select(idx); - template.onChange = idx => onChange(dataElement.setting.enum[idx]); + template.onChange = idx => onChange(dataElement.setting.enum![idx]); template.enumDescriptionElement.innerHTML = ''; } @@ -889,7 +889,7 @@ export class SettingNumberRenderer extends AbstractSettingRenderer implements IT super.renderSettingElement(element, index, templateData); } - protected renderValue(dataElement: SettingsTreeSettingElement, template: ISettingNumberItemTemplate, onChange: (value: number) => void): void { + protected renderValue(dataElement: SettingsTreeSettingElement, template: ISettingNumberItemTemplate, onChange: (value: number | null) => void): void { const numParseFn = (dataElement.valueType === 'integer' || dataElement.valueType === 'nullable-integer') ? parseInt : parseFloat; @@ -898,9 +898,12 @@ export class SettingNumberRenderer extends AbstractSettingRenderer implements IT const label = this.setElementAriaLabels(dataElement, SETTINGS_NUMBER_TEMPLATE_ID, template); - template.onChange = null; + template.onChange = undefined; template.inputBox.value = dataElement.value; - template.onChange = value => { renderValidations(dataElement, template, false, label); onChange(nullNumParseFn(value)); }; + template.onChange = value => { + renderValidations(dataElement, template, false, label); + onChange(nullNumParseFn(value)); + }; renderValidations(dataElement, template, true, label); } @@ -930,7 +933,7 @@ export class SettingBoolRenderer extends AbstractSettingRenderer implements ITre const deprecationWarningElement = DOM.append(container, $('.setting-item-deprecation-message')); const toDispose: IDisposable[] = []; - const checkbox = new Checkbox({ actionClassName: 'setting-value-checkbox', isChecked: true, title: '', inputActiveOptionBorder: null }); + const checkbox = new Checkbox({ actionClassName: 'setting-value-checkbox', isChecked: true, title: '', inputActiveOptionBorder: undefined }); controlElement.appendChild(checkbox.domNode); toDispose.push(checkbox); toDispose.push(checkbox.onChange(() => { @@ -949,7 +952,7 @@ export class SettingBoolRenderer extends AbstractSettingRenderer implements ITre // Toggle target checkbox if (targetElement.tagName.toLowerCase() !== 'a' && targetId === template.checkbox.domNode.id) { template.checkbox.checked = template.checkbox.checked ? false : true; - template.onChange(checkbox.checked); + template.onChange!(checkbox.checked); } DOM.EventHelper.stop(e); })); @@ -993,7 +996,7 @@ export class SettingBoolRenderer extends AbstractSettingRenderer implements ITre } protected renderValue(dataElement: SettingsTreeSettingElement, template: ISettingBoolItemTemplate, onChange: (value: boolean) => void): void { - template.onChange = null; + template.onChange = undefined; template.checkbox.checked = dataElement.value; template.onChange = onChange; @@ -1067,17 +1070,17 @@ export class SettingTreeRenderers { } showContextMenu(element: SettingsTreeSettingElement, settingDOMElement: HTMLElement): void { - const toolbarElement: HTMLElement = settingDOMElement.querySelector('.toolbar-toggle-more'); + const toolbarElement = settingDOMElement.querySelector('.toolbar-toggle-more'); if (toolbarElement) { this._contextMenuService.showContextMenu({ getActions: () => this.settingActions, - getAnchor: () => toolbarElement, + getAnchor: () => toolbarElement, getActionsContext: () => element }); } } - getSettingDOMElementForDOMElement(domElement: HTMLElement): HTMLElement { + getSettingDOMElementForDOMElement(domElement: HTMLElement): HTMLElement | null { const parent = DOM.findParentWithClass(domElement, AbstractSettingRenderer.CONTENTS_CLASS); if (parent) { return parent; @@ -1090,12 +1093,12 @@ export class SettingTreeRenderers { return treeContainer.querySelectorAll(`[${AbstractSettingRenderer.SETTING_KEY_ATTR}="${key}"]`); } - getKeyForDOMElementInSetting(element: HTMLElement): string { + getKeyForDOMElementInSetting(element: HTMLElement): string | null { const settingElement = this.getSettingDOMElementForDOMElement(element); return settingElement && settingElement.getAttribute(AbstractSettingRenderer.SETTING_KEY_ATTR); } - getIdForDOMElementInSetting(element: HTMLElement): string { + getIdForDOMElementInSetting(element: HTMLElement): string | null { const settingElement = this.getSettingDOMElementForDOMElement(element); return settingElement && settingElement.getAttribute(AbstractSettingRenderer.SETTING_ID_ATTR); } @@ -1108,11 +1111,11 @@ function renderValidations(dataElement: SettingsTreeSettingElement, template: IS DOM.addClass(template.containerElement, 'invalid-input'); template.validationErrorMessageElement.innerText = errMsg; const validationError = localize('validationError', "Validation Error."); - template.inputBox.inputElement.parentElement.setAttribute('aria-label', [originalAriaLabel, validationError, errMsg].join(' ')); + template.inputBox.inputElement.parentElement!.setAttribute('aria-label', [originalAriaLabel, validationError, errMsg].join(' ')); if (!calledOnStartup) { ariaAlert(validationError + ' ' + errMsg); } return; } else { - template.inputBox.inputElement.parentElement.setAttribute('aria-label', originalAriaLabel); + template.inputBox.inputElement.parentElement!.setAttribute('aria-label', originalAriaLabel); } } DOM.removeClass(template.containerElement, 'invalid-input'); diff --git a/src/vs/workbench/contrib/preferences/browser/settingsTreeModels.ts b/src/vs/workbench/contrib/preferences/browser/settingsTreeModels.ts index 879d9a9e49f..4e7f56b60d8 100644 --- a/src/vs/workbench/contrib/preferences/browser/settingsTreeModels.ts +++ b/src/vs/workbench/contrib/preferences/browser/settingsTreeModels.ts @@ -24,7 +24,7 @@ export interface ISettingsEditorViewState { export abstract class SettingsTreeElement { id: string; - parent: SettingsTreeGroupElement; + parent?: SettingsTreeGroupElement; /** * Index assigned in display order, used for paging. @@ -130,7 +130,7 @@ export class SettingsTreeSettingElement extends SettingsTreeElement { } private initLabel(): void { - const displayKeyFormat = settingKeyToDisplayFormat(this.setting.key, this.parent.id); + const displayKeyFormat = settingKeyToDisplayFormat(this.setting.key, this.parent!.id); this._displayLabel = displayKeyFormat.label; this._displayCategory = displayKeyFormat.category; } @@ -161,7 +161,7 @@ export class SettingsTreeSettingElement extends SettingsTreeElement { } if (this.setting.tags) { - this.setting.tags.forEach(tag => this.tags.add(tag)); + this.setting.tags.forEach(tag => this.tags!.add(tag)); } } @@ -207,7 +207,7 @@ export class SettingsTreeSettingElement extends SettingsTreeElement { if (this.tags) { let hasFilteredTag = true; tagFilters.forEach(tag => { - hasFilteredTag = hasFilteredTag && this.tags.has(tag); + hasFilteredTag = hasFilteredTag && this.tags!.has(tag); }); return hasFilteredTag; } else { @@ -261,12 +261,12 @@ export class SettingsTreeModel { } } - getElementById(id: string): SettingsTreeElement { - return this._treeElementsById.get(id); + getElementById(id: string): SettingsTreeElement | null { + return this._treeElementsById.get(id) || null; } - getElementsByName(name: string): SettingsTreeSettingElement[] { - return this._treeElementsBySettingName.get(name); + getElementsByName(name: string): SettingsTreeSettingElement[] | null { + return this._treeElementsBySettingName.get(name) || null; } updateElementsByName(name: string): void { @@ -274,7 +274,7 @@ export class SettingsTreeModel { return; } - this._treeElementsBySettingName.get(name).forEach(element => { + this._treeElementsBySettingName.get(name)!.forEach(element => { const inspectResult = inspectSetting(element.setting.key, this._viewState.settingsTarget, this._configurationService); element.update(inspectResult); }); @@ -428,7 +428,7 @@ export const enum SearchResultIdx { export class SearchResultModel extends SettingsTreeModel { private rawSearchResults: ISearchResult[]; - private cachedUniqueSearchResults: ISearchResult[]; + private cachedUniqueSearchResults: ISearchResult[] | undefined; private newExtensionSearchResults: ISearchResult; readonly id = 'searchResultModel'; @@ -473,8 +473,8 @@ export class SearchResultModel extends SettingsTreeModel { return this.rawSearchResults; } - setResult(order: SearchResultIdx, result: ISearchResult): void { - this.cachedUniqueSearchResults = null; + setResult(order: SearchResultIdx, result: ISearchResult | null): void { + this.cachedUniqueSearchResults = undefined; this.rawSearchResults = this.rawSearchResults || []; if (!result) { delete this.rawSearchResults[order]; diff --git a/src/vs/workbench/contrib/preferences/browser/tocTree.ts b/src/vs/workbench/contrib/preferences/browser/tocTree.ts index 273471fdcfa..d3fdbdcc6f5 100644 --- a/src/vs/workbench/contrib/preferences/browser/tocTree.ts +++ b/src/vs/workbench/contrib/preferences/browser/tocTree.ts @@ -22,7 +22,7 @@ const $ = DOM.$; export class TOCTreeModel { - private _currentSearchModel: SearchResultModel; + private _currentSearchModel: SearchResultModel | null; private _settingsTreeRoot: SettingsTreeGroupElement; constructor(private _viewState: ISettingsEditorViewState) { @@ -37,7 +37,11 @@ export class TOCTreeModel { this.update(); } - set currentSearchModel(model: SearchResultModel) { + get currentSearchModel(): SearchResultModel | null { + return this._currentSearchModel; + } + + set currentSearchModel(model: SearchResultModel | null) { this._currentSearchModel = model; this.update(); } @@ -61,7 +65,7 @@ export class TOCTreeModel { const childCount = group.children .filter(child => child instanceof SettingsTreeGroupElement) - .reduce((acc, cur) => acc + (cur).count, 0); + .reduce((acc, cur) => acc + (cur).count!, 0); group.count = childCount + this.getGroupCount(group); } diff --git a/src/vs/workbench/contrib/preferences/electron-browser/settingsEditor2.ts b/src/vs/workbench/contrib/preferences/electron-browser/settingsEditor2.ts index efdfdce5654..8dccdbea88c 100644 --- a/src/vs/workbench/contrib/preferences/electron-browser/settingsEditor2.ts +++ b/src/vs/workbench/contrib/preferences/electron-browser/settingsEditor2.ts @@ -107,14 +107,14 @@ export class SettingsEditor2 extends BaseEditor { private delayedFilterLogging: Delayer; private localSearchDelayer: Delayer; private remoteSearchThrottle: ThrottledDelayer; - private searchInProgress: CancellationTokenSource; + private searchInProgress: CancellationTokenSource | null; private settingFastUpdateDelayer: Delayer; private settingSlowUpdateDelayer: Delayer; - private pendingSettingUpdate: { key: string, value: any }; + private pendingSettingUpdate: { key: string, value: any } | null; private readonly viewState: ISettingsEditorViewState; - private _searchResultModel: SearchResultModel; + private _searchResultModel: SearchResultModel | null; private tocRowFocused: IContextKey; private inSettingsEditorContextKey: IContextKey; @@ -128,7 +128,7 @@ export class SettingsEditor2 extends BaseEditor { private editorMemento: IEditorMemento; - private tocFocusedElement: SettingsTreeGroupElement; + private tocFocusedElement: SettingsTreeGroupElement | null; private settingsTreeScrollTop = 0; constructor( @@ -180,18 +180,19 @@ export class SettingsEditor2 extends BaseEditor { return this.searchResultModel || this.settingsTreeModel; } - private get searchResultModel(): SearchResultModel { + private get searchResultModel(): SearchResultModel | null { return this._searchResultModel; } - private set searchResultModel(value: SearchResultModel) { + private set searchResultModel(value: SearchResultModel | null) { this._searchResultModel = value; DOM.toggleClass(this.rootElement, 'search-mode', !!this._searchResultModel); } - private get currentSettingsContextMenuKeyBindingLabel() { - return this.keybindingService.lookupKeybinding(SETTINGS_EDITOR_COMMAND_SHOW_CONTEXT_MENU).getAriaLabel(); + private get currentSettingsContextMenuKeyBindingLabel(): string { + const keybinding = this.keybindingService.lookupKeybinding(SETTINGS_EDITOR_COMMAND_SHOW_CONTEXT_MENU); + return (keybinding && keybinding.getAriaLabel()) || ''; } createEditor(parent: HTMLElement): void { @@ -233,7 +234,7 @@ export class SettingsEditor2 extends BaseEditor { } private restoreCachedState(): void { - const cachedState = this.editorMemento.loadEditorState(this.group, this.input); + const cachedState = this.group && this.input && this.editorMemento.loadEditorState(this.group, this.input); if (cachedState && typeof cachedState.target === 'object') { cachedState.target = URI.revive(cachedState.target); } @@ -268,7 +269,10 @@ export class SettingsEditor2 extends BaseEditor { clearInput(): void { this.inSettingsEditorContextKey.set(false); - this.editorMemento.clearEditorState(this.input, this.group); + if (this.input) { + this.editorMemento.clearEditorState(this.input, this.group); + } + super.clearInput(); } @@ -315,7 +319,12 @@ export class SettingsEditor2 extends BaseEditor { } showContextMenu(): void { - const settingDOMElement = this.settingRenderers.getSettingDOMElementForDOMElement(this.getActiveElementInSettingsTree()); + const activeElement = this.getActiveElementInSettingsTree(); + if (!activeElement) { + return; + } + + const settingDOMElement = this.settingRenderers.getSettingDOMElementForDOMElement(activeElement); if (!settingDOMElement) { return; } @@ -383,9 +392,10 @@ export class SettingsEditor2 extends BaseEditor { this._register(attachStylerCallback(this.themeService, { badgeBackground, contrastBorder, badgeForeground }, colors => { const background = colors.badgeBackground ? colors.badgeBackground.toString() : null; const border = colors.contrastBorder ? colors.contrastBorder.toString() : null; + const foreground = colors.badgeForeground ? colors.badgeForeground.toString() : null; this.countElement.style.backgroundColor = background; - this.countElement.style.color = colors.badgeForeground.toString(); + this.countElement.style.color = foreground; this.countElement.style.borderWidth = border ? '1px' : null; this.countElement.style.borderStyle = border ? 'solid' : null; @@ -412,6 +422,10 @@ export class SettingsEditor2 extends BaseEditor { const elements = this.currentSettingsModel.getElementsByName(evt.targetKey); if (elements && elements[0]) { let sourceTop = this.settingsTree.getRelativeTop(evt.source); + if (typeof sourceTop !== 'number') { + return; + } + if (sourceTop < 0) { // e.g. clicked a searched element, now the search has been cleared sourceTop = 0.5; @@ -435,12 +449,12 @@ export class SettingsEditor2 extends BaseEditor { } } - switchToSettingsFile(): Promise { + switchToSettingsFile(): Promise { const query = parseQuery(this.searchWidget.getValue()); return this.openSettingsFile(query.query); } - private openSettingsFile(query?: string): Promise { + private openSettingsFile(query?: string): Promise { const currentSettingsTarget = this.settingsTargetsWidget.settingsTarget; const options: ISettingsEditorOptions = { query }; @@ -550,7 +564,7 @@ export class SettingsEditor2 extends BaseEditor { this.viewState)); this._register(this.tocTree.onDidChangeFocus(e => { - const element: SettingsTreeGroupElement = e.elements[0]; + const element: SettingsTreeGroupElement | null = e.elements[0]; if (this.tocFocusedElement === element) { return; } @@ -559,7 +573,7 @@ export class SettingsEditor2 extends BaseEditor { this.tocTree.setSelection(element ? [element] : []); if (this.searchResultModel) { if (this.viewState.filterToCategory !== element) { - this.viewState.filterToCategory = element; + this.viewState.filterToCategory = element || undefined; this.renderTree(); this.settingsTree.scrollTop = 0; } @@ -672,6 +686,10 @@ export class SettingsEditor2 extends BaseEditor { this.tocTree.reveal(element); const elementTop = this.tocTree.getRelativeTop(element); + if (typeof elementTop !== 'number') { + return; + } + this.tocTree.collapseAll(); ancestors.forEach(e => this.tocTree.expand(e)); @@ -738,17 +756,17 @@ export class SettingsEditor2 extends BaseEditor { }); } - private reportModifiedSetting(props: { key: string, query: string, searchResults: ISearchResult[], rawResults: ISearchResult[], showConfiguredOnly: boolean, isReset: boolean, settingsTarget: SettingsTarget }): void { + private reportModifiedSetting(props: { key: string, query: string, searchResults: ISearchResult[] | null, rawResults: ISearchResult[] | null, showConfiguredOnly: boolean, isReset: boolean, settingsTarget: SettingsTarget }): void { this.pendingSettingUpdate = null; - const remoteResult = props.searchResults && props.searchResults[SearchResultIdx.Remote]; - const localResult = props.searchResults && props.searchResults[SearchResultIdx.Local]; - - let groupId = undefined; - let nlpIndex = undefined; - let displayIndex = undefined; + let groupId: string | undefined = undefined; + let nlpIndex: number | undefined = undefined; + let displayIndex: number | undefined = undefined; if (props.searchResults) { - const localIndex = arrays.firstIndex(localResult.filterMatches, m => m.setting.key === props.key); + const remoteResult = props.searchResults[SearchResultIdx.Remote]; + const localResult = props.searchResults[SearchResultIdx.Local]; + + const localIndex = arrays.firstIndex(localResult!.filterMatches, m => m.setting.key === props.key); groupId = localIndex >= 0 ? 'local' : 'remote'; @@ -860,9 +878,9 @@ export class SettingsEditor2 extends BaseEditor { } const commonlyUsed = resolveSettingsTree(commonlyUsedData, dividedGroups.core); - resolvedSettingsRoot.children.unshift(commonlyUsed.tree); + resolvedSettingsRoot.children!.unshift(commonlyUsed.tree); - resolvedSettingsRoot.children.push(resolveExtensionsSettings(dividedGroups.extension || [])); + resolvedSettingsRoot.children!.push(resolveExtensionsSettings(dividedGroups.extension || [])); if (this.searchResultModel) { this.searchResultModel.updateChildren(); @@ -872,7 +890,7 @@ export class SettingsEditor2 extends BaseEditor { this.settingsTreeModel.update(resolvedSettingsRoot); // Make sure that all extensions' settings are included in search results - const cachedState = this.editorMemento.loadEditorState(this.group, this.input); + const cachedState = this.group && this.input && this.editorMemento.loadEditorState(this.group, this.input); if (cachedState && cachedState.searchQuery) { this.triggerSearch(cachedState.searchQuery); } else { @@ -895,7 +913,7 @@ export class SettingsEditor2 extends BaseEditor { private updateElementsByKey(keys: string[]): Promise { if (keys.length) { if (this.searchResultModel) { - keys.forEach(key => this.searchResultModel.updateElementsByName(key)); + keys.forEach(key => this.searchResultModel!.updateElementsByName(key)); } if (this.settingsTreeModel) { @@ -923,7 +941,8 @@ export class SettingsEditor2 extends BaseEditor { } // If a setting control is currently focused, schedule a refresh for later - const focusedSetting = this.settingRenderers.getSettingDOMElementForDOMElement(this.getActiveElementInSettingsTree()); + const activeElement = this.getActiveElementInSettingsTree(); + const focusedSetting = activeElement && this.settingRenderers.getSettingDOMElementForDOMElement(activeElement); if (focusedSetting && !force) { // If a single setting is being refreshed, it's ok to refresh now if that is not the focused setting if (key) { @@ -978,7 +997,7 @@ export class SettingsEditor2 extends BaseEditor { const isModified = dataElements && dataElements[0] && dataElements[0].isConfigured; // all elements are either configured or not const elements = this.settingRenderers.getDOMElementsForSettingKey(this.settingsTree.getHTMLElement(), key); if (elements && elements[0]) { - DOM.toggleClass(elements[0], 'is-configured', isModified); + DOM.toggleClass(elements[0], 'is-configured', !!isModified); } } @@ -987,12 +1006,12 @@ export class SettingsEditor2 extends BaseEditor { this.delayedFilterLogging.cancel(); this.triggerSearch(query.replace(/›/g, ' ')).then(() => { if (query && this.searchResultModel) { - this.delayedFilterLogging.trigger(() => this.reportFilteringUsed(query, this.searchResultModel.getUniqueResults())); + this.delayedFilterLogging.trigger(() => this.reportFilteringUsed(query, this.searchResultModel!.getUniqueResults())); } }); } - private parseSettingFromJSON(query: string): string { + private parseSettingFromJSON(query: string): string | null { const match = query.match(/"([a-zA-Z.]+)": /); return match && match[1]; } @@ -1002,7 +1021,7 @@ export class SettingsEditor2 extends BaseEditor { if (query) { const parsedQuery = parseQuery(query); query = parsedQuery.query; - parsedQuery.tags.forEach(tag => this.viewState.tagFilters.add(tag)); + parsedQuery.tags.forEach(tag => this.viewState.tagFilters!.add(tag)); } if (query && query !== '@') { @@ -1023,7 +1042,7 @@ export class SettingsEditor2 extends BaseEditor { this.searchInProgress = null; } - this.viewState.filterToCategory = null; + this.viewState.filterToCategory = undefined; this.tocTreeModel.currentSearchModel = this.searchResultModel; this.onSearchModeToggled(); @@ -1043,7 +1062,7 @@ export class SettingsEditor2 extends BaseEditor { this.refreshTOCTree(); } - return Promise.resolve(null); + return Promise.resolve(); } /** @@ -1121,18 +1140,18 @@ export class SettingsEditor2 extends BaseEditor { if (result && !result.exactMatch) { this.remoteSearchThrottle.trigger(() => { return searchInProgress && !searchInProgress.token.isCancellationRequested ? - this.remoteSearchPreferences(query, this.searchInProgress.token) : - Promise.resolve(null); + this.remoteSearchPreferences(query, this.searchInProgress!.token) : + Promise.resolve(); }); } }); } else { - return Promise.resolve(null); + return Promise.resolve(); } }); } - private localFilterPreferences(query: string, token?: CancellationToken): Promise { + private localFilterPreferences(query: string, token?: CancellationToken): Promise { const localSearchProvider = this.preferencesSearchService.getLocalSearchProvider(query); return this.filterOrSearchPreferences(query, SearchResultIdx.Local, localSearchProvider, token); } @@ -1147,7 +1166,7 @@ export class SettingsEditor2 extends BaseEditor { ]).then(() => { }); } - private filterOrSearchPreferences(query: string, type: SearchResultIdx, searchProvider: ISearchProvider, token?: CancellationToken): Promise { + private filterOrSearchPreferences(query: string, type: SearchResultIdx, searchProvider?: ISearchProvider, token?: CancellationToken): Promise { return this._filterOrSearchPreferencesModel(query, this.defaultSettingsEditorModel, searchProvider, token).then(result => { if (token && token.isCancellationRequested) { // Handle cancellation like this because cancellation is lost inside the search provider due to async/await @@ -1165,7 +1184,7 @@ export class SettingsEditor2 extends BaseEditor { } this.tocTree.setSelection([]); - this.viewState.filterToCategory = null; + this.viewState.filterToCategory = undefined; this.tocTree.expandAll(); return this.renderTree(undefined, true).then(() => result); @@ -1193,7 +1212,7 @@ export class SettingsEditor2 extends BaseEditor { } } - private _filterOrSearchPreferencesModel(filter: string, model: ISettingsEditorModel, provider: ISearchProvider, token?: CancellationToken): Promise { + private _filterOrSearchPreferencesModel(filter: string, model: ISettingsEditorModel, provider?: ISearchProvider, token?: CancellationToken): Promise { const searchP = provider ? provider.searchModel(model, token) : Promise.resolve(null); return searchP .then(null, err => { @@ -1212,7 +1231,7 @@ export class SettingsEditor2 extends BaseEditor { this.telemetryService.publicLog('settingsEditor.searchError', { message, filter }); this.logService.info('Setting search error: ' + message); } - return null; + return Promise.resolve(null); } }); } @@ -1232,7 +1251,9 @@ export class SettingsEditor2 extends BaseEditor { if (this.isVisible()) { const searchQuery = this.searchWidget.getValue().trim(); const target = this.settingsTargetsWidget.settingsTarget as SettingsTarget; - this.editorMemento.saveEditorState(this.group, this.input, { searchQuery, target }); + if (this.group && this.input) { + this.editorMemento.saveEditorState(this.group, this.input, { searchQuery, target }); + } } super.saveState(); From 6da595150d8e32ea1a57d8d78d0252bfebedbb68 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Wed, 6 Mar 2019 19:29:45 +0100 Subject: [PATCH 9/9] Fix simple file picker in empty workspace --- .../services/dialogs/electron-browser/remoteFileDialog.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/services/dialogs/electron-browser/remoteFileDialog.ts b/src/vs/workbench/services/dialogs/electron-browser/remoteFileDialog.ts index 1861f1c0bdf..299ac57f673 100644 --- a/src/vs/workbench/services/dialogs/electron-browser/remoteFileDialog.ts +++ b/src/vs/workbench/services/dialogs/electron-browser/remoteFileDialog.ts @@ -97,8 +97,8 @@ export class RemoteFileDialog { } private getOptions(options: ISaveDialogOptions | IOpenDialogOptions): IOpenDialogOptions | undefined { - const defaultUri = options.defaultUri ? options.defaultUri : URI.from({ scheme: REMOTE_HOST_SCHEME, authority: this.remoteAuthority, path: '/' }); - if (!this.remoteFileService.canHandleResource(defaultUri)) { + const defaultUri = options.defaultUri ? options.defaultUri : URI.from({ scheme: this.scheme, authority: this.remoteAuthority, path: '/' }); + if ((this.scheme !== Schemas.file) && !this.remoteFileService.canHandleResource(defaultUri)) { this.notificationService.info(nls.localize('remoteFileDialog.notConnectedToRemote', 'File system provider for {0} is not available.', defaultUri.toString())); return undefined; } @@ -119,7 +119,7 @@ export class RemoteFileDialog { private async pickResource(options: IOpenDialogOptions, isSave: boolean = false): Promise { this.allowFolderSelection = !!options.canSelectFolders; this.allowFileSelection = !!options.canSelectFiles; - let homedir: URI = options.defaultUri && options.defaultUri.scheme === REMOTE_HOST_SCHEME ? options.defaultUri : this.workspaceContextService.getWorkspace().folders[0].uri; + let homedir: URI = options.defaultUri ? options.defaultUri : this.workspaceContextService.getWorkspace().folders[0].uri; let trailing: string | undefined; let stat: IFileStat | undefined; let ext: string = resources.extname(options.defaultUri);