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.

Sign up for GitHub

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

Jump to bottom

awesome: fix luaModules using pkgs.lua instead of awesome.lua #3258

Merged
merged 1 commit into from
Nov 1, 2022
Merged

awesome: fix luaModules using pkgs.lua instead of awesome.lua #3258

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/services/window-managers/awesome.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ let

cfg = config.xsession.windowManager.awesome;
awesome = cfg.package;
getLuaPath = lib: dir: "${lib}/${dir}/lua/${pkgs.luaPackages.lua.luaversion}";
getLuaPath = lib: dir: "${lib}/${dir}/lua/${awesome.lua.luaversion}";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would like to avoid out-of-tree reimplementation of lua utilities such as getLuaPath here. I would like to use lua flat installs in the future but that requires packages not to replicate their own lua wrapping code NixOS/nixpkgs#56398 (comment) .

I have some ongoing work to improve the lua ecosystem https://github.com/NixOS/nixpkgs/pull/177556/files#diff-558d30a16316ebf76080fb16d8b82a341313e48617e224e2acf30b5f64952990 but until then could you use https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/lua-modules/lib.nix#L39 instead ?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can merge this as-is for now and do the improvement in a future PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, while I appreciate the effort of reducing duplicate code/boilerplate when dealing with Lua that you seem to be working on, this PR is a oneline fix and just mirrors the same fix I made in nixpkgs a little while ago.

I am totally out of context for the Lua ecosystem improvements you're making and a jump from 'oneline fix' to 'use the entirely new better lua system' was a bit too much for me so I forgot about this PR 😅

makeSearchPath = lib.concatMapStrings (path:
" --search ${getLuaPath path "share"}"
+ " --search ${getLuaPath path "lib"}");
Expand Down