Skip to content

Commit 3d0862d

Browse files
ulugbeknaulugbekna
authored
nes: create a setting to allow/disallow whitespace-only NES (#838)
* nes: implement filtering whitespace-only changes issue with this is that currently filters are registered on XtabProvider creation and cannot be changed afterwards (which's bad in case user changes the setting -- they don't expect they'd need to reload the window) * nes: refactor edit-filtering and fix non-dynamic whitespace-only edit filtering
1 parent 4a3b8de commit 3d0862d

File tree

7 files changed

+139
-79
lines changed

7 files changed

+139
-79
lines changed

‎package.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2254,6 +2254,16 @@
22542254
"markdownDescription": "%github.copilot.nextEditSuggestions.fixes%",
22552255
"scope": "language-overridable"
22562256
},
2257+
"github.copilot.nextEditSuggestions.allowWhitespaceOnlyChanges": {
2258+
"type": "boolean",
2259+
"default": true,
2260+
"tags": [
2261+
"nextEditSuggestions",
2262+
"onExp"
2263+
],
2264+
"markdownDescription": "%github.copilot.nextEditSuggestions.allowWhitespaceOnlyChanges%",
2265+
"scope": "language-overridable"
2266+
},
22572267
"github.copilot.chat.agent.autoFix": {
22582268
"type": "boolean",
22592269
"default": true,

‎package.nls.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
"github.copilot.config.codesearch.enabled": "Whether to enable agentic codesearch when using `#codebase`.",
9494
"github.copilot.nextEditSuggestions.enabled": "Whether to enable next edit suggestions (NES).\n\nNES can propose a next edit based on your recent changes. [Learn more](https://aka.ms/vscode-nes) about next edit suggestions.",
9595
"github.copilot.nextEditSuggestions.fixes": "Whether to offer fixes for diagnostics via next edit suggestions (NES).",
96+
"github.copilot.nextEditSuggestions.allowWhitespaceOnlyChanges": "Whether to allow whitespace-only changes be proposed by next edit suggestions (NES).",
9697
"github.copilot.chat.copilotDebugCommand.enabled": "Whether the `copilot-debug` command is enabled in the terminal.",
9798
"github.copilot.config.terminalChatLocation": "Controls where chat queries from the terminal should be opened.",
9899
"github.copilot.config.terminalChatLocation.chatView": "Open the chat view.",

‎src/extension/inlineEdits/node/importFiltering.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,16 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { StatelessNextEditDocument } from '../../../platform/inlineEdits/common/statelessNextEditProvider';
7-
import { EditFilterAspect } from '../../../platform/inlineEdits/common/statelessNextEditProviders';
87
import { coalesce } from '../../../util/vs/base/common/arrays';
98
import { LineReplacement } from '../../../util/vs/editor/common/core/edits/lineEdit';
109
import { isImportStatement } from '../../prompt/common/importStatement';
1110

12-
export class IgnoreImportChangesAspect extends EditFilterAspect {
11+
export class IgnoreImportChangesAspect {
1312
public static isImportChange(edit: LineReplacement, languageId: string, lines: string[]): boolean {
1413
return edit.newLines.some(l => isImportStatement(l, languageId)) || getOldLines(edit, lines).some(l => isImportStatement(l, languageId));
1514
}
1615

17-
override filterEdit(resultDocument: StatelessNextEditDocument, singleEdits: readonly LineReplacement[]): readonly LineReplacement[] {
16+
public static filterEdit(resultDocument: StatelessNextEditDocument, singleEdits: readonly LineReplacement[]): readonly LineReplacement[] {
1817
const languageId = resultDocument.languageId;
1918
const filteredEdits = singleEdits.filter(e => !IgnoreImportChangesAspect.isImportChange(e, languageId, resultDocument.documentLinesBeforeEdit));
2019
return filteredEdits;

‎src/extension/xtab/node/xtabProvider.ts‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ import * as xtabPromptOptions from '../../../platform/inlineEdits/common/dataTyp
1818
import { LanguageContextLanguages, LanguageContextOptions } from '../../../platform/inlineEdits/common/dataTypes/xtabPromptOptions';
1919
import { InlineEditRequestLogContext } from '../../../platform/inlineEdits/common/inlineEditLogContext';
2020
import { ResponseProcessor } from '../../../platform/inlineEdits/common/responseProcessor';
21-
import { NoNextEditReason, PushEdit, ShowNextEditPreference, StatelessNextEditDocument, StatelessNextEditRequest, StatelessNextEditResult, StatelessNextEditTelemetryBuilder } from '../../../platform/inlineEdits/common/statelessNextEditProvider';
22-
import { ChainedStatelessNextEditProvider, IgnoreTriviaWhitespaceChangesAspect } from '../../../platform/inlineEdits/common/statelessNextEditProviders';
21+
import { IStatelessNextEditProvider, NoNextEditReason, PushEdit, ShowNextEditPreference, StatelessNextEditDocument, StatelessNextEditRequest, StatelessNextEditResult, StatelessNextEditTelemetryBuilder } from '../../../platform/inlineEdits/common/statelessNextEditProvider';
22+
import { IgnoreEmptyLineAndLeadingTrailingWhitespaceChanges, IgnoreWhitespaceOnlyChanges } from '../../../platform/inlineEdits/common/statelessNextEditProviders';
2323
import { ILanguageContextProviderService } from '../../../platform/languageContextProvider/common/languageContextProviderService';
2424
import { ILanguageDiagnosticsService } from '../../../platform/languages/common/languageDiagnosticsService';
2525
import { ContextKind, SnippetContext } from '../../../platform/languageServer/common/languageContextService';
@@ -73,10 +73,12 @@ const enum RetryState {
7373
RetryingWithExpandedWindow
7474
}
7575

76-
export class XtabProvider extends ChainedStatelessNextEditProvider {
76+
export class XtabProvider implements IStatelessNextEditProvider {
7777

7878
public static readonly ID = XTabProviderId;
7979

80+
public readonly ID = XtabProvider.ID;
81+
8082
public readonly dependsOnSelection = true;
8183
public readonly showNextEditPreference = ShowNextEditPreference.Always;
8284

@@ -97,11 +99,6 @@ export class XtabProvider extends ChainedStatelessNextEditProvider {
9799
@ILanguageDiagnosticsService private readonly langDiagService: ILanguageDiagnosticsService,
98100
@IIgnoreService private readonly ignoreService: IIgnoreService,
99101
) {
100-
super(XtabProvider.ID, [
101-
base => new IgnoreImportChangesAspect(base),
102-
base => new IgnoreTriviaWhitespaceChangesAspect(base),
103-
]);
104-
105102
this.delayer = new Delayer(this.configService, this.expService);
106103
this.tracer = createTracer(['NES', 'XtabProvider'], (s) => this.logService.trace(s));
107104
}
@@ -114,7 +111,39 @@ export class XtabProvider extends ChainedStatelessNextEditProvider {
114111
this.delayer.handleRejection();
115112
}
116113

117-
public async provideNextEditBase(request: StatelessNextEditRequest, pushEdit: PushEdit, logContext: InlineEditRequestLogContext, cancellationToken: CancellationToken): Promise<StatelessNextEditResult> {
114+
public provideNextEdit(request: StatelessNextEditRequest, pushEdit: PushEdit, logContext: InlineEditRequestLogContext, cancellationToken: CancellationToken): Promise<StatelessNextEditResult> {
115+
const filteringPushEdit: PushEdit = (result) => {
116+
if (result.isError()) {
117+
pushEdit(result);
118+
return;
119+
}
120+
const { edit } = result.val;
121+
const filteredEdits = this.filterEdit(request.getActiveDocument(), [edit]);
122+
if (filteredEdits.length === 0) { // do not invoke pushEdit
123+
return;
124+
}
125+
pushEdit(result);
126+
};
127+
128+
return this._provideNextEdit(request, filteringPushEdit, logContext, cancellationToken);
129+
}
130+
131+
private filterEdit(activeDoc: StatelessNextEditDocument, edits: readonly LineReplacement[]): readonly LineReplacement[] {
132+
type EditFilter = (edits: readonly LineReplacement[]) => readonly LineReplacement[];
133+
134+
const filters: EditFilter[] = [
135+
(edits) => IgnoreImportChangesAspect.filterEdit(activeDoc, edits),
136+
(edits) => IgnoreEmptyLineAndLeadingTrailingWhitespaceChanges.filterEdit(activeDoc, edits),
137+
];
138+
139+
if (!this.configService.getExperimentBasedConfig(ConfigKey.InlineEditsAllowWhitespaceOnlyChanges, this.expService)) {
140+
filters.push((edits) => IgnoreWhitespaceOnlyChanges.filterEdit(activeDoc, edits));
141+
}
142+
143+
return filters.reduce((acc, filter) => filter(acc), edits);
144+
}
145+
146+
public async _provideNextEdit(request: StatelessNextEditRequest, pushEdit: PushEdit, logContext: InlineEditRequestLogContext, cancellationToken: CancellationToken): Promise<StatelessNextEditResult> {
118147
const telemetry = new StatelessNextEditTelemetryBuilder(request);
119148

120149
logContext.setProviderStartTime();

‎src/platform/configuration/common/configurationService.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,7 @@ export namespace ConfigKey {
770770
export const CodeSearchAgentEnabled = defineSetting<boolean>('chat.codesearch.enabled', false);
771771
export const InlineEditsEnabled = defineExpSetting<boolean>('nextEditSuggestions.enabled', { defaultValue: false, teamDefaultValue: true });
772772
export const InlineEditsEnableDiagnosticsProvider = defineExpSetting<boolean>('nextEditSuggestions.fixes', { defaultValue: true, teamDefaultValue: true });
773+
export const InlineEditsAllowWhitespaceOnlyChanges = defineExpSetting<boolean>('nextEditSuggestions.allowWhitespaceOnlyChanges', true);
773774
export const NewWorkspaceCreationAgentEnabled = defineSetting<boolean>('chat.newWorkspaceCreation.enabled', true);
774775
export const NewWorkspaceUseContext7 = defineSetting<boolean>('chat.newWorkspace.useContext7', false);
775776
export const SummarizeAgentConversationHistory = defineExpSetting<boolean>('chat.summarizeAgentConversationHistory.enabled', true);

‎src/platform/inlineEdits/common/statelessNextEditProviders.ts‎

Lines changed: 20 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -3,78 +3,16 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { CancellationToken } from '../../../util/vs/base/common/cancellation';
76
import { LineReplacement } from '../../../util/vs/editor/common/core/edits/lineEdit';
8-
import { InlineEditRequestLogContext } from './inlineEditLogContext';
9-
import { IStatelessNextEditProvider, PushEdit, StatelessNextEditDocument, StatelessNextEditRequest, StatelessNextEditResult } from './statelessNextEditProvider';
7+
import { StatelessNextEditDocument } from './statelessNextEditProvider';
108

11-
export function chainStatelessNextEditProviders(base: IStatelessNextEditProvider, ...decorators: ((provider: IStatelessNextEditProvider) => IStatelessNextEditProvider)[]): IStatelessNextEditProvider {
12-
let result: IStatelessNextEditProvider = base;
13-
for (const decorator of decorators) {
14-
result = decorator(result);
15-
}
16-
return result;
17-
}
18-
19-
export abstract class ChainedStatelessNextEditProvider implements IStatelessNextEditProvider {
20-
private _impl: IStatelessNextEditProvider;
21-
22-
constructor(
23-
public readonly ID: string,
24-
private readonly _providers: ((next: IStatelessNextEditProvider) => IStatelessNextEditProvider)[],
25-
) {
26-
const self: IStatelessNextEditProvider = {
27-
ID: this.ID,
28-
provideNextEdit: (request: StatelessNextEditRequest, pushEdit: PushEdit, logContext: InlineEditRequestLogContext, cancellationToken: CancellationToken): Promise<StatelessNextEditResult> => {
29-
return this.provideNextEditBase(request, pushEdit, logContext, cancellationToken);
30-
}
31-
};
32-
this._impl = chainStatelessNextEditProviders(self, ...this._providers);
33-
34-
}
35-
36-
public provideNextEdit(request: StatelessNextEditRequest, pushEdit: PushEdit, logContext: InlineEditRequestLogContext, cancellationToken: CancellationToken): Promise<StatelessNextEditResult> {
37-
return this._impl.provideNextEdit(request, pushEdit, logContext, cancellationToken);
38-
}
39-
40-
abstract provideNextEditBase(request: StatelessNextEditRequest, pushEdit: PushEdit, logContext: InlineEditRequestLogContext, cancellationToken: CancellationToken): Promise<StatelessNextEditResult>;
41-
}
42-
43-
export abstract class EditFilterAspect implements IStatelessNextEditProvider {
44-
get ID(): string { return this._baseProvider.ID; }
45-
46-
constructor(
47-
private readonly _baseProvider: IStatelessNextEditProvider,
48-
) {
49-
}
50-
51-
async provideNextEdit(request: StatelessNextEditRequest, pushEdit: PushEdit, logContext: InlineEditRequestLogContext, cancellationToken: CancellationToken): Promise<StatelessNextEditResult> {
52-
const filteringPushEdit: PushEdit = (result) => {
53-
if (result.isError()) {
54-
pushEdit(result);
55-
return;
56-
}
57-
const { edit } = result.val;
58-
const filteredEdits = this.filterEdit(request.getActiveDocument(), [edit]);
59-
if (filteredEdits.length === 0) { // do not invoke pushEdit
60-
return;
61-
}
62-
pushEdit(result);
63-
};
64-
65-
return this._baseProvider.provideNextEdit(request, filteringPushEdit, logContext, cancellationToken);
66-
}
67-
68-
abstract filterEdit(resultDocument: StatelessNextEditDocument, singleEdits: readonly LineReplacement[]): readonly LineReplacement[];
69-
}
70-
71-
export class IgnoreTriviaWhitespaceChangesAspect extends EditFilterAspect {
72-
override filterEdit(resultDocument: StatelessNextEditDocument, singleEdits: readonly LineReplacement[]): readonly LineReplacement[] {
73-
const filteredEdits = singleEdits.filter(e => !this._isWhitespaceOnlyChange(e, resultDocument.documentAfterEditsLines));
9+
export class IgnoreEmptyLineAndLeadingTrailingWhitespaceChanges {
10+
public static filterEdit(resultDocument: StatelessNextEditDocument, singleEdits: readonly LineReplacement[]): readonly LineReplacement[] {
11+
const filteredEdits = singleEdits.filter(e => !IgnoreEmptyLineAndLeadingTrailingWhitespaceChanges._isWhitespaceOnlyChange(e, resultDocument.documentAfterEditsLines));
7412
return filteredEdits;
7513
}
7614

77-
private _isWhitespaceOnlyChange(edit: LineReplacement, baseLines: string[]): boolean {
15+
private static _isWhitespaceOnlyChange(edit: LineReplacement, baseLines: string[]): boolean {
7816
const originalLines = edit.lineRange.toOffsetRange().slice(baseLines);
7917
const newLines = edit.newLines;
8018

@@ -104,3 +42,18 @@ export class IgnoreTriviaWhitespaceChangesAspect extends EditFilterAspect {
10442
return true;
10543
}
10644
}
45+
46+
export class IgnoreWhitespaceOnlyChanges {
47+
public static filterEdit(resultDocument: StatelessNextEditDocument, singleEdits: readonly LineReplacement[]): readonly LineReplacement[] {
48+
return singleEdits.filter(e => !IgnoreWhitespaceOnlyChanges._isFormattingOnlyChange(resultDocument.documentAfterEditsLines, e));
49+
}
50+
51+
/**
52+
* @remarks public only for testing
53+
*/
54+
public static _isFormattingOnlyChange(baseLines: string[], singleEdit: LineReplacement): boolean {
55+
const originalLines = singleEdit.lineRange.toOffsetRange().slice(baseLines).join('').replace(/\s/g, '');
56+
const newLines = singleEdit.newLines.join('').replace(/\s/g, '');
57+
return originalLines === newLines;
58+
}
59+
}

‎src/platform/inlineEdits/test/common/statelessNextEditProviers.spec.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import { describe, expect, it } from 'vitest';
7+
import { LineReplacement } from '../../../../util/vs/editor/common/core/edits/lineEdit';
8+
import { LineRange } from '../../../../util/vs/editor/common/core/ranges/lineRange';
9+
import { IgnoreWhitespaceOnlyChanges } from '../../common/statelessNextEditProviders';
10+
11+
describe('IgnoreFormattingChangesAspect', () => {
12+
// Helper to create test cases with less boilerplate
13+
function createEdit(baseLines: string[], newLines: string[]): LineReplacement {
14+
return new LineReplacement(new LineRange(1, baseLines.length + 1), newLines);
15+
}
16+
17+
function isFormattingOnly(base: string[], edited: string[]): boolean {
18+
return IgnoreWhitespaceOnlyChanges._isFormattingOnlyChange(base, createEdit(base, edited));
19+
}
20+
21+
// Test the core algorithm: formatting-only changes preserve content after whitespace removal
22+
it('identifies formatting vs content changes correctly', () => {
23+
// Formatting-only: content identical after removing whitespace
24+
expect(isFormattingOnly(['x=1;'], ['x = 1;'])).toBe(true);
25+
expect(isFormattingOnly([' x'], ['x'])).toBe(true);
26+
expect(isFormattingOnly(['a', 'b'], ['a b'])).toBe(true);
27+
28+
// Content changes: content differs after removing whitespace
29+
expect(isFormattingOnly(['x=1;'], ['x=2;'])).toBe(false);
30+
expect(isFormattingOnly(['x'], ['x+1'])).toBe(false);
31+
expect(isFormattingOnly(['a'], ['a', 'b'])).toBe(false);
32+
});
33+
34+
// Representative examples of common scenarios
35+
describe('common scenarios', () => {
36+
const testCases = [
37+
// Formatting-only changes
38+
{ name: 'indentation', base: [' code'], edited: [' code'], expected: true },
39+
{ name: 'space normalization', base: ['a b'], edited: ['a b'], expected: true },
40+
{ name: 'line breaks', base: ['a;', 'b;'], edited: ['a; b;'], expected: true },
41+
{ name: 'empty lines', base: [' '], edited: ['\t'], expected: true },
42+
43+
// Content changes
44+
{ name: 'value change', base: ['x=1'], edited: ['x=2'], expected: false },
45+
{ name: 'added code', base: ['f()'], edited: ['f()', 'g()'], expected: false },
46+
{ name: 'removed code', base: ['a', 'b'], edited: ['a'], expected: false },
47+
];
48+
49+
it.each(testCases)('$name', ({ base, edited, expected }) => {
50+
expect(isFormattingOnly(base, edited)).toBe(expected);
51+
});
52+
});
53+
54+
// Edge cases that could break the algorithm
55+
describe('edge cases', () => {
56+
it('handles empty content correctly', () => {
57+
expect(isFormattingOnly([''], [''])).toBe(true);
58+
expect(isFormattingOnly([''], [' '])).toBe(true);
59+
expect(isFormattingOnly([' '], [''])).toBe(true);
60+
});
61+
62+
it('handles single character changes', () => {
63+
expect(isFormattingOnly(['a'], ['a '])).toBe(true);
64+
expect(isFormattingOnly(['a'], ['b'])).toBe(false);
65+
});
66+
});
67+
});

0 commit comments

Comments
 (0)