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

[Lua]: Add annotations to the core codebase. #1265

Merged
merged 15 commits into from
Jan 6, 2024
Merged

[Lua]: Add annotations to the core codebase. #1265

merged 15 commits into from
Jan 6, 2024

Conversation

vhyrro
Copy link
Member

@vhyrro vhyrro commented Jan 6, 2024

This PR adds LuaCATS annotations (i.e. the annotation standard used by the lua language server) to the core of Neorg (not to all modules, but just to the core).
LuaCATS is a more modern spinoff of EmmyLua.

This gives proper type checking, better autocompletion and verifying the contents of tables provided to the setup() function. It's even useful to users as they can have autocompletion for their config :D

@vhyrro
Copy link
Member Author

vhyrro commented Jan 6, 2024

It seems to me like the CI is using some older version of LuaLS which does not use LuaCATS. @pysan3 do we pin the luals-typecheck language server to a specific version?

@vhyrro
Copy link
Member Author

vhyrro commented Jan 6, 2024

It seems that there are general issues with the typechecking CI, but lua-ls itself is not having any issues when parsing the codebase for me locally. I'll merge the PR as-is and I hope the other issues will fix themselves later on.

@vhyrro vhyrro merged commit 6c46f8b into main Jan 6, 2024
2 of 4 checks passed
@vhyrro vhyrro deleted the neorg-luacats branch January 6, 2024 17:14
@pysan3
Copy link
Contributor

pysan3 commented Jan 6, 2024

The type checking is using this repo, which I have no control over.

https://github.com/mrcjkb/lua-typecheck-action

I'm not very sure, but as far as I understand, it uses nix script? to setup the environment, and as nix has to be run against a lock file (flake.lock), I think the lang server version is locked.

It seems that the repo auto generates a new release when a new version of luals is released, so we would need to always keep the @v0.2.* in the workflow yaml file up to date.

GitHub is not clever enough to resolve @v0.2 as the latest release of v0.2.*, and we would need to ask the repo owner to make a branch named v0.2 and update it recordingly to match the latest @v0.2.* release if we just want to go with @v0.2.

Ref; https://github.com/orgs/community/discussions/39519

@pysan3
Copy link
Contributor

pysan3 commented Jan 6, 2024

Oh, you seem to have pinned it to the latest version: fb23d2e

@vhyrro
Copy link
Member Author

vhyrro commented Jan 6, 2024

Yeah I tried to fix it by pinning to the latest version but it's still failing for some reason 🤔

It's unable to find the neodev files, despite them being there (I've been debugging the CI for the best part of half an hour now :/)

@pysan3
Copy link
Contributor

pysan3 commented Jan 6, 2024

I think upstream v0.2 fails to load .luarc.json and is running the LS without any config file, hence does not recognize neodev.

IDK how to fix it tho...

@pysan3
Copy link
Contributor

pysan3 commented Jan 6, 2024

LuaLS seems to accept the command line arg --config-path as stated here https://luals.github.io/wiki/configuration/, but the action passes --configpath (without - before path), and ignores that arg i think.

I don't know when it changed, but I think this is the reason.

EDIT: Hmm, it seems that the doc is wrong, and --configpath is correct.

@pysan3
Copy link
Contributor

pysan3 commented Jan 6, 2024

@vhyrro Found the reason!

Abs path to the work dir has changed from v0.1.* to v0.2.* in mrcjkb/lua-typecheck-action since it moved from Dockerfile to nix action setup.

So we had to change this in .github/workflows/.luarc.json.

image

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