improve test for truecolor support #32

Merged
merged 5 commits into from Jun 23, 2016

Projects

None yet

2 participants

@ninrod
Contributor
ninrod commented Jun 23, 2016 edited

This improves limelight account for truecolor support in both vim and nvim.

improves the treatment of the problem described by #30.

rationale:

exists('+termguicolors') works for both vim and nvim, if this feature is present.
nvim before version 0.1.5 also supports truecolors even without +termguicolors.

@ninrod ninrod referenced this pull request Jun 23, 2016
@junegunn Fix #30 - Add support for termguicolors e93c801
@junegunn junegunn and 1 other commented on an outdated diff Jun 23, 2016
autoload/limelight.vim
@@ -139,7 +139,7 @@ function! s:dim(coeff)
let fg = synIDattr(synid, 'fg#')
let bg = synIDattr(synid, 'bg#')
- if has('gui_running') || has('termguicolors') && &termguicolors
+ if has('gui_running') || exists('+termguicolors') || has('nvim') && $NVIM_TUI_ENABLE_TRUE_COLOR
@junegunn
junegunn Jun 23, 2016 Owner

Any reason to change has('termguicolors') && &termguicolors to exists('+termguicolors')? Shouldn't we check the value of &termguicolors? It can be either on or off.

@ninrod
ninrod Jun 23, 2016 Contributor

yes @junegunn, you're right. I'll submit a change.

@junegunn junegunn and 1 other commented on an outdated diff Jun 23, 2016
autoload/limelight.vim
@@ -139,7 +139,7 @@ function! s:dim(coeff)
let fg = synIDattr(synid, 'fg#')
let bg = synIDattr(synid, 'bg#')
- if has('gui_running') || exists('+termguicolors') || has('nvim') && $NVIM_TUI_ENABLE_TRUE_COLOR
+ if has('gui_running') || (exists('+termguicolors') && termguicolors)|| has('nvim') && $NVIM_TUI_ENABLE_TRUE_COLOR
@junegunn
junegunn Jun 23, 2016 Owner

A minor nit: there's no space between ) and ||. And actually we don't need parentheses as && takes precedence over ||. Another question: why change has('termguicolors') to exists('+termguicolors') when the former is perfectly fine?

@ninrod
ninrod Jun 23, 2016 Contributor

@junegunn 2 hours ago in gitter the neovim developers told me that exists('+termguicolors') is better if you want to mantain compatibility between nvim and vim.

I'll fix the space.

@junegunn
junegunn Jun 23, 2016 Owner

I looked it up, but the conversation wasn't about has() vs. exists(). has() has long been a conventional way of checking the existence of a feature. "If it ain't broke, don't fix it."

@ninrod
Contributor
ninrod commented Jun 23, 2016

@junegunn, now I think is finished.

@junegunn junegunn merged commit 106fb57 into junegunn:master Jun 23, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@junegunn
Owner

Squashed and merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment