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

feat: add undercurl support #2678

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

strongholder
Copy link

Context

Modern development environments, especially those utilizing Neovim or Helix, frequently rely on the Language Server Protocol (LSP) to use the undercurl style for feedback. This underlining style, prevalent in many spell checkers over time, serves as an intuitive indicator for mistakes and errors. Recognizing its broad acceptance and utility, I am convinced of the importance of undercurl support for Zellij. The absence of this feature could significantly impact my continued engagement with the project.

Scope and Implementation

While I recognize the diverse underline styles supported by various terminal emulators (including double, curly, dotted, and dashed), the prominence and functionality of undercurl make it a priority. Hence, this PR exclusively addresses the implementation of undercurl.

Related Issues

#1594
#2287

Tests

I've tested this implementation manually with Kitty and Alacritty

Kitty NeoVim:
kitty_neovim

Alacritty NeoVim:
alacritty_nvim

Kitty Terminal:
kitty_terminal

Alacritty Terminal:
alacritty_terminal

@crabdancing
Copy link

Awesome! Finally :D

@eatgrass
Copy link
Contributor

eatgrass commented Aug 25, 2023

image

Is it possible to implement along with all these styles as well?

vim/vim#9553

@strongholder
Copy link
Author

strongholder commented Aug 28, 2023

@eatgrass That would be nice, however there's already a PR aiming to implement multiple underline styles, unfortunately it's still not merged, so I was hoping that we could at least get undercurl. Here's the PR in question - #2730. I really hope that the maintainers consider merging any of these PRs because it seems that a lot of people are interested in this feature.

@eatgrass
Copy link
Contributor

@strongholder, thanks to you.
I was trying to add those styles based on your commit first; however, I found it a little tricky to manage underline rendering with multiple states, so I opened #2730. I'm sorry I didn't provide further notification here at that time.
I'm also looking forward to the maintainers merging these features.

@strongholder strongholder temporarily deployed to cachix August 28, 2023 14:00 — with GitHub Actions Inactive
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.

3 participants