-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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: remove internal implementation of util.path.dirname #3460
Conversation
Instead, just return the result of vim.fs.dirname.
LGTM.. Actually it can be removed. see #3434 |
@justinmk pulled out a deprecation strategy in #2079 (comment), I interpret his comment to mean we shouldn't remove any functions from util yet. |
Yes. A safe approach :D. But I want to make lspconfig become a pure configuration collection. It would be helpful to exploit some of the functions of lspconfig step by step? If possible, is it a good idea to use deprecate here? Point to vim.fs.dirname so that any other plugins or users who use this function will remove the dependency on util.path.dirname. |
Yeah, kinda agree. I've written a complaint in the issue. |
This is a good example of why removing As we hollow-out the guts of So this is a good change, and we should continue to make this kind of change. The possible future breaking change can wait until later. There is no need to do it right now. Let's just focus on shrinking the implementation, first. |
@@ -155,20 +155,7 @@ M.path = (function() | |||
--- @param path T | |||
--- @return T | |||
local function dirname(path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a @deprecated
token would help too, and the docstring can mention "use vim.fs"
done in #3462
Instead, just return the result of vim.fs.dirname.