microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.67k stars 29.44k forks source link

Color picker inserts text repeatedly when switching between different presentations #200569

Closed DanTup closed 11 months ago

DanTup commented 11 months ago

Originally noted at https://github.com/microsoft/vscode/issues/136965#issuecomment-1850252538. When cycling between colour presentations, rather than updating the same regions, the edits/inserts are now made on every click, resulting in broken code:

https://github.com/microsoft/vscode/assets/1078012/0eed2879-fca2-463f-af1b-240d26db3bf3

To repro, create a new extension with this code, open a file with a few lines, and click on the colour picker header to cycle between the different "presentations" of the colour.

const colorRange = new vscode.Range(new vscode.Position(5, 0), new vscode.Position(5, 1));
const otherEditRange = new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0));
context.subscriptions.push(vscode.languages.registerColorProvider({ scheme: 'file' }, {
    provideDocumentColors(document: vscode.TextDocument, token: vscode.CancellationToken): vscode.ProviderResult<vscode.ColorInformation[]> {
        return [
            new vscode.ColorInformation(
                colorRange,
                new vscode.Color(1, 0, 0, 1),
            ),
        ];
    },

    provideColorPresentations(color: vscode.Color, context: { document: vscode.TextDocument, range: vscode.Range }, token: vscode.CancellationToken): vscode.ProviderResult<vscode.ColorPresentation[]> {
        const one = new vscode.ColorPresentation('one');
        one.textEdit = new vscode.TextEdit(colorRange, one.label);
        one.additionalTextEdits = [
            new vscode.TextEdit(otherEditRange, 'IMPORT')
        ];
        const two = new vscode.ColorPresentation('two');
        two.textEdit = new vscode.TextEdit(colorRange, two.label);
        two.additionalTextEdits = [
            new vscode.TextEdit(otherEditRange, 'IMPORT')
        ];
        return [
            one,
            two,
        ];
    }
}));
aiday-mar commented 11 months ago

This issue has helped me uncover a bug in the current color picker code for which I opened a PR (linked above).

However by looking again at the provider you included as a test example, I believe the code is working as expected in your GIF. The reason I say that is because the color range is defined as

const colorRange = new vscode.Range(new vscode.Position(5, 0), new vscode.Position(5, 1));

You define the color as being defined only by the first character on line 5. When the color picker is used to change the color it changes only the first character of line 5 as is designed by the code.

DanTup commented 11 months ago

Ah, you're right - it seems the provider is re-called between each click so it can give an updated range and I forgot about that. In my real implementation this doesn't occur:

https://github.com/microsoft/vscode/assets/1078012/4a9412ac-6656-4a4f-af50-41af45e240fd

Sorry for the bogus report!