Integrate inline color boxes & API finalization.

[rebornix]

Fix #32295. Move inline color boxes rendering to core/contrib as we already ask for color ranges.

This adds a new option renderDecorator to color range, which decides whether we should render inline box for a color range. cc @jrieken @joaomoreno

[mention-bot]

@rebornix, thanks for your PR! By analyzing the history of the files in this pull request, we identified @joaomoreno and @jrieken to be potential reviewers.

[jrieken]

Shouldn't this be a user setting?

[joaomoreno]

Interesting... I wonder if this would function better as a user setting.

[rebornix]

Users need to have settings to control the visibility of color decorator, but I'd like to give the decision to extensions.

Right now, our internal CSS and JSON extensions have four settings for decorators rendering:

For pure CSS/JSON, the setting can be a single boolean. In this case, having a setting in the core which can be language-wise is good enough.

However, if the document has embedded languages, like CSS+JS in HTML, turning on Color Decorators only for CSS sounds fair and to make that happen in the core, we need either scoping, or asking the extension in which range should we render the decorators. Considering we already ask extensions for color range, we can ask them whether to render the inline color decorator for a specific range or not at the same time.

If we decide to make ColorPicker and Color Decorators two standalone features, then I prefer to have a user setting to control Color Decorators, and do following

1. Configuration migration, from css.colorDecorators.enable to "[css]": { "editor.colorDecorators": true }.
2. Maybe need a new API to ask extensions for which range of document are allowed to show inline decorators and what's the proper value they stand for.

#2 will make it pretty similar to what we already have. So my take here is

• DocumentColorProvider is for Color related features, including Color Picker, inline Color Decorator and even more in the future.
• Give the decision to extensions: show inline Color Decorator or not, show Color Picker as Hoever or Peek, etc. Extensions decide what kind of settings they want to expose to users.

Look forward to your comments.

[jrieken]

Give the decision to extensions:

Disagree, we control the UI and the extensions just provide the data. I think a core experience of our editor is that it always behaves the same, no matter in what language you are coding.

[joaomoreno]

I'd go with a simple user setting for now, which can easily be language scoped, and wait for users requesting the very specific I want color boxes here but not there request. We can always add that later.

[rebornix]

I'm convinced of a core experience of our editor is that it always behaves the same . I only have one question

we control the UI and the extensions just provide the data

Is it reasonable that the provider provides view state? AFAIK, we already did that in TreeDataProvider, which feeds us TreeItem and TreeItem has a view state collapsibleState (whether it's collapsed or expanded). In our case, renderDecorator decides the visibility of the inline color decorator. @jrieken thoughts?

[rebornix]

Talked with @kieferrm about this and we thought adding a separate option for controlling the visibility of color decorator makes things more complex. Inline Color Box relies on Color Range, Color Range is controlled by extensions, allowing users to disable color box but no way to disable color picker seems weird. So we decided to keep the API clean:

• provideDocumentColors still just returns a list of color ranges. We will render the color box for it, when the mouse hovers on this range, the Color Picker shows up.
• Extensions trigger onDidChangeColors event when necessary, e.g., settings change.
• Extensions do the scoping themselves by slicing the color ranges. For example, HTML extension can just return color ranges in <style> tag and then only styles have the color box and color picker.

If ppl think we should separate these two features, then we add options in the future.

FYI @aeschli , I removed the color box decorations and use the new API instead.

[joaomoreno]

Do we really need a onDidChangeColors? Note that we don't have a onDidChangeDocumentLinks. We really just need to listen on model change events.

[rebornix]

@joaomoreno onDidChangeColors allows extensions to set configuration for enabling/disabling color picker / color boxes. Right now CSS/JSON extensions already have colorDecorators.enable configs, if there is no way to update the color ranges, users have to restart VSCode after changing configs.

Besides, if we listen to configuration change internally and ask for color ranges each time we receive a configuration change, DocumentColorProvider.provideDocumentColors needs to check their settings synchronously in order to get the accurate setting value. If extensions check settings asynchronously, they may run into bugs.

[rebornix]

Latest commit already covered Monaco API and what we discussed in #32235 (comment) .

Speaking of Monaco API, one blocker is #32470. I already pushed a fix for it and let's see if that works perfectly.

[joaomoreno]

I don't understand why we'd need to keep JSON and CSS extension color-related settings.

All color settings should be in core, and they should be customizable per language using scoping. JSON and CSS need to drop their own settings.

The core should listen on the configuration change and, depending on its value, ask the providers to give it colors.

[aeschli]

+1 on Joaos suggestion. A new editor.colorDecorator settings would be much nicer than what we have now.

[aeschli]

@rebornix I'm also looking to having the actual colors value (r/g/b) provided by the language server. That way we can get rid of the color parsing/convertion code in the client and can remove the Color.fromHex API. Let me know if I should wait for your change or if I should go ahead on the current state.

[rebornix]

The latest code change removes the onDidChange event, add new option editor.colorDecorators and deprecate old options in CSS/JSON languages.

• provideDocumentColors provides a list color ranges, it's up to the extension if it wants to do scope restriction. For example, if HTML extension can return color ranges in <style>
• if users set css.colorDecorators.enable to false, then we don't show the decorator and color picker in CSS file
• Otherwise it's controlled by editor.colorDecorators.

@aeschli feel free to make changes, no need to wait for this PR but I think it's pretty close to merge. This PR won't cover the Color Format discussion @joaomoreno

[rebornix]

Have the PR merged first, Monaco API will be marked as @internal as we didn't finalize the format/api yet.