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

Add all Iosevka build plans, including Aile, Etoile, and Sparkle #98489

Closed
wants to merge 1 commit into from

Conversation

jbaum98
Copy link
Contributor

@jbaum98 jbaum98 commented Sep 22, 2020

Motivation for this change

This adds all of the Iosevka variants.

I know it's a lot of copy-paste code in all-packages.nix, but I think if we want top-level packages (maybe we don't) then we need to do this, or have some attrset merge at the bottom of the file. We also could, if we wanted, generate this list by looking at build-plans.toml in the Iosekva source.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@rileyinman rileyinman left a comment

Choose a reason for hiding this comment

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

Hmm, I'm not sure how I feel about this. On the one hand other repos (like the AUR) put all the variants at the top level, but on the other hand we might be able to do it in a more organized way. I'm not actually sure how this would be done though, your idea about generating automatically from the build plans could work. Maybe it's something that could be done in the new update script for the package?

@jbaum98
Copy link
Contributor Author

jbaum98 commented Oct 1, 2020

I see two separate questions here:

  1. Should the variants all live as top-level packages, or should it be iosevka.default, iosevka.aile, etc. The awkward one is the default Iosevka one. Another option is leaving that as top-level iosevka, and then having a iosevkaVariants attrset containing all the other ones.
  2. Should the variants be hardcoded or dynamically generated? I would classify even using an update script to generate the variants as hard-coded. Dynamically generating would be passing the TOML in some way in nix code in order to define the iosevka attrset.

@deviant
Copy link
Member

deviant commented Oct 30, 2020

#101132 might be of interest to you.

@jbaum98
Copy link
Contributor Author

jbaum98 commented Nov 6, 2020

Oh, exciting! I do have to say, I don't love the interface of passing in the variant as a string when we know there are only a few variants we'll accept, but if I can come up with a better interface I'll submit a new PR.

@jbaum98 jbaum98 closed this Nov 6, 2020
@deviant
Copy link
Member

deviant commented Nov 6, 2020

The list of variants is subject to change over time. Some have been removed (IIRC), while some (such as the variable width ones) have been added.

@jbaum98 jbaum98 deleted the iosevka-variants branch November 6, 2020 04:16
@jbaum98
Copy link
Contributor Author

jbaum98 commented Nov 6, 2020

That's pretty fair.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants