Color Picker support in LSP #2053

predragnikolic · 2022-09-14T08:31:36Z

This is a draft and will probably take time to make right :)

Here is how I expect the color picker to behave in this PR:

it should not block ST:
✅ Linux
✅ Mac
❌ Windows
The color picker should close when we click outside of a color picker (when ST.View is activated)""
✅ Linux - the color picker window gains focus when clicking on a color box, thus clicking on a ST view will trigger a on_activated call which will close the Color Picker.
❌ Mac - the color picker window DOESN't gains focus when clicking on a color box, thus clicking on a ST view will NOT trigger a on_activated call, so the ColorPicker will still not be closed.
❌ Windows - I think that instead of cc.hwndOwner = sublime.active_window().hwnd() it would be better to have cc.hwndOwner = None and somehow programatically bring the color picker window in front, beacuse with cc.hwndOwner = None it will appear behind the current st window.
Each time a user chooses a color, the color picker will normailze the color to match the Color type.
✅ Linux
❌ Mac - When we chose a color, OSX output contains the following warning (and a color at the end):
2022-09-14 10:13:38.606 osascript[66572:9484254] It's not legal to call -layoutSubtreeIfNeeded on a view which is already being laid out. If you are implementing the view's -layout method, you can call -[super layout] instead. Break on void _NSDetectedLayoutRecursion(void) to debug. This will be logged only once. This may break in the future.
[0.16188296675682068,1,0.7679865956306458]

I could write a regex to extract the last line and parse the color. But I would like to see if there is a better way to fix it.
✅ Windows

Supports aplha channel:
✅ Linux
❌ Mac
❌ Windows

TODO:
Apart form me not being able to fix all steps marked as ❌.
If anyone sees a way to fix any of the ❌ feel free to directly push a fix on this branch,
or leave a fix in the comment :)
Linux support is currently limited to GTK, I do not think that all Linux distributions have GTK installed,
so I think that we can improve that by providing a fallback.

Other options I have considered

Telling the user to install the ColorPicker plugin from PackageControl, and use that in LSP.
But that approach is not the best user experinece. (the code for that is at https://github.com/sublimelsp/LSP/tree/feat/add-color-picker-support)

Here are some of the cons:

it blocks ST, ideally they should not block ST
there can be multiple ColorPicker Windows left open, ideally they should be automatically closed when clicked outside
there is not a unified output of color, ideally each color picker should output should be standardised.

closes #1291

- linux for now - this gives us more control like: - kill the picker process when the view is focused - kill the picker process when ST is exited - fix bug that made preselect color to not work correctly on linux with the ColorPicker plugin. - also with this commit I fixed a bug that cause the text edit to be inserted in the wrong view

I am on linux so I will test this once on windows

…returns the same thing

I am on linux tying to make a pikcer work on windows... :D all the cahnges I do are on linux XD

Ideally on windows I would love to have a picker that supports RGBA and to not block the ST UI. Like it is implemented on Linux.

TODO: - there is a warning printed in stdout, or stderr to be precise. find a way to fix this. - bring the color window to front, there is an app.activate() method, but it doesn't work. - there is a bug with killing a process if the process is not alive

…new color

… parse errors

…-for-more-control

predragnikolic · 2022-09-14T08:47:19Z

Here is how it looks on Linux.

Note at the end of the gif(which is hard to spot), I am clicking outside of the color picker which closes the color picker

rchl · 2022-09-14T08:54:58Z

I'm a bit concerned that the native pickers might not support all the features that the LSP spec might support. For example choosing the presentation of the color (whether it's inserted as hex/rgb/hsl etc.)

Ideally there would be a custom unified color picker for all platforms I guess but not sure if that's feasible.

(Also I'm afraid of crashing ST when going so low-level.)

predragnikolic · 2022-09-14T11:59:36Z

Yeah, I would also love a unified cross OS color picker.

Here is a list of my failed attempts :D

I searched Github and could not find a cross platform color picker.
Also looked at a way to see if I could find Chromes color picker anywhere , because it seems like it is cross platform. But couldn't find a way to do that.
Tied to see if rust has a cross platform GUI lib to create a color picker app ... to be hard core and write the app myself and compile it to different platforms... I am not hard core :)

... At the end this PR is unfortunately the best of my ability :)

For example choosing the presentation of the color (whether it's inserted as hex/rgb/hsl etc.)

Currently I implemented the color picker to insert hex values
and on purpose left out the textDocument/colorPresentation for a future PR.

VS Code shows the color presentation in the color picker
and the user can select the desired color presentation in the editor:

In this PR, I am using system pickers(which are limited in functionality, OSX and Windows color pickers do not support alpha channel).
Just to give one way of achieving the color presentation in Sublime Text with OS color pickers,
is when someone selects a color, send a textDocument/colorPresentation with the selected color, and show the response in a popup: (the same way that ColorHelper package does):

IMO, for the start, inserting just hex values seems just enough,
especially because most of more important stuff is still not done in this PR :)

rchl

I'm very skeptical about this approach but I've still made some comments.

Feels like it would be a pain to support and maintain. Ideally ST should be providing a color picker API.

ind.css

plugin/color_picker/__init__.py

rchl · 2022-09-15T19:46:21Z

plugin/color_picker/linux_executable.py

@@ -0,0 +1,23 @@
+#!/usr/bin/env python
+import gi


I assume this package is not installed by default?

It is installed by default in GNOME and other GTK distrons,
but I guess it is not installed on KDE or other distros that use Qt.

That said, currently this script doesn't support all linux versions.
We could do something like ColorHelper https://github.com/facelessuser/ColorHelper/releases/tag/st3-3.1.0
.

Or something like ColorPicker, to have a fallback in the script
https://github.com/weslly/ColorPicker/blob/master/lib/linux_colorpicker.py#L21

rchl · 2022-09-15T19:46:29Z

plugin/color_picker/linux_executable.py

@@ -0,0 +1,23 @@
+#!/usr/bin/env python


Is shebang necessary?

Will check when I'm on linux.

plugin/session_buffer.py

plugin/color_picker/__init__.py

rchl · 2022-09-15T20:02:27Z

Only half of the color box is clickable on Mac:

Screen.Recording.2022-09-15.at.21.57.37.mov

predragnikolic · 2022-09-16T09:24:03Z

I'm very skeptical about this approach but I've still made some comments.

I'm also not that much in favour of this PR.
And would love to have a consistent color picker across all OS. (and that could be achieved with minihtml, facelessuser showed that it is possible, but for that time is needed)

While it certainly is better to have a color picker, than not to have any color picker at all.
I will keep this draft open.

Co-authored-by: Rafał Chłodnicki <[email protected]>

rchl · 2022-09-16T09:30:39Z

While it certainly is better to have a color picker, than not to have any color picker at all.

That could be argued. :)
Depends on the usefulness of the feature and the amount of problems and/or maintenance that it might come with. :)

jwortmann · 2022-09-16T18:03:43Z

It looks like a quite nice feature, but I'm also a bit skeptical about this platform-dependent approach. I had only tested the Windows implementation some time earlier, and this blocking behavior which freezes ST seems far from optimal. Also this old color picker from the Win32 API looks really ugly and seems to be a legacy from Windows 95. Unfortunately there doesn't seem to be any other feasible alternative if you want to use a builtin color picker on Windows.

There is also a win_colorpicker.exe file in the diff, which should be removed.

But I'm also unsure about whether LSP should really provide a color picker at all. I agree that it would be much better if either Sublime Text would provide a platform independent color picker API, or at least if we could use some dependecy from Package Control to create the color picker.

But I could imagine that instead of a color picker, LSP could run textDocument/colorPresentation (if the server supports it) when you click on the color box. Then the results could be shown in a quick panel. That should be much easier to implement and maintain.

IMO This is better than the font-size hack

plugin/core/views.py

predragnikolic · 2022-09-20T12:42:02Z

been experimenting locally.
minihtml can be used to create a unified color picker (that supports alpha and colorPresentation)

It is possible, but it should be a ST dependency on its own...

predragnikolic · 2022-09-20T12:49:05Z

Would you think that it would be better to have a ST minihtml color picker then to use the native OS color picker?

jwortmann · 2022-09-20T14:38:14Z

I think I would prefer the minihtml color picker. It looks very impressive! But maybe I would make it a little bit smaller, so that there are no scroll bars needed. Or try to adjust the max_width and max_height for View.show_popup.

I guess that "SELECT" replaces the color in the view, and the link in the top right corner would run the textDocument/colorPresentation request?

predragnikolic · 2022-09-20T14:56:16Z

I guess that "SELECT" replaces the color in the view, and the link in the top right corner would run the textDocument/colorPresentation request?

Yes, that is what I had in mind.

Note, none of the functionality of the picker works.
I just generated clickable color divs :⁠-⁠)

I would need more knowledge for understanding colors.
It is a fun topic to learn.

predragnikolic · 2022-09-20T14:59:25Z

I will close this draft.

If I ever get a fully functional minihtml implementation for the color picker, I will open a PR.

jwortmann · 2022-09-20T15:02:45Z

I would need more knowledge for understanding colors. It is a fun topic to learn.

It's a bit offtopic and not relevant for LSP, but there is a really interesting blog post at https://ciechanow.ski/color-spaces/ about how color spaces work. It's a good read for a long evening and fun to play with the interactive widgets.

predragnikolic and others added 27 commits September 7, 2022 15:27

make color box clickable

07223ea

Finalize color picker implementation

d34c06c

fix box rendering and import the command from the right place

b5e292b

smaller color boxes

1c02247

add back margin top to make the color box look centered

e3de718

fix stylelint

f26d2d9

add abstraction for creating Color Pickers

113f339

add windows executable,

23957e9

I am on linux so I will test this once on windows

add windows color picker executable

c464502

add a normalize_color to enusre that each ColorPicker implementatoin …

4fc9bcd

…returns the same thing

fix lint

ea17403

float map, also add fallback

b0da79c

no need to fallback

fa50534

remove ColorPickResult

df001a4

move everthing to one file

751858b

add a windows_executable

ef59300

asdfjklc

4c60249

I am on linux tying to make a pikcer work on windows... :D all the cahnges I do are on linux XD

make to color picker window on display on top

09f8008

this is the best I can do on windows for now

81fcfbf

Ideally on windows I would love to have a picker that supports RGBA and to not block the ST UI. Like it is implemented on Linux.

add mac picker

03d3ea0

TODO: - there is a warning printed in stdout, or stderr to be precise. find a way to fix this. - bring the color window to front, there is an app.activate() method, but it doesn't work. - there is a bug with killing a process if the process is not alive

reduce margin top

0d02c85

make sure to close the previous color picker before trying to pick a …

4754306

…new color

add comment

4ac908e

empty string is also a string, add check if string is truthy to avoid…

12fbff0

… parse errors

ignore errors if the process was already killed

9f1a540

Merge branch 'main' into feat/add-color-picker-support-v2-bundle-them…

1661538

…-for-more-control

rchl reviewed Sep 15, 2022

View reviewed changes

predragnikolic and others added 3 commits September 16, 2022 11:25

Update plugin/color_picker/__init__.py

ab8352b

Co-authored-by: Rafał Chłodnicki <[email protected]>

add sys import

51eb05f

remove test file

ebb9f4d

predrag-codetribe added 3 commits September 16, 2022 11:52

add back the if check

b1ac99b

add platform checks

7dd6a3b

hack - make the hole box clickable

8a1e5cd

an image and display block will make the whole color box clickable

ad43def

IMO This is better than the font-size hack

rchl reviewed Sep 19, 2022

View reviewed changes

plugin/core/views.py Outdated Show resolved Hide resolved

predrag-codetribe added 2 commits September 20, 2022 10:12

use  

f06895c

remove win_colorpicker.exe

a6d1e47

predragnikolic closed this Sep 20, 2022

rchl mentioned this pull request Sep 23, 2022

Request color presentations when clicking on a color box #2065

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Color Picker support in LSP #2053

Color Picker support in LSP #2053

predragnikolic commented Sep 14, 2022 •

edited

Loading

predragnikolic commented Sep 14, 2022

rchl commented Sep 14, 2022

predragnikolic commented Sep 14, 2022 •

edited

Loading

rchl left a comment

rchl Sep 15, 2022

predragnikolic Sep 16, 2022

rchl Sep 15, 2022

predragnikolic Sep 16, 2022

rchl commented Sep 15, 2022

predragnikolic commented Sep 16, 2022

rchl commented Sep 16, 2022

jwortmann commented Sep 16, 2022

predragnikolic commented Sep 20, 2022

predragnikolic commented Sep 20, 2022

jwortmann commented Sep 20, 2022

predragnikolic commented Sep 20, 2022

predragnikolic commented Sep 20, 2022

jwortmann commented Sep 20, 2022

Color Picker support in LSP #2053

Color Picker support in LSP #2053

Conversation

predragnikolic commented Sep 14, 2022 • edited Loading

Other options I have considered

predragnikolic commented Sep 14, 2022

rchl commented Sep 14, 2022

predragnikolic commented Sep 14, 2022 • edited Loading

rchl left a comment

Choose a reason for hiding this comment

rchl Sep 15, 2022

Choose a reason for hiding this comment

predragnikolic Sep 16, 2022

Choose a reason for hiding this comment

rchl Sep 15, 2022

Choose a reason for hiding this comment

predragnikolic Sep 16, 2022

Choose a reason for hiding this comment

rchl commented Sep 15, 2022

predragnikolic commented Sep 16, 2022

rchl commented Sep 16, 2022

jwortmann commented Sep 16, 2022

predragnikolic commented Sep 20, 2022

predragnikolic commented Sep 20, 2022

jwortmann commented Sep 20, 2022

predragnikolic commented Sep 20, 2022

predragnikolic commented Sep 20, 2022

jwortmann commented Sep 20, 2022

predragnikolic commented Sep 14, 2022 •

edited

Loading

predragnikolic commented Sep 14, 2022 •

edited

Loading