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

refactor(lsp): replace util.buf_versions with changedtick #28943

Merged
merged 1 commit into from
May 30, 2024

Conversation

mfussenegger
Copy link
Member

lsp.util.buf_versions was already derived from changedtick (on_lines
from buf_attach synced the version)

As far as I can tell there is no need to keep track of the state in a
separate table.

@github-actions github-actions bot added lsp refactor changes that are not features or bugfixes labels May 23, 2024
@icholy
Copy link
Contributor

icholy commented May 23, 2024

FYI: https://github.com/neovim/nvim-lspconfig/blob/2c1877081b237a643e52ebdebaf36c84a2695639/lua/lspconfig/server_configurations/eslint.lua#L29

Also: https://sourcegraph.com/search?q=context:global+vim.lsp.util.buf_versions&patternType=keyword&sm=0

Could do something like

---@nodoc
---@deprecated
M.buf_versions = setmetatable({}, {
  __index = function(_, bufnr)
    vim.notify_once("vim.lsp.util.buf_versions is deprecated", vim.log.levels.WARN)
    return vim.b[bufnr].changedtick
  end
})

@icholy
Copy link
Contributor

icholy commented May 23, 2024

Might make sense to hide the vim.b[bufnr].changedtick inside a util.buf_version(bufnr) function:

function M.buf_version(bufnr)
  return vim.b[bufnr].changedtick
end

This way it can be changed in the future without breaking anything.

@mfussenegger mfussenegger marked this pull request as ready for review May 23, 2024 19:15
@github-actions github-actions bot requested a review from MariaSolOs May 23, 2024 19:16
@mfussenegger
Copy link
Member Author

mfussenegger commented May 30, 2024

Might make sense to hide the vim.b[bufnr].changedtick inside a util.buf_version(bufnr) function:

function M.buf_version(bufnr)
  return vim.b[bufnr].changedtick
end

This way it can be changed in the future without breaking anything.

I don't think that's necessary and it would defeat the purpose of this PR (getting rid of unnecessary extra API).
util.buf_versions wasn't intended as public API (hence the @nodoc) and I was even inclined to skip the deprecation step because of that.

`lsp.util.buf_versions` was already derived from changedtick (`on_lines`
from `buf_attach` synced the version)

As far as I can tell there is no need to keep track of the state in a
separate table.
@mfussenegger mfussenegger merged commit 5c33815 into neovim:master May 30, 2024
29 checks passed
@mfussenegger mfussenegger deleted the lsp-buf-versions branch May 30, 2024 08:46
@github-actions github-actions bot removed the request for review from MariaSolOs May 30, 2024 08:46
huangyingw pushed a commit to huangyingw/neovim that referenced this pull request May 31, 2024
`lsp.util.buf_versions` was already derived from changedtick (`on_lines`
from `buf_attach` synced the version)

As far as I can tell there is no need to keep track of the state in a
separate table.
mfussenegger added a commit to mfussenegger/neovim that referenced this pull request Jun 3, 2024
mfussenegger added a commit to mfussenegger/neovim that referenced this pull request Jun 6, 2024
mfussenegger added a commit that referenced this pull request Jun 7, 2024
* Revert "fix(lsp): account for changedtick version gap on modified reset (#29170)"

This reverts commit 2e6d295.

* Revert "refactor(lsp): replace util.buf_versions with changedtick (#28943)"

This reverts commit 5c33815.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lsp refactor changes that are not features or bugfixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants