facelessuser / ColorHelper

Sublime plugin that provides helpful color previews and tooltips
https://facelessuser.github.io/ColorHelper/
MIT License
253 stars 30 forks source link

[PackageDev] Add support for var() #186

Open MattDMo opened 3 years ago

MattDMo commented 3 years ago

Description

The support for PackageDev themes and color schemes is great - it's what I most often use this plugin for. Would it be possible to add previews for the simple "var(some_color_variable)" syntax?

For example, in a color scheme, I define a variable and then use it for a global:

{
    "variables": {
        "blue": "#0042FF"
    },
    "globals": {
        "selection": "var(blue)"
    }
}

The color preview shows up next to the #0042FF as it should, but I would like that same preview to show up next to var(blue). I don't use any of the color() mod function options, but I imagine those that do would probably like to see previews of those, too (if possible, given they're implemented within Sublime and not exposed via the API, that I know of). However, even if you can't do that or it would take a long time, I should think it would be fairly straightforward to highlight var(). Unless the variables themselves are made using color()...

facelessuser commented 3 years ago

Here's the thing. I have the capability to parse, that part isn't hard. But this requires us to parse the whole file every time to get the context of all the var() values.

Currently, ColorHelper works on a sliding window. Everything that is visible (both horizontally and vertically) gets parsed and has previews added. Everything outside that window does not.

This is mainly done for performance and is done in a general way. Additionally, any file that does something like this must have language context. In PackageDev, we must parse the file as JSON grab anything in variables and store them. In something like SCSS, we have to look for things like $var: color. I mention this because if I add support for one language, others will be asking for it in others.

In such cases, you almost need something like a language server to provide such info asynchronously.

The idea was to keep things generic but simple. Or as simple as can be expected considering the complexity of specifying different color types per file along with other syntax-specific preferences. I'll be the first to admit the config file is fairly complex currently.

Adding in variable detection definitely elevates the level of complexity, and then we must solve performance-related issues as we would then be scanning the entire file each time. A sliding window is no longer going to cut it. And the approach most assuredly has to be flexible enough to some mechanism to provide language-specific parsing.

For these reasons, such a feature has not been added as of yet.

MattDMo commented 3 years ago

I see, I had forgotten about the sliding window part of it. Would pairing up with LSP-json and LSP-css help? I have no idea, just throwing things out there...

facelessuser commented 3 years ago

@rwols I believe had made a suggestion while I was kneedeep in re-writing ColorHelper about somehow connecting LSP with ColorHelper. At the time I was so overwhelmed with work that I never had a chance to fully look into it.

Theoretically, we have the capability to apply the variables if we know them: https://github.com/facelessuser/ColorHelper/blob/master/custom/st_colormod.py#L528.

I'm not sure at this time what is possible with LSP and what is not. For instance, I'm not sure if LSP completely resolves colors in CSS or not, or just resolves variables. Can I get that on-demand or is the only way for LSP to send that info when it is ready, and you just have to be prepared to accept it?

So, is it possible? Maybe. We'd have to figure out how to get these two separate plugin systems talking. If we just have to be ready for LSP at any time, maybe there could be a way that ColorHelper just accepts the context data we need for a given view, and then when we are evaluating patterns in a view, we then check if we have context data.

They fundamentally approach things differently, but if we could work out some kind of way to pass context, maybe it would be possible. But, the important thing is that whatever we do on the ColorHelper side has to be generalized as we don't want language-specific code in the core, and if we do need language specific code, it has to be done in such a way that we provide a hook and point it at some external code that we can run on the fly.

AmjadHD commented 2 years ago

Can you make it work for built-in variables like --background and --accent etc. ?

facelessuser commented 2 years ago

Can you make it work for built-in variables like --background and --accent etc. ?

The problems for built-in variables and other custom variables remain the same. Nothing is made easier or faster by only adding support for built-in variables.

AmjadHD commented 2 years ago

Oh, I meant in the context of the command palette.

facelessuser commented 2 years ago

Can you elaborate? I'm not sure what your mean.

AmjadHD commented 2 years ago

image

facelessuser commented 2 years ago

Are you proposing it always loads the variables from the currently loaded color-scheme? Looks at the current active buffer and loads variables from there? Some combination?

While I'm not sure whether we'd go this route or not, I'm not quite sure I understand the expectation with this proposal.

AmjadHD commented 2 years ago

var(--name) comes from the global CS, and var(view.--name) from the view.

facelessuser commented 2 years ago

I don't think var(view.--name) is a thing.

AmjadHD commented 2 years ago

Why not ?

facelessuser commented 2 years ago

One of the reasons I'm not too keen on including variables is because it isn't even as simple as just get the current variables.

There are complexities when trying to perform such variable resolution with Sublime themes. You have to take in consideration file overloads and such which would require us to scan all color schemes every time. We'd have to find all the like-named color schemes, overlay them in the proper order, then find all the variables, and then resolve them all. ScopeHunter used to do this to get proper color scheme variables when it was still manually parsing color schemes.

Why not ?

Because var(view.--name) doesn't exist as a convention. It breaks the CSS variable identifier rule with the dot. This makes it a little more complicated to provided patterns for it. I assume you are proposing this as a new thing. I assume the thinking is that the current global scheme is considered unless prefixing view. and then the view's color scheme (which may not match the global one) is then considered? Or do you mean the scheme loaded in the view's buffer (the text)? If the latter, I assume we'd have to check the path against Sublime's package path to see if it is within the said path and if so, try to resolve it with overloads? Or maybe just ignore overloads?

Anyways, you can see how this gets more and more complicated.

Now, we could probably simplify all this by simplifying this process by only evaluating the text in the given active view and not scanning all the overloads.

Anyways, I get the gist of what you are proposing. I'm not sure how I feel about going down this road right now, but I'll keep it in mind as a possibility.

AmjadHD commented 2 years ago

I don't think considering overloads is necessary, just go the easy way: var(view.--name) is got from view.style() and var(--name) from sublime.ui_info(), all values resolved. As for view. prefix, you can make it opt-in and/or only available in the the command palette.

facelessuser commented 2 years ago

I don't think considering overloads is necessary, just go the easy way: var(view.--name) is got from view.style() and var(--name) from sublime.ui_info(), all values resolved. As for view. prefix, you can make it opt-in and/or only available in the the command palette.

If all you want is background or accent, sure, but it really isn't exposing variables as much as it is exposing some color scheme settings. I'm not sure it's worth it for such limited variable access. TBH, if I need to mix background with something, I just use ScopeHunter, copy the background, and plug it into Sublime ColorMod mixer.

I'm also not really a fan of view.--name if I'm being honest as well.