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

Add configuration option to "forcefully" toggle undercurl when not detected. #6196

Closed
filipdutescu opened this issue Mar 5, 2023 · 5 comments · Fixed by #6253
Closed

Add configuration option to "forcefully" toggle undercurl when not detected. #6196

filipdutescu opened this issue Mar 5, 2023 · 5 comments · Fixed by #6253
Labels
C-enhancement Category: Improvements E-easy Call for participation: Experience needed to fix: Easy / not much E-good-first-issue Call for participation: Issues suitable for new contributors E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR

Comments

@filipdutescu
Copy link
Contributor

Description

Due to the way some distributions and package managers package the TermInfo file of various emulators, it is possible to have a false negative reading in Helix that the used emulator does not support undercurls. Even if it actually does (for example, Alacritty in ArchLinux shows as not supporting undercurls, when it acutally does).

Proposal

A configuration option could be added to "force" Helix to use undercurls even if its automated detection mechanism says they are not supported.

@filipdutescu filipdutescu added the C-enhancement Category: Improvements label Mar 5, 2023
@pascalkuthe
Copy link
Member

The necessary configuration should be part of the normal editor configuration and passed to CrossTermBackend::new and overwrite the autodetected value there

pub fn new(buffer: W) -> CrosstermBackend<W> {

@pascalkuthe pascalkuthe added E-good-first-issue Call for participation: Issues suitable for new contributors E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR E-easy Call for participation: Experience needed to fix: Easy / not much labels Mar 5, 2023
@the-mikedavis
Copy link
Member

We have a special case for WezTerm for the same term-info-installation reasons: 98c121c

We could add one for Alacritty too since it has been supported for some time now alacritty/alacritty#5837

@filipdutescu
Copy link
Contributor Author

@pascalkuthe mentioned it might be better to handle it this way since it increases burden on us adding more and more exceptions.

@pascalkuthe
Copy link
Member

pascalkuthe commented Mar 5, 2023

We have a special case for WezTerm for the same term-info-installation reasons: 98c121c

We could add one for Alacritty too since it has been supported for some time now alacritty/alacritty#5837

just to note: Alacritty is detected just fine on every distro I tried but archlinux (and it's derivatives like manjaro), I probably missed some distros but this is not that the detection is not working for alacritty, just that the package is wrong. If you install the terminfo file from the alactritty repo there is no problem.

That's why sort of suggested adding an overwrite here because it doesn't seem sustainable to hardcode every emulator here by hand where any distro has ever messed up the terminfo file (even worse how do you deal with ssh...)

@the-mikedavis
Copy link
Member

It's a little hacky but you could also set the VTE_VERSION environment variable to 5102 as a workaround without changing the Helix codebase.

A config option is probably fine for this though, we have a similar one for forcing true-color when the detection fails.

jpttrssn added a commit to jpttrssn/helix that referenced this issue Mar 10, 2023
If set to 'true' this option will force terminal undercurl support.
jpttrssn added a commit to jpttrssn/helix that referenced this issue Mar 10, 2023
If set to 'true' this option will force terminal undercurl support.
jpttrssn added a commit to jpttrssn/helix that referenced this issue Mar 11, 2023
If set to 'true' this option will force terminal undercurl support.
archseer pushed a commit that referenced this issue Mar 14, 2023
If set to 'true' this option will force terminal undercurl support.
sagnibak pushed a commit to sagnibak/helix that referenced this issue Mar 21, 2023
If set to 'true' this option will force terminal undercurl support.
wes-adams pushed a commit to wes-adams/helix that referenced this issue Jul 4, 2023
If set to 'true' this option will force terminal undercurl support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Improvements E-easy Call for participation: Experience needed to fix: Easy / not much E-good-first-issue Call for participation: Issues suitable for new contributors E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants