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
nixos: unify virtual console options #71473
Conversation
c7e2b2d
to
211afb4
Compare
8230905
to
f6d1d3e
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/80 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/what-are-your-goals-for-20-03/4773/4 |
Reviewed points
Possible improvements
|
Thank you for the review, @pstch.
I thought about it but I made these changes simultaneously so it would require some nontrivial history rewriting. |
I missed something in my review:
I'm rebasing your commits, fixing both of these issues, and will push my branch so that you can use it if you want. I also removed the changes to the color palette application by diffing the two files, so that we can add them in another PR. |
@rnhmjoj I rebased your PR on recent master, applying the following steps:
I have tested these commits on my machine by rebasing them on 19.09. |
@pstch Thank you. I fixed the merge conflict and followed your suggestions. |
Fixed yet another conflict. |
This time it was me. I wonder why git can't automaticallt merge this... |
This commit moves all the virtual console related options to a dedicated config/console.nix NixOS module. Currently most of these are defined in config/i18n.nix with a "console" prefix like `i18n.consoleFont`, `i18n.consoleColors` or under `boot` and are implemented in tasks/kbd.nix. Since they have little to do with actual internationalisation and are (informally) in an attrset already, it makes sense to move them to a specific module.
This commit changes the console colors implementation to use the kernel parameters instead of relying on terminal escape sequences. This means the palette is applied by the kernel itself with no custom code running in the initrd and works for all virtual terminals (not only tty0).
@infinisil What to do you think about adding a new top level attribute? I'm not too keen but I didn't find a suitable existing one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a good idea to me, it really doesn't fit anywhere else.
I tested this in a VM, seems to work fine, let's merge :)
Thank you! |
Motivation for this change
As per commit message:
This commit moves all the virtual console related options
to a dedicated config/console.nix NixOS module.
Currently most of these are defined in config/i18n.nix
with a "console" prefix like
i18n.consoleFont
,i18n.consoleColors
or underboot
and are implementedin tasks/kbd.nix.
Since they have little to do with actual internationalisation
and are (informally) in an attrset already, it makes sense to
move them to a specific module.
Also, this commit changes the console colors implementation
to use the kernel parameters instead of relying on terminal
escape sequences. This means the palette is applied by the
kernel itself with no for custom code running in the initrd
and works for virtual terminals (not only tty0).
Things done
Notify maintainers
cc @vcunat @abbradar @greedy @the-kenny