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

feat(dirman)!: use pathlib for all dirman operations #1354

Merged
merged 4 commits into from
Mar 28, 2024

Conversation

pysan3
Copy link
Contributor

@pysan3 pysan3 commented Mar 26, 2024

Pathlib.nvim

pathlib.nvim is a plugin which aims to decrease the difficulties of path management across mutliple OSs in neovim. The plugin API is heavily inspired by Python’s pathlib.path with tweaks to fit neovim usage.

You can search through the methods at: https://pysan3.github.io/pathlib.nvim/search.html

Commits

feat(spec): add pathlib.nvim to dependency

This adds pathlib.nvim to neorg's dependencies managed by .github/workflows/luarocks.yml and build.lua.

This commit also changes the typecheck CI to install all its dependencies defined by *.rockspec and use the packages for type annotations. Thanks to this change, types like PathlibPath (defined in pathlib.nvim) also get annotated as a valid type and will be type-checked against.

You will want to do this with local development as well.

$ luarocks init --no-gitignore  # RUN THIS ONLY ONCE

$ luarocks install --only-deps ./neorg-scm-1.rockspec  # Run this each time dependency changes

After running these commands, packages will be installed under ./lua_modules and those packages will be picked up by lua_ls thanks to config in ./.luarc.json and you will get all nvim-cmp completions!

docs(annotation): add better type annotations

Some updates to type annotations unrelated to dirman to make life easier with completions.

feat(dirman)!: use pathlib for all dirman operations

This commit contains all the breaking changes.

Some of the public facing APIs from dirman now returns a PathlibPath object instead of a string.
Besides, there is a new function dirman.utils.expand_pathlib which is meant to replace dirman.utils.expand_path.
I have not deprecated expand_path just yet since it is used in some places in the codebase, and if I deprecate it now, typecheck CI will complain about it resulting to ❌.

Some notable changes to modules other than dirman are as follows.

  • core.completion
    • This uses dirman.get_norg_files to get the list of norg files under the workspace.
    • It does string manipulation of the file paths to generate the completion options.
    • I changed the code to make it work with pathlib which result in a much cleaner code.
  • core.summary
    • path:len() == string.len(tostring(path)) so,
    • string.len(ws_root) won't work but ws_root:len() will call Path.len or string.len based on the type but result in the same value.
      • How clever!

@vhyrro I know you are busy but please take your time to read through the changes.
I tried my best to keep the diff as little as possible so that the diff explains what pathlib brings to the table (especially this commit).
I hope after you read through the changes, you already have some good understanding of how to use the library.
Of course if you have any question, feel free to ping me anytime.

I labeled this PR with feat! flagging it as a breaking change (which will increase the major version). Do you agree?
I think neorg v9 is labeled to bring the v3 parser so I'm not sure if introducing pathlib is worth making a major jump already.

--- @return neorg.module.public?
--- @generic T
--- @param module_name `T` The name of the module to retrieve.
--- @return T?
function modules.get_module(module_name)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vhyrro You see that with this change, the string passed to module_name will literally mean the return type and type annotation works awesome.

Screenshot_20240328_021520

@pysan3 pysan3 force-pushed the pathlib-for-dirman branch from ff76653 to 46bcdf0 Compare March 28, 2024 18:03
@vhyrro
Copy link
Member

vhyrro commented Mar 28, 2024

All LGTM! Tested all of the functions and they seem to work in the same way, good work.

Since the old function is still supported (simply deprecated in the annotations), I think we can merge this without a breaking change. After all, adding dependencies is no longer a breaker :)

@vhyrro vhyrro merged commit 1190dc7 into nvim-neorg:main Mar 28, 2024
2 checks passed
sahinf pushed a commit to sahinf/neorg that referenced this pull request May 15, 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.

2 participants