Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
212 changes: 106 additions & 106 deletions src/client/datascience/interactive-common/synchronization.ts

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions src/client/datascience/interactive-common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

'use strict';

import { CommonActionType } from '../../../datascience-ui/interactive-common/redux/reducers/types';
import { CssMessages, SharedMessages } from '../messages';
import { InteractiveWindowMessages } from './interactiveWindowTypes';
import { MessageType } from './synchronization';

// Stuff common to React and Extensions.
Expand Down Expand Up @@ -32,3 +35,8 @@ export type BaseReduxActionPayload<T = never | undefined> = T extends never
? BaseData
: BaseDataWithPayload<T>
: BaseDataWithPayload<T>;
export type SyncPayload = {
type: InteractiveWindowMessages | SharedMessages | CommonActionType | CssMessages;
// tslint:disable-next-line: no-any
payload: BaseReduxActionPayload<any>;
};
8 changes: 8 additions & 0 deletions src/client/datascience/interactive-ipynb/nativeEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ import {
IThemeFinder,
WebViewViewChangeEventArgs
} from '../types';
import { NativeEditorSynchronizer } from './nativeEditorSynchronizer';

import { nbformat } from '@jupyterlab/coreutils';
// tslint:disable-next-line: no-require-imports
Expand Down Expand Up @@ -164,6 +165,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
@inject(ICommandManager) commandManager: ICommandManager,
@inject(INotebookExporter) jupyterExporter: INotebookExporter,
@inject(IWorkspaceService) workspaceService: IWorkspaceService,
@inject(NativeEditorSynchronizer) private readonly synchronizer: NativeEditorSynchronizer,
@inject(INotebookEditorProvider) private editorProvider: INotebookEditorProvider,
@inject(IDataViewerProvider) dataExplorerProvider: IDataViewerProvider,
@inject(IJupyterVariables) jupyterVariables: IJupyterVariables,
Expand Down Expand Up @@ -211,6 +213,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
switcher
);
asyncRegistry.push(this);

this.synchronizer.subscribeToUserActions(this, this.postMessage.bind(this));
}

public dispose(): Promise<void> {
Expand Down Expand Up @@ -247,6 +251,10 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
public onMessage(message: string, payload: any) {
super.onMessage(message, payload);
switch (message) {
case InteractiveWindowMessages.Sync:
this.synchronizer.notifyUserAction(payload, this);
break;

case InteractiveWindowMessages.ReExecuteCells:
this.executedEvent.fire(this);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
} from '../types';
import { NativeEditor } from './nativeEditor';
import { NativeEditorStorage } from './nativeEditorStorage';
import { NativeEditorSynchronizer } from './nativeEditorSynchronizer';

enum AskForSaveResult {
Yes,
Expand Down Expand Up @@ -85,6 +86,7 @@ export class NativeEditorOldWebView extends NativeEditor {
@inject(ICommandManager) commandManager: ICommandManager,
@inject(INotebookExporter) jupyterExporter: INotebookExporter,
@inject(IWorkspaceService) workspaceService: IWorkspaceService,
@inject(NativeEditorSynchronizer) synchronizer: NativeEditorSynchronizer,
@inject(INotebookEditorProvider) editorProvider: INotebookEditorProvider,
@inject(IDataViewerProvider) dataExplorerProvider: IDataViewerProvider,
@inject(IJupyterVariables) jupyterVariables: IJupyterVariables,
Expand Down Expand Up @@ -114,6 +116,7 @@ export class NativeEditorOldWebView extends NativeEditor {
commandManager,
jupyterExporter,
workspaceService,
synchronizer,
editorProvider,
dataExplorerProvider,
jupyterVariables,
Expand All @@ -127,6 +130,8 @@ export class NativeEditorOldWebView extends NativeEditor {
switcher
);
asyncRegistry.push(this);
// No ui syncing in old notebooks.
synchronizer.disable();
}
public async load(model: INotebookModel, webViewPanel: WebviewPanel): Promise<void> {
await super.load(model, webViewPanel);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

'use strict';

import { inject, injectable } from 'inversify';
import { IFileSystem } from '../../common/platform/types';
import { IInteractiveWindowMapping, InteractiveWindowMessages } from '../interactive-common/interactiveWindowTypes';
import { SyncPayload } from '../interactive-common/types';
import { INotebookEditor } from '../types';

// tslint:disable: no-any

type UserActionNotificationCallback = <M extends IInteractiveWindowMapping, T extends keyof M>(
type: T,
payload?: M[T]
) => void;

@injectable()
export class NativeEditorSynchronizer {
private registeredNotebooks = new Map<INotebookEditor, UserActionNotificationCallback>();
private enabled = true;
constructor(@inject(IFileSystem) private readonly fs: IFileSystem) {}
public notifyUserAction(message: SyncPayload, editor: INotebookEditor) {
if (!this.enabled) {
return;
}
this.registeredNotebooks.forEach((cb, item) => {
if (item !== editor && this.fs.arePathsSame(item.file.fsPath, editor.file.fsPath)) {
cb(InteractiveWindowMessages.Sync, message as any);
}
});
}
public subscribeToUserActions(editor: INotebookEditor, cb: UserActionNotificationCallback) {
this.registeredNotebooks.set(editor, cb);
}
public disable() {
this.enabled = false;
this.registeredNotebooks.clear();
}
}
4 changes: 2 additions & 2 deletions src/client/datascience/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import { NativeEditorOldWebView } from './interactive-ipynb/nativeEditorOldWebVi
import { NativeEditorProvider } from './interactive-ipynb/nativeEditorProvider';
import { NativeEditorProviderOld } from './interactive-ipynb/nativeEditorProviderOld';
import { NativeEditorStorage } from './interactive-ipynb/nativeEditorStorage';
import { NativeEditorSynchronizer } from './interactive-ipynb/nativeEditorSynchronizer';
import { InteractiveWindow } from './interactive-window/interactiveWindow';
import { InteractiveWindowCommandListener } from './interactive-window/interactiveWindowCommandListener';
import { InteractiveWindowProvider } from './interactive-window/interactiveWindowProvider';
Expand Down Expand Up @@ -158,8 +159,6 @@ export function registerTypes(serviceManager: IServiceManager) {
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, JupyterInterpreterSelectionCommand);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, PreWarmActivatedJupyterEnvironmentVariables);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, ServerPreload);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, ServerPreload);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, ServerPreload);
serviceManager.addSingleton<IInteractiveWindowListener>(IInteractiveWindowListener, DataScienceSurveyBannerLogger);
serviceManager.addSingleton<IInteractiveWindowProvider>(IInteractiveWindowProvider, InteractiveWindowProvider);
serviceManager.addSingleton<IJupyterDebugger>(IJupyterDebugger, JupyterDebugger, undefined, [ICellHashListener]);
Expand Down Expand Up @@ -188,6 +187,7 @@ export function registerTypes(serviceManager: IServiceManager) {
serviceManager.addSingleton<KernelSwitcherCommand>(KernelSwitcherCommand, KernelSwitcherCommand);
serviceManager.addSingleton<NotebookStarter>(NotebookStarter, NotebookStarter);
serviceManager.addSingleton<ProgressReporter>(ProgressReporter, ProgressReporter);
serviceManager.addSingleton<NativeEditorSynchronizer>(NativeEditorSynchronizer, NativeEditorSynchronizer);

// Temporary code, to allow users to revert to the old behavior.
const cfg = serviceManager.get<IWorkspaceService>(IWorkspaceService).getConfiguration('python.dataScience', undefined);
Expand Down
28 changes: 21 additions & 7 deletions src/datascience-ui/interactive-common/redux/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
MessageType,
shouldRebroadcast
} from '../../../client/datascience/interactive-common/synchronization';
import { BaseReduxActionPayload } from '../../../client/datascience/interactive-common/types';
import { BaseReduxActionPayload, SyncPayload } from '../../../client/datascience/interactive-common/types';
import { CssMessages, SharedMessages } from '../../../client/datascience/messages';
import { QueueAnotherFunc } from '../../react-common/reduxUtils';
import { CommonActionType, CommonActionTypeMapping } from './reducers/types';
Expand Down Expand Up @@ -87,7 +87,7 @@ export function postActionToExtension(originalReducerArg: ReducerArg, message: a
const newPayload: BaseReduxActionPayload<any> = ({
data: payload,
messageDirection: 'outgoing',
messageType: MessageType.userAction
messageType: MessageType.other
// tslint:disable-next-line: no-any
} as any) as BaseReduxActionPayload<any>;
const action = { type: CommonActionType.PostOutgoingMessage, payload: { payload: newPayload, type: message } };
Expand All @@ -102,15 +102,29 @@ export function unwrapPostableAction(
return { type, payload };
}

/**
* Whether this is a message type that indicates it is part of a scynchronization message.
*/
export function isSyncingMessage(messageType?: MessageType) {
if (!messageType) {
return false;
}

return (
(messageType && MessageType.syncAcrossSameNotebooks) === MessageType.syncAcrossSameNotebooks ||
(messageType && MessageType.syncWithLiveShare) === MessageType.syncWithLiveShare
);
}
export function reBroadcastMessageIfRequired(
dispatcher: Function,
message: InteractiveWindowMessages | SharedMessages | CommonActionType | CssMessages,
payload?: BaseReduxActionPayload<{}>
) {
const messageType = payload?.messageType || 0;
if (
message === InteractiveWindowMessages.Sync ||
payload?.messageType === MessageType.syncAcrossSameNotebooks ||
payload?.messageType === MessageType.syncWithLiveShare ||
(messageType && MessageType.syncAcrossSameNotebooks) === MessageType.syncAcrossSameNotebooks ||
(messageType && MessageType.syncWithLiveShare) === MessageType.syncWithLiveShare ||
payload?.messageDirection === 'outgoing'
) {
return;
Expand All @@ -127,8 +141,8 @@ export function reBroadcastMessageIfRequired(
messageDirection: 'incoming'
};
// tslint:disable-next-line: no-any
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you can ditch the no-any now.

const syncPayload = { type: message, payload: syncPayloadData } as any;
// Send this out.
dispatcher(InteractiveWindowMessages.Sync, syncPayload);
const syncPayload: SyncPayload = { type: message, payload: syncPayloadData };
// First focus on UX perf, hence the setTimeout (i.e. ensure other code in event loop executes).
setTimeout(() => dispatcher(InteractiveWindowMessages.Sync, syncPayload), 1);
}
}
9 changes: 1 addition & 8 deletions src/datascience-ui/interactive-common/redux/postOffice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,7 @@ export function generatePostOfficeSendReducer(postOffice: PostOffice): Redux.Red
// Do not rebroadcast messages that have been sent through as part of a synchronization packet.
// If `messageType` is a number, then its some part of a synchronization packet.
if (payload?.messageDirection === 'incoming') {
// We can delay this, first focus on UX perf.
setTimeout(() => {
reBroadcastMessageIfRequired(
postOffice.sendMessage.bind(postOffice),
action.type,
action?.payload
);
}, 1);
reBroadcastMessageIfRequired(postOffice.sendMessage.bind(postOffice), action.type, action?.payload);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import { CssMessages } from '../../../../client/datascience/messages';
import { ICell } from '../../../../client/datascience/types';
import { extractInputText, getSelectedAndFocusedInfo, IMainState } from '../../mainState';
import { postActionToExtension } from '../helpers';
import { isSyncingMessage, postActionToExtension } from '../helpers';
import { Helpers } from './helpers';
import {
CommonActionType,
Expand Down Expand Up @@ -231,7 +231,11 @@ export namespace Transfer {
// when focus is lost
const index = arg.prevState.cellVMs.findIndex(c => c.cell.id === arg.payload.data.cellId);
const selectionInfo = getSelectedAndFocusedInfo(arg.prevState);
if (index >= 0 && selectionInfo.focusedCellId === arg.payload.data.cellId) {
// If this is the focused cell, then user is editing it, hence it needs to be updated.
const isThisTheFocusedCell = selectionInfo.focusedCellId === arg.payload.data.cellId;
// If this edit is part of a sycning comging from another notebook, then we need to update it again.
const isSyncFromAnotherNotebook = isSyncingMessage(arg.payload.messageType);
if (index >= 0 && (isThisTheFocusedCell || isSyncFromAnotherNotebook)) {
const newVMs = [...arg.prevState.cellVMs];
const current = arg.prevState.cellVMs[index];
const newCell = {
Expand Down
6 changes: 4 additions & 2 deletions src/datascience-ui/interactive-common/redux/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function createSendInfoMiddleware(): Redux.Middleware<{}, IStore> {
const afterState = store.getState();

// If the action is part of a sync message, then do not send it to the extension.
const messageType = (action?.payload as BaseReduxActionPayload).messageType ?? MessageType.userAction;
const messageType = (action?.payload as BaseReduxActionPayload).messageType ?? MessageType.other;
const isSyncMessage =
(messageType & MessageType.syncAcrossSameNotebooks) === MessageType.syncAcrossSameNotebooks &&
(messageType & MessageType.syncAcrossSameNotebooks) === MessageType.syncWithLiveShare;
Expand Down Expand Up @@ -341,11 +341,13 @@ export function createStore<M>(
// This is a message that has been sent from extension purely for synchronization purposes.
// Unwrap the message.
message = payload.type;
// This is a message that came in as a result of an outgoing message from another view.
basePayload.messageDirection = 'outgoing';
basePayload.messageType = payload.payload.messageType ?? MessageType.syncAcrossSameNotebooks;
basePayload.data = payload.payload.data;
} else {
// Messages result of some user action.
basePayload.messageType = basePayload.messageType ?? MessageType.userAction;
basePayload.messageType = basePayload.messageType ?? MessageType.other;
}
store.dispatch({ type: message, payload: basePayload });
}
Expand Down
5 changes: 4 additions & 1 deletion src/test/datascience/dataScienceIocContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ import { NativeEditor } from '../../client/datascience/interactive-ipynb/nativeE
import { NativeEditorCommandListener } from '../../client/datascience/interactive-ipynb/nativeEditorCommandListener';
import { NativeEditorOldWebView } from '../../client/datascience/interactive-ipynb/nativeEditorOldWebView';
import { NativeEditorStorage } from '../../client/datascience/interactive-ipynb/nativeEditorStorage';
import { NativeEditorSynchronizer } from '../../client/datascience/interactive-ipynb/nativeEditorSynchronizer';
import { InteractiveWindow } from '../../client/datascience/interactive-window/interactiveWindow';
import { InteractiveWindowCommandListener } from '../../client/datascience/interactive-window/interactiveWindowCommandListener';
import { JupyterCommandFactory } from '../../client/datascience/jupyter/interpreter/jupyterCommand';
Expand Down Expand Up @@ -1061,7 +1062,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
this.serviceManager.addSingleton<IJupyterPasswordConnect>(IJupyterPasswordConnect, JupyterPasswordConnect);
this.serviceManager.addSingleton<IProcessLogger>(IProcessLogger, ProcessLogger);
}

this.serviceManager.addSingleton<NativeEditorSynchronizer>(NativeEditorSynchronizer, NativeEditorSynchronizer);
// Disable syncrhonizing edits
this.serviceContainer.get<NativeEditorSynchronizer>(NativeEditorSynchronizer).disable();
const dummyDisposable = {
dispose: () => {
return;
Expand Down