Skip to content

Commit

Permalink
Ignore indentation rules when interacting with a string (#210761)
Browse files Browse the repository at this point in the history
* ignore indenation when interacting with a string

* changes from discussion

* remove the export keyword

* adding changes to the PR

* removing the console log

* adding one more comment

* extracting the method which forces the tokenization
  • Loading branch information
aiday-mar authored May 27, 2024
1 parent 356d2ac commit b085f37
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 11 deletions.
10 changes: 10 additions & 0 deletions src/vs/editor/common/tokens/lineTokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

import { ILanguageIdCodec } from 'vs/editor/common/languages';
import { FontStyle, ColorId, StandardTokenType, MetadataConsts, TokenMetadata, ITokenPresentation } from 'vs/editor/common/encodedTokenAttributes';
import { IPosition } from 'vs/editor/common/core/position';
import { ITextModel } from 'vs/editor/common/model';

export interface IViewLineTokens {
languageIdCodec: ILanguageIdCodec;
Expand Down Expand Up @@ -367,3 +369,11 @@ class SliceLineTokens implements IViewLineTokens {
}
}
}

export function getStandardTokenTypeAtPosition(model: ITextModel, position: IPosition): StandardTokenType {
model.tokenization.forceTokenization(position.lineNumber);
const lineTokens = model.tokenization.getLineTokens(position.lineNumber);
const tokenIndex = lineTokens.findTokenIndexAtOffset(position.column - 1);
const tokenType = lineTokens.getStandardTokenType(tokenIndex);
return tokenType;
}
14 changes: 13 additions & 1 deletion src/vs/editor/contrib/indentation/browser/indentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import * as nls from 'vs/nls';
import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput';
import { getGoodIndentForLine, getIndentMetadata } from 'vs/editor/common/languages/autoIndent';
import { getReindentEditOperations } from '../common/indentation';
import { getStandardTokenTypeAtPosition } from 'vs/editor/common/tokens/lineTokens';
import { Position } from 'vs/editor/common/core/position';

export class IndentationToSpacesAction extends EditorAction {
public static readonly ID = 'editor.action.indentationToSpaces';
Expand Down Expand Up @@ -416,7 +418,9 @@ export class AutoIndentOnPaste implements IEditorContribution {
if (!model) {
return;
}

if (isStartOrEndInString(model, range)) {
return;
}
if (!model.tokenization.isCheapToTokenize(range.getStartPosition().lineNumber)) {
return;
}
Expand Down Expand Up @@ -565,6 +569,14 @@ export class AutoIndentOnPaste implements IEditorContribution {
}
}

function isStartOrEndInString(model: ITextModel, range: Range): boolean {
const isPositionInString = (position: Position): boolean => {
const tokenType = getStandardTokenTypeAtPosition(model, position);
return tokenType === StandardTokenType.String;
};
return isPositionInString(range.getStartPosition()) || isPositionInString(range.getEndPosition());
}

function getIndentationEditOperations(model: ITextModel, builder: IEditOperationBuilder, tabSize: number, tabsToSpaces: boolean): void {
if (model.getLineCount() === 1 && model.getLineMaxColumn(1) === 1) {
// Model is empty
Expand Down
9 changes: 9 additions & 0 deletions src/vs/editor/contrib/indentation/common/indentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ShiftCommand } from 'vs/editor/common/commands/shiftCommand';
import { EditOperation, ISingleEditOperation } from 'vs/editor/common/core/editOperation';
import { normalizeIndentation } from 'vs/editor/common/core/indentation';
import { Selection } from 'vs/editor/common/core/selection';
import { StandardTokenType } from 'vs/editor/common/encodedTokenAttributes';
import { ILanguageConfigurationService } from 'vs/editor/common/languages/languageConfigurationRegistry';
import { ProcessedIndentRulesSupport } from 'vs/editor/common/languages/supports/indentationLineProcessor';
import { ITextModel } from 'vs/editor/common/model';
Expand Down Expand Up @@ -71,6 +72,9 @@ export function getReindentEditOperations(model: ITextModel, languageConfigurati

// Calculate indentation adjustment for all following lines
for (let lineNumber = startLineNumber; lineNumber <= endLineNumber; lineNumber++) {
if (doesLineStartWithString(model, lineNumber)) {
continue;
}
const text = model.getLineContent(lineNumber);
const oldIndentation = strings.getLeadingWhitespace(text);
const currentIdealIndent = idealIndentForNextLine;
Expand Down Expand Up @@ -101,3 +105,8 @@ export function getReindentEditOperations(model: ITextModel, languageConfigurati

return indentEdits;
}

function doesLineStartWithString(model: ITextModel, lineNumber: number): boolean {
const lineTokens = model.tokenization.getLineTokens(lineNumber);
return lineTokens.getStandardTokenType(0) === StandardTokenType.String;
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export interface StandardTokenTypeData {
standardTokenType: StandardTokenType;
}

export function registerTokenizationSupport(instantiationService: TestInstantiationService, tokens: StandardTokenTypeData[][], languageId: string): IDisposable {
export function registerTokenizationSupport(instantiationService: TestInstantiationService, tokens: StandardTokenTypeData[][], languageId: Language): IDisposable {
let lineIndex = 0;
const languageService = instantiationService.get(ILanguageService);
const tokenizationSupport: ITokenizationSupport = {
Expand Down Expand Up @@ -556,6 +556,43 @@ suite('Auto Indent On Paste - TypeScript/JavaScript', () => {
});
});

test('issue #209859: do not do change indentation when pasted inside of a string', () => {

// issue: https://github.com/microsoft/vscode/issues/209859
// issue: https://github.com/microsoft/vscode/issues/209418

const initialText = [
'const foo = "some text',
' which is strangely',
' indented"'
].join('\n');
const model = createTextModel(initialText, languageId, {});
disposables.add(model);

withTestCodeEditor(model, { autoIndent: 'full' }, (editor, viewModel, instantiationService) => {
const tokens: StandardTokenTypeData[][] = [
[
{ startIndex: 0, standardTokenType: StandardTokenType.Other },
{ startIndex: 12, standardTokenType: StandardTokenType.String },
],
[
{ startIndex: 0, standardTokenType: StandardTokenType.String },
],
[
{ startIndex: 0, standardTokenType: StandardTokenType.String },
]
];
disposables.add(registerLanguage(instantiationService, languageId));
disposables.add(registerTokenizationSupport(instantiationService, tokens, languageId));

editor.setSelection(new Selection(2, 10, 2, 15));
viewModel.paste('which', true, undefined, 'keyboard');
const autoIndentOnPasteController = editor.registerAndInstantiateContribution(AutoIndentOnPaste.ID, AutoIndentOnPaste);
autoIndentOnPasteController.trigger(new Range(2, 1, 2, 28));
assert.strictEqual(model.getValue(), initialText);
});
});

// Failing tests found in issues...

test.skip('issue #181065: Incorrect paste of object within comment', () => {
Expand Down
59 changes: 50 additions & 9 deletions src/vs/workbench/contrib/codeEditor/test/node/autoindent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { ILanguageService } from 'vs/editor/common/languages/language';
import { EncodedTokenizationResult, IState, ITokenizationSupport, TokenizationRegistry } from 'vs/editor/common/languages';
import { NullState } from 'vs/editor/common/languages/nullTokenize';
import { MetadataConsts, StandardTokenType } from 'vs/editor/common/encodedTokenAttributes';
import { ITextModel } from 'vs/editor/common/model';

function getIRange(range: IRange): IRange {
return {
Expand All @@ -36,12 +37,22 @@ const enum LanguageId {
TypeScript = 'ts-test'
}

function forceTokenizationFromLineToLine(model: ITextModel, startLine: number, endLine: number): void {
for (let line = startLine; line <= endLine; line++) {
model.tokenization.forceTokenization(line);
}
}

function registerLanguage(instantiationService: TestInstantiationService, languageId: LanguageId): IDisposable {
const languageService = instantiationService.get(ILanguageService)
return languageService.registerLanguage({ id: languageId });
const disposables = new DisposableStore();
const languageService = instantiationService.get(ILanguageService);
disposables.add(registerLanguageConfiguration(instantiationService, languageId));
disposables.add(languageService.registerLanguage({ id: languageId }));
return disposables;
}

function registerLanguageConfiguration(languageConfigurationService: ILanguageConfigurationService, languageId: LanguageId): IDisposable {
function registerLanguageConfiguration(instantiationService: TestInstantiationService, languageId: LanguageId): IDisposable {
const languageConfigurationService = instantiationService.get(ILanguageConfigurationService);
let configPath: string;
switch (languageId) {
case LanguageId.TypeScript:
Expand All @@ -61,7 +72,7 @@ interface StandardTokenTypeData {
standardTokenType: StandardTokenType;
}

function registerTokenizationSupport(instantiationService: TestInstantiationService, tokens: StandardTokenTypeData[][], languageId: string): IDisposable {
function registerTokenizationSupport(instantiationService: TestInstantiationService, tokens: StandardTokenTypeData[][], languageId: LanguageId): IDisposable {
let lineIndex = 0;
const languageService = instantiationService.get(ILanguageService);
const tokenizationSupport: ITokenizationSupport = {
Expand Down Expand Up @@ -97,7 +108,7 @@ suite('Auto-Reindentation - TypeScript/JavaScript', () => {
languageConfigurationService = instantiationService.get(ILanguageConfigurationService);
disposables.add(instantiationService);
disposables.add(registerLanguage(instantiationService, languageId));
disposables.add(registerLanguageConfiguration(languageConfigurationService, languageId));
disposables.add(registerLanguageConfiguration(instantiationService, languageId));
});

teardown(() => {
Expand Down Expand Up @@ -218,8 +229,7 @@ suite('Auto-Reindentation - TypeScript/JavaScript', () => {
];
disposables.add(registerTokenizationSupport(instantiationService, tokens, languageId));
const model = disposables.add(instantiateTextModel(instantiationService, fileContents, languageId, options));
model.tokenization.forceTokenization(1);
model.tokenization.forceTokenization(2);
forceTokenizationFromLineToLine(model, 1, 2);
const editOperations = getReindentEditOperations(model, languageConfigurationService, 1, model.getLineCount());
assert.deepStrictEqual(editOperations.length, 1);
const operation = editOperations[0];
Expand Down Expand Up @@ -338,8 +348,7 @@ suite('Auto-Reindentation - TypeScript/JavaScript', () => {
];
disposables.add(registerTokenizationSupport(instantiationService, tokens, languageId));
const model = disposables.add(instantiateTextModel(instantiationService, fileContents, languageId, options));
model.tokenization.forceTokenization(1);
model.tokenization.forceTokenization(2);
forceTokenizationFromLineToLine(model, 1, 2);
const editOperations = getReindentEditOperations(model, languageConfigurationService, 1, model.getLineCount());
assert.deepStrictEqual(editOperations.length, 1);
const operation = editOperations[0];
Expand Down Expand Up @@ -373,6 +382,38 @@ suite('Auto-Reindentation - TypeScript/JavaScript', () => {
assert.deepStrictEqual(editOperations.length, 0);
});

test('Issue #209859: do not do reindentation for tokens inside of a string', () => {

// issue: https://github.com/microsoft/vscode/issues/209859

const tokens: StandardTokenTypeData[][] = [
[
{ startIndex: 0, standardTokenType: StandardTokenType.Other },
{ startIndex: 12, standardTokenType: StandardTokenType.String },
],
[
{ startIndex: 0, standardTokenType: StandardTokenType.String },
],
[
{ startIndex: 0, standardTokenType: StandardTokenType.String },
],
[
{ startIndex: 0, standardTokenType: StandardTokenType.String },
]
];
disposables.add(registerTokenizationSupport(instantiationService, tokens, languageId));
const fileContents = [
'const foo = `some text',
' which is strangely',
' indented. It should',
' not be reindented.`'
].join('\n');
const model = disposables.add(instantiateTextModel(instantiationService, fileContents, languageId, options));
forceTokenizationFromLineToLine(model, 1, 4);
const editOperations = getReindentEditOperations(model, languageConfigurationService, 1, model.getLineCount());
assert.deepStrictEqual(editOperations.length, 0);
});

// Failing tests inferred from the current regexes...

test.skip('Incorrect deindentation after `*/}` string', () => {
Expand Down

0 comments on commit b085f37

Please sign in to comment.