Skip to content

Commit 7736bd0

Browse files
karthiknadigkarthiknadig
authored
Update code selection to handle more scenarios (microsoft#14479)
* Update code that handles code selection * Update tests * Typo
1 parent a70c2eb commit 7736bd0

File tree

2 files changed

+243
-13
lines changed

2 files changed

+243
-13
lines changed

‎src/client/terminals/codeExecution/helper.ts‎

Lines changed: 113 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import '../../common/extensions';
44

55
import { inject, injectable } from 'inversify';
6-
import { Range, TextEditor, Uri } from 'vscode';
6+
import { Position, Range, TextEditor, Uri } from 'vscode';
77

88
import { IApplicationShell, IDocumentManager } from '../../common/application/types';
99
import { PYTHON_LANGUAGE } from '../../common/constants';
@@ -20,12 +20,14 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
2020
private readonly applicationShell: IApplicationShell;
2121
private readonly processServiceFactory: IProcessServiceFactory;
2222
private readonly interpreterService: IInterpreterService;
23+
2324
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
2425
this.documentManager = serviceContainer.get<IDocumentManager>(IDocumentManager);
2526
this.applicationShell = serviceContainer.get<IApplicationShell>(IApplicationShell);
2627
this.processServiceFactory = serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory);
2728
this.interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
2829
}
30+
2931
public async normalizeLines(code: string, resource?: Uri): Promise<string> {
3032
try {
3133
if (code.trim().length === 0) {
@@ -75,16 +77,124 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
7577
let code: string;
7678
if (selection.isEmpty) {
7779
code = textEditor.document.lineAt(selection.start.line).text;
80+
} else if (selection.isSingleLine) {
81+
code = this.getSingleLineSelectionText(textEditor);
7882
} else {
79-
const textRange = new Range(selection.start, selection.end);
80-
code = textEditor.document.getText(textRange);
83+
code = this.getMultiLineSelectionText(textEditor);
8184
}
8285
return code;
8386
}
87+
8488
public async saveFileIfDirty(file: Uri): Promise<void> {
8589
const docs = this.documentManager.textDocuments.filter((d) => d.uri.path === file.path);
8690
if (docs.length === 1 && docs[0].isDirty) {
8791
await docs[0].save();
8892
}
8993
}
94+
95+
private getSingleLineSelectionText(textEditor: TextEditor): string {
96+
const selection = textEditor.selection;
97+
const selectionRange = new Range(selection.start, selection.end);
98+
const selectionText = textEditor.document.getText(selectionRange);
99+
const fullLineText = textEditor.document.lineAt(selection.start.line).text;
100+
101+
if (selectionText.trim() === fullLineText.trim()) {
102+
// This handles the following case:
103+
// if (x):
104+
// print(x)
105+
// ↑------↑ <--- selection range
106+
//
107+
// We should return:
108+
// print(x)
109+
// ↑----------↑ <--- text including the initial white space
110+
return fullLineText;
111+
}
112+
113+
// This is where part of the line is selected:
114+
// if(isPrime(x) || isFibonacci(x)):
115+
// ↑--------↑ <--- selection range
116+
//
117+
// We should return just the selection:
118+
// isPrime(x)
119+
return selectionText;
120+
}
121+
122+
private getMultiLineSelectionText(textEditor: TextEditor): string {
123+
const selection = textEditor.selection;
124+
const selectionRange = new Range(selection.start, selection.end);
125+
const selectionText = textEditor.document.getText(selectionRange);
126+
127+
const fullTextRange = new Range(
128+
new Position(selection.start.line, 0),
129+
new Position(selection.end.line, textEditor.document.lineAt(selection.end.line).text.length)
130+
);
131+
const fullText = textEditor.document.getText(fullTextRange);
132+
133+
// This handles case where:
134+
// def calc(m, n):
135+
// ↓<------------------------------- selection start
136+
// print(m)
137+
// print(n)
138+
// ↑<------------------------ selection end
139+
// if (m == 0):
140+
// return n + 1
141+
// if (m > 0 and n == 0):
142+
// return calc(m - 1 , 1)
143+
// return calc(m - 1, calc(m, n - 1))
144+
//
145+
// We should return:
146+
// ↓<---------------------------------- From here
147+
// print(m)
148+
// print(n)
149+
// ↑<----------------------- To here
150+
if (selectionText.trim() === fullText.trim()) {
151+
return fullText;
152+
}
153+
154+
const fullStartLineText = textEditor.document.lineAt(selection.start.line).text;
155+
const selectionFirstLineRange = new Range(
156+
selection.start,
157+
new Position(selection.start.line, fullStartLineText.length)
158+
);
159+
const selectionFirstLineText = textEditor.document.getText(selectionFirstLineRange);
160+
161+
// This handles case where:
162+
// def calc(m, n):
163+
// ↓<------------------------------ selection start
164+
// if (m == 0):
165+
// return n + 1
166+
// ↑<------------------- selection end (notice " + 1" is not selected)
167+
// if (m > 0 and n == 0):
168+
// return calc(m - 1 , 1)
169+
// return calc(m - 1, calc(m, n - 1))
170+
//
171+
// We should return:
172+
// ↓<---------------------------------- From here
173+
// if (m == 0):
174+
// return n + 1
175+
// ↑<------------------- To here (notice " + 1" is not selected)
176+
if (selectionFirstLineText.trimLeft() === fullStartLineText.trimLeft()) {
177+
return fullStartLineText + selectionText.substr(selectionFirstLineText.length);
178+
}
179+
180+
// If you are here then user has selected partial start and partial end lines:
181+
// def calc(m, n):
182+
183+
// if (m == 0):
184+
// return n + 1
185+
186+
// ↓<------------------------------- selection start
187+
// if (m > 0
188+
// and n == 0):
189+
// ↑<-------------------- selection end
190+
// return calc(m - 1 , 1)
191+
// return calc(m - 1, calc(m, n - 1))
192+
//
193+
// We should return:
194+
// ↓<---------------------------------- From here
195+
// (m > 0
196+
// and n == 0)
197+
// ↑<---------------- To here
198+
return selectionText;
199+
}
90200
}

‎src/test/terminals/codeExecution/helper.test.ts‎

Lines changed: 130 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { EOL } from 'os';
99
import * as path from 'path';
1010
import { SemVer } from 'semver';
1111
import * as TypeMoq from 'typemoq';
12-
import { Range, Selection, TextDocument, TextEditor, TextLine, Uri } from 'vscode';
12+
import { Position, Range, Selection, TextDocument, TextEditor, TextLine, Uri } from 'vscode';
1313
import { IApplicationShell, IDocumentManager } from '../../../client/common/application/types';
1414
import { EXTENSION_ROOT_DIR, PYTHON_LANGUAGE } from '../../../client/common/constants';
1515
import '../../../client/common/extensions';
@@ -271,8 +271,8 @@ suite('Terminal - Code Execution Helper', () => {
271271
document.verify((doc) => doc.save(), TypeMoq.Times.never());
272272
});
273273

274-
test('Returns current line if nothing is selected', async () => {
275-
const lineContents = 'Line Contents';
274+
test('Selection is empty, return current line', async () => {
275+
const lineContents = ' Line Contents';
276276
editor.setup((e) => e.selection).returns(() => new Selection(3, 0, 3, 0));
277277
const textLine = TypeMoq.Mock.ofType<TextLine>();
278278
textLine.setup((t) => t.text).returns(() => lineContents);
@@ -282,17 +282,137 @@ suite('Terminal - Code Execution Helper', () => {
282282
expect(content).to.be.equal(lineContents);
283283
});
284284

285-
test('Returns selected text', async () => {
286-
const lineContents = 'Line Contents';
287-
editor.setup((e) => e.selection).returns(() => new Selection(3, 0, 10, 5));
285+
test('Single line: text selection without whitespace ', async () => {
286+
// This test verifies following case:
287+
// 1: if (x):
288+
// 2: print(x)
289+
// 3: ↑------↑ <--- selection range
290+
const expected = ' print(x)';
291+
editor.setup((e) => e.selection).returns(() => new Selection(2, 4, 2, 12));
288292
const textLine = TypeMoq.Mock.ofType<TextLine>();
289-
textLine.setup((t) => t.text).returns(() => lineContents);
293+
textLine.setup((t) => t.text).returns(() => ' print(x)');
294+
document.setup((d) => d.lineAt(TypeMoq.It.isAny())).returns(() => textLine.object);
295+
document.setup((d) => d.getText(TypeMoq.It.isAny())).returns(() => 'print(x)');
296+
297+
const content = await helper.getSelectedTextToExecute(editor.object);
298+
expect(content).to.be.equal(expected);
299+
});
300+
301+
test('Single line: partial text selection without whitespace ', async () => {
302+
// This test verifies following case:
303+
// 1: if (isPrime(x) || isFibonacci(x)):
304+
// 2: ↑--------↑ <--- selection range
305+
const expected = 'isPrime(x)';
306+
editor.setup((e) => e.selection).returns(() => new Selection(1, 4, 1, 14));
307+
const textLine = TypeMoq.Mock.ofType<TextLine>();
308+
textLine.setup((t) => t.text).returns(() => 'if (isPrime(x) || isFibonacci(x)):');
309+
document.setup((d) => d.lineAt(TypeMoq.It.isAny())).returns(() => textLine.object);
310+
document.setup((d) => d.getText(TypeMoq.It.isAny())).returns(() => 'isPrime(x)');
311+
312+
const content = await helper.getSelectedTextToExecute(editor.object);
313+
expect(content).to.be.equal(expected);
314+
});
315+
316+
test('Multi-line: text selection without whitespace ', async () => {
317+
// This test verifies following case:
318+
// 1: def calc(m, n):
319+
// ↓<------------------------------- selection start
320+
// 2: print(m)
321+
// 3: print(n)
322+
// ↑<------------------------ selection end
323+
const expected = ' print(m)\n print(n)';
324+
const selection = new Selection(2, 4, 3, 12);
325+
editor.setup((e) => e.selection).returns(() => selection);
326+
const textLine = TypeMoq.Mock.ofType<TextLine>();
327+
textLine.setup((t) => t.text).returns(() => 'def calc(m, n):');
328+
const textLine2 = TypeMoq.Mock.ofType<TextLine>();
329+
textLine2.setup((t) => t.text).returns(() => ' print(m)');
330+
const textLine3 = TypeMoq.Mock.ofType<TextLine>();
331+
textLine3.setup((t) => t.text).returns(() => ' print(n)');
332+
const textLines = [textLine, textLine2, textLine3];
333+
document.setup((d) => d.lineAt(TypeMoq.It.isAny())).returns((r: number) => textLines[r - 1].object);
334+
document
335+
.setup((d) => d.getText(new Range(selection.start, selection.end)))
336+
.returns(() => 'print(m)\n print(n)');
337+
document
338+
.setup((d) => d.getText(new Range(new Position(selection.start.line, 0), selection.end)))
339+
.returns(() => ' print(m)\n print(n)');
340+
341+
const content = await helper.getSelectedTextToExecute(editor.object);
342+
expect(content).to.be.equal(expected);
343+
});
344+
345+
test('Multi-line: text selection without whitespace and partial last line ', async () => {
346+
// This test verifies following case:
347+
// 1: def calc(m, n):
348+
// ↓<------------------------------ selection start
349+
// 2: if (m == 0):
350+
// 3: return n + 1
351+
// ↑<------------------- selection end (notice " + 1" is not selected)
352+
const expected = ' if (m == 0):\n return n';
353+
const selection = new Selection(2, 4, 3, 16);
354+
editor.setup((e) => e.selection).returns(() => selection);
355+
const textLine = TypeMoq.Mock.ofType<TextLine>();
356+
textLine.setup((t) => t.text).returns(() => 'def calc(m, n):');
357+
const textLine2 = TypeMoq.Mock.ofType<TextLine>();
358+
textLine2.setup((t) => t.text).returns(() => ' if (m == 0):');
359+
const textLine3 = TypeMoq.Mock.ofType<TextLine>();
360+
textLine3.setup((t) => t.text).returns(() => ' return n + 1');
361+
const textLines = [textLine, textLine2, textLine3];
362+
document.setup((d) => d.lineAt(TypeMoq.It.isAny())).returns((r: number) => textLines[r - 1].object);
363+
document
364+
.setup((d) => d.getText(new Range(selection.start, selection.end)))
365+
.returns(() => 'if (m == 0):\n return n');
366+
document
367+
.setup((d) =>
368+
d.getText(new Range(new Position(selection.start.line, 4), new Position(selection.start.line, 16)))
369+
)
370+
.returns(() => 'if (m == 0):');
371+
document
372+
.setup((d) =>
373+
d.getText(new Range(new Position(selection.start.line, 0), new Position(selection.end.line, 20)))
374+
)
375+
.returns(() => ' if (m == 0):\n return n + 1');
376+
377+
const content = await helper.getSelectedTextToExecute(editor.object);
378+
expect(content).to.be.equal(expected);
379+
});
380+
381+
test('Multi-line: partial first and last line', async () => {
382+
// This test verifies following case:
383+
// 1: def calc(m, n):
384+
// ↓<------------------------------- selection start
385+
// 2: if (m > 0
386+
// 3: and n == 0):
387+
// ↑<-------------------- selection end
388+
// 4: pass
389+
const expected = '(m > 0\n and n == 0)';
390+
const selection = new Selection(2, 7, 3, 19);
391+
editor.setup((e) => e.selection).returns(() => selection);
392+
const textLine = TypeMoq.Mock.ofType<TextLine>();
393+
textLine.setup((t) => t.text).returns(() => 'def calc(m, n):');
394+
const textLine2 = TypeMoq.Mock.ofType<TextLine>();
395+
textLine2.setup((t) => t.text).returns(() => ' if (m > 0');
396+
const textLine3 = TypeMoq.Mock.ofType<TextLine>();
397+
textLine3.setup((t) => t.text).returns(() => ' and n == 0)');
398+
const textLines = [textLine, textLine2, textLine3];
399+
document.setup((d) => d.lineAt(TypeMoq.It.isAny())).returns((r: number) => textLines[r - 1].object);
400+
document
401+
.setup((d) => d.getText(new Range(selection.start, selection.end)))
402+
.returns(() => '(m > 0\n and n == 0)');
403+
document
404+
.setup((d) =>
405+
d.getText(new Range(new Position(selection.start.line, 7), new Position(selection.start.line, 13)))
406+
)
407+
.returns(() => '(m > 0');
290408
document
291-
.setup((d) => d.getText(TypeMoq.It.isAny()))
292-
.returns((r: Range) => `${r.start.line}.${r.start.character}.${r.end.line}.${r.end.character}`);
409+
.setup((d) =>
410+
d.getText(new Range(new Position(selection.start.line, 0), new Position(selection.end.line, 19)))
411+
)
412+
.returns(() => ' if (m > 0\n and n == 0)');
293413

294414
const content = await helper.getSelectedTextToExecute(editor.object);
295-
expect(content).to.be.equal('3.0.10.5');
415+
expect(content).to.be.equal(expected);
296416
});
297417

298418
test('saveFileIfDirty will not fail if file is not opened', async () => {

0 commit comments

Comments
 (0)