Skip to content
This repository has been archived by the owner on Oct 13, 2021. It is now read-only.

Correct boolean interpretation for config variables #365

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

baco
Copy link

@baco baco commented Apr 3, 2021

Instead of comparing configuration values read from local config files
with integer values 1 or 0, use their boolean interpretation as they can
be directly set to v:true or v:false respectively in config files.

Solves #364

Instead of comparing configuration values read from local config files
with integer values 1 or 0, use their boolean interpretation as they can
be directly set to v:true or v:false respectively in config files.
@baco baco force-pushed the correct-boolean-options branch from ddb3665 to 6e26f0e Compare April 3, 2021 04:03
@baco baco changed the title Correct boolean interpretation of config variables Correct boolean interpretation for config variables Apr 3, 2021
@@ -98,7 +98,7 @@ local function applyAddtionalTextEdits(completed_item)
if completed_item.user_data.lsp ~= nil then
local item = completed_item.user_data.lsp.completion_item
-- vim-vsnip have better additional text edits...
if vim.fn.exists('g:loaded_vsnip_integ') == 1 then
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that the == 1 is mandatory here, as VimL functions return an integer.

Copy link
Author

Choose a reason for hiding this comment

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

Result may be an integer, but according to documentation, any value other than zero is considered True, and only 0 is considered False:

                                                       exists()                                            
exists({expr})  The result is a Number, which is TRUE if {expr} is                                          
                defined, zero otherwise.

So which integer doesn't really matters, as long as it can be interpreted the same way boolean expressions are interpreted in C language.

Restricting the comparison to == 1 may lead to unexpected behaviors if for instance, on runtime, the exists() function decides to return 2; which according to documentation is also a valid return value when the expression is defined.

Copy link
Contributor

@theHamsta theHamsta Apr 17, 2021

Choose a reason for hiding this comment

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

But 0 is true in Lua. You could do if XX ~= 0

This comment was marked as outdated.

Copy link
Author

Choose a reason for hiding this comment

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

But 0 is true in Lua. You could do if XX ~= 0

Ok, true, corrected the PR

@vigoux
Copy link
Collaborator

vigoux commented Apr 18, 2021

I think the change should be done in get_option so that it returns a boolean if needed, instead of changing the various statements like you did in the first version of the PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants