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: add feature flags to module configuration #16

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mzonski
Copy link
Contributor

@mzonski mzonski commented Feb 8, 2025

Adds an ifFeatured option to provide granular control over module behavior without requiring full custom configuration. This sits alongside the existing enable flag, allowing for more flexible module control while maintaining simplicity.

Example configuration can be found here

This results in proper feature flag evaluation, as shown in the trace output:

❯ home-manager build --flake .#zonni@desktop --show-trace
trace: foooobar/c2_foo feature enabled
trace: child1/c1_baz feature enabled
trace: child1/c1_bar feature enabled
trace: child2 (foooobar) feature enabled
trace: child1 feature enabled

What do you think about this change?

@yunfachi
Copy link
Owner

yunfachi commented Feb 8, 2025

A bit confusing. Do you have a real example of using this feature?
Also, keep in mind that Denix aims for simplicity in logic and readability.

Also, a note for the documentation:
(from your denix-demo)

 options = { featured, ... }: delib.featuresEnableOption featured [ "c1_foo" ] { name = "child1"; };

(the more preferred option)

 options = { featured, ... } @ args: delib.featuresEnableOption featured [ "c1_foo" ] args;

@mzonski
Copy link
Contributor Author

mzonski commented Feb 8, 2025

I want to have a little more than just an enable option, but without writing custom options. It's possible to create nested features by passing down the featured variable, but I think one-level deep feature flags alongside the enable option should cover most cases while keeping the ability to set part of the module as active. I like how I solved one-level structures; nested structures are a bit uglier because I didn't want to combine feature flags name with module name to create another option level. I also didn't want to add another option function because I thought it would bloat the options. Correct me if you think otherwise.

{ delib, ... }:
let
  extendFeatureOptions = name: defaultFeatures: { featured, ... }:
    delib.featuresEnableOption featured defaultFeatures { inherit name; };
in
delib.module {
  name = "parent";
  feature = "child1";

  options = extendFeatureOptions "child1" [ "c1_foo" ];
  # ...
}

Here's real world example of refactored audio module

Idea here is like:
I want to keep my wireplumber/pipewire rules in a single folder hidden either by feature flags or enabled option. By having feature flags I can, for example enable noisy audio suspension on a single machine, but on another I can result to normal audio suspension rule.

While I could mix isEnabled with inherited config variables, feature flags feel much cleaner and more elegant, using them feels smooth

@yunfachi
Copy link
Owner

yunfachi commented Feb 8, 2025

Tomorrow I'll take a closer look at this, but for now, check out my Neovim configuration here: https://github.com/yunfachi/nix-config/blob/master/modules/programs/nixvim/default.nix. Each plugin is a separate module with its own options, like this one: https://github.com/yunfachi/nix-config/blob/master/modules/programs/nixvim/plugins/which-key.nix. Is this similar to your issue?

@mzonski
Copy link
Contributor Author

mzonski commented Feb 8, 2025

Yup, and you're controlling modules through the enabled flag. With my approach you could modularize your nvim config even further:

nixvim/default.nix

  options = featuresEnableOption true [ "plugin-which-key" ];

plugins/which-key.nix

{ delib, ... }:
delib.module {
  name = "programs.nixvim";
  feature = "plugin-which-key";

  home.ifFeatured.programs.nixvim.plugins.which-key = {
    enable = true;
  };
}

This way which-key plugin will be conditionally included based on featuresEnableOption defaults, but you can still override the features list in your host or rice directories if needed.

@yunfachi
Copy link
Owner

Tomorrow I'll take a closer look at this

This really looks good for some cases, but it has a few drawbacks:

  • If there are many features, as in my nixvim configuration, it would require manually listing all enabled features.
  • The logic for removing a feature from the list, when it's already there, is not very intuitive.

To avoid making the library more complex, I suggest creating a directory for addons after completing #5 and adding this there.

It would look something like this:

delib.configurations {
  # ...
  libExtensions = [
    ./myCustomLib
    delib.addons.featureFlags
  ];
}

What do you think?

@mzonski
Copy link
Contributor Author

mzonski commented Feb 10, 2025

This way is the way to go, you proposed a way better solution than integrating feature flags into the project core. I will think on how extending the lib could be solved at the end of the week if you don't tackle this issue beforehand. It would be a great exercise for improving my Nix skills :)

@yunfachi
Copy link
Owner

Thanks for understanding! What do you think about optionally adding an enum to this option? For example, enumFeaturesEnableOption - I think it would be useful in modules where only one feature can be enabled.

@mzonski
Copy link
Contributor Author

mzonski commented Feb 13, 2025

No worries! I discovered your library by accident, and really like it as it made my modules so beautiful! 🥇

Regarding naming conventions, I would suggest not prefixing the feature name with a specific type. Instead, I prefer using singular words rather than words that indicate multiple options. When developing ui components, I frequently work with properties like type, kind or variant. Initially, I considered using variantEnableOption for enums. However, this approach would potentially double the number of additional fields inside each module. Therefore, my alternative suggestion is to use singleEnableOption paired with featureEnableOption for enums, and featuresEnableOptions for lists. By using this naming convention, we'll achieve a level of abstraction where the underlying type becomes irrelevant to the builtin option

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