-
-
Notifications
You must be signed in to change notification settings - Fork 283
feat(tuigreet): add TUIgreet target and ANSI theme helpers #1893
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
base: master
Are you sure you want to change the base?
Conversation
trueNAHO
left a comment
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.
The pre-commit and treefmt checks are failing. Also, add a testbed for this new module.
The PR is very verbose, hard to understand, and does seemingly unnecessary things. This needs an overhaul or further explanations.
Was this generated with the help of AI?
| { | ||
| name = "TUIgreet"; | ||
| homepage = "https://github.com/apognu/tuigreet"; | ||
| maintainers = []; |
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.
New modules must have maintainers.
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.
Why add a new public function for a single module?
|
I see a few questions spread throughout comments. Responding here:
The function is quite generic (ANSI mapping) and not specific to TUIgreet, so it seemed better to make it public, as it might be accessed in the future for modules theming other tools that require ANSI colours.
Seems to me to be issues with the checks related to formatting. If different and specific versions of the formatter are required on different architectures (unclear why that would be), this might be best to explain in contrib, or better to include in workspace settings.
Don't have capacity for TDD on a relatively tiny module. Others feel free to contribute.
It's a single commit with probably 20 lines of code if you exclude colour definitions.
That's stylix's prerogative, I'm just providing useful code. Users of TUIgreet can cherry pick the commit if they need this.
No. Was this review automated? |
feat(tuigreet): add TUIgreet target and ANSI theme helpers
Introduces first-class support for TUIgreet by generating an ANSI theme derived from the active Base16 scheme. Also includes a warning scheme nudging users to wire it into greetd, but does not interact with their configuration directly (intentionally -- user config is out of scope).
config.lib.stylix.ansihelpers:themeMapNearest: maps UI components to the nearest ANSI color name.themeStringNearest: produces a single-line--themestring liketext=…;time=…;….bright-black,bright-red,bright-green,bright-yellow,bright-blue,bright-magenta,bright-cyan,bright-white.tuigreet:services.greetd.enable = trueand itsdefault_session.commandcontains “tuigreet”./etc/tuigreet/stylix.themewith the computed--themestring (newline-terminated).--themeis not present in the greetd command.--theme "$(cat /etc/tuigreet/stylix.theme)".Testing performed
config.lib.stylix.ansi.themeStringNearestand ensure the file is generated as/etc/tuigreet/stylix.theme.