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

Allow using a list of paths in emacsWithPackagesFromUsePackage #465

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TLATER
Copy link

@TLATER TLATER commented Feb 3, 2025

This permits creating use-package/leaf package lists for full configuration directories, instead of just single big config files, at least unless I misunderstand something quite a bit.

@TLATER
Copy link
Author

TLATER commented Feb 3, 2025

This can be done downstream, of course, but I think it's neater to have actual support for it.

@TLATER TLATER marked this pull request as draft February 3, 2025 14:50
@TLATER TLATER force-pushed the tlater/list-packages branch 2 times, most recently from 5e2b7c0 to 9971ad4 Compare February 3, 2025 14:54
elisp.nix Outdated
@@ -40,6 +40,8 @@ let
else if type == "string" then config
# - A config path { config = ./config.el; }
else if type == "path" then builtins.readFile config
# - A list of paths { config = [ ./config.el ./custom.el ] }
else if type == "list" then lib.concatMapStringsSep "\n" (fname: builtins.readFile fname) config
Copy link
Member

Choose a reason for hiding this comment

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

Eta reduction

Suggested change
else if type == "list" then lib.concatMapStringsSep "\n" (fname: builtins.readFile fname) config
else if type == "list" then lib.concatMapStringsSep "\n" builtins.readFile config

Copy link
Member

Choose a reason for hiding this comment

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

re #465 (comment)

Presumably because you do not apply my suggestion as is and instead an extra map is added which breaks the code.

Copy link
Author

@TLATER TLATER Feb 3, 2025

Choose a reason for hiding this comment

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

I did, both lib.concatMapStringsSep and the indirect lib.concatStringsSep (map ... result in unresolved thunks. This commit only uses the latter because in a narrow scenario that did resolve them, whereas lib.concatMapStringsSep did not. I'd figured that it's maybe because lib.concatMapStringsSep demands specifically strings from its input list, but that's clearly not it (and would not make any sense).

I'm stumped, but I'll have to try again another time. It works fine downstream fwiw, maybe this is a weird edge case of ${self} in flakes.

elisp.nix Outdated
Comment on lines 43 to 44
# - A list of paths { config = [ ./config.el ./custom.el ] }
else if type == "list" then lib.concatMapStringsSep "\n" (fname: builtins.readFile fname) config
Copy link
Member

@jian-lin jian-lin Feb 3, 2025

Choose a reason for hiding this comment

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

I am slightly against this because the check type == "list" is weaker than a list of paths.

EDIT: This PR seems to be under active working and not stable yet. I unsubscribed this PR to stop receiving email notifications about "pushed 1 commit". Please @ me if you want me to receive your message or re-request my review using a github button when this PR is ready.

Copy link
Author

Choose a reason for hiding this comment

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

Yep, agreed, I suppose a check of what the list elements actually are is possible.

This permits creating use-package/leaf package lists for full
configuration directories, instead of just single big config files.
@TLATER TLATER force-pushed the tlater/list-packages branch from 9971ad4 to 5de1626 Compare February 3, 2025 15:01
@TLATER
Copy link
Author

TLATER commented Feb 3, 2025

This... Doesn't seem to work in practice, somehow, I'd only tested it without going through the full entrypoint. Somehow going here makes nix not resolve the <thunk>s pointing into my source into strings before passing them to builtins.readFile, which breaks everything.

Unsure how this is even possible, but I'll have to dig into this a bit deeper than I can right this evening.

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