Skip to content

Conversation

@GetPsyched
Copy link
Member

This one took me a while and I still feel more can be done. I've listed them in the first comment.

Writing a module for this was a nightmare and I lost the battle trying to make i3.commands be a proper Nix type rather than just a multi-line string. It's either that or going down the HM route of defining submodule options for each command there is. A few things I wanted to note/discuss:

  • I could do something similar to the niri module which only abstracts certain things like binds, spawn-on-startup, etc. Should I do this?
  • About sourcing env vars (Properly sourcing environmental variables #17), from what I gathered, i3's config doesn't support this at all and instead relies on files like .xsession, .xinitrc, and friends to export env vars before i3 launches. Those should be different modules and I didn't research enough on them to confidently write a module. I also thought about using i3.extraSessionCommands from the nixpkgs nixos module to export env vars but idk if that's good practice or not. Need more folks to comment on this.

Meta

Related Issue(s): #17 (see above)

AI used to generate code included in this PR?: No, but I yoinked some stuff from the niri module

All Submissions:

  • Formatted commit message in accordance with CONTRIBUTING.md guidelines
  • Filled in all meta items
  • Mentioned any blockers before the PR can merge
  • Verified there are no conflicting PRs open

New Module Submissions:

  • Followed the general API laid out in CONTRIBUTING.md
  • Wrote tests (or expressed a need for help on tests)

Comment on lines +58 to +59
# Assert that the i3 config is valid
machine.succeed("${lib.getExe pkgs.i3} -c %s -C -d all" % "/home/bob/.config/i3/config")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is redundant because the checkPhase in writeTextFile will cause the config drv to fail. Should I remove it?

And generally, is there more I can add to the tests? I couldn't think of much to put in here.

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.

1 participant