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 undercurl config option #6253

Merged
merged 1 commit into from
Mar 14, 2023
Merged

Conversation

jpttrssn
Copy link
Contributor

@jpttrssn jpttrssn commented Mar 10, 2023

Fixes #6196. If set to 'true' this option will force terminal undercurl support.

I don't have ArchLinux so tested this by setting undercurl = true in my config file and running a version of helix where has_extended_underlines was only set by the config option.

I didn't find any related integration tests that could help me in writing one for this. If needed (and someone can point me in the right direction) I may be able to add that as well.

@jpttrssn jpttrssn changed the title Add undercurl config option #6196 Add undercurl config option Mar 10, 2023
pascalkuthe
pascalkuthe previously approved these changes Mar 10, 2023
Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

LGTM thanks for taking this on 👍 This will also be useful for people using ssh. Sadly terminfo can be unreliable both because some distros don't package it correctly but also because there is no solution for situations like ssh

@pascalkuthe pascalkuthe added E-easy Call for participation: Experience needed to fix: Easy / not much A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Mar 10, 2023
@the-mikedavis the-mikedavis self-requested a review March 10, 2023 23:04
@pascalkuthe
Copy link
Member

pascalkuthe commented Mar 11, 2023

Looks like you forgot to update some function arguments in tests @jpttrssn sorry I forgot about tests and didn't wait for CI to finish before posting my review

If set to 'true' this option will force terminal undercurl support.
@jpttrssn
Copy link
Contributor Author

I think I've fixed the tests. I didn't know how to run them! I created a PR that updates the contributing documentation on how to run tests for Rust newbies like myself. #6268

@jpttrssn jpttrssn requested review from pascalkuthe and removed request for the-mikedavis March 12, 2023 06:46
Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

LGTM now 👍

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Thanks! Sadly I need this option too, despite tinkering with my tmux config for ages I'm not able to get it working without an override :/

@archseer archseer merged commit d479adf into helix-editor:master Mar 14, 2023
sagnibak pushed a commit to sagnibak/helix that referenced this pull request 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 pull request 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
A-helix-term Area: Helix term improvements E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add configuration option to "forcefully" toggle undercurl when not detected.
3 participants