Repository navigation
ColorProvider API: Let extensions define the color formats #34366
Description
Activity
Additional suggestion:
RenameColorRangetoColorInformation. This makes the API consistent with the DocumentProviders API👍 for
ColorInformation, also 👍 form more details inColorInformation, tho unsure aboutinsertTextjust being a string with a range. Either we make it a TextEdit (similar things we deprecated on the CompletionItem) or we add an optional range (defaults to the information range). Or we just have one array that is just TextEdits...I like the proposal but would make everything text edits.
I like the proposal but would make everything text edits.
I like that. We only have to think if we would ever want to support snippet-style-color insertion? E.g one could insert a choice snippet with all color formats?
I agree that we should not have formats in the API.
I also agree with Johannes Rieken (@jrieken) and Dirk Bäumer (@dbaeumer).
Thanks all for your input on this ❤️
Previously the only concern I have about delegating formatting to extensions is it might lead to potential performance issue (flooding extension host when dragging inside saturation box, re #32235 (comment) ). Since we already reduce document changes with #34001 , I did testing again with functional formatter, the performance is reasonable: my idle CPU usage of Code is 20%, keep dragging inside saturation box will increase the usage to 40-50% on my machine. The number is acceptable as none one keeps dragging quickly forever, even they do so the CPU usage adds 20 - 30 % is still not bad.
Based on Martin Aeschlimann (@aeschli) 's proposal and suggestions from Jo and Joao in #32235 , now I change the API to
export class ColorPresentation { /** * The label of this color presentation. It will be shown on the color * picker header. By default this is also the text that is inserted when selecting * this color presentation. */ label: string; /** * An [edit](#TextEdit) which is applied to a document when selecting * this presentation for the color. When `falsy` the [label](#ColorPresentation.label) * is used. */ textEdit?: TextEdit; /** * An optional array of additional [text edits](#TextEdit) that are applied when * selecting this color presentation. Edits must not overlap with the main [edit](#ColorPresentation.textEdit) nor with themselves. */ additionalTextEdits?: TextEdit[]; } export interface DocumentColorProvider { provideDocumentColors(document: TextDocument, token: CancellationToken): ProviderResult<ColorInformation[]>; provideColorPresentations(colorInfo: ColorInformation, token: CancellationToken): ProviderResult<ColorPresentation[]>; }
What it covers:
- For languages like CSS/JSON, a list of
labelis enough. - If extensions want to display different things on the color picker header, they can add a
textEditfor color change. For example, we may want to show RGB on header while insertingUIColor()while writing Swift. - Extensions can use
additionalTextEditsto make additional content change like adding imports.
I separate
textEditandadditionalTextEditsas we need to track the primary edit (which is used to change the color) for range update. I went through iOS/Android/UWP color definitions, didn't find a very strong case for snippet so at this moment, we can add support for this in the future.- For languages like CSS/JSON, a list of
- added a commit that references this issue
on Sep 19, 2017 Synced with Martin Aeschlimann (@aeschli) , the
provideColorPresentationsapi is closer toprovideDocumentColorsinstead ofresolve.*API, adocumentparameter is necessary so the latest one isprovideColorPresentations(document: TextDocument, colorInfo: ColorInformation, token: CancellationToken): ProviderResult<ColorPresentation[]>;I like it but I'd replace
textEdit?: TextEdit;withinsertText: string; range: Rangeto be align with what we have completion items. Martin Aeschlimann (@aeschli) Let's discuss on Monday and make this stableI always thought that we have insertText and range on a completion item is some sort of backwards compatibility. Wouldn't it make more sense to always express this as a text edit ?
I always thought that we have insertText and range on a completion item is some sort of backwards compatibility.
No, moved
rangeup and deprecated thetextEditto support snippet completions. We did not make the text edit support snippets because a snippet needs an editor and a text edit only needs a model. That's why we have mixed the edit into the completion item.Marking as fixed. API is up-to-date, and extensions and language servers have adopted the new API. We can create new issues for follow-up issues.
- locked and limited conversation to collaborators
on Nov 17, 2017
The proposed API for the color provider currently has a knowledge of color formats. Problem is these are language specific. In the case of CSS there are even more formats coming.
Therefore I'd suggest the following API instead of the currently proposed resolveDocumentColor
Given a color value (e.g.
{red:1, green:0, blue:0}) , the provider returns a number of possible presentations: (#FF0000,rgb(255, 0, 0),hsl(0, 100%, 50%),red) that apply at the given range.The color picker can let the user toggle through the different presentations.
While the color picker is open and the user looks at different colors, new presentation requests will be sent to the provider for each color value. The color picker label will be updated once the result arrives.
labelfield which is used as label to show to the user in the color picker.insertTextis provided, theinsertTextwill be used if the when the color picker decides to apply that color presentation at the given color range. If noinsertTextis provided, thelabelwill be used asinsertText.If there is a need for it,
insertTextcould also supportSnippetString.additionalTextEditsare set, these edits can be additionally applied (e.g. for adding an import statement)typefile is optional. The idea is that the same type of presentation (e.g. hsl) always gets the same type (type: hsl). That way the color picker can stick with the same presentation type while the user browses through different colors.