Skip to content

Commit

Permalink
Strongly typed instead of any for nb config (#209493)
Browse files Browse the repository at this point in the history
* Strongly types instead of `any` for nb config

* fix swapped setting fallbacks

* Address review comments

---------

Co-authored-by: Michael Lively <[email protected]>
  • Loading branch information
DonJayamanne and Yoyokrazy authored Apr 4, 2024
1 parent 95885d7 commit a64b518
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,9 @@ export class NotebookIndentationStatus extends Disposable implements IWorkbenchC
}

const cellEditorOverridesRaw = editor.notebookOptions.getDisplayOptions().editorOptionsCustomizations;
const indentSize = cellEditorOverridesRaw['editor.indentSize'] ?? cellOptions?.indentSize;
const insertSpaces = cellEditorOverridesRaw['editor.insertSpaces'] ?? cellOptions?.tabSize;
const tabSize = cellEditorOverridesRaw['editor.tabSize'] ?? cellOptions?.insertSpaces;
const indentSize = cellEditorOverridesRaw?.['editor.indentSize'] ?? cellOptions?.indentSize;
const insertSpaces = cellEditorOverridesRaw?.['editor.insertSpaces'] ?? cellOptions?.insertSpaces;
const tabSize = cellEditorOverridesRaw?.['editor.tabSize'] ?? cellOptions?.tabSize;

const width = typeof indentSize === 'number' ? indentSize : tabSize;

Expand Down
14 changes: 12 additions & 2 deletions src/vs/workbench/contrib/notebook/browser/notebookOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { PixelRatio } from 'vs/base/browser/pixelRatio';
import { CodeWindow } from 'vs/base/browser/window';
import { Emitter } from 'vs/base/common/event';
import { Disposable } from 'vs/base/common/lifecycle';
import { isObject } from 'vs/base/common/types';
import { URI } from 'vs/base/common/uri';
import { FontMeasurements } from 'vs/editor/browser/config/fontMeasurements';
import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService';
Expand Down Expand Up @@ -45,7 +46,11 @@ export interface NotebookDisplayOptions { // TODO @Yoyokrazy rename to a more ge
outputFontFamily: string;
outputLineHeight: number;
markupFontSize: number;
editorOptionsCustomizations: any | undefined;
editorOptionsCustomizations: Partial<{
'editor.indentSize': 'tabSize' | number;
'editor.tabSize': number;
'editor.insertSpaces': boolean;
}> | undefined;
}

export interface NotebookLayoutConfiguration {
Expand Down Expand Up @@ -152,7 +157,12 @@ export class NotebookOptions extends Disposable {
// const { bottomToolbarGap, bottomToolbarHeight } = this._computeBottomToolbarDimensions(compactView, insertToolbarPosition, insertToolbarAlignment);
const fontSize = this.configurationService.getValue<number>('editor.fontSize');
const markupFontSize = this.configurationService.getValue<number>(NotebookSetting.markupFontSize);
const editorOptionsCustomizations = this.configurationService.getValue(NotebookSetting.cellEditorOptionsCustomizations);
let editorOptionsCustomizations = this.configurationService.getValue<Partial<{
'editor.indentSize': 'tabSize' | number;
'editor.tabSize': number;
'editor.insertSpaces': boolean;
}>>(NotebookSetting.cellEditorOptionsCustomizations) ?? {};
editorOptionsCustomizations = isObject(editorOptionsCustomizations) ? editorOptionsCustomizations : {};
const interactiveWindowCollapseCodeCells: InteractiveWindowCollapseCodeCells = this.configurationService.getValue(NotebookSetting.interactiveWindowCollapseCodeCells);

// TOOD @rebornix remove after a few iterations of deprecated setting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,15 @@ export class CellEditorOptions extends CellContentPart implements ITextModelUpda

// TODO @Yoyokrazy find a different way to get the editor overrides, this is not the right way
const cellEditorOverridesRaw = this.notebookOptions.getDisplayOptions().editorOptionsCustomizations;
const indentSize = cellEditorOverridesRaw['editor.indentSize'];
const indentSize = cellEditorOverridesRaw?.['editor.indentSize'];
if (indentSize !== undefined) {
this.indentSize = indentSize;
}
const insertSpaces = cellEditorOverridesRaw['editor.insertSpaces'];
const insertSpaces = cellEditorOverridesRaw?.['editor.insertSpaces'];
if (insertSpaces !== undefined) {
this.insertSpaces = insertSpaces;
}
const tabSize = cellEditorOverridesRaw['editor.tabSize'];
const tabSize = cellEditorOverridesRaw?.['editor.tabSize'];
if (tabSize !== undefined) {
this.tabSize = tabSize;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,13 @@ export class BaseCellEditorOptions extends Disposable implements IBaseCellEditor

private _computeEditorOptions() {
const editorOptions = deepClone(this.configurationService.getValue<IEditorOptions>('editor', { overrideIdentifier: this.language }));
const editorOptionsOverrideRaw = this.notebookOptions.getDisplayOptions().editorOptionsCustomizations ?? {};
const editorOptionsOverride: { [key: string]: any } = {};
for (const key in editorOptionsOverrideRaw) {
if (key.indexOf('editor.') === 0) {
editorOptionsOverride[key.substring(7)] = editorOptionsOverrideRaw[key];
const editorOptionsOverrideRaw = this.notebookOptions.getDisplayOptions().editorOptionsCustomizations;
const editorOptionsOverride: Record<string, any> = {};
if (editorOptionsOverrideRaw) {
for (const key in editorOptionsOverrideRaw) {
if (key.indexOf('editor.') === 0) {
editorOptionsOverride[key.substring(7)] = editorOptionsOverrideRaw[key as keyof typeof editorOptionsOverrideRaw];
}
}
}
const computed = Object.freeze({
Expand Down
1 change: 0 additions & 1 deletion src/vs/workbench/contrib/notebook/common/notebookCommon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,6 @@ export interface INotebookCellStatusBarItemList {
}

export type ShowCellStatusBarType = 'hidden' | 'visible' | 'visibleAfterExecute';

export const NotebookSetting = {
displayOrder: 'notebook.displayOrder',
cellToolbarLocation: 'notebook.cellToolbarLocation',
Expand Down

0 comments on commit a64b518

Please sign in to comment.