Skip to content

Remove whitespace around code when showing in interactive window #10057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 11, 2020
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
1 change: 1 addition & 0 deletions news/2 Fixes/9116.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove whitespace from code before pushing to the interactive window
31 changes: 31 additions & 0 deletions src/datascience-ui/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,37 @@ export function splitMultilineString(source: nbformat.MultilineString): string[]
return [];
}

export function removeLinesFromFrontAndBack(code: string): string {

Choose a reason for hiding this comment

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

I think its good to add unit tests for this function.

Copy link
Author

Choose a reason for hiding this comment

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

I did. See the tests in cellFactory.unit.test

const lines = code.splitLines({ trim: false, removeEmptyEntries: false });
let foundNonEmptyLine = false;
let lastNonEmptyLine = -1;
let result: string[] = [];
parseForComments(
lines,
(_s, i) => {
result.push(lines[i]);
lastNonEmptyLine = i;
},
(s, i) => {
const trimmed = s.trim();
if (foundNonEmptyLine || trimmed) {
result.push(lines[i]);
foundNonEmptyLine = true;
}
if (trimmed) {
lastNonEmptyLine = i;
}
}
);

// Remove empty lines off the bottom too
if (lastNonEmptyLine < lines.length - 1) {
result = result.slice(0, result.length - (lines.length - 1 - lastNonEmptyLine));
}

return result.join('\n');
}

// Strip out comment lines from code
export function stripComments(str: string): string {
let result: string = '';
Expand Down
13 changes: 11 additions & 2 deletions src/datascience-ui/history-react/redux/reducers/creation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { Identifiers } from '../../../../client/datascience/constants';
import { InteractiveWindowMessages } from '../../../../client/datascience/interactive-common/interactiveWindowTypes';
import { ICell, IDataScienceExtraSettings } from '../../../../client/datascience/types';
import { removeLinesFromFrontAndBack } from '../../../common';
import { createCellVM, extractInputText, ICellViewModel, IMainState } from '../../../interactive-common/mainState';
import { createPostableAction } from '../../../interactive-common/redux/postOffice';
import { Helpers } from '../../../interactive-common/redux/reducers/helpers';
Expand All @@ -19,6 +20,14 @@ export namespace Creation {
return true;
}

function extractInputBlockText(cellVM: ICellViewModel, settings?: IDataScienceExtraSettings) {
// Use the base function first
const text = extractInputText(cellVM, settings);

// Then remove text on the front and back. We only do this for the interactive window
return removeLinesFromFrontAndBack(text);
}

export function alterCellVM(cellVM: ICellViewModel, settings?: IDataScienceExtraSettings, visible?: boolean, expanded?: boolean): ICellViewModel {
if (cellVM.cell.data.cell_type === 'code') {
// If we are already in the correct state, return back our initial cell vm
Expand All @@ -41,13 +50,13 @@ export namespace Creation {
if (cellVM.inputBlockOpen !== expanded && cellVM.inputBlockCollapseNeeded && cellVM.inputBlockShow) {
if (expanded) {
// Expand the cell
const newText = extractInputText(cellVM, settings);
const newText = extractInputBlockText(cellVM, settings);

newCellVM.inputBlockOpen = true;
newCellVM.inputBlockText = newText;
} else {
// Collapse the cell
let newText = extractInputText(cellVM, settings);
let newText = extractInputBlockText(cellVM, settings);
if (newText.length > 0) {
newText = newText.split('\n', 1)[0];
newText = newText.slice(0, 255); // Slice to limit length, slicing past length is fine
Expand Down
49 changes: 48 additions & 1 deletion src/test/datascience/cellFactory.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
'use strict';
import { assert } from 'chai';
import { generateCells } from '../../client/datascience/cellFactory';
import { stripComments } from '../../datascience-ui/common';
import { removeLinesFromFrontAndBack, stripComments } from '../../datascience-ui/common';

// tslint:disable: max-func-body-length
suite('Data Science CellFactory', () => {
Expand Down Expand Up @@ -150,4 +150,51 @@ class Pizza(object):
nonComments = stripComments(multilineQuoteInFunc);
assert.equal(nonComments.splitLines().length, 6, 'Splitting quote in func wrong number of lines');
});

test('Line removal', () => {
const entry1 = `# %% CELL

first line`;
const expected1 = `# %% CELL
first line`;
const entry2 = `# %% CELL

first line

`;
const expected2 = `# %% CELL
first line`;
const entry3 = `# %% CELL

first line

second line

`;
const expected3 = `# %% CELL
first line

second line`;

const entry4 = `

if (foo):
print('stuff')

print('some more')

`;
const expected4 = `if (foo):
print('stuff')

print('some more')`;
let removed = removeLinesFromFrontAndBack(entry1);
assert.equal(removed, expected1);
removed = removeLinesFromFrontAndBack(entry2);
assert.equal(removed, expected2);
removed = removeLinesFromFrontAndBack(entry3);
assert.equal(removed, expected3);
removed = removeLinesFromFrontAndBack(entry4);
assert.equal(removed, expected4);
});
});