Skip to content

Commit

Permalink
Take Smart Send Out of Experiment (microsoft#23067)
Browse files Browse the repository at this point in the history
Resolves: microsoft#23045
  • Loading branch information
anthonykim1 committed Mar 14, 2024
1 parent 8f726c5 commit 7eabfbb
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 52 deletions.
6 changes: 1 addition & 5 deletions python_files/normalizeSelection.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,7 @@ def get_next_block_lineno(which_line_next):
data = None
which_line_next = 0

if (
empty_Highlight
and contents.get("smartSendExperimentEnabled")
and contents.get("smartSendSettingsEnabled")
):
if empty_Highlight and contents.get("smartSendSettingsEnabled"):
result = traverse_file(
contents["wholeFileContent"],
vscode_start_line,
Expand Down
4 changes: 0 additions & 4 deletions src/client/common/experiments/groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ export enum DiscoveryUsingWorkers {
export enum EnableTestAdapterRewrite {
experiment = 'pythonTestAdapter',
}
// Experiment to enable smart shift+enter, advance cursor.
export enum EnableREPLSmartSend {
experiment = 'pythonREPLSmartSend',
}

// Experiment to recommend installing the tensorboard extension.
export enum RecommendTensobardExtension {
Expand Down
35 changes: 10 additions & 25 deletions src/client/terminals/codeExecution/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ import { IInterpreterService } from '../../interpreter/contracts';
import { IServiceContainer } from '../../ioc/types';
import { ICodeExecutionHelper } from '../types';
import { traceError } from '../../logging';
import { IConfigurationService, IExperimentService, Resource } from '../../common/types';
import { EnableREPLSmartSend } from '../../common/experiments/groups';
import { IConfigurationService, Resource } from '../../common/types';
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';

Expand Down Expand Up @@ -93,7 +92,6 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
const startLineVal = activeEditor?.selection?.start.line ?? 0;
const endLineVal = activeEditor?.selection?.end.line ?? 0;
const emptyHighlightVal = activeEditor?.selection?.isEmpty ?? true;
const smartSendExperimentEnabledVal = pythonSmartSendEnabled(this.serviceContainer);
let smartSendSettingsEnabledVal = true;
const configuration = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
if (configuration) {
Expand All @@ -107,7 +105,6 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
startLine: startLineVal,
endLine: endLineVal,
emptyHighlight: emptyHighlightVal,
smartSendExperimentEnabled: smartSendExperimentEnabledVal,
smartSendSettingsEnabled: smartSendSettingsEnabledVal,
});
observable.proc?.stdin?.write(input);
Expand All @@ -117,12 +114,7 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
const result = await normalizeOutput.promise;
const object = JSON.parse(result);

if (
activeEditor?.selection &&
smartSendExperimentEnabledVal &&
smartSendSettingsEnabledVal &&
object.normalized !== 'deprecated'
) {
if (activeEditor?.selection && smartSendSettingsEnabledVal && object.normalized !== 'deprecated') {
const lineOffset = object.nextBlockLineno - activeEditor!.selection.start.line - 1;
await this.moveToNextBlock(lineOffset, activeEditor);
}
Expand All @@ -145,16 +137,15 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
*/
// eslint-disable-next-line class-methods-use-this
private async moveToNextBlock(lineOffset: number, activeEditor?: TextEditor): Promise<void> {
if (pythonSmartSendEnabled(this.serviceContainer)) {
if (activeEditor?.selection?.isEmpty) {
await this.commandManager.executeCommand('cursorMove', {
to: 'down',
by: 'line',
value: Number(lineOffset),
});
await this.commandManager.executeCommand('cursorEnd');
}
if (activeEditor?.selection?.isEmpty) {
await this.commandManager.executeCommand('cursorMove', {
to: 'down',
by: 'line',
value: Number(lineOffset),
});
await this.commandManager.executeCommand('cursorEnd');
}

return Promise.resolve();
}

Expand Down Expand Up @@ -314,9 +305,3 @@ function getMultiLineSelectionText(textEditor: TextEditor): string {
// ↑<---------------- To here
return selectionText;
}

function pythonSmartSendEnabled(serviceContainer: IServiceContainer): boolean {
const experiment = serviceContainer.get<IExperimentService>(IExperimentService);

return experiment ? experiment.inExperimentSync(EnableREPLSmartSend.experiment) : false;
}
18 changes: 0 additions & 18 deletions src/test/terminals/codeExecution/smartSend.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { IConfigurationService, IExperimentService, IPythonSettings } from '../.
import { CodeExecutionHelper } from '../../../client/terminals/codeExecution/helper';
import { IServiceContainer } from '../../../client/ioc/types';
import { ICodeExecutionHelper } from '../../../client/terminals/types';
import { EnableREPLSmartSend } from '../../../client/common/experiments/groups';
import { Commands, EXTENSION_ROOT_DIR } from '../../../client/common/constants';
import { EnvironmentType, PythonEnvironment } from '../../../client/pythonEnvironments/info';
import { PYTHON_PATH } from '../../common';
Expand Down Expand Up @@ -117,10 +116,6 @@ suite('REPL - Smart Send', () => {
});

test('Cursor is not moved when explicit selection is present', async () => {
experimentService
.setup((exp) => exp.inExperimentSync(TypeMoq.It.isValue(EnableREPLSmartSend.experiment)))
.returns(() => true);

const activeEditor = TypeMoq.Mock.ofType<TextEditor>();
const firstIndexPosition = new Position(0, 0);
const selection = TypeMoq.Mock.ofType<Selection>();
Expand Down Expand Up @@ -164,15 +159,10 @@ suite('REPL - Smart Send', () => {
});

test('Smart send should perform smart selection and move cursor', async () => {
experimentService
.setup((exp) => exp.inExperimentSync(TypeMoq.It.isValue(EnableREPLSmartSend.experiment)))
.returns(() => true);

configurationService
.setup((c) => c.getSettings(TypeMoq.It.isAny()))
.returns({
REPL: {
EnableREPLSmartSend: true,
REPLSmartSend: true,
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -228,9 +218,6 @@ suite('REPL - Smart Send', () => {

// Do not perform smart selection when there is explicit selection
test('Smart send should not perform smart selection when there is explicit selection', async () => {
experimentService
.setup((exp) => exp.inExperimentSync(TypeMoq.It.isValue(EnableREPLSmartSend.experiment)))
.returns(() => true);
const activeEditor = TypeMoq.Mock.ofType<TextEditor>();
const firstIndexPosition = new Position(0, 0);
const selection = TypeMoq.Mock.ofType<Selection>();
Expand All @@ -257,15 +244,10 @@ suite('REPL - Smart Send', () => {
});

test('Smart Send should provide warning when code is not valid', async () => {
experimentService
.setup((exp) => exp.inExperimentSync(TypeMoq.It.isValue(EnableREPLSmartSend.experiment)))
.returns(() => true);

configurationService
.setup((c) => c.getSettings(TypeMoq.It.isAny()))
.returns({
REPL: {
EnableREPLSmartSend: true,
REPLSmartSend: true,
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down

0 comments on commit 7eabfbb

Please sign in to comment.