Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colors for items in the completion menu #12299

Merged
merged 30 commits into from
Dec 20, 2024

Conversation

nik-rev
Copy link
Contributor

@nik-rev nik-rev commented Dec 19, 2024

This PR adds little colored boxes to completion items if its a color

image

It works on any Color completion item, so it works with CSS, tailwindcss, any other language server which provides "colors".

@nik-rev nik-rev changed the title feat: "Color" completion items now have a little box next to them showing the actual color feat: Color completion items now have a little box next to them showing the actual color Dec 19, 2024
@nik-rev nik-rev changed the title feat: Color completion items now have a little box next to them showing the actual color feat: Preview color for completion items Dec 19, 2024
@nik-rev nik-rev changed the title feat: Preview color for completion items Preview color for completion items Dec 19, 2024
@nik-rev nik-rev changed the title Preview color for completion items Color for completion items Dec 19, 2024
@nik-rev nik-rev changed the title Color for completion items Colors in the completion menu for items with CompletionItemKind::COLOR Dec 19, 2024
@nik-rev nik-rev changed the title Colors in the completion menu for items with CompletionItemKind::COLOR Colors for items in the completion menu Dec 19, 2024
helix-term/src/ui/completion.rs Outdated Show resolved Hide resolved
helix-term/src/ui/completion.rs Outdated Show resolved Hide resolved
helix-term/src/ui/completion.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@TornaxO7 TornaxO7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just my two cents 👀 You can decline them. But nice feature!

helix-term/src/ui/completion.rs Outdated Show resolved Hide resolved
helix-view/src/graphics.rs Show resolved Hide resolved
helix-term/src/ui/completion.rs Outdated Show resolved Hide resolved
helix-term/src/ui/completion.rs Outdated Show resolved Hide resolved
@tveskag
Copy link

tveskag commented Dec 20, 2024

This looks like it will conflict with #12151. It would be logical to combine the two if possible

@nik-rev
Copy link
Contributor Author

nik-rev commented Dec 20, 2024

This looks like it will conflict with #12151. It would be logical to combine the two if possible

It's not possible to think about all 272 PRs and whether this one will conflict with any of them

depending on which PR gets merged first, the other has to resolve merge conflicts

@nik-rev
Copy link
Contributor Author

nik-rev commented Dec 20, 2024

Not for this PR, but a good follow up would be to add the square before any colour string in a file, not just on LSP completions.

I'm actually working on that https://github.com/NikitaRevenco/helix/tree/textDocument/documentColor :)

@DaveTJones
Copy link

Just a thought, is the word 'color' before every color maybe a little redundant? The only scenario in which I can see this being useful is one in which the color to be rendered happens to match the background.

An alternative use for this space might be to show the hex/RGB/HSL representation of the color?

@nik-rev
Copy link
Contributor Author

nik-rev commented Dec 20, 2024

Just a thought, is the word 'color' before every color maybe a little redundant? The only scenario in which I can see this being useful is one in which the color to be rendered happens to match the background.

An alternative use for this space might be to show the hex/RGB/HSL representation of the color?

I think it's fine to keep it as is, if we have a bunch of hsl/hex/rgb colors it will get messy + the only information we actually do have is hex colors.

We would have to manually convert them to hsl/rgb if we needed to. Then, which should be pick - rgb or hsl?"Should it be a config option? + You can already see what the color is from the completion menu. It introduces so much unnecessary complexity and not worth it at all imo

@the-mikedavis the-mikedavis merged commit ba6e6dc into helix-editor:master Dec 20, 2024
6 checks passed
@DaveTJones
Copy link

I think it's fine to keep it as is, if we have a bunch of hsl/hex/rgb colors it will get messy + the only information we actually do have is hex colors.

We would have to manually convert them to hsl/rgb if we needed to. Then, which should be pick - rgb or hsl?"Should it be a config option? + You can already see what the color is from the completion menu. It introduces so much unnecessary complexity and not worth it at all imo

I agree that adding a RGB or HSL option is probably not worth the effort. My main point was that annotating each color as 'color' is redundant.

@nik-rev
Copy link
Contributor Author

nik-rev commented Dec 20, 2024

I think it's fine to keep it as is, if we have a bunch of hsl/hex/rgb colors it will get messy + the only information we actually do have is hex colors.
We would have to manually convert them to hsl/rgb if we needed to. Then, which should be pick - rgb or hsl?"Should it be a config option? + You can already see what the color is from the completion menu. It introduces so much unnecessary complexity and not worth it at all imo

I agree that adding a RGB or HSL option is probably not worth the effort. My main point was that annotating each color as 'color' is redundant.

it's for accessibility, since not everyone can recognize differences between colors

@nferhat
Copy link

nferhat commented Dec 27, 2024

Nice stuff! Can be easily merged with #12151

CedricMeu pushed a commit to CedricMeu/helix that referenced this pull request Jan 2, 2025
GladkihEgor pushed a commit to GladkihEgor/helix that referenced this pull request Jan 4, 2025
diucicd pushed a commit to diucicd/helix that referenced this pull request Jan 8, 2025
@atheeq-rhxn
Copy link

The colors preview is not working for dart

helix_color_completion_not_working_for_dart.mp4

is it a problem with dart lsp?

@nik-rev
Copy link
Contributor Author

nik-rev commented Jan 11, 2025

The colors preview is not working for dart
helix_color_completion_not_working_for_dart.mp4

is it a problem with dart lsp?

yes. And there is nothing we can do about it.

The LSP needs to send documentation containing the hex code like #fafafa, which is what all other LSPs which have color completions do

@the-mikedavis
Copy link
Member

What does the dart language server send for color completions? What does the log say when running in verbose mode (hx -v) when completing a color?

@atheeq-rhxn
Copy link

atheeq-rhxn commented Jan 12, 2025

What does the dart language server send for color completions? What does the log say when running in verbose mode (hx -v) when completing a color?

They seem to directly give the png of color

The logs:

2025-01-12T13:00:04.015 helix_term::handlers::signature_help [ERROR] signature help request failed: failed to parse: invalid value: integer `-1`, expected u32
2025-01-12T13:00:04.044 helix_lsp::transport [INFO] dart -> {"jsonrpc":"2.0","method":"completionItem/resolve","params":{"detail":"Color","documentation":"Completely opaque black.\n\n![](https://flutter.github.io/assets-for-api-docs/assets/material/Colors.blacks.png)\n\nSee also:\n\n * [black87], [black54], [black45], [black38], [black26], [black12], which\n   are variants on this color but with different opacities.\n * [white], a solid white color.\n * [transparent], a fully-transparent color.\n\n#000000","kind":16,"label":"black","sortText":"9416","textEdit":{"newText":"black","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}},"id":6}
2025-01-12T13:00:04.045 helix_lsp::transport [INFO] dart <- {"id":6,"jsonrpc":"2.0","result":{"detail":"Color","documentation":"Completely opaque black.\n\n![](https://flutter.github.io/assets-for-api-docs/assets/material/Colors.blacks.png)\n\nSee also:\n\n * [black87], [black54], [black45], [black38], [black26], [black12], which\n   are variants on this color but with different opacities.\n * [white], a solid white color.\n * [transparent], a fully-transparent color.\n\n#000000","kind":16,"label":"black","sortText":"9416","textEdit":{"newText":"black","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}}}
2025-01-12T13:00:04.046 helix_lsp::transport [INFO] dart <- {"detail":"Color","documentation":"Completely opaque black.\n\n![](https://flutter.github.io/assets-for-api-docs/assets/material/Colors.blacks.png)\n\nSee also:\n\n * [black87], [black54], [black45], [black38], [black26], [black12], which\n   are variants on this color but with different opacities.\n * [white], a solid white color.\n * [transparent], a fully-transparent color.\n\n#000000","kind":16,"label":"black","sortText":"9416","textEdit":{"newText":"black","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}}
2025-01-12T13:00:04.510 helix_lsp::transport [INFO] dart -> {"jsonrpc":"2.0","method":"textDocument/signatureHelp","params":{"position":{"character":29,"line":8},"textDocument":{"uri":"file:///home/atheeq/Documents/code/dart/x/lib/currency_converter.dart"}},"id":7}
2025-01-12T13:00:04.512 helix_lsp::transport [INFO] dart <- {"id":7,"jsonrpc":"2.0","result":{"activeParameter":-1,"activeSignature":0,"signatures":[{"documentation":{"kind":"markdown","value":"Creates the side of a border.\n\nBy default, the border is 1.0 logical pixels wide and solid black."},"label":"BorderSide({Color color = const Color(0xFF000000), double width = 1.0, BorderStyle style = BorderStyle.solid, double strokeAlign = strokeAlignInside})","parameters":[{"label":"Color color = const Color(0xFF000000)"},{"label":"double width = 1.0"},{"label":"BorderStyle style = BorderStyle.solid"},{"label":"double strokeAlign = strokeAlignInside"}]}]}}
2025-01-12T13:00:04.512 helix_lsp::transport [INFO] dart <- {"activeParameter":-1,"activeSignature":0,"signatures":[{"documentation":{"kind":"markdown","value":"Creates the side of a border.\n\nBy default, the border is 1.0 logical pixels wide and solid black."},"label":"BorderSide({Color color = const Color(0xFF000000), double width = 1.0, BorderStyle style = BorderStyle.solid, double strokeAlign = strokeAlignInside})","parameters":[{"label":"Color color = const Color(0xFF000000)"},{"label":"double width = 1.0"},{"label":"BorderStyle style = BorderStyle.solid"},{"label":"double strokeAlign = strokeAlignInside"}]}]}
2025-01-12T13:00:04.512 helix_term::handlers::signature_help [ERROR] signature help request failed: failed to parse: invalid value: integer `-1`, expected u32
2025-01-12T13:00:04.540 helix_lsp::transport [INFO] dart -> {"jsonrpc":"2.0","method":"completionItem/resolve","params":{"detail":"Color","documentation":"Black with 87% opacity.\n\nThis is a good contrasting color for text in light themes.\n\n![](https://flutter.github.io/assets-for-api-docs/assets/material/Colors.blacks.png)\n\nSee also:\n\n * [Typography.black], which uses this color for its text styles.\n * [Theme.of], which allows you to select colors from the current theme\n   rather than hard-coding colors in your build methods.\n * [black], [black54], [black45], [black38], [black26], [black12], which\n   are variants on this color but with different opacities.\n\n#000000","kind":16,"label":"black87","sortText":"9416","textEdit":{"newText":"black87","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}},"id":8}
2025-01-12T13:00:04.542 helix_lsp::transport [INFO] dart <- {"id":8,"jsonrpc":"2.0","result":{"detail":"Color","documentation":"Black with 87% opacity.\n\nThis is a good contrasting color for text in light themes.\n\n![](https://flutter.github.io/assets-for-api-docs/assets/material/Colors.blacks.png)\n\nSee also:\n\n * [Typography.black], which uses this color for its text styles.\n * [Theme.of], which allows you to select colors from the current theme\n   rather than hard-coding colors in your build methods.\n * [black], [black54], [black45], [black38], [black26], [black12], which\n   are variants on this color but with different opacities.\n\n#000000","kind":16,"label":"black87","sortText":"9416","textEdit":{"newText":"black87","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}}}
2025-01-12T13:00:04.542 helix_lsp::transport [INFO] dart <- {"detail":"Color","documentation":"Black with 87% opacity.\n\nThis is a good contrasting color for text in light themes.\n\n![](https://flutter.github.io/assets-for-api-docs/assets/material/Colors.blacks.png)\n\nSee also:\n\n * [Typography.black], which uses this color for its text styles.\n * [Theme.of], which allows you to select colors from the current theme\n   rather than hard-coding colors in your build methods.\n * [black], [black54], [black45], [black38], [black26], [black12], which\n   are variants on this color but with different opacities.\n\n#000000","kind":16,"label":"black87","sortText":"9416","textEdit":{"newText":"black87","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}}
2025-01-12T13:00:05.008 helix_lsp::transport [INFO] dart -> {"jsonrpc":"2.0","method":"textDocument/signatureHelp","params":{"position":{"character":29,"line":8},"textDocument":{"uri":"file:///home/atheeq/Documents/code/dart/x/lib/currency_converter.dart"}},"id":9}
2025-01-12T13:00:05.011 helix_lsp::transport [INFO] dart <- {"id":9,"jsonrpc":"2.0","result":{"activeParameter":-1,"activeSignature":0,"signatures":[{"documentation":{"kind":"markdown","value":"Creates the side of a border.\n\nBy default, the border is 1.0 logical pixels wide and solid black."},"label":"BorderSide({Color color = const Color(0xFF000000), double width = 1.0, BorderStyle style = BorderStyle.solid, double strokeAlign = strokeAlignInside})","parameters":[{"label":"Color color = const Color(0xFF000000)"},{"label":"double width = 1.0"},{"label":"BorderStyle style = BorderStyle.solid"},{"label":"double strokeAlign = strokeAlignInside"}]}]}}
2025-01-12T13:00:05.011 helix_lsp::transport [INFO] dart <- {"activeParameter":-1,"activeSignature":0,"signatures":[{"documentation":{"kind":"markdown","value":"Creates the side of a border.\n\nBy default, the border is 1.0 logical pixels wide and solid black."},"label":"BorderSide({Color color = const Color(0xFF000000), double width = 1.0, BorderStyle style = BorderStyle.solid, double strokeAlign = strokeAlignInside})","parameters":[{"label":"Color color = const Color(0xFF000000)"},{"label":"double width = 1.0"},{"label":"BorderStyle style = BorderStyle.solid"},{"label":"double strokeAlign = strokeAlignInside"}]}]}
2025-01-12T13:00:05.011 helix_term::handlers::signature_help [ERROR] signature help request failed: failed to parse: invalid value: integer `-1`, expected u32

@nik-rev
Copy link
Contributor Author

nik-rev commented Jan 12, 2025

What does the dart language server send for color completions? What does the log say when running in verbose mode (hx -v) when completing a color?

They seem to directly give the png of color

The logs:

2025-01-12T13:00:04.015 helix_term::handlers::signature_help [ERROR] signature help request failed: failed to parse: invalid value: integer -1, expected u32 2025-01-12T13:00:04.044 helix_lsp::transport [INFO] dart -> {"jsonrpc":"2.0","method":"completionItem/resolve","params":{"detail":"Color","documentation":"Completely opaque black.\n\n\n\nSee also:\n\n * [black87], [black54], [black45], [black38], [black26], [black12], which\n are variants on this color but with different opacities.\n * [white], a solid white color.\n * [transparent], a fully-transparent color.\n\n#000000","kind":16,"label":"black","sortText":"9416","textEdit":{"newText":"black","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}},"id":6} 2025-01-12T13:00:04.045 helix_lsp::transport [INFO] dart <- {"id":6,"jsonrpc":"2.0","result":{"detail":"Color","documentation":"Completely opaque black.\n\n\n\nSee also:\n\n * [black87], [black54], [black45], [black38], [black26], [black12], which\n are variants on this color but with different opacities.\n * [white], a solid white color.\n * [transparent], a fully-transparent color.\n\n#000000","kind":16,"label":"black","sortText":"9416","textEdit":{"newText":"black","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}}} 2025-01-12T13:00:04.046 helix_lsp::transport [INFO] dart <- {"detail":"Color","documentation":"Completely opaque black.\n\n\n\nSee also:\n\n * [black87], [black54], [black45], [black38], [black26], [black12], which\n are variants on this color but with different opacities.\n * [white], a solid white color.\n * [transparent], a fully-transparent color.\n\n#000000","kind":16,"label":"black","sortText":"9416","textEdit":{"newText":"black","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}} 2025-01-12T13:00:04.510 helix_lsp::transport [INFO] dart -> {"jsonrpc":"2.0","method":"textDocument/signatureHelp","params":{"position":{"character":29,"line":8},"textDocument":{"uri":"file:///home/atheeq/Documents/code/dart/x/lib/currency_converter.dart"}},"id":7} 2025-01-12T13:00:04.512 helix_lsp::transport [INFO] dart <- {"id":7,"jsonrpc":"2.0","result":{"activeParameter":-1,"activeSignature":0,"signatures":[{"documentation":{"kind":"markdown","value":"Creates the side of a border.\n\nBy default, the border is 1.0 logical pixels wide and solid black."},"label":"BorderSide({Color color = const Color(0xFF000000), double width = 1.0, BorderStyle style = BorderStyle.solid, double strokeAlign = strokeAlignInside})","parameters":[{"label":"Color color = const Color(0xFF000000)"},{"label":"double width = 1.0"},{"label":"BorderStyle style = BorderStyle.solid"},{"label":"double strokeAlign = strokeAlignInside"}]}]}} 2025-01-12T13:00:04.512 helix_lsp::transport [INFO] dart <- {"activeParameter":-1,"activeSignature":0,"signatures":[{"documentation":{"kind":"markdown","value":"Creates the side of a border.\n\nBy default, the border is 1.0 logical pixels wide and solid black."},"label":"BorderSide({Color color = const Color(0xFF000000), double width = 1.0, BorderStyle style = BorderStyle.solid, double strokeAlign = strokeAlignInside})","parameters":[{"label":"Color color = const Color(0xFF000000)"},{"label":"double width = 1.0"},{"label":"BorderStyle style = BorderStyle.solid"},{"label":"double strokeAlign = strokeAlignInside"}]}]} 2025-01-12T13:00:04.512 helix_term::handlers::signature_help [ERROR] signature help request failed: failed to parse: invalid value: integer -1, expected u32 2025-01-12T13:00:04.540 helix_lsp::transport [INFO] dart -> {"jsonrpc":"2.0","method":"completionItem/resolve","params":{"detail":"Color","documentation":"Black with 87% opacity.\n\nThis is a good contrasting color for text in light themes.\n\n\n\nSee also:\n\n * [Typography.black], which uses this color for its text styles.\n * [Theme.of], which allows you to select colors from the current theme\n rather than hard-coding colors in your build methods.\n * [black], [black54], [black45], [black38], [black26], [black12], which\n are variants on this color but with different opacities.\n\n#000000","kind":16,"label":"black87","sortText":"9416","textEdit":{"newText":"black87","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}},"id":8} 2025-01-12T13:00:04.542 helix_lsp::transport [INFO] dart <- {"id":8,"jsonrpc":"2.0","result":{"detail":"Color","documentation":"Black with 87% opacity.\n\nThis is a good contrasting color for text in light themes.\n\n\n\nSee also:\n\n * [Typography.black], which uses this color for its text styles.\n * [Theme.of], which allows you to select colors from the current theme\n rather than hard-coding colors in your build methods.\n * [black], [black54], [black45], [black38], [black26], [black12], which\n are variants on this color but with different opacities.\n\n#000000","kind":16,"label":"black87","sortText":"9416","textEdit":{"newText":"black87","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}}} 2025-01-12T13:00:04.542 helix_lsp::transport [INFO] dart <- {"detail":"Color","documentation":"Black with 87% opacity.\n\nThis is a good contrasting color for text in light themes.\n\n\n\nSee also:\n\n * [Typography.black], which uses this color for its text styles.\n * [Theme.of], which allows you to select colors from the current theme\n rather than hard-coding colors in your build methods.\n * [black], [black54], [black45], [black38], [black26], [black12], which\n are variants on this color but with different opacities.\n\n#000000","kind":16,"label":"black87","sortText":"9416","textEdit":{"newText":"black87","range":{"end":{"character":22,"line":8},"start":{"character":22,"line":8}}}} 2025-01-12T13:00:05.008 helix_lsp::transport [INFO] dart -> {"jsonrpc":"2.0","method":"textDocument/signatureHelp","params":{"position":{"character":29,"line":8},"textDocument":{"uri":"file:///home/atheeq/Documents/code/dart/x/lib/currency_converter.dart"}},"id":9} 2025-01-12T13:00:05.011 helix_lsp::transport [INFO] dart <- {"id":9,"jsonrpc":"2.0","result":{"activeParameter":-1,"activeSignature":0,"signatures":[{"documentation":{"kind":"markdown","value":"Creates the side of a border.\n\nBy default, the border is 1.0 logical pixels wide and solid black."},"label":"BorderSide({Color color = const Color(0xFF000000), double width = 1.0, BorderStyle style = BorderStyle.solid, double strokeAlign = strokeAlignInside})","parameters":[{"label":"Color color = const Color(0xFF000000)"},{"label":"double width = 1.0"},{"label":"BorderStyle style = BorderStyle.solid"},{"label":"double strokeAlign = strokeAlignInside"}]}]}} 2025-01-12T13:00:05.011 helix_lsp::transport [INFO] dart <- {"activeParameter":-1,"activeSignature":0,"signatures":[{"documentation":{"kind":"markdown","value":"Creates the side of a border.\n\nBy default, the border is 1.0 logical pixels wide and solid black."},"label":"BorderSide({Color color = const Color(0xFF000000), double width = 1.0, BorderStyle style = BorderStyle.solid, double strokeAlign = strokeAlignInside})","parameters":[{"label":"Color color = const Color(0xFF000000)"},{"label":"double width = 1.0"},{"label":"BorderStyle style = BorderStyle.solid"},{"label":"double strokeAlign = strokeAlignInside"}]}]} 2025-01-12T13:00:05.011 helix_term::handlers::signature_help [ERROR] signature help request failed: failed to parse: invalid value: integer -1, expected u32

What about colors other than black? From what I can see the documentation for each completion request includes the hex color at the end.

@nik-rev
Copy link
Contributor Author

nik-rev commented Jan 12, 2025

Could you build this branch and let me know if it works for you? https://github.com/nik-rev/helix/tree/colored-color-completions-fix

@atheeq-rhxn
Copy link

atheeq-rhxn commented Jan 12, 2025

Could you build this branch and let me know if it works for you? https://github.com/nik-rev/helix/tree/colored-color-completions-fix

Its working!!

image

Why does building from source brings no theme though?

@nferhat
Copy link

nferhat commented Jan 12, 2025

Why does building from source brings no theme though?

Because it looks for runtime files elsewhere

@nik-rev
Copy link
Contributor Author

nik-rev commented Jan 12, 2025

Could you build this branch and let me know if it works for you? https://github.com/nik-rev/helix/tree/colored-color-completions-fix

Its working!!

image

Why does building from source brings no theme though?

Alright, good to know! I've made a pull request for that: #12501

@nik-rev nik-rev deleted the colored-color-completions branch January 12, 2025 11:53
rmburg pushed a commit to rmburg/helix that referenced this pull request Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants