Save variable explorer height persistently (microsoft#12086)

* started working * redux setup done * mostly working * clean code * small refactors * communication done * finished linking * formatted * changed to local storage * some small bugs - mostly working * some small bugs * working! * clean up * cleaned up code * fixed linting bugs * fixed broken tests * removed duplicated comment * addressed comments * added sourceUri to store old filename * fixed untitled persistence save * made untitled notebooks NOT restore previous height * fixed comments * started working * redux setup done * mostly working * clean code * small refactors * communication done * finished linking * formatted * changed to local storage * some small bugs - mostly working * some small bugs * working! * clean up * cleaned up code * fixed linting bugs * fixed broken tests * removed duplicated comment * addressed comments * added sourceUri to store old filename * fixed untitled persistence save * made untitled notebooks NOT restore previous height * fixed comments * added export * fixed

Author: techwithtim

src/client/datascience/interactive-common/interactiveBase.ts

@@ -62,8 +62,10 @@ import {
  IRemoteReexecuteCode,
  IShowDataViewer,
  ISubmitNewCell,
- SysInfoReason
+ SysInfoReason,
+ VariableExplorerStateKeys
 } from '../interactive-common/interactiveWindowTypes';
+import { isUntitledFile } from '../interactive-ipynb/nativeEditorStorage';
 import { JupyterInvalidKernelError } from '../jupyter/jupyterInvalidKernelError';
 import { JupyterKernelPromiseFailedError } from '../jupyter/kernels/jupyterKernelPromiseFailedError';
 import { KernelSwitcher } from '../jupyter/kernels/kernelSwitcher';
@@ -144,6 +146,7 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
  @unmanaged() protected errorHandler: IDataScienceErrorHandler,
  @unmanaged() protected readonly commandManager: ICommandManager,
  @unmanaged() protected globalStorage: Memento,
+ @unmanaged() protected workspaceStorage: Memento,
  @unmanaged() rootPath: string,
  @unmanaged() scripts: string[],
  @unmanaged() title: string,
@@ -228,6 +231,14 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
  // Send the loc strings (skip during testing as it takes up a lot of memory)
  const locStrings = isTestExecution() ? '{}' : localize.getCollectionJSON();
  this.postMessageInternal(SharedMessages.LocInit, locStrings).ignoreErrors();
+ this.variableExplorerHeightRequest()
+ .then((data) =>
+ this.postMessageInternal(
+ InteractiveWindowMessages.VariableExplorerHeightResponse,
+ data
+ ).ignoreErrors()
+ )
+ .catch(); // do nothing
  break;
 
  case InteractiveWindowMessages.GotoCodeCell:
@@ -278,6 +289,10 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
  this.variableExplorerToggle(payload);
  break;
 
+ case InteractiveWindowMessages.SetVariableExplorerHeight:
+ this.setVariableExplorerHeight(payload).ignoreErrors();
+ break;
+
  case InteractiveWindowMessages.AddedSysInfo:
  this.handleMessage(message, payload, this.onAddedSysInfo);
  break;
@@ -1388,6 +1403,44 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
  }
  };
 
+ // tslint:disable-next-line: no-any
+ private async setVariableExplorerHeight(payload?: any) {
+ // Store variable explorer height based on file name in workspace storage
+ if (payload !== undefined) {
+ const updatedHeights = payload as { containerHeight: number; gridHeight: number };
+ const uri = await this.getOwningResource(); // Get file name
+
+ if (!uri) {
+ return;
+ }
+ // Storing an object that looks like
+ // { "fully qualified Path to 1.ipynb": 1234,
+ // "fully qualifieid path to 2.ipynb": 1234 }
+
+ // tslint:disable-next-line: no-any
+ const value = this.workspaceStorage.get(VariableExplorerStateKeys.height, {} as any);
+ value[uri.toString()] = updatedHeights;
+ this.workspaceStorage.update(VariableExplorerStateKeys.height, value);
+ }
+ }
+
+ private async variableExplorerHeightRequest(): Promise<
+ { containerHeight: number; gridHeight: number } | undefined
+ > {
+ const uri = await this.getOwningResource(); // Get file name
+
+ if (!uri || isUntitledFile(uri)) {
+ return; // don't resotre height of untitled notebooks
+ }
+
+ // tslint:disable-next-line: no-any
+ const value = this.workspaceStorage.get(VariableExplorerStateKeys.height, {} as any);
+ const uriString = uri.toString();
+ if (uriString in value) {
+ return value[uriString];
+ }
+ }
+
  private async requestTmLanguage(languageId: string) {
  // Get the contents of the appropriate tmLanguage file.
  traceInfo('Request for tmlanguage file.');

src/client/datascience/interactive-common/interactiveWindowTypes.ts

@@ -11,6 +11,7 @@ import {
  CommonActionType,
  IAddCellAction,
  ILoadIPyWidgetClassFailureAction,
+ IVariableExplorerHeight,
  LoadIPyWidgetClassLoadAction,
  NotifyIPyWidgeWidgetVersionNotSupportedAction
 } from '../../../datascience-ui/interactive-common/redux/reducers/types';
@@ -68,6 +69,8 @@ export enum InteractiveWindowMessages {
  GetVariablesRequest = 'get_variables_request',
  GetVariablesResponse = 'get_variables_response',
  VariableExplorerToggle = 'variable_explorer_toggle',
+ SetVariableExplorerHeight = 'set_variable_explorer_height',
+ VariableExplorerHeightResponse = 'variable_explorer_height_response',
  ForceVariableRefresh = 'force_variable_refresh',
  ProvideCompletionItemsRequest = 'provide_completion_items_request',
  CancelCompletionItemsRequest = 'cancel_completion_items_request',
@@ -177,6 +180,10 @@ export interface ICopyCode {
  source: string;
 }
 
+export enum VariableExplorerStateKeys {
+ height = 'NBVariableHeights'
+}
+
 export enum SysInfoReason {
  Start,
  Restart,
@@ -351,6 +358,7 @@ export interface INotebookModelSaved extends INotebookModelChange {
 export interface INotebookModelSavedAs extends INotebookModelChange {
  kind: 'saveAs';
  target: Uri;
+ sourceUri: Uri;
 }
 
 export interface INotebookModelRemoveAllChange extends INotebookModelChange {
@@ -561,6 +569,8 @@ export class IInteractiveWindowMapping {
  public [InteractiveWindowMessages.GetVariablesRequest]: IJupyterVariablesRequest;
  public [InteractiveWindowMessages.GetVariablesResponse]: IJupyterVariablesResponse;
  public [InteractiveWindowMessages.VariableExplorerToggle]: boolean;
+ public [InteractiveWindowMessages.SetVariableExplorerHeight]: IVariableExplorerHeight;
+ public [InteractiveWindowMessages.VariableExplorerHeightResponse]: IVariableExplorerHeight;
  public [CssMessages.GetCssRequest]: IGetCssRequest;
  public [CssMessages.GetCssResponse]: IGetCssResponse;
  public [CssMessages.GetMonacoThemeRequest]: IGetMonacoThemeRequest;

src/client/datascience/interactive-common/synchronization.ts

@@ -84,6 +84,7 @@ const messageWithMessageTypes: MessageMapping<IInteractiveWindowMapping> & Messa
  [CommonActionType.TOGGLE_LINE_NUMBERS]: MessageType.syncWithLiveShare,
  [CommonActionType.TOGGLE_OUTPUT]: MessageType.syncWithLiveShare,
  [CommonActionType.TOGGLE_VARIABLE_EXPLORER]: MessageType.syncWithLiveShare,
+ [CommonActionType.SET_VARIABLE_EXPLORER_HEIGHT]: MessageType.other,
  [CommonActionType.UNFOCUS_CELL]: MessageType.syncWithLiveShare,
  [CommonActionType.UNMOUNT]: MessageType.other,
  [CommonActionType.PostOutgoingMessage]: MessageType.other,
@@ -193,6 +194,8 @@ const messageWithMessageTypes: MessageMapping<IInteractiveWindowMapping> & Messa
  [InteractiveWindowMessages.UpdateKernel]: MessageType.syncAcrossSameNotebooks | MessageType.syncWithLiveShare,
  [InteractiveWindowMessages.UpdateDisplayData]: MessageType.syncWithLiveShare,
  [InteractiveWindowMessages.VariableExplorerToggle]: MessageType.other,
+ [InteractiveWindowMessages.SetVariableExplorerHeight]: MessageType.other,
+ [InteractiveWindowMessages.VariableExplorerHeightResponse]: MessageType.other,
  [InteractiveWindowMessages.VariablesComplete]: MessageType.other,
  [InteractiveWindowMessages.ConvertUriForUseInWebViewRequest]: MessageType.other,
  [InteractiveWindowMessages.ConvertUriForUseInWebViewResponse]: MessageType.other,

src/client/datascience/interactive-ipynb/nativeEditor.ts

@@ -36,7 +36,8 @@ import {
  IDisposableRegistry,
  IExperimentsManager,
  IMemento,
- Resource
+ Resource,
+ WORKSPACE_MEMENTO
 } from '../../common/types';
 import { createDeferred, Deferred } from '../../common/utils/async';
 import * as localize from '../../common/utils/localize';
@@ -54,7 +55,8 @@ import {
  IRunByLine,
  ISubmitNewCell,
  NotebookModelChange,
- SysInfoReason
+ SysInfoReason,
+ VariableExplorerStateKeys
 } from '../interactive-common/interactiveWindowTypes';
 import {
  CellState,
@@ -174,6 +176,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
  @inject(INotebookImporter) protected readonly importer: INotebookImporter,
  @inject(IDataScienceErrorHandler) errorHandler: IDataScienceErrorHandler,
  @inject(IMemento) @named(GLOBAL_MEMENTO) globalStorage: Memento,
+ @inject(IMemento) @named(WORKSPACE_MEMENTO) workspaceStorage: Memento,
  @inject(IExperimentsManager) experimentsManager: IExperimentsManager,
  @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry,
  @inject(KernelSwitcher) switcher: KernelSwitcher,
@@ -202,6 +205,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
  errorHandler,
  commandManager,
  globalStorage,
+ workspaceStorage,
  nativeEditorDir,
  [
  path.join(nativeEditorDir, 'require.js'),
@@ -560,6 +564,15 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
  // VS code is telling us to broadcast this to our UI. Tell the UI about the new change
  await this.postMessage(InteractiveWindowMessages.UpdateModel, change);
  }
+ if (change.kind === 'saveAs' && change.model) {
+ const newFileName = change.model.file.toString();
+ const oldFileName = change.sourceUri.toString();
+
+ if (newFileName !== oldFileName) {
+ // If the filename has changed
+ this.renameVariableExplorerHeights(oldFileName, newFileName);
+ }
+ }
 
  // Use the current state of the model to indicate dirty (not the message itself)
  if (this.model && change.newDirty !== change.oldDirty) {
@@ -572,6 +585,21 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
  }
  }
  }
+
+ private renameVariableExplorerHeights(name: string, updatedName: string) {
+ // Updates the workspace storage to reflect the updated name of the notebook
+ // should be called if the name of the notebook changes
+ // tslint:disable-next-line: no-any
+ const value = this.workspaceStorage.get(VariableExplorerStateKeys.height, {} as any);
+ if (!(name in value)) {
+ return; // Nothing to update
+ }
+
+ value[updatedName] = value[name];
+ delete value[name];
+ this.workspaceStorage.update(VariableExplorerStateKeys.height, value);
+ }
+
  private interruptExecution() {
  this.executeCancelTokens.forEach((t) => t.cancel());
  }

src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts

@@ -24,7 +24,8 @@ import {
  IConfigurationService,
  IDisposableRegistry,
  IExperimentsManager,
- IMemento
+ IMemento,
+ WORKSPACE_MEMENTO
 } from '../../common/types';
 import * as localize from '../../common/utils/localize';
 import { noop } from '../../common/utils/misc';
@@ -96,6 +97,7 @@ export class NativeEditorOldWebView extends NativeEditor {
  @inject(INotebookImporter) importer: INotebookImporter,
  @inject(IDataScienceErrorHandler) errorHandler: IDataScienceErrorHandler,
  @inject(IMemento) @named(GLOBAL_MEMENTO) globalStorage: Memento,
+ @inject(IMemento) @named(WORKSPACE_MEMENTO) workspaceStorage: Memento,
  @inject(IExperimentsManager) experimentsManager: IExperimentsManager,
  @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry,
  @inject(KernelSwitcher) switcher: KernelSwitcher,
@@ -128,6 +130,7 @@ export class NativeEditorOldWebView extends NativeEditor {
  importer,
  errorHandler,
  globalStorage,
+ workspaceStorage,
  experimentsManager,
  asyncRegistry,
  switcher,

src/client/datascience/interactive-ipynb/nativeEditorStorage.ts

@@ -35,7 +35,7 @@ interface INativeEditorStorageState {
  notebookJson: Partial<nbformat.INotebookContent>;
 }
 
-function isUntitledFile(file?: Uri) {
+export function isUntitledFile(file?: Uri) {
  return file?.scheme === 'untitled';
 }
 export function isUntitled(model?: INotebookModel): boolean {
@@ -528,7 +528,8 @@ export class NativeEditorStorage implements INotebookStorage {
  kind: 'saveAs',
  oldDirty: model.isDirty,
  newDirty: false,
- target: file
+ target: file,
+ sourceUri: model.file
  });
  this.savedAs.fire({ new: file, old });
  this.clearHotExit(old).ignoreErrors();

src/client/datascience/interactive-window/interactiveWindow.ts

@@ -25,7 +25,8 @@ import {
  IExperimentsManager,
  IMemento,
  IPersistentStateFactory,
- Resource
+ Resource,
+ WORKSPACE_MEMENTO
 } from '../../common/types';
 import * as localize from '../../common/utils/localize';
 import { EXTENSION_ROOT_DIR } from '../../constants';
@@ -110,6 +111,7 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi
  @inject(IDataScienceErrorHandler) errorHandler: IDataScienceErrorHandler,
  @inject(IPersistentStateFactory) private readonly stateFactory: IPersistentStateFactory,
  @inject(IMemento) @named(GLOBAL_MEMENTO) globalStorage: Memento,
+ @inject(IMemento) @named(WORKSPACE_MEMENTO) workspaceStorage: Memento,
  @inject(IExperimentsManager) experimentsManager: IExperimentsManager,
  @inject(KernelSwitcher) switcher: KernelSwitcher,
  @inject(INotebookProvider) notebookProvider: INotebookProvider,
@@ -137,6 +139,7 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi
  errorHandler,
  commandManager,
  globalStorage,
+ workspaceStorage,
  historyReactDir,
  [
  path.join(historyReactDir, 'require.js'),

src/datascience-ui/history-react/interactivePanel.tsx

@@ -307,13 +307,16 @@ ${buildSettingsCss(this.props.settings)}`}</style>
  toolbarHeight = this.mainPanelToolbarRef.current.offsetHeight;
  }
  return {
+ gridHeight: this.props.variableState.gridHeight,
+ containerHeight: this.props.variableState.containerHeight,
  variables: this.props.variableState.variables,
  debugging: this.props.debugging,
  busy: this.props.busy,
  showDataExplorer: this.props.showDataViewer,
  skipDefault: this.props.skipDefault,
  testMode: this.props.testMode,
  closeVariableExplorer: this.props.toggleVariableExplorer,
+ setVariableExplorerHeight: this.props.setVariableExplorerHeight,
  baseTheme: baseTheme,
  pageIn: this.pageInVariableData,
  fontSize: this.props.font.size,

src/datascience-ui/history-react/redux/actions.ts

@@ -18,7 +18,8 @@ import {
  ILinkClickAction,
  IOpenSettingsAction,
  IScrollAction,
- IShowDataViewerAction
+ IShowDataViewerAction,
+ IVariableExplorerHeight
 } from '../../interactive-common/redux/reducers/types';
 import { IMonacoModelContentChangeEvent } from '../../react-common/monacoHelpers';
 
@@ -73,6 +74,8 @@ export const actionCreators = {
  submitInput: (code: string, cellId: string): CommonAction<ICodeAction> =>
  createIncomingActionWithPayload(CommonActionType.SUBMIT_INPUT, { code, cellId }),
  toggleVariableExplorer: (): CommonAction => createIncomingAction(CommonActionType.TOGGLE_VARIABLE_EXPLORER),
+ setVariableExplorerHeight: (containerHeight: number, gridHeight: number): CommonAction<IVariableExplorerHeight> =>
+ createIncomingActionWithPayload(CommonActionType.SET_VARIABLE_EXPLORER_HEIGHT, { containerHeight, gridHeight }),
  expandAll: (): CommonAction => createIncomingAction(InteractiveWindowMessages.ExpandAll),
  collapseAll: (): CommonAction => createIncomingAction(InteractiveWindowMessages.CollapseAll),
  export: (): CommonAction => createIncomingAction(CommonActionType.EXPORT),

src/datascience-ui/interactive-common/redux/reducers/types.ts

@@ -24,7 +24,6 @@ import { CursorPos, IMainState } from '../../mainState';
  * 5) Add a new handler for the action under the 'reducer's folder. Handle the expected state change
  * 6) Add the handler to the main reducer map in reducers\index.ts
  */
-
 export enum CommonActionType {
  ADD_AND_FOCUS_NEW_CELL = 'action.add_new_cell_and_focus_cell',
  INSERT_ABOVE_AND_FOCUS_NEW_CELL = 'action.insert_above_and_focus_cell',
@@ -74,6 +73,7 @@ export enum CommonActionType {
  SCROLL = 'action.scroll',
  SELECT_CELL = 'action.select_cell',
  SELECT_SERVER = 'action.select_server',
+ SET_VARIABLE_EXPLORER_HEIGHT = 'action.set_variable_explorer_height',
  SEND_COMMAND = 'action.send_command',
  SHOW_DATA_VIEWER = 'action.show_data_viewer',
  STEP = 'action.step',
@@ -134,6 +134,7 @@ export type CommonActionTypeMapping = {
  [CommonActionType.CODE_CREATED]: ICodeCreatedAction;
  [CommonActionType.GET_VARIABLE_DATA]: IJupyterVariablesRequest;
  [CommonActionType.TOGGLE_VARIABLE_EXPLORER]: never | undefined;
+ [CommonActionType.SET_VARIABLE_EXPLORER_HEIGHT]: IVariableExplorerHeight;
  [CommonActionType.PostOutgoingMessage]: never | undefined;
  [CommonActionType.REFRESH_VARIABLES]: never | undefined;
  [CommonActionType.OPEN_SETTINGS]: IOpenSettingsAction;
@@ -230,6 +231,12 @@ export interface ILoadIPyWidgetClassFailureAction {
  error: any;
  timedout: boolean;
 }
+
+export interface IVariableExplorerHeight {
+ containerHeight: number;
+ gridHeight: number;
+}
+
 export type LoadIPyWidgetClassDisabledAction = {
  className: string;
  moduleName: string;