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

shrink util implementation by using upstream functions, avoid util in configs #2079

Open
ranjithshegde opened this issue Aug 23, 2022 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@ranjithshegde
Copy link
Contributor

ranjithshegde commented Aug 23, 2022

Functions to deprecate

  1. path
  2. search_ancestors
  3. root_pattern
  4. find_git_ancestors
  5. find_mercurial_ancestors
  6. find_node_modules_ancestors
  7. find_package_json_ancestors

All of the above can be replaced with relevant functions from vim.fs module.

  1. parse_user_command_options
  2. create_module_commands

Both of these functions can be made private in the next releases seeing as user_commands part has already been signaled for eventual deprecation.

Proposal

  1. Soft deprecate it after the release of nvim 0.8 + stabilization period.
  2. Internally redirect the calls to the relevant calls from vim.fs module
  3. Replace the functions inside configs/[server].lua wherever relevant
  4. Update the CI "lint" job to prevent usage of any util functions that have an alternative in the minimum supported Nvim.
  5. Provide examples/docs that use the recommended functions
@justinmk

This comment was marked as outdated.

@glepnir

This comment was marked as outdated.

@justinmk

This comment was marked as outdated.

@glepnir

This comment was marked as outdated.

@justinmk
Copy link
Member

We don't necessarily need to remove lspconfig.util.root_pattern, we can start by updating their implementations to use vim.fs and minimizing code where possible. And where possible, show a deprecation message that explains how to use vim.fs directly instead of e.g. lspconfig.util.root_pattern. And update all of the examples in lspconfig docs to use vim.fs directly, instead of lspconfig.util.

@justinmk

This comment was marked as outdated.

@glepnir

This comment was marked as outdated.

@justinmk
Copy link
Member

justinmk commented Oct 2, 2024

Plan

what general strategy is for removing lspconfig functions

we should keep the util stuff around to avoid breaking downstream. meanwhile, in this repo:

  • CI should fail if any config uses a deprecated util function
  • we should remove all docs that suggest a deprecated util function

once we have isolated most of util in that way, then we can think about when/how to fully remove them.

@dundargoc
Copy link
Member

Since we don't actually test/run any of the lua configs in CI to my knowledge, the best we can hope for is to make a string search of changed lines on PRs and complain if the deprecated function is used.

@justinmk
Copy link
Member

justinmk commented Oct 4, 2024

The "Proposal" in the main description of this issue is updated and correct. To be clear:

  • we don't need to remove anything on util, its interface can stay frozen for back-compat.
  • internally, the implementation of util should shrink as we update it to use stdlib features of the minimum-supported Nvim version.
  • as we bump the minimum required Nvim, we will add lint rules to the lspconfig CI which prevent configs from using util functions if the minimum supported Nvim (currently 0.9) has alternatives.

For example, search_ancestors currently doesn't use vim.fs.parents even though it has been available since Nvim 0.8

function M.search_ancestors(startpath, func)

@justinmk justinmk changed the title Replace functions from utils with upstream functions shrink util implementation by using upstream functions, avoid util in configs Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants