Skip to content

Commit 17c2c7f

Browse files
rchiodorchiodo
authored
Another issue with triplequoted strings (#5027)
* Fix multiline string being used as an actual string Add test for tabs to make sure they stay in comments * Handle case where comment delimiter doesn't match original
1 parent ca08e17 commit 17c2c7f

3 files changed

Lines changed: 117 additions & 26 deletions

File tree

‎news/2 Fixes/4029.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix tabs in comments to come out in cells.

‎src/client/datascience/common.ts‎

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { nbformat } from '@jupyterlab/coreutils/lib/nbformat';
55

66
import { noop } from '../../test/core';
77

8+
const SingleQuoteMultiline = '\'\'\'';
9+
const DoubleQuoteMultiline = '\"\"\"';
810
export function concatMultilineString(str: nbformat.MultilineString): string {
911
if (Array.isArray(str)) {
1012
let result = '';
@@ -80,37 +82,49 @@ export function parseForComments(
8082
foundCommentLine: (s: string, i: number) => void,
8183
foundNonCommentLine: (s: string, i: number) => void) {
8284
// Check for either multiline or single line comments
83-
let insideMultiline = false;
85+
let insideMultilineComment: string | undefined ;
86+
let insideMultilineQuote: string | undefined;
8487
let pos = 0;
8588
for (const l of lines) {
8689
const trim = l.trim();
8790
// Multiline is triple quotes of either kind
88-
const isMultiline = trim.startsWith('\'\'\'') || trim.startsWith('\"\"\"');
89-
if (insideMultiline) {
90-
if (!isMultiline) {
91+
const isMultilineComment = trim.startsWith(SingleQuoteMultiline) ?
92+
SingleQuoteMultiline : trim.startsWith(DoubleQuoteMultiline) ? DoubleQuoteMultiline : undefined;
93+
const isMultilineQuote = trim.includes(SingleQuoteMultiline) ?
94+
SingleQuoteMultiline : trim.includes(DoubleQuoteMultiline) ? DoubleQuoteMultiline : undefined;
95+
96+
// Check for ending quotes of multiline string
97+
if (insideMultilineQuote) {
98+
if (insideMultilineQuote === isMultilineQuote) {
99+
insideMultilineQuote = undefined;
100+
}
101+
foundNonCommentLine(l, pos);
102+
// Not inside quote, see if inside a comment
103+
} else if (insideMultilineComment) {
104+
if (insideMultilineComment === isMultilineComment) {
105+
insideMultilineComment = undefined;
106+
}
107+
if (insideMultilineComment) {
91108
foundCommentLine(l, pos);
92-
} else {
93-
insideMultiline = false;
109+
}
110+
// Not inside either, see if starting a quote
111+
} else if (isMultilineQuote && !isMultilineComment) {
112+
insideMultilineQuote = isMultilineQuote;
113+
foundNonCommentLine(l, pos);
114+
// Not starting a quote, might be starting a comment
115+
} else if (isMultilineComment) {
116+
insideMultilineComment = isMultilineComment;
94117

95-
// Might end with text too
96-
if (trim.length > 3) {
97-
foundNonCommentLine(trim.slice(3), pos);
98-
}
118+
// Might end with text too
119+
if (trim.length > 3) {
120+
foundCommentLine(trim.slice(3), pos);
99121
}
100122
} else {
101-
if (!isMultiline) {
102-
if (trim.startsWith('#')) {
103-
foundCommentLine(trim.slice(1), pos);
104-
} else {
105-
foundNonCommentLine(l, pos);
106-
}
123+
// Normal line
124+
if (trim.startsWith('#')) {
125+
foundCommentLine(trim.slice(1), pos);
107126
} else {
108-
insideMultiline = true;
109-
110-
// Might end with text too
111-
if (trim.length > 3) {
112-
foundCommentLine(trim.slice(3), pos);
113-
}
127+
foundNonCommentLine(l, pos);
114128
}
115129
}
116130
pos += 1;

‎src/test/datascience/datascience.unit.test.ts‎

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { generateCells } from '../../client/datascience/cellFactory';
77
import { formatStreamText } from '../../client/datascience/common';
88
import { InputHistory } from '../../datascience-ui/history-react/inputHistory';
99

10+
// tslint:disable-next-line: max-func-body-length
1011
suite('Data Science Tests', () => {
1112

1213
test('formatting stream text', async () => {
@@ -96,18 +97,93 @@ suite('Data Science Tests', () => {
9697
assert.equal(cells.length, 1, 'Markdown cell multline failed');
9798
assert.equal(cells[0].data.cell_type, 'markdown', 'Markdown cell not generated');
9899
assert.equal(cells[0].data.source.length, 2, 'Lines for markdown not emitted');
99-
cells = generateCells(undefined, '#%% [markdown]\n\"\"\"\n# a\nb\n\'\'\'', 'foo', 0, true, '1');
100+
cells = generateCells(undefined, '#%% [markdown]\n\"\"\"\n# a\nb\n\"\"\"', 'foo', 0, true, '1');
100101
assert.equal(cells.length, 1, 'Markdown cell multline failed');
101102
assert.equal(cells[0].data.cell_type, 'markdown', 'Markdown cell not generated');
102103
assert.equal(cells[0].data.source.length, 2, 'Lines for markdown not emitted');
103-
cells = generateCells(undefined, '#%% \n\"\"\"\n# a\nb\n\'\'\'', 'foo', 0, true, '1');
104+
cells = generateCells(undefined, '#%% \n\"\"\"\n# a\nb\n\"\"\"', 'foo', 0, true, '1');
104105
assert.equal(cells.length, 1, 'Code cell multline failed');
105106
assert.equal(cells[0].data.cell_type, 'code', 'Code cell not generated');
106107
assert.equal(cells[0].data.source.length, 5, 'Lines for cell not emitted');
107-
cells = generateCells(undefined, '#%% [markdown] \n\"\"\"# a\nb\n\'\'\'', 'foo', 0, true, '1');
108+
cells = generateCells(undefined, '#%% [markdown] \n\"\"\"# a\nb\n\"\"\"', 'foo', 0, true, '1');
108109
assert.equal(cells.length, 1, 'Markdown cell multline failed');
109110
assert.equal(cells[0].data.cell_type, 'markdown', 'Markdown cell not generated');
110111
assert.equal(cells[0].data.source.length, 2, 'Lines for cell not emitted');
111-
});
112+
113+
// tslint:disable-next-line: no-multiline-string
114+
const multilineCode = `#%%
115+
myvar = """ # Lorem Ipsum
116+
Lorem ipsum dolor sit amet, consectetur adipiscing elit.
117+
Nullam eget varius ligula, eget fermentum mauris.
118+
Cras ultrices, enim sit amet iaculis ornare, nisl nibh aliquet elit, sed ultrices velit ipsum dignissim nisl.
119+
Nunc quis orci ante. Vivamus vel blandit velit.
120+
Sed mattis dui diam, et blandit augue mattis vestibulum.
121+
Suspendisse ornare interdum velit. Suspendisse potenti.
122+
Morbi molestie lacinia sapien nec porttitor. Nam at vestibulum nisi.
123+
"""`;
124+
// tslint:disable-next-line: no-multiline-string
125+
const multilineTwo = `#%%
126+
""" # Lorem Ipsum
127+
Lorem ipsum dolor sit amet, consectetur adipiscing elit.
128+
Nullam eget varius ligula, eget fermentum mauris.
129+
Cras ultrices, enim sit amet iaculis ornare, nisl nibh aliquet elit, sed ultrices velit ipsum dignissim nisl.
130+
Nunc quis orci ante. Vivamus vel blandit velit.
131+
Sed mattis dui diam, et blandit augue mattis vestibulum.
132+
Suspendisse ornare interdum velit. Suspendisse potenti.
133+
Morbi molestie lacinia sapien nec porttitor. Nam at vestibulum nisi.
134+
""" print('bob')`;
135+
136+
cells = generateCells(undefined, multilineCode, 'foo', 0, true, '1');
137+
assert.equal(cells.length, 1, 'code cell multline failed');
138+
assert.equal(cells[0].data.cell_type, 'code', 'Code cell not generated');
139+
assert.equal(cells[0].data.source.length, 10, 'Lines for cell not emitted');
140+
cells = generateCells(undefined, multilineTwo, 'foo', 0, true, '1');
141+
assert.equal(cells.length, 1, 'code cell multline failed');
142+
assert.equal(cells[0].data.cell_type, 'code', 'Code cell not generated');
143+
assert.equal(cells[0].data.source.length, 10, 'Lines for cell not emitted');
144+
// tslint:disable-next-line: no-multiline-string
145+
assert.equal(cells[0].data.source[9], `""" print('bob')`, 'Lines for cell not emitted');
146+
// tslint:disable-next-line: no-multiline-string
147+
const multilineMarkdown = `#%% [markdown]
148+
# ## Block of Interest
149+
#
150+
# ### Take a look
151+
#
152+
#
153+
# 1. Item 1
154+
#
155+
# - Item 1-a
156+
# 1. Item 1-a-1
157+
# - Item 1-a-1-a
158+
# - Item 1-a-1-b
159+
# 2. Item 1-a-2
160+
# - Item 1-a-2-a
161+
# - Item 1-a-2-b
162+
# 3. Item 1-a-3
163+
# - Item 1-a-3-a
164+
# - Item 1-a-3-b
165+
# - Item 1-a-3-c
166+
#
167+
# 2. Item 2`;
168+
cells = generateCells(undefined, multilineMarkdown, 'foo', 0, true, '1');
169+
assert.equal(cells.length, 1, 'markdown cell multline failed');
170+
assert.equal(cells[0].data.cell_type, 'markdown', 'markdown cell not generated');
171+
assert.equal(cells[0].data.source.length, 20, 'Lines for cell not emitted');
172+
assert.equal(cells[0].data.source[17], ' - Item 1-a-3-c\n', 'Lines for markdown not emitted');
173+
174+
// tslint:disable-next-line: no-multiline-string
175+
const multilineQuoteWithOtherDelimiter = `#%% [markdown]
176+
'''
177+
### Take a look
178+
2. Item 2
179+
""" Not a comment delimiter
180+
'''
181+
`;
182+
cells = generateCells(undefined, multilineQuoteWithOtherDelimiter, 'foo', 0, true, '1');
183+
assert.equal(cells.length, 1, 'markdown cell multline failed');
184+
assert.equal(cells[0].data.cell_type, 'markdown', 'markdown cell not generated');
185+
assert.equal(cells[0].data.source.length, 3, 'Lines for cell not emitted');
186+
assert.equal(cells[0].data.source[2], '""" Not a comment delimiter', 'Lines for markdown not emitted');
187+
});
112188

113189
});

0 commit comments

Comments
 (0)