uga-rosa / ccc.nvim

Color picker and highlighter plugin for Neovim.
MIT License
731 stars 12 forks source link

Option to keep the color format when using `:CccPick` #54

Closed chrisgrieser closed 1 year ago

chrisgrieser commented 1 year ago

Request for additional color space

No response

Request for other features

It would be useful there was an option for :CccPick to automatically preserve the color format under the cursor. So if I trigger the picker while my cursor is on a hex-formatted color, the output should switch to hex, if I trigger it on a hsl-formatted color, the output should switch to hsl automatically.

This would be very useful when you regularly work with different color formats.

(I am unsure whether this is maybe already possible with the plugin? At least I didn't figure out how to do this after reading the full docs.)

uga-rosa commented 1 year ago

Good suggestion. This is a feature that is currently not possible. I will add options for input and output that will be automatically set to the color format picked up.

uga-rosa commented 1 year ago

Can you try #59?

chrisgrieser commented 1 year ago

sure. What do I need to add to the config to activate that? Can't find any new options in the docs

uga-rosa commented 1 year ago

I have enabled it by default to try it out for now. I will try it for a while and if it looks good, I will write documentation and merge it.

chrisgrieser commented 1 year ago

Hmm, I tried the dev-branch with default settings (ccc.setup({})), and it converts all colors to hex for me, regardless of the color format under the cursor.

other issues that only occurred now with the dev branch:

uga-rosa commented 1 year ago

That is probably because you have not set the inputs/outputs option. The default registered hex, rgb(), hsl() should work though.

chrisgrieser commented 1 year ago

okay, I tried latest version on the dev branch, and now it works.

However, alpha being added and a character in front of the color being eaten still persist here.

uga-rosa commented 1 year ago

Maybe you are not using the latest version of the main branch. The addition of alpha values should not be specific to the dev branch. And perhaps the reason for this is that LSP automatically completes and returns missing alpha values.

uga-rosa commented 1 year ago

I think the same is true for spaces; the color range returned by the LSP may have included the previous space. I think the issue of spaces can be resolved, but the alpha is harder to resolve (since LSP does more advanced parsing than the mere lexical parsing that this plugin does).

chrisgrieser commented 1 year ago

ok, so which branch should I use for testing then? dev or main?

The issue with the alpha and the eaten spaces did not occur with previous versions of ccc.nvim though? 🤔

uga-rosa commented 1 year ago

Please try to reproduce it with the latest main branch. My point is that it is not appropriate to talk about the problem here if it reproduces in the latest main branch. This issue is for feature request.

chrisgrieser commented 1 year ago

I simply assumed this bugs were related to the implementation of the feature request, since both issues only occurred since the introduction of the feature. But I can open a separate issue, sure.

I tested on the latest main branch, and used only the minimal default config:

opt.termguicolors = true
local ccc = require("ccc")
ccc.setup{}

Upon using :CccPick, it still converts to hex (with alpha) by default, regardless of the color format under the cursor.

uga-rosa commented 1 year ago

Maybe you are not using the latest version of the main branch. The addition of alpha values should not be specific to the dev branch. And perhaps the reason for this is that LSP automatically completes and returns missing alpha values.

This command may help you understand what I am talking about.

vim.api.nvim_create_user_command("ShowLspColor", function()
    for _, client in pairs(vim.lsp.get_active_clients()) do
        if client.server_capabilities.colorProvider then
            local param = { textDocument = vim.lsp.util.make_text_document_params() }
            client.request("textDocument/documentColor", param, function(err, color_informations)
                if err or color_informations == nil then
                    return
                end
                for _, color_info in ipairs(color_informations) do
                    local color = color_info.color
                    vim.pretty_print("color:", color)
                end
            end)
        end
    end
end, {})

foo.css

body { color: #ffffff }

Let's run this command on foo.css. This is the result I got using cssls.

"color:"    {
  alpha = 1,
  blue = 1,
  green = 1,
  red = 1
}

As you can see, LSP completes the alpha value on its own, and I have no way of knowing that. However, this plugin was not originally LSP-compatible, and this is a feature that was added later. The original feature, mere lexical analysis, would be better to display an alpha slider if an alpha value was detected. The result is that the slider reads an alpha value that does not exist and the slider is displayed.

uga-rosa commented 1 year ago

So it is not a bug that the alpha value appears. It would be possible to add option alphaslider to make the display of the alpha slider configurable when the ui is opened. ("auto", "show", "hide").

chrisgrieser commented 1 year ago

yes, the LSP indeed seems to be the issue (see #60).

However, with or without LSP for highlighting enabled, the color format (this FR) is not never picked up for me.

uga-rosa commented 1 year ago

Since this is a feature to be added in PR #59, it would be obvious that it is not possible with the current main. What is your point?

chrisgrieser commented 1 year ago

Since this is a feature to be added in PR https://github.com/uga-rosa/ccc.nvim/pull/59, it would be obvious that it is not possible with the current main. What is your point?

Now I am confused. You asked me to test the new feature, I switched to the dev branch (that is being added in #59) to do so. When I asked what config/branch I should use for testing the recognize feature, you told me to use the main branch. And now the feature actually isn't included in the main branch?

Like I am super confused right now, sorry 🙈

uga-rosa commented 1 year ago

You are confused because you are talking about two things in parallel. The two bugs you mentioned (the one related to alpha and spaces) are not caused by the dev branch but reproduced in the latest main branch, which is a completely separate issue from this feature request issue.

chrisgrieser commented 1 year ago

Ah I see, you meant switching to main for the two bugs, I see. Sorry for the mix-up.

Okay then, so I switched to the dev branch, and it initially did not work. Checking #59, I noticed that you changed it to be disabled by default (was confused, cause you initially said you enabled it by default).

Reading a bit, I also now notice that you mention that input/output has to be set, I thought you meant the input/output top level settings, but now realize you meant recognize.input/recognize.output.

Anyway, long story short, I tested #59, and knowing all this, everything works as intended. Thanks a lot, and again sorry for the confusion!

uwla commented 1 month ago

Hello, I'm still experiencing this issue (original color format not kept). How can I keep the original?