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

fix(utils): Changed to be able to refer to default settings proposed … #973

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

misumisumi
Copy link
Collaborator

Changed to be able to refer to default settings from extend_config and load_plugin functions proposed in #972

@CharlesChiuGit
Copy link
Collaborator

from: #972 (comment)

If i want to add new settings based on default setting, i can just pass in a table, i.e.

settings["lsp_deps"] = {
	"bashls",
	"clangd",
	"html",
	"jsonls",
	"lua_ls",
	"pylsp",
	"zls", -- zig lsp
}

If i want to replace/remove some default settings, i need to pass in a function, i.e.

settings["lsp_deps"] = function()
    return {
		"lua_ls",
		"pylsp",
		"zls", -- zig lsp
    }
end

Did I missing understanding anything?

@misumisumi
Copy link
Collaborator Author

misumisumi commented Aug 28, 2023

The override method is fine
Here's a PR for a proposed change that allows users to reference the default settings table when creating functions.

@CharlesChiuGit
Copy link
Collaborator

CharlesChiuGit commented Aug 28, 2023

aha, got it.

So for referring default settings and adding new settings:

local default_setting = require("core.settings")
settings["lsp_deps"] = function(default_settings.lsp_deps)
    return {
		"zls", -- zig lsp
    }
end

For rewriting default settings:

settings["lsp_deps"] = function()
    return {
		"lua_ls",
		"pylsp",
		"zls", -- zig lsp
    }
end

am i right?

@misumisumi
Copy link
Collaborator Author

misumisumi commented Aug 28, 2023

The suggestion is to leave room for users to do their own merges.
The default setting is:

settings["lsp_deps"] = {
	"bashls",
	"clangd",
	"html",
	"jsonls",
	"lua_ls",
	"pylsp",
}

We can change the settings as follows. (use extend_config())

  • Merge setting
settings["lsp_deps"] = {
  "zls", -- zig lsp
}
-- Return:
-- {
-- "bashls",
-- "clangd",
-- "html",
-- "jsonls",
-- "lua_ls",
-- "pylsp",
--  "zls"
-- }
  • Override
settings["lsp_deps"] = function(default)
  return {
    default[5] -- "lua_ls"
    default[6] -- "pylsp"
    "zls", -- zig lsp
  }
end
-- default:
-- {
-- "bashls",
-- "clangd",
-- "html",
-- "jsonls",
-- "lua_ls",
-- "pylsp",
-- }

-- Return:
-- {
-- "lua_ls",
-- "pylsp",
-- "zls"
-- }

load_plugin() will have default as the whole configuration.
That is, as follows.

-- Override bufferline
return function(default)
  return {
		options = {
			number = nil,
 			...
			color_icons = default.options.color_icons, -- reference default opt
    }
  }
end
-- default:
-- options = {
--   number = nil,
--   modified_icon = icons.ui.Modified,
--   buffer_close_icon = icons.ui.Close,
--   ...
--}

@CharlesChiuGit
Copy link
Collaborator

tks for the explanations!

Copy link
Collaborator

@CharlesChiuGit CharlesChiuGit left a comment

Choose a reason for hiding this comment

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

lgtm

@ayamir ayamir merged commit 973a8cc into ayamir:main Aug 28, 2023
2 checks passed
@misumisumi misumisumi deleted the fix-utils branch August 28, 2023 20:25
singlemancombat pushed a commit to singlemancombat/nvim-config that referenced this pull request Aug 29, 2023
ImAaronChou pushed a commit to ImAaronChou/nvimdots that referenced this pull request Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants