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

treewide: avoid IFD #159

Open
18 tasks done
lovesegfault opened this issue Oct 7, 2023 · 12 comments
Open
18 tasks done

treewide: avoid IFD #159

lovesegfault opened this issue Oct 7, 2023 · 12 comments
Labels
performance Improvements to or issues about performance

Comments

@lovesegfault
Copy link
Contributor

lovesegfault commented Oct 7, 2023

The main drawback of using stylix is how much it relies on IFD for setting themes, which bogs down evaluation times and causes basic commands like nix flake show to stop working.

In my config, I found that the following targets all rely on IFD:

Looking at the modules, though, they mostly don't need IFD, and could be reworked to avoid it. One example is bat, where I was able to remove the need for IFD by improving the underlying HomeManager module.

I think most of the others on the list can also avoid IFD by just linking the theme file, and then referencing the linked path instead of trying to embed the theme in the configuration files.

The exception to this is xresources, where include directives rely on a C preprocessor, which is disabled by a lot of display managers.

@lovesegfault
Copy link
Contributor Author

I closed some of the PRs because they didn't actually solve the IFD, but rather just moved it around.

I think the right solution there might be to turn most of those fetchFromGitHubs into flake inputs

@lovesegfault
Copy link
Contributor Author

I guess all the templates would need to be added as flake inputs, and then we could have a moduleArgs inputs that would let individual modules reference inputs.base16-alacritty, for example.

@danth
Copy link
Owner

danth commented Oct 8, 2023

Also, some of the templates are quite short, so we could consider just embedding them as a string.

@lovesegfault
Copy link
Contributor Author

Alright, with the two open PRs listed above and #165 we get rid of all IFD except for xresources, which I can't see a clear way to do without IFD, but folks can also just disable it :)

@lovesegfault
Copy link
Contributor Author

Another plus of the approach taken in #165 is that it's not possible to easily override the templates used, and they get auto-updated with the rest of the flake inputs :)

@lovesegfault
Copy link
Contributor Author

I'm now running all these changes on my systems, everything working AOK and evaluation is much faster 🚀

@lovesegfault
Copy link
Contributor Author

lovesegfault commented Oct 11, 2023

I may have a solution for xresources.

All home-manager does is create the .Xresources file: https://github.com/nix-community/home-manager/blob/master/modules/xresources.nix#L90-L100

And then configure the xsession to merge it to xrdb: https://github.com/nix-community/home-manager/blob/master/modules/xresources.nix#L102

And that is just a command that points to the file: https://github.com/nix-community/home-manager/blob/master/modules/xresources.nix#L26

We could do something similar with:

{
  home.file.".Xstylix" = {
    source = theme;
    onChange = ''
      if [[ -v DISPLAY ]]; then
        ${pkgs.xorg.xrdb}/bin/xrdb -merge ${config.home.homeDirectory}/.Xstylix
      fi
    '';
  };

  xsession.initExtra = "${pkgs.xorg.xrdb}/bin/xrdb -merge ${config.home.homeDirectory}/.Xstylix";
}

And that does no IFD.

I can't really test this because I don't know what the .Xresources theme even impacts.

cc. @danth

@danth
Copy link
Owner

danth commented Oct 11, 2023

It should have an effect on rxvt and xterm.

I think we can avoid linking the file too:

let
  # theme = ;
  command = "${pkgs.xorg.xrdb}/bin/xrdb -merge ${theme}";

in {
  xsession.initExtra = command;

  home.activation.Xresources = lib.hm.dag.mkEntryAfter [ "writeBoundary" ] ''
    if [[ -v DISPLAY ]]; then
      $DRY_RUN_CMD ${command}
    fi
  '';
}

On the other hand the template is fairly simple, so we could translate it to xresources.properties and remove the dependency:

{
  xresources.properties =
    with config.lib.stylix.colors.withHashtag;
    {
      "*foreground" = base05;
      "*cursorColor" = base05;
      ...
    };
}

Then it goes through the Home Manager module as usual.

@lovesegfault
Copy link
Contributor Author

Ah, good idea, let me give it a shot :)

@lovesegfault
Copy link
Contributor Author

Seems to work! Done in #168

@danth danth added the performance Improvements to or issues about performance label Oct 28, 2023
@trueNAHO
Copy link
Collaborator

Looking at the modules, though, they mostly don't need IFD, and could be reworked to avoid it. One example is bat, where I was able to remove the need for IFD by improving the underlying HomeManager module.

I think most of the others on the list can also avoid IFD by just linking the theme file, and then referencing the linked path instead of trying to embed the theme in the configuration files.

(Source: #159 (comment))

Would replacing the current Mustache templates with upstream Home Manager and NixOS options not further improve evaluation time? Additionally, unlike Mustache templates, Nix options can be seamlessly overwritten by end users in Stylix, Home Manager, or NixOS.

I guess all the templates would need to be added as flake inputs, and then we could have a moduleArgs inputs that would let individual modules reference inputs.base16-alacritty, for example.

(Source: #159 (comment))

For reference, this is quite similar to what #222 does.

@danth
Copy link
Owner

danth commented Feb 12, 2024

Yes, using Nix options is a better option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Improvements to or issues about performance
Projects
None yet
Development

No branches or pull requests

3 participants