Skip to content

Commit db68b03

Browse files
author
Mikhail Arkhipov
authored
Fix 'formatOnType add unwanted space to *args' (#2083)
* LS symbol providers * formatOnType add unwanted space to *args * Add news * Fix floating point number tokenization * Add test * Add news
1 parent b19f71f commit db68b03

6 files changed

Lines changed: 103 additions & 16 deletions

File tree

‎news/2 Fixes/2048.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Format on type no longer adds space after * in multiline arguments.

‎news/2 Fixes/2079.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Format on type is more reliable handling floating point numbers.

‎src/client/formatters/lineFormatter.ts‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -149,19 +149,16 @@ export class LineFormatter {
149149
this.builder.append('*');
150150
return;
151151
}
152+
if (this.handleStarOperator(t, prev)) {
153+
return;
154+
}
152155
break;
153156
default:
154157
break;
155158
}
156159
} else if (t.length === 2) {
157160
if (this.text.charCodeAt(t.start) === Char.Asterisk && this.text.charCodeAt(t.start + 1) === Char.Asterisk) {
158-
if (!prev || (prev.type !== TokenType.Identifier && prev.type !== TokenType.Number)) {
159-
this.builder.append('**');
160-
return;
161-
}
162-
if (prev && this.isKeyword(prev, 'lambda')) {
163-
this.builder.softAppendSpace();
164-
this.builder.append('**');
161+
if (this.handleStarOperator(t, prev)) {
165162
return;
166163
}
167164
}
@@ -185,6 +182,28 @@ export class LineFormatter {
185182
this.builder.softAppendSpace();
186183
}
187184

185+
private handleStarOperator(current: IToken, prev: IToken): boolean {
186+
if (this.text.charCodeAt(current.start) === Char.Asterisk && this.text.charCodeAt(current.start + 1) === Char.Asterisk) {
187+
if (!prev || (prev.type !== TokenType.Identifier && prev.type !== TokenType.Number)) {
188+
this.builder.append('**');
189+
return true;
190+
}
191+
if (prev && this.isKeyword(prev, 'lambda')) {
192+
this.builder.softAppendSpace();
193+
this.builder.append('**');
194+
return true;
195+
}
196+
}
197+
// Check previous line for the **/* condition
198+
const lastLine = this.getPreviousLineTokens();
199+
const lastToken = lastLine && lastLine.count > 0 ? lastLine.getItemAt(lastLine.count - 1) : undefined;
200+
if (lastToken && (this.isOpenBraceType(lastToken.type) || lastToken.type === TokenType.Comma)) {
201+
this.builder.append(this.text.substring(current.start, current.end));
202+
return true;
203+
}
204+
return false;
205+
}
206+
188207
private handleEqual(t: IToken, index: number): void {
189208
if (this.isMultipleStatements(index) && !this.braceCounter.isOpened(TokenType.OpenBrace)) {
190209
// x = 1; x, y = y, x
@@ -411,4 +430,12 @@ export class LineFormatter {
411430
}
412431
return -1;
413432
}
433+
434+
private getPreviousLineTokens(): ITextRangeCollection<IToken> | undefined {
435+
if (!this.document || this.lineNumber === 0) {
436+
return undefined; // unable to determine
437+
}
438+
const line = this.document.lineAt(this.lineNumber - 1);
439+
return new Tokenizer().tokenize(line.text);
440+
}
414441
}

‎src/client/language/tokenizer.ts‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -279,15 +279,15 @@ export class Tokenizer implements ITokenizer {
279279
}
280280
}
281281

282-
// Floating point
283-
if ((this.cs.currentChar >= Char._0 && this.cs.currentChar <= Char._9) || this.cs.currentChar === Char.Period) {
284-
while (!isWhiteSpace(this.cs.currentChar)) {
285-
this.cs.moveNext();
286-
}
287-
const text = this.cs.getText().substr(start, this.cs.position - start);
288-
if (!isNaN(parseFloat(text))) {
289-
this.tokens.push(new Token(TokenType.Number, start, this.cs.position - start));
290-
return true;
282+
// Floating point. Sign was already skipped over.
283+
if ((this.cs.currentChar >= Char._0 && this.cs.currentChar <= Char._9) ||
284+
(this.cs.currentChar === Char.Period && this.cs.nextChar >= Char._0 && this.cs.nextChar <= Char._9)) {
285+
if (this.skipFloatingPointCandidate(false)) {
286+
const text = this.cs.getText().substr(start, this.cs.position - start);
287+
if (!isNaN(parseFloat(text))) {
288+
this.tokens.push(new Token(TokenType.Number, start, this.cs.position - start));
289+
return true;
290+
}
291291
}
292292
}
293293

@@ -467,4 +467,34 @@ export class Tokenizer implements ITokenizer {
467467
}
468468
this.cs.advance(3);
469469
}
470+
471+
private skipFloatingPointCandidate(allowSign: boolean): boolean {
472+
// Determine end of the potential floating point number
473+
const start = this.cs.position;
474+
this.skipFractionalNumber(allowSign);
475+
if (this.cs.position > start) {
476+
if (this.cs.currentChar === Char.e || this.cs.currentChar === Char.E) {
477+
this.cs.moveNext(); // Optional exponent sign
478+
}
479+
this.skipDecimalNumber(true); // skip exponent value
480+
}
481+
return this.cs.position > start;
482+
}
483+
484+
private skipFractionalNumber(allowSign: boolean): void {
485+
this.skipDecimalNumber(allowSign);
486+
if (this.cs.currentChar === Char.Period) {
487+
this.cs.moveNext(); // Optional period
488+
}
489+
this.skipDecimalNumber(false);
490+
}
491+
492+
private skipDecimalNumber(allowSign: boolean): void {
493+
if (allowSign && (this.cs.currentChar === Char.Hyphen || this.cs.currentChar === Char.Plus)) {
494+
this.cs.moveNext(); // Optional sign
495+
}
496+
while (isDecimal(this.cs.currentChar)) {
497+
this.cs.moveNext(); // skip integer part
498+
}
499+
}
470500
}

‎src/test/format/extension.lineFormatter.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,14 @@ suite('Formatting - line formatter', () => {
134134
test('lambda arguments', () => {
135135
testFormatMultiline('l4= lambda x =lambda y =lambda z= 1: z: y(): x()', 0, 'l4 = lambda x=lambda y=lambda z=1: z: y(): x()');
136136
});
137+
test('star in multiline arguments', () => {
138+
testFormatMultiline('x = [\n * param1,\n * param2\n]', 1, ' *param1,');
139+
testFormatMultiline('x = [\n * param1,\n * param2\n]', 2, ' *param2');
140+
});
141+
test('arrow operator', () => {
142+
//testFormatMultiline('def f(a, b: 1, e: 3 = 4, f =5, * g: 6, ** k: 11) -> 12: pass', 0, 'def f(a, b: 1, e: 3 = 4, f=5, *g: 6, **k: 11) -> 12: pass');
143+
testFormatMultiline('def f(a, \n ** k: 11) -> 12: pass', 1, ' **k: 11) -> 12: pass');
144+
});
137145

138146
test('Multiline function call', () => {
139147
testFormatMultiline('def foo(x = 1)', 0, 'def foo(x=1)');

‎src/test/language/tokenizer.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,26 @@ suite('Language.Tokenizer', () => {
310310
assert.equal(tokens.getItemAt(5).type, TokenType.Number);
311311
assert.equal(tokens.getItemAt(5).length, 5);
312312
});
313+
test('Floating point numbers with braces', () => {
314+
const t = new Tokenizer();
315+
const tokens = t.tokenize('(3.0) (.2) (+.3e+12, .4e1; 0)');
316+
assert.equal(tokens.count, 13);
317+
318+
assert.equal(tokens.getItemAt(1).type, TokenType.Number);
319+
assert.equal(tokens.getItemAt(1).length, 3);
320+
321+
assert.equal(tokens.getItemAt(4).type, TokenType.Number);
322+
assert.equal(tokens.getItemAt(4).length, 2);
323+
324+
assert.equal(tokens.getItemAt(7).type, TokenType.Number);
325+
assert.equal(tokens.getItemAt(7).length, 7);
326+
327+
assert.equal(tokens.getItemAt(9).type, TokenType.Number);
328+
assert.equal(tokens.getItemAt(9).length, 4);
329+
330+
assert.equal(tokens.getItemAt(11).type, TokenType.Number);
331+
assert.equal(tokens.getItemAt(11).length, 1);
332+
});
313333
test('Underscore numbers', () => {
314334
const t = new Tokenizer();
315335
const tokens = t.tokenize('+1_0_0_0 0_0 .5_00_3e-4 0xCAFE_F00D 10_000_000.0 0b_0011_1111_0100_1110');

0 commit comments

Comments
 (0)