Conversation
There was a problem hiding this comment.
Since the GitHub UI does not let me annotate specific lines, here is a normal review:
From 6d3d807f46b94f4de7dd5bec510f8a88420c0c6e Mon Sep 17 00:00:00 2001 From: Mirza Arnaut <mirza.arnaut45@gmail.com> Date: Fri, 27 Dec 2024 02:26:00 +0100 Subject: [PATCH] Add ghostty module
Rename the commit to ghostty: init.
--- modules/ghostty/hm.nix | 49 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 modules/ghostty/hm.nix diff --git a/modules/ghostty/hm.nix b/modules/ghostty/hm.nix new file mode 100644 index 00000000..f1cdd09a --- /dev/null +++ b/modules/ghostty/hm.nix @@ -0,0 +1,49 @@ +# Documentation is available at: +# - https://ghostty.org/docs/config/reference +# - `man 5 ghostty` +{ config, lib, ... }: +{ + options.stylix.targets.ghostty.enable = config.lib.stylix.mkEnableTarget "Ghostty" true; + + config = + lib.mkIf + ( + config.stylix.enable && config.stylix.targets.ghostty.enable && config.stylix.targets.ghostty.enable + )
You are repeating the config.stylix.targets.ghostty.enable condition twice:
- (
- config.stylix.enable && config.stylix.targets.ghostty.enable && config.stylix.targets.ghostty.enable
- )
+ (config.stylix.enable && config.stylix.targets.ghostty.enable)+ { + programs.ghostty.settings = with config.stylix; { + theme = "stylix"; + font-family = fonts.monospace.name; + font-style = "Reg";
Is this font-style generally compatible or only with your specific one?
+ font-size = fonts.sizes.terminal; + background-opacity = opacity.terminal; + }; + + home.file.".config/ghostty/themes/stylix".text = + (with config.lib.stylix.colors.withHashtag; '' + palette = 0=${base03} + palette = 1=${base08} + palette = 2=${base0B} + palette = 3=${base0A} + palette = 4=${base0D} + palette = 5=${base0F} + palette = 6=${base0C} + palette = 7=${base05} + palette = 8=${base04} + palette = 9=${base08} + palette = 10=${base0B} + palette = 11=${base0A} + palette = 12=${base0D} + palette = 13=${base0F} + palette = 14=${base0C} + palette = 15=${base05} + '') + + (with config.lib.stylix.colors; '' + background = ${base00} + foreground = ${base05} + cursor-color = ${base06} + selection-background = ${base02} + selection-foreground = ${base05} + ''); + }; +}
Avoid the with pattern:
{
home.file.".config/ghostty/themes/stylix".text = let
inherit (colors) withHashtag;
inherit (config.lib.stylix) colors;
in ''
palette = 0=${withHashtag.base03}
background = ${colors.base00}
'';
}Also, does the Home Manager module support generating theme files as follows:
{
programs.ghostty.theme.stylix = let
inherit (config.lib.stylix) colors;
in {
background = colors.base00;
cursor-color = colors.base06;
foreground = colors.base05;
palette = let
inherit (config.lib.stylix.colors) withHashtag;
in [
withHashtag.base03
withHashtag.base08
withHashtag.base0B
withHashtag.base0A
withHashtag.base0D
withHashtag.base0F
withHashtag.base0C
withHashtag.base05
withHashtag.base04
withHashtag.base08
withHashtag.base0B
withHashtag.base0A
withHashtag.base0D
withHashtag.base0F
withHashtag.base0C
withHashtag.base05
];
selection-background = colors.base02;
selection-foreground = colors.base05;
};
}Maybe you can add or request this functionality to the future Home Manager module.
|
Thanks for the review!
✅
✅ no idea how that slipped through...
❎ I somehow thought that this was universal, but different fonts implement a different
✅ I used some other modules on stylix as inspiration. Even tho I don't like
It does not.. for now. I asked in the PR and it seems like it will be implemented soonish! On a sidenote: I added the emoji font too. |
|
The ghostty HM PR just pushed a commit with the needed themes option! |
trueNAHO
left a comment
trueNAHO
left a comment
There was a problem hiding this comment.
Avoid the
withpattern:✅ I used some other modules on stylix as inspiration. Even tho I don't like
withpersonally, I thought it was used heavily in stylix.
Once #519 merges, I plan on cleaning up the codebase and removing legacies, like with.
trueNAHO
left a comment
trueNAHO
left a comment
There was a problem hiding this comment.
LGTM. Great job!
Remember to mark this PR as ready for review and update the nixpkgs and home-manager inputs once the Ghostty patches are available.
Will do so! I am subscribed to both the nixpkgs and hm PRs, so I should get notified when they are done. |
Same. Keep in mind that the Nixpkgs PR has to land into |
The package PR has landed onto Is there a reason why |
I was thinking the same when I saw this. I opened #712. |
|
The ghostty module got merged into master an hour ago! |
trueNAHO
left a comment
trueNAHO
left a comment
There was a problem hiding this comment.
The ghostty module got merged into master an hour ago!
Please update the nixpkgs and home-manager inputs appropriately for the CI to pass.
Done! While I was at it, I looked at how the tests are implemented and added a |
trueNAHO
left a comment
trueNAHO
left a comment
There was a problem hiding this comment.
Please update the
nixpkgsandhome-managerinputs appropriately for the CI to pass.Done! While I was at it, I looked at how the tests are implemented and added a
testbed.nixto ghostty 😄
Great job!
|
I just saw that #712 got merged. I hope me syncing |
The |
Link: nix-community#703 Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com> (cherry picked from commit 6eb0597)
Link: nix-community#703 Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com> (cherry picked from commit 6eb0597)
2 participants
Ghostty just came out, and I wanted to try it out right away!
If I am gonna do this, I could also do it the right way from the start so I can share my config. Here is my current progress. Feel free to chime in with corrections :D
Colorscheme is derived by comparing cattpuccin macchiato from their official ghostty repo and the tera file for compiling the theme, and the base16 version of the theme. They named everything nicely! The only things missing are:
This will be kept as a draft, until the ghostty PR is merged into home-manager.