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: remove missing required fields diagnostic from config #812

Merged
merged 1 commit into from
Nov 29, 2023
Merged

fix: remove missing required fields diagnostic from config #812

merged 1 commit into from
Nov 29, 2023

Conversation

tj-moody
Copy link
Contributor

@tj-moody tj-moody commented Sep 7, 2023

As discussed in the mentioned issue, it appears that some changes to lua_ls mean that not including non-optional fields in a config table throws a diagnostic warning, as shown in the screenshot. As the issue mentions, it's possible to disable this behavior globally or with a ---@diagnostic disable-next-line missing-fields comment before the table, but I think it would still be greatly desirable to have bufferline not throw warnings for non-mandatory fields by default.

I've tried my best to only make the non-mandatory fields optional, but I probably over or under-modified the types, so please feel free to add/remove ?s around the types file.

LuaLS/lua-language-server#2214 (comment)
Screenshot 2023-09-07 at 11 48 57 AM

@tj-moody
Copy link
Contributor Author

tj-moody commented Sep 7, 2023

As a note here is an analogous pr in for nvim-cmp, which the original issue was about: hrsh7th/nvim-cmp#1694

---@field style "underline" | "icon" | "none"
---@field icon string?
---@field style? "underline" | "icon" | "none"
---@field icon? string?

Choose a reason for hiding this comment

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

If adding ? after the field/param name, then maybe it might be a good idea to also remove the ? after the type to make it consistent.

Suggested change
---@field icon? string?
---@field icon? string

I think there are also others like this in this file:
lines 15, 37, 42, 50*, 181*

* = for those lines I am not completely sure

@akinsho akinsho merged commit 1a33975 into akinsho:main Nov 29, 2023
sstallion pushed a commit to sstallion/bufferline.nvim that referenced this pull request Jul 3, 2024
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