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 terminus_font to installation isos' console.packages #84476

Merged
merged 1 commit into from Apr 29, 2020

Conversation

jakobrs
Copy link
Contributor

@jakobrs jakobrs commented Apr 6, 2020

Motivation for this change

On some modern HiDPI laptop displays (in particular, Apple's Retina displays), the default font is absolutely tiny due to the high DPI. This adds pkgs.terminus_font to console.packages in installation-cd-base.nix to make more font sizes of the default Terminus font available.

Things done

Built using nix build -f nixos/release.nix iso_image_new_kernel.x86_64-linux.

Note: I have tested the changes on nixpkgs-channels:nixos-20.03 (specifically, 0bb3515) instead of master not to have to compile the entire Linux kernel.

  • 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/) (N/A)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
    • Closure size increased by ~2.1 MiB
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@worldofpeace
Copy link
Contributor

How about we just use a better default console font in general @jakobrs?
Last time I checked even ubuntu used terminus.

It's best not to deviate from user experience in the representative media, and then have it be different on an actual installation.

@jakobrs
Copy link
Contributor Author

jakobrs commented Apr 14, 2020

I believe we already use Lat2-Terminus16 by default (8x16 terminus font). This just makes the higher-DPI versions available on the installation media.

I should probably mention that the terminus-font package uses a completely different naming scheme:

  • Lat2-Terminus16 is called ter-116n
  • Lat2-Terminus32 is called ter-132n
    and so on.

The reason for doing this is pretty much only because of very-high-DPI screens like those found on MacBooks, because having to nixos-rebuild inside the image just to make the text readable is clearly not ideal.

This doesn't actually tell the user that the fonts are available. The login motd could be modified to tell the user that

Run `setfont ter-132n` for a higher-resolution font if the text is nearly unreadable.

or mention it in the manual.

Edit: https://github.com/NixOS/nixpkgs/blob/d696d9e2cba4ade26a4feb3ef8caba3be658fc67/nixos/modules/config/console.nix#L47

Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

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

Looks good; Should be mentioned in the manual as well.

@jakobrs
Copy link
Contributor Author

jakobrs commented Apr 14, 2020

Should I include a list of all terminus font sizes, or just ter-132n?

@Mic92
Copy link
Member

Mic92 commented Apr 15, 2020

Should I include a list of all terminus font sizes, or just ter-132n?

Just mention one good one for HiDpi. Less choices makes it less confusing for the reader.

@jakobrs
Copy link
Contributor Author

jakobrs commented Apr 15, 2020

For reference, the names of the Terminus fonts are described in $(nix-build '<nixpkgs>' kbd)/share/consolefonts/README.Lat2-Terminus16

@Mic92
Copy link
Member

Mic92 commented Apr 16, 2020

@GrahamcOfBorg test installer

@Mic92 Mic92 merged commit b0196ca into NixOS:master Apr 29, 2020
@jakobrs jakobrs deleted the installation-cd-base-terminus branch April 29, 2020 16:21
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