Skip to content

Commit d0d356f

Browse files
rchiodorchiodo
authored
Fix undo in notebooks (#11167)
* Put back ctrl+z and 'z' for undo. Refactor to use one value to compute custom editor support * Add news entry
1 parent bd8d32b commit d0d356f

File tree

15 files changed

+47
-31
lines changed

15 files changed

+47
-31
lines changed

‎news/2 Fixes/11160.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix CTRL+Z and Z for undo on notebooks.

‎src/client/datascience/data-viewing/dataViewer.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import * as path from 'path';
88
import { ViewColumn } from 'vscode';
99

1010
import { IApplicationShell, IWebPanelProvider, IWorkspaceService } from '../../common/application/types';
11-
import { EXTENSION_ROOT_DIR } from '../../common/constants';
11+
import { EXTENSION_ROOT_DIR, UseCustomEditorApi } from '../../common/constants';
1212
import { WebHostNotebook } from '../../common/experimentGroups';
1313
import { traceError } from '../../common/logger';
1414
import { IConfigurationService, IDisposable, IExperimentsManager, Resource } from '../../common/types';
@@ -39,7 +39,8 @@ export class DataViewer extends WebViewHost<IDataViewerMapping> implements IData
3939
@inject(IWorkspaceService) workspaceService: IWorkspaceService,
4040
@inject(IJupyterVariables) private variableManager: IJupyterVariables,
4141
@inject(IApplicationShell) private applicationShell: IApplicationShell,
42-
@inject(IExperimentsManager) experimentsManager: IExperimentsManager
42+
@inject(IExperimentsManager) experimentsManager: IExperimentsManager,
43+
@inject(UseCustomEditorApi) useCustomEditorApi: boolean
4344
) {
4445
super(
4546
configuration,
@@ -52,7 +53,8 @@ export class DataViewer extends WebViewHost<IDataViewerMapping> implements IData
5253
[path.join(dataExplorereDir, 'commons.initial.bundle.js'), path.join(dataExplorereDir, 'dataExplorer.js')],
5354
localize.DataScience.dataExplorerTitle(),
5455
ViewColumn.One,
55-
experimentsManager.inExperiment(WebHostNotebook.experiment)
56+
experimentsManager.inExperiment(WebHostNotebook.experiment),
57+
useCustomEditorApi
5658
);
5759

5860
// Load the web panel using our current directory as we don't expect to load any other files

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
150150
@unmanaged() viewColumn: ViewColumn,
151151
@unmanaged() experimentsManager: IExperimentsManager,
152152
@unmanaged() private switcher: KernelSwitcher,
153-
@unmanaged() private readonly notebookProvider: INotebookProvider
153+
@unmanaged() private readonly notebookProvider: INotebookProvider,
154+
@unmanaged() useCustomEditorApi: boolean
154155
) {
155156
super(
156157
configuration,
@@ -163,7 +164,8 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
163164
scripts,
164165
title,
165166
viewColumn,
166-
experimentsManager.inExperiment(WebHostNotebook.experiment)
167+
experimentsManager.inExperiment(WebHostNotebook.experiment),
168+
useCustomEditorApi
167169
);
168170

169171
// Create our unique id. We use this to skip messages we send to other interactive windows

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ import { nbformat } from '@jupyterlab/coreutils';
9090
import cloneDeep = require('lodash/cloneDeep');
9191
import { concatMultilineStringInput } from '../../../datascience-ui/common';
9292
import { ServerStatus } from '../../../datascience-ui/interactive-common/mainState';
93-
import { isTestExecution } from '../../common/constants';
93+
import { isTestExecution, UseCustomEditorApi } from '../../common/constants';
9494
import { KernelSwitcher } from '../jupyter/kernels/kernelSwitcher';
9595

9696
const nativeEditorDir = path.join(EXTENSION_ROOT_DIR, 'out', 'datascience-ui', 'notebook');
@@ -177,7 +177,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
177177
@inject(IExperimentsManager) experimentsManager: IExperimentsManager,
178178
@inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry,
179179
@inject(KernelSwitcher) switcher: KernelSwitcher,
180-
@inject(INotebookProvider) notebookProvider: INotebookProvider
180+
@inject(INotebookProvider) notebookProvider: INotebookProvider,
181+
@inject(UseCustomEditorApi) useCustomEditorApi: boolean
181182
) {
182183
super(
183184
listeners,
@@ -212,7 +213,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
212213
ViewColumn.Active,
213214
experimentsManager,
214215
switcher,
215-
notebookProvider
216+
notebookProvider,
217+
useCustomEditorApi
216218
);
217219
asyncRegistry.push(this);
218220

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
IWebPanelProvider,
1616
IWorkspaceService
1717
} from '../../common/application/types';
18+
import { UseCustomEditorApi } from '../../common/constants';
1819
import { traceError } from '../../common/logger';
1920
import { IFileSystem } from '../../common/platform/types';
2021
import {
@@ -95,7 +96,8 @@ export class NativeEditorOldWebView extends NativeEditor {
9596
@inject(IExperimentsManager) experimentsManager: IExperimentsManager,
9697
@inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry,
9798
@inject(KernelSwitcher) switcher: KernelSwitcher,
98-
@inject(INotebookProvider) notebookProvider: INotebookProvider
99+
@inject(INotebookProvider) notebookProvider: INotebookProvider,
100+
@inject(UseCustomEditorApi) useCustomEditorApi: boolean
99101
) {
100102
super(
101103
listeners,
@@ -124,7 +126,8 @@ export class NativeEditorOldWebView extends NativeEditor {
124126
experimentsManager,
125127
asyncRegistry,
126128
switcher,
127-
notebookProvider
129+
notebookProvider,
130+
useCustomEditorApi
128131
);
129132
asyncRegistry.push(this);
130133
// No ui syncing in old notebooks.

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
IWebPanelProvider,
1414
IWorkspaceService
1515
} from '../../common/application/types';
16+
import { UseCustomEditorApi } from '../../common/constants';
1617
import { ContextKey } from '../../common/contextKey';
1718
import '../../common/extensions';
1819
import { traceError } from '../../common/logger';
@@ -108,7 +109,8 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi
108109
@inject(IMemento) @named(GLOBAL_MEMENTO) globalStorage: Memento,
109110
@inject(IExperimentsManager) experimentsManager: IExperimentsManager,
110111
@inject(KernelSwitcher) switcher: KernelSwitcher,
111-
@inject(INotebookProvider) notebookProvider: INotebookProvider
112+
@inject(INotebookProvider) notebookProvider: INotebookProvider,
113+
@inject(UseCustomEditorApi) useCustomEditorApi: boolean
112114
) {
113115
super(
114116
listeners,
@@ -143,7 +145,8 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi
143145
ViewColumn.Two,
144146
experimentsManager,
145147
switcher,
146-
notebookProvider
148+
notebookProvider,
149+
useCustomEditorApi
147150
);
148151

149152
// Send a telemetry event to indicate window is opening

‎src/client/datascience/plotting/plotViewer.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Event, EventEmitter, ViewColumn } from 'vscode';
1010
import { traceInfo } from '../../../client/common/logger';
1111
import { createDeferred } from '../../../client/common/utils/async';
1212
import { IApplicationShell, IWebPanelProvider, IWorkspaceService } from '../../common/application/types';
13-
import { EXTENSION_ROOT_DIR } from '../../common/constants';
13+
import { EXTENSION_ROOT_DIR, UseCustomEditorApi } from '../../common/constants';
1414
import { WebHostNotebook } from '../../common/experimentGroups';
1515
import { traceError } from '../../common/logger';
1616
import { IFileSystem } from '../../common/platform/types';
@@ -35,7 +35,8 @@ export class PlotViewer extends WebViewHost<IPlotViewerMapping> implements IPlot
3535
@inject(IWorkspaceService) workspaceService: IWorkspaceService,
3636
@inject(IApplicationShell) private applicationShell: IApplicationShell,
3737
@inject(IFileSystem) private fileSystem: IFileSystem,
38-
@inject(IExperimentsManager) experimentsManager: IExperimentsManager
38+
@inject(IExperimentsManager) experimentsManager: IExperimentsManager,
39+
@inject(UseCustomEditorApi) useCustomEditorApi: boolean
3940
) {
4041
super(
4142
configuration,
@@ -48,7 +49,8 @@ export class PlotViewer extends WebViewHost<IPlotViewerMapping> implements IPlot
4849
[path.join(plotDir, 'commons.initial.bundle.js'), path.join(plotDir, 'plotViewer.js')],
4950
localize.DataScience.plotViewerTitle(),
5051
ViewColumn.One,
51-
experimentsManager.inExperiment(WebHostNotebook.experiment)
52+
experimentsManager.inExperiment(WebHostNotebook.experiment),
53+
useCustomEditorApi
5254
);
5355
// Load the web panel using our current directory as we don't expect to load any other files
5456
super.loadWebPanel(process.cwd()).catch(traceError);

‎src/client/datascience/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,7 @@ export interface IDataScienceExtraSettings extends IDataScienceSettings {
652652
fontFamily: string;
653653
};
654654
theme: string;
655+
useCustomEditorApi: boolean;
655656
};
656657
intellisenseOptions: {
657658
quickSuggestions: {

‎src/client/datascience/webViewHost.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ export abstract class WebViewHost<IMapping> implements IDisposable {
5050
@unmanaged() private scripts: string[],
5151
@unmanaged() private title: string,
5252
@unmanaged() private viewColumn: ViewColumn,
53-
@unmanaged() private readonly useWebViewServer: boolean
53+
@unmanaged() private readonly useWebViewServer: boolean,
54+
@unmanaged() protected readonly useCustomEditorApi: boolean
5455
) {
5556
// Create our message listener for our web panel.
5657
this.messageListener = messageListenerCtor(
@@ -193,7 +194,8 @@ export abstract class WebViewHost<IMapping> implements IDisposable {
193194
fontSize: this.getValue(editor, 'fontSize', 14),
194195
fontFamily: this.getValue(editor, 'fontFamily', "Consolas, 'Courier New', monospace")
195196
},
196-
theme: theme
197+
theme: theme,
198+
useCustomEditorApi: this.useCustomEditorApi
197199
},
198200
intellisenseOptions: {
199201
quickSuggestions: {

‎src/datascience-ui/native-editor/nativeCell.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { CellOutput } from '../interactive-common/cellOutput';
1919
import { ExecutionCount } from '../interactive-common/executionCount';
2020
import { InformationMessages } from '../interactive-common/informationMessages';
2121
import { CursorPos, ICellViewModel, IFont } from '../interactive-common/mainState';
22-
import { getOSType, UseCustomEditor } from '../react-common/constants';
22+
import { getOSType } from '../react-common/constants';
2323
import { IKeyboardEvent } from '../react-common/event';
2424
import { Image, ImageName } from '../react-common/image';
2525
import { ImageButton } from '../react-common/imageButton';
@@ -51,6 +51,7 @@ interface INativeCellBaseProps {
5151
themeMatplotlibPlots: boolean | undefined;
5252
focusPending: number;
5353
busy: boolean;
54+
useCustomEditorApi: boolean;
5455
}
5556

5657
type INativeCellProps = INativeCellBaseProps & typeof actionCreators;
@@ -356,7 +357,7 @@ export class NativeCell extends React.Component<INativeCellProps> {
356357
break;
357358
case 'z':
358359
case 'Z':
359-
if (!this.isFocused() && !UseCustomEditor.enabled) {
360+
if (!this.isFocused() && !this.props.useCustomEditorApi) {
360361
if (e.shiftKey && !e.ctrlKey && !e.altKey) {
361362
e.stopPropagation();
362363
this.props.redo();
@@ -657,7 +658,7 @@ export class NativeCell extends React.Component<INativeCellProps> {
657658
keyDown={this.keyDownInput}
658659
showLineNumbers={this.props.cellVM.showLineNumbers}
659660
font={this.props.font}
660-
disableUndoStack={UseCustomEditor.enabled}
661+
disableUndoStack={this.props.useCustomEditorApi}
661662
codeVersion={this.props.cellVM.codeVersion ? this.props.cellVM.codeVersion : 1}
662663
focusPending={this.props.focusPending}
663664
/>

‎src/datascience-ui/native-editor/nativeEditor.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { handleLinkClick } from '../interactive-common/handlers';
1111
import { getSelectedAndFocusedInfo, ICellViewModel, IMainState } from '../interactive-common/mainState';
1212
import { IMainWithVariables, IStore } from '../interactive-common/redux/store';
1313
import { IVariablePanelProps, VariablePanel } from '../interactive-common/variablePanel';
14-
import { getOSType, UseCustomEditor } from '../react-common/constants';
14+
import { getOSType } from '../react-common/constants';
1515
import { ErrorBoundary } from '../react-common/errorBoundary';
1616
import { getLocString } from '../react-common/locReactSide';
1717
import { Progress } from '../react-common/progress';
@@ -193,7 +193,10 @@ ${buildSettingsCss(this.props.settings)}`}</style>
193193
}
194194
case 'z':
195195
case 'Z':
196-
if (!getSelectedAndFocusedInfo(this.props).focusedCellId && !UseCustomEditor.enabled) {
196+
if (
197+
!getSelectedAndFocusedInfo(this.props).focusedCellId &&
198+
!this.props.settings?.extraSettings.useCustomEditorApi
199+
) {
197200
if (event.shiftKey && !event.ctrlKey && !event.altKey) {
198201
event.stopPropagation();
199202
this.props.redo();
@@ -283,6 +286,7 @@ ${buildSettingsCss(this.props.settings)}`}</style>
283286
// Focus pending does not apply to native editor.
284287
focusPending={0}
285288
busy={this.props.busy}
289+
useCustomEditorApi={this.props.settings?.extraSettings.useCustomEditorApi}
286290
/>
287291
</ErrorBoundary>
288292
{lastLine}

‎src/datascience-ui/react-common/constants.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,3 @@ export function getOSType() {
2323
return OSType.Unknown;
2424
}
2525
}
26-
27-
export const UseCustomEditor = { enabled: true };

‎src/datascience-ui/react-common/settingsReactSide.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ export function getDefaultSettings() {
4646
fontSize: 14,
4747
fontFamily: "Consolas, 'Courier New', monospace"
4848
},
49-
theme: 'Default Dark+'
49+
theme: 'Default Dark+',
50+
useCustomEditorApi: false
5051
},
5152
intellisenseOptions: {
5253
quickSuggestions: {

‎src/test/datascience/nativeEditor.functional.test.tsx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ import { ExecutionCount } from '../../datascience-ui/interactive-common/executio
3838
import { CommonActionType } from '../../datascience-ui/interactive-common/redux/reducers/types';
3939
import { NativeCell } from '../../datascience-ui/native-editor/nativeCell';
4040
import { NativeEditor } from '../../datascience-ui/native-editor/nativeEditor';
41-
import { UseCustomEditor } from '../../datascience-ui/react-common/constants';
4241
import { IKeyboardEvent } from '../../datascience-ui/react-common/event';
4342
import { ImageButton } from '../../datascience-ui/react-common/imageButton';
4443
import { IMonacoEditorState, MonacoEditor } from '../../datascience-ui/react-common/monacoEditor';
@@ -130,7 +129,6 @@ suite('DataScience Native Editor', () => {
130129
};
131130

132131
setup(async () => {
133-
UseCustomEditor.enabled = useCustomEditorApi;
134132
ioc = new DataScienceIocContainer();
135133
ioc.registerDataScienceTypes(useCustomEditorApi);
136134
await ioc.activate();
@@ -906,7 +904,6 @@ df.head()`;
906904
cleanupCallback: Function;
907905
};
908906
function initIoc() {
909-
UseCustomEditor.enabled = useCustomEditorApi;
910907
ioc = new DataScienceIocContainer();
911908
ioc.registerDataScienceTypes(useCustomEditorApi);
912909
return ioc.activate();

‎src/test/datascience/uiTests/ipywidget.ui.functional.test.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import * as path from 'path';
1414
import * as sinon from 'sinon';
1515
import { Disposable } from 'vscode';
1616
import { EXTENSION_ROOT_DIR } from '../../../client/constants';
17-
import { UseCustomEditor } from '../../../datascience-ui/react-common/constants';
1817
import { retryIfFail as retryIfFailOriginal } from '../../common';
1918
import { mockedVSCodeNamespaces } from '../../vscode-mock';
2019
import { DataScienceIocContainer } from '../dataScienceIocContainer';
@@ -37,15 +36,13 @@ use(chaiAsPromised);
3736

3837
suiteSetup(function () {
3938
// These are UI tests, hence nothing to do with platforms.
40-
UseCustomEditor.enabled = useCustomEditorApi;
4139
this.timeout(30_000); // UI Tests, need time to start jupyter.
4240
if (!process.env.VSCODE_PYTHON_ROLLING) {
4341
// Skip all tests unless using real jupyter
4442
this.skip();
4543
}
4644
});
4745
setup(async () => {
48-
UseCustomEditor.enabled = useCustomEditorApi;
4946
ioc = new DataScienceIocContainer(true);
5047
ioc.registerDataScienceTypes(useCustomEditorApi);
5148
await ioc.activate();

0 commit comments

Comments
 (0)