:chore: some code cleanup

- use readonly for static ids
- prefer undefined over null in my code
- remove some obsolete todos
- do not require Promise from action.run()

src/vs/base/common/actions.ts

@@ -30,14 +30,14 @@ export interface IAction extends IDisposable {
 	class: string | undefined;
 	enabled: boolean;
 	checked: boolean;
-	run(event?: unknown): Promise<unknown>;
+	run(event?: unknown): unknown;
 }
 
 export interface IActionRunner extends IDisposable {
 	readonly onDidRun: Event<IRunEvent>;
 	readonly onBeforeRun: Event<IRunEvent>;
 
-	run(action: IAction, context?: unknown): Promise<unknown>;
+	run(action: IAction, context?: unknown): unknown;
 }
 
 export interface IActionChangeEvent {
@@ -59,9 +59,9 @@ export class Action extends Disposable implements IAction {
 	protected _cssClass: string | undefined;
 	protected _enabled: boolean = true;
 	protected _checked: boolean = false;
-	protected readonly _actionCallback?: (event?: unknown) => Promise<unknown>;
+	protected readonly _actionCallback?: (event?: unknown) => unknown;
 
-	constructor(id: string, label: string = '', cssClass: string = '', enabled: boolean = true, actionCallback?: (event?: unknown) => Promise<unknown>) {
+	constructor(id: string, label: string = '', cssClass: string = '', enabled: boolean = true, actionCallback?: (event?: unknown) => unknown) {
 		super();
 		this._id = id;
 		this._label = label;

src/vs/code/electron-main/app.ts

@@ -247,7 +247,7 @@ export class CodeApplication extends Disposable {
 		//#endregion
 
 		let macOpenFileURIs: IWindowOpenable[] = [];
-		let runningTimeout: NodeJS.Timeout | null = null;
+		let runningTimeout: NodeJS.Timeout | undefined = undefined;
 		app.on('open-file', (event, path) => {
 			this.logService.trace('app#open-file: ', path);
 			event.preventDefault();
@@ -256,9 +256,9 @@ export class CodeApplication extends Disposable {
 			macOpenFileURIs.push(this.getWindowOpenableFromPathSync(path));
 
 			// Clear previous handler if any
-			if (runningTimeout !== null) {
+			if (runningTimeout !== undefined) {
 				clearTimeout(runningTimeout);
-				runningTimeout = null;
+				runningTimeout = undefined;
 			}
 
 			// Handle paths delayed in case more are coming!
@@ -272,7 +272,7 @@ export class CodeApplication extends Disposable {
 				});
 
 				macOpenFileURIs = [];
-				runningTimeout = null;
+				runningTimeout = undefined;
 			}, 100);
 		});
 

src/vs/editor/contrib/folding/folding.ts

@@ -51,7 +51,7 @@ interface FoldingStateMemento {
 
 export class FoldingController extends Disposable implements IEditorContribution {
 
-	public static ID = 'editor.contrib.folding';
+	public static readonly ID = 'editor.contrib.folding';
 
 	static readonly MAX_FOLDING_REGIONS = 5000;
 

src/vs/editor/contrib/snippet/snippetController2.ts

@@ -43,7 +43,7 @@ const _defaultOptions: ISnippetInsertOptions = {
 
 export class SnippetController2 implements IEditorContribution {
 
-	public static ID = 'snippetController2';
+	public static readonly ID = 'snippetController2';
 
 	static get(editor: ICodeEditor): SnippetController2 {
 		return editor.getContribution<SnippetController2>(SnippetController2.ID);

src/vs/platform/actions/browser/menuEntryActionViewItem.ts

@@ -133,13 +133,15 @@ export class MenuEntryActionViewItem extends ActionViewItem {
 		return this._wantsAltCommand && this._menuItemAction.alt || this._menuItemAction;
 	}
 
-	override onClick(event: MouseEvent): void {
+	override async onClick(event: MouseEvent): Promise<void> {
 		event.preventDefault();
 		event.stopPropagation();
 
-		this.actionRunner
-			.run(this._commandAction, this._context)
-			.catch(err => this._notificationService.error(err));
+		try {
+			await this.actionRunner.run(this._commandAction, this._context);
+		} catch (err) {
+			this._notificationService.error(err);
+		}
 	}
 
 	override render(container: HTMLElement): void {

src/vs/platform/actions/common/actions.ts

@@ -326,7 +326,7 @@ export class ExecuteCommandAction extends Action {
 		super(id, label);
 	}
 
-	override run(...args: any[]): Promise<any> {
+	override run(...args: any[]): Promise<void> {
 		return this._commandService.executeCommand(this.id, ...args);
 	}
 }
@@ -417,7 +417,7 @@ export class MenuItemAction implements IAction {
 		// to bridge into the rendering world.
 	}
 
-	run(...args: any[]): Promise<any> {
+	run(...args: any[]): Promise<void> {
 		let runArgs: any[] = [];
 
 		if (this._options?.arg) {
@@ -525,7 +525,7 @@ export interface IAction2Options extends ICommandAction {
 
 export abstract class Action2 {
 	constructor(readonly desc: Readonly<IAction2Options>) { }
-	abstract run(accessor: ServicesAccessor, ...args: any[]): any;
+	abstract run(accessor: ServicesAccessor, ...args: any[]): void;
 }
 
 export function registerAction2(ctor: { new(): Action2 }): IDisposable {

src/vs/platform/files/common/files.ts

@@ -400,7 +400,7 @@ export interface IFileSystemProvider {
 	readonly capabilities: FileSystemProviderCapabilities;
 	readonly onDidChangeCapabilities: Event<void>;
 
-	readonly onDidErrorOccur?: Event<string>; // TODO@bpasero remove once file watchers are solid
+	readonly onDidErrorOccur?: Event<string>;
 
 	readonly onDidChangeFile: Event<readonly IFileChange[]>;
 	watch(resource: URI, opts: IWatchOptions): IDisposable;

src/vs/platform/files/node/diskFileSystemProvider.ts

@@ -542,7 +542,7 @@ export class DiskFileSystemProvider extends Disposable implements
 			return this.watchRecursive(resource, opts.excludes);
 		}
 
-		return this.watchNonRecursive(resource); // TODO@bpasero ideally the same watcher can be used in both cases
+		return this.watchNonRecursive(resource);
 	}
 
 	private watchRecursive(resource: URI, excludes: string[]): IDisposable {

src/vs/platform/lifecycle/electron-main/lifecycleMainService.ts

@@ -188,10 +188,10 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 	private oneTimeListenerTokenGenerator = 0;
 	private windowCounter = 0;
 
-	private pendingQuitPromise: Promise<boolean> | null = null;
-	private pendingQuitPromiseResolve: { (veto: boolean): void } | null = null;
+	private pendingQuitPromise: Promise<boolean> | undefined = undefined;
+	private pendingQuitPromiseResolve: { (veto: boolean): void } | undefined = undefined;
 
-	private pendingWillShutdownPromise: Promise<void> | null = null;
+	private pendingWillShutdownPromise: Promise<void> | undefined = undefined;
 
 	private readonly phaseWhen = new Map<LifecycleMainPhase, Barrier>();
 
@@ -454,8 +454,8 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 	private resolvePendingQuitPromise(veto: boolean): void {
 		if (this.pendingQuitPromiseResolve) {
 			this.pendingQuitPromiseResolve(veto);
-			this.pendingQuitPromiseResolve = null;
-			this.pendingQuitPromise = null;
+			this.pendingQuitPromiseResolve = undefined;
+			this.pendingQuitPromise = undefined;
 		}
 	}
 

src/vs/platform/product/common/product.ts

@@ -54,7 +54,7 @@ else {
 	// Running out of sources
 	if (Object.keys(product).length === 0) {
 		Object.assign(product, {
-			version: '1.56.0-dev',
+			version: '1.57.0-dev',
 			nameShort: isWeb ? 'Code Web - OSS Dev' : 'Code - OSS Dev',
 			nameLong: isWeb ? 'Code Web - OSS Dev' : 'Code - OSS Dev',
 			applicationName: 'code-oss',

src/vs/platform/windows/electron-main/windowsFinder.ts

@@ -8,7 +8,7 @@ import { IWorkspaceIdentifier, IResolvedWorkspace, isWorkspaceIdentifier, isSing
 import { extUriBiasedIgnorePathCase } from 'vs/base/common/resources';
 import { ICodeWindow } from 'vs/platform/windows/electron-main/windows';
 
-export function findWindowOnFile(windows: ICodeWindow[], fileUri: URI, localWorkspaceResolver: (workspace: IWorkspaceIdentifier) => IResolvedWorkspace | null): ICodeWindow | undefined {
+export function findWindowOnFile(windows: ICodeWindow[], fileUri: URI, localWorkspaceResolver: (workspace: IWorkspaceIdentifier) => IResolvedWorkspace | undefined): ICodeWindow | undefined {
 
 	// First check for windows with workspaces that have a parent folder of the provided path opened
 	for (const window of windows) {

src/vs/platform/windows/electron-main/windowsMainService.ts

@@ -409,7 +409,7 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic
 			let windowToUseForFiles: ICodeWindow | undefined = undefined;
 			if (fileToCheck?.fileUri && !openFilesInNewWindow) {
 				if (openConfig.context === OpenContext.DESKTOP || openConfig.context === OpenContext.CLI || openConfig.context === OpenContext.DOCK) {
-					windowToUseForFiles = findWindowOnFile(windows, fileToCheck.fileUri, workspace => workspace.configPath.scheme === Schemas.file ? this.workspacesManagementMainService.resolveLocalWorkspaceSync(workspace.configPath) : null);
+					windowToUseForFiles = findWindowOnFile(windows, fileToCheck.fileUri, workspace => workspace.configPath.scheme === Schemas.file ? this.workspacesManagementMainService.resolveLocalWorkspaceSync(workspace.configPath) : undefined);
 				}
 
 				if (!windowToUseForFiles) {

src/vs/platform/windows/test/electron-main/windowsFinder.test.ts

@@ -28,7 +28,7 @@ suite('WindowsFinder', () => {
 	};
 
 	const testWorkspaceFolders = toWorkspaceFolders([{ path: join(fixturesFolder, 'vscode_workspace_1_folder') }, { path: join(fixturesFolder, 'vscode_workspace_2_folder') }], testWorkspace.configPath, extUriBiasedIgnorePathCase);
-	const localWorkspaceResolver = (workspace: any) => { return workspace === testWorkspace ? { id: testWorkspace.id, configPath: workspace.configPath, folders: testWorkspaceFolders } : null; };
+	const localWorkspaceResolver = (workspace: any) => { return workspace === testWorkspace ? { id: testWorkspace.id, configPath: workspace.configPath, folders: testWorkspaceFolders } : undefined; };
 
 	function createTestCodeWindow(options: { lastFocusTime: number, openedFolderUri?: URI, openedWorkspace?: IWorkspaceIdentifier }): ICodeWindow {
 		return new class implements ICodeWindow {

src/vs/platform/workspaces/common/workspaces.ts

@@ -39,7 +39,7 @@ export interface IWorkspacesService {
 	readonly _serviceBrand: undefined;
 
 	// Workspaces Management
-	enterWorkspace(path: URI): Promise<IEnterWorkspaceResult | null>;
+	enterWorkspace(path: URI): Promise<IEnterWorkspaceResult | undefined>;
 	createUntitledWorkspace(folders?: IWorkspaceFolderCreationData[], remoteAuthority?: string): Promise<IWorkspaceIdentifier>;
 	deleteUntitledWorkspace(workspace: IWorkspaceIdentifier): Promise<void>;
 	getWorkspaceIdentifier(workspacePath: URI): Promise<IWorkspaceIdentifier>;
@@ -320,7 +320,7 @@ export function toWorkspaceFolders(configuredFolders: IStoredWorkspaceFolder[],
 
 	const relativeTo = extUri.dirname(workspaceConfigFile);
 	for (let configuredFolder of configuredFolders) {
-		let uri: URI | null = null;
+		let uri: URI | undefined = undefined;
 		if (isRawFileWorkspaceFolder(configuredFolder)) {
 			if (configuredFolder.path) {
 				uri = extUri.resolvePath(relativeTo, configuredFolder.path);
@@ -328,16 +328,16 @@ export function toWorkspaceFolders(configuredFolders: IStoredWorkspaceFolder[],
 		} else if (isRawUriWorkspaceFolder(configuredFolder)) {
 			try {
 				uri = URI.parse(configuredFolder.uri);
-				// this makes sure all workspace folder are absolute
 				if (uri.path[0] !== '/') {
-					uri = uri.with({ path: '/' + uri.path });
+					uri = uri.with({ path: '/' + uri.path }); // this makes sure all workspace folder are absolute
 				}
 			} catch (e) {
-				console.warn(e);
-				// ignore
+				console.warn(e); // ignore
 			}
 		}
+
 		if (uri) {
+
 			// remove duplicates
 			let comparisonKey = extUri.getComparisonKey(uri);
 			if (!seen.has(comparisonKey)) {
@@ -369,11 +369,9 @@ export function rewriteWorkspaceFileForNewLocation(rawWorkspaceContents: string,
 		const folderURI = isRawFileWorkspaceFolder(folder) ? extUri.resolvePath(sourceConfigFolder, folder.path) : URI.parse(folder.uri);
 		let absolute;
 		if (isFromUntitledWorkspace) {
-			// if it was an untitled workspace, try to make paths relative
-			absolute = false;
+			absolute = false; // if it was an untitled workspace, try to make paths relative
 		} else {
-			// for existing workspaces, preserve whether a path was absolute or relative
-			absolute = !isRawFileWorkspaceFolder(folder) || isAbsolute(folder.path);
+			absolute = !isRawFileWorkspaceFolder(folder) || isAbsolute(folder.path); // for existing workspaces, preserve whether a path was absolute or relative
 		}
 		rewrittenFolders.push(getStoredWorkspaceFolder(folderURI, absolute, folder.name, targetConfigFolder, slashForPath, extUri));
 	}

src/vs/platform/workspaces/electron-main/workspacesMainService.ts

@@ -25,13 +25,13 @@ export class WorkspacesMainService implements AddFirstParameterToFunctions<IWork
 
 	//#region Workspace Management
 
-	async enterWorkspace(windowId: number, path: URI): Promise<IEnterWorkspaceResult | null> {
+	async enterWorkspace(windowId: number, path: URI): Promise<IEnterWorkspaceResult | undefined> {
 		const window = this.windowsMainService.getWindowById(windowId);
 		if (window) {
 			return this.workspacesManagementMainService.enterWorkspace(window, this.windowsMainService.getWindows(), path);
 		}
 
-		return null;
+		return undefined;
 	}
 
 	createUntitledWorkspace(windowId: number, folders?: IWorkspaceFolderCreationData[], remoteAuthority?: string): Promise<IWorkspaceIdentifier> {

src/vs/platform/workspaces/electron-main/workspacesManagementMainService.ts

@@ -41,7 +41,7 @@ export interface IWorkspacesManagementMainService {
 	readonly onDidDeleteUntitledWorkspace: Event<IWorkspaceIdentifier>;
 	readonly onDidEnterWorkspace: Event<IWorkspaceEnteredEvent>;
 
-	enterWorkspace(intoWindow: ICodeWindow, openedWindows: ICodeWindow[], path: URI): Promise<IEnterWorkspaceResult | null>;
+	enterWorkspace(intoWindow: ICodeWindow, openedWindows: ICodeWindow[], path: URI): Promise<IEnterWorkspaceResult | undefined>;
 
 	createUntitledWorkspace(folders?: IWorkspaceFolderCreationData[], remoteAuthority?: string): Promise<IWorkspaceIdentifier>;
 	createUntitledWorkspaceSync(folders?: IWorkspaceFolderCreationData[]): IWorkspaceIdentifier;
@@ -52,7 +52,7 @@ export interface IWorkspacesManagementMainService {
 	getUntitledWorkspacesSync(): IUntitledWorkspaceInfo[];
 	isUntitledWorkspace(workspace: IWorkspaceIdentifier): boolean;
 
-	resolveLocalWorkspaceSync(path: URI): IResolvedWorkspace | null;
+	resolveLocalWorkspaceSync(path: URI): IResolvedWorkspace | undefined;
 	getWorkspaceIdentifier(workspacePath: URI): Promise<IWorkspaceIdentifier>;
 }
 
@@ -83,20 +83,20 @@ export class WorkspacesManagementMainService extends Disposable implements IWork
 		super();
 	}
 
-	resolveLocalWorkspaceSync(uri: URI): IResolvedWorkspace | null {
+	resolveLocalWorkspaceSync(uri: URI): IResolvedWorkspace | undefined {
 		if (!this.isWorkspacePath(uri)) {
-			return null; // does not look like a valid workspace config file
+			return undefined; // does not look like a valid workspace config file
 		}
 
 		if (uri.scheme !== Schemas.file) {
-			return null;
+			return undefined;
 		}
 
 		let contents: string;
 		try {
 			contents = readFileSync(uri.fsPath, 'utf8');
 		} catch (error) {
-			return null; // invalid workspace
+			return undefined; // invalid workspace
 		}
 
 		return this.doResolveWorkspace(uri, contents);
@@ -106,7 +106,7 @@ export class WorkspacesManagementMainService extends Disposable implements IWork
 		return isUntitledWorkspace(uri, this.environmentMainService) || hasWorkspaceFileExtension(uri);
 	}
 
-	private doResolveWorkspace(path: URI, contents: string): IResolvedWorkspace | null {
+	private doResolveWorkspace(path: URI, contents: string): IResolvedWorkspace | undefined {
 		try {
 			const workspace = this.doParseStoredWorkspace(path, contents);
 			const workspaceIdentifier = getWorkspaceIdentifier(path);
@@ -120,7 +120,7 @@ export class WorkspacesManagementMainService extends Disposable implements IWork
 			this.logService.warn(error.toString());
 		}
 
-		return null;
+		return undefined;
 	}
 
 	private doParseStoredWorkspace(path: URI, contents: string): IStoredWorkspace {
@@ -238,19 +238,19 @@ export class WorkspacesManagementMainService extends Disposable implements IWork
 		return untitledWorkspaces;
 	}
 
-	async enterWorkspace(window: ICodeWindow, windows: ICodeWindow[], path: URI): Promise<IEnterWorkspaceResult | null> {
+	async enterWorkspace(window: ICodeWindow, windows: ICodeWindow[], path: URI): Promise<IEnterWorkspaceResult | undefined> {
 		if (!window || !window.win || !window.isReady) {
-			return null; // return early if the window is not ready or disposed
+			return undefined; // return early if the window is not ready or disposed
 		}
 
 		const isValid = await this.isValidTargetWorkspacePath(window, windows, path);
 		if (!isValid) {
-			return null; // return early if the workspace is not valid
+			return undefined; // return early if the workspace is not valid
 		}
 
 		const result = this.doEnterWorkspace(window, getWorkspaceIdentifier(path));
 		if (!result) {
-			return null;
+			return undefined;
 		}
 
 		// Emit as event
@@ -287,9 +287,9 @@ export class WorkspacesManagementMainService extends Disposable implements IWork
 		return true; // OK
 	}
 
-	private doEnterWorkspace(window: ICodeWindow, workspace: IWorkspaceIdentifier): IEnterWorkspaceResult | null {
+	private doEnterWorkspace(window: ICodeWindow, workspace: IWorkspaceIdentifier): IEnterWorkspaceResult | undefined {
 		if (!window.config) {
-			return null;
+			return undefined;
 		}
 
 		window.focus();

src/vs/workbench/contrib/codeEditor/browser/diffEditorHelper.ts

@@ -21,7 +21,7 @@ const enum WidgetState {
 
 class DiffEditorHelperContribution extends Disposable implements IDiffEditorContribution {
 
-	public static ID = 'editor.contrib.diffEditorHelper';
+	public static readonly ID = 'editor.contrib.diffEditorHelper';
 
 	private _helperWidget: FloatingClickWidget | null;
 	private _helperWidgetListener: IDisposable | null;

src/vs/workbench/contrib/debug/browser/welcomeView.ts

@@ -32,8 +32,8 @@ const CONTEXT_DEBUGGER_INTERESTED_IN_ACTIVE_EDITOR = new RawContextKey<boolean>(
 
 export class WelcomeView extends ViewPane {
 
-	static ID = 'workbench.debug.welcome';
-	static LABEL = localize('run', "Run");
+	static readonly ID = 'workbench.debug.welcome';
+	static readonly LABEL = localize('run', "Run");
 
 	private debugStartLanguageContext: IContextKey<string | undefined>;
 	private debuggerInterestedContext: IContextKey<boolean>;

src/vs/workbench/contrib/output/common/outputChannelModel.ts

@@ -128,7 +128,6 @@ export abstract class AbstractFileOutputChannelModel extends Disposable implemen
 	}
 }
 
-// TODO@bpasero see if new watchers can cope with spdlog and avoid polling then
 class OutputFileListener extends Disposable {
 
 	private readonly _onDidContentChange = new Emitter<number | undefined>();

src/vs/workbench/contrib/timeline/browser/timelinePane.ts

@@ -1051,13 +1051,14 @@ export class TimelineIdentityProvider implements IIdentityProvider<TreeElement>
 
 class TimelineActionRunner extends ActionRunner {
 
-	override runAction(action: IAction, { uri, item }: TimelineActionContext): Promise<any> {
+	override async runAction(action: IAction, { uri, item }: TimelineActionContext): Promise<void> {
 		if (!isTimelineItem(item)) {
 			// TODO@eamodio do we need to do anything else?
-			return action.run();
+			await action.run();
+			return;
 		}
 
-		return action.run(...[
+		await action.run(...[
 			{
 				$mid: 11,
 				handle: item.handle,

src/vs/workbench/contrib/welcome/gettingStarted/browser/gettingStarted.ts

@@ -77,7 +77,7 @@ type GettingStartedActionEvent = {
 
 export class GettingStartedPage extends EditorPane {
 
-	public static ID = 'gettingStartedPage';
+	public static readonly ID = 'gettingStartedPage';
 
 	private editorInput!: GettingStartedInput;
 	private inProgressScroll = Promise.resolve();

src/vs/workbench/services/workspaces/browser/abstractWorkspaceEditingService.ts

@@ -317,7 +317,7 @@ export abstract class AbstractWorkspaceEditingService implements IWorkspaceEditi
 
 	abstract enterWorkspace(path: URI): Promise<void>;
 
-	protected async doEnterWorkspace(path: URI): Promise<IEnterWorkspaceResult | null> {
+	protected async doEnterWorkspace(path: URI): Promise<IEnterWorkspaceResult | undefined> {
 		if (!!this.environmentService.extensionTestsLocationURI) {
 			throw new Error('Entering a new workspace is not possible in tests.');
 		}

src/vs/workbench/services/workspaces/browser/workspacesService.ts

@@ -125,7 +125,7 @@ export class BrowserWorkspacesService extends Disposable implements IWorkspacesS
 
 	//#region Workspace Management
 
-	async enterWorkspace(path: URI): Promise<IEnterWorkspaceResult | null> {
+	async enterWorkspace(path: URI): Promise<IEnterWorkspaceResult | undefined> {
 		return { workspace: await this.getWorkspaceIdentifier(path) };
 	}
 

src/vs/workbench/test/browser/workbenchTestServices.ts

@@ -1560,7 +1560,7 @@ export class TestWorkspacesService implements IWorkspacesService {
 	async clearRecentlyOpened(): Promise<void> { }
 	async getRecentlyOpened(): Promise<IRecentlyOpened> { return { files: [], workspaces: [] }; }
 	async getDirtyWorkspaces(): Promise<(URI | IWorkspaceIdentifier)[]> { return []; }
-	async enterWorkspace(path: URI): Promise<IEnterWorkspaceResult | null> { throw new Error('Method not implemented.'); }
+	async enterWorkspace(path: URI): Promise<IEnterWorkspaceResult | undefined> { throw new Error('Method not implemented.'); }
 	async getWorkspaceIdentifier(workspacePath: URI): Promise<IWorkspaceIdentifier> { throw new Error('Method not implemented.'); }
 }