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

Make font handling match NixOS #665

Merged
merged 2 commits into from
Jul 20, 2023
Merged

Make font handling match NixOS #665

merged 2 commits into from
Jul 20, 2023

Conversation

quentinmit
Copy link
Contributor

@quentinmit quentinmit commented May 18, 2023

Previously, nix-darwin would look in the share/fonts directory in each of the packages mentioned in the fonts.fonts option. Not all fonts packaged in nixpkgs place their fonts in this directory. It looks like this logic recently changed to just link every file in every package, whether it's a font or not. This PR changes the handling of fonts.fonts to match NixOS's behavior, which is to use find to find all .ttf, .ttc, and .otf files in any of the passed-in paths. This allows using fonts.fonts with more font packages.

…tf files in any directory in the passed packages
ln -sf "$f" $out/Library/Fonts
done
font_regexp='.*\.\(ttf\|ttc\|otf\)'
find -L ${toString cfg.fonts} -regex "$font_regexp" -type f -print0 | while IFS= read -rd "" f; do

Choose a reason for hiding this comment

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

This prevents non-fonts from being iterated-over in the activationScripts, correct?

Copy link

@sielicki sielicki left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Collaborator

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Seems reasonable. The NixOS behaviour includes other font types, but I don't think macOS supports any of them.

@emilazy
Copy link
Collaborator

emilazy commented Jul 10, 2023

Can you rebase this against latest master? It can't be merged because the checks added between your commit and now haven't run.

@quentinmit
Copy link
Contributor Author

quentinmit commented Jul 19, 2023

Can you rebase this against latest master? It can't be merged because the checks added between your commit and now haven't run.

Apologies for the delay, I just saw this comment. I've merged in the current master.

@emilazy emilazy merged commit 531c3de into LnL7:master Jul 20, 2023
6 checks passed
@emilazy
Copy link
Collaborator

emilazy commented Jul 20, 2023

Thanks!

emilazy added a commit to emilazy/nix-darwin that referenced this pull request Aug 3, 2023
Ensure that LnL7#665 works correctly.
emilazy added a commit to emilazy/nix-darwin that referenced this pull request Jun 13, 2024
Ensure that LnL7#665 works correctly.
347Online pushed a commit to 347Online/nix-darwin that referenced this pull request Jun 16, 2024
kamushadenes pushed a commit to kamushadenes/nix-darwin that referenced this pull request Jul 8, 2024
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.

None yet

3 participants