Fix backwards compatability with Lua.workspace.checkThirdParty

[9999years]

I attempted to maintain backwards compatibility in #2354 but didn't fully understand the config type system. Here I checked for the booleans true and false:

lua-language-server/script/library.lua

Lines 611 to 617 in 3bbc8dd

	local checkThirdParty = config.get(uri, 'Lua.workspace.checkThirdParty')
	-- Backwards compatability: `checkThirdParty` used to be a boolean.
	if not checkThirdParty or checkThirdParty == 'Disable' then
	return
	elseif checkThirdParty == true then
	checkThirdParty = 'Ask'
	end

But the config value is now defined as a string:

lua-language-server/script/config/template.lua

Lines 320 to 325 in 5a763b0

	['Lua.workspace.checkThirdParty'] = Type.String >> 'Ask' << {
	'Ask',
	'Apply',
	'ApplyInMemory',
	'Disable',
	},

So when the config is loaded as a boolean, it gets converted to a string:

lua-language-server/script/config/template.lua

Lines 81 to 85 in 5a763b0

	register('String', '', function (self, v)
	return type(v) == 'string'
	end, function (self, v)
	return tostring(v)
	end)

We can fix this by checking for the string values 'true' and 'false', rather than the boolean values true and false.

[sumneko]

Thank you!

[pysan3]

Hi @9999years @sumneko ,

Reading the code, I believe when workspace.checkThirdParty is set to a boolean value, this code prevents from loading the value and fallbacks to default value in the first place.

lua-language-server/script/config/config.lua

Lines 56 to 71 in 8e880ad

	function m.setByScope(scp, key, value)
	local unit = template[key]
	if not unit then
	return false
	end
	local raw = m.getRawTable(scp)
	if util.equal(raw[key], value) then
	return false
	end
	if unit:checker(value) then
	update(scp, key, unit:loader(value), value)
	else
	update(scp, key, unit.default, unit.default)
	end
	return true
	end

Since Type.String checks type(v) == 'string' as defined here.

lua-language-server/script/config/template.lua

Lines 81 to 85 in 8e880ad

	register('String', '', function (self, v)
	return type(v) == 'string'
	end, function (self, v)
	return tostring(v)
	end)

And it is an enum as well so util:checker also checks if the value is within the enum values, hence "false" does not work either.

lua-language-server/script/config/template.lua

Lines 27 to 44 in 8e880ad

	function mt:__shl(enums)
	self.enums = enums
	return self
	end
	function mt:checker(v)
	if self.enums then
	local ok
	for _, enum in ipairs(self.enums) do
	if util.equal(enum, v) then
	ok = true
	break
	end
	end
	if not ok then
	return false
	end
	end

Related: #2407

[9999years]

Hmm, this is unfortunate. I'm not sure if there's a way around this other than extending the configuration type system.

[pysan3]

I guess if you want to preserve backwards compatibility, the best way would be to inject hardcoded converter inside this function.

lua-language-server/script/config/config.lua

Line 56 in 8e880ad

function m.setByScope(scp, key, value)

if key == ... and type(value) == "boolean" then
  value = value and "Apply" or "Disable"
end

Or, actually keep Lua.workspace.checkThirdParty key as before and introduce a new key Lua.workspace.checkThirdParty2 and change the behavior where it is referenced.

If it is hard to keep backwards compatibility I believe this should be announced as a breaking change since a lot of people depend on this option (at least in neovim community I hear a lot of people using setting this option to false before).

[9999years]

Yeah, I'll support reverting these changes until we can figure out the backwards compatibility story.

[pysan3]

Actually, I do like your change and if we can keep backwards compat, I will be happy to have the option.

If there is a way to notify breaking changes, or we could somehow write a workaround, it'll be awesome but I'd like to hand it to the core developers.

[sumneko]

Have you tried Type.Or ?