feat(home-manager): add support for firefox-based browsers #451
Conversation
Anomalocaridid
force-pushed
the
firefox
branch
3 times, most recently
from
ef006cf to
01fe782
Compare
modules/home-manager/firefox.nix
Outdated
| profiles.default.extensions = [ | ||
| # Only package desired theme addon to not pollute themes list | ||
| # and to avoid flooding the user with extension added notifications | ||
| # Code adapted from `buildFirefoxXpiAddon` from | ||
| # https://github.com/nix-community/nur-combined/blob/master/repos/rycee/pkgs/firefox-addons/default.nix | ||
| (pkgs.runCommand "catppuccin-firefox" { } '' | ||
| dst="$out/share/mozilla/extensions/{ec8030f7-c20a-464f-9b0e-13a3a9e97384}" | ||
| mkdir -p "$dst" | ||
| install -v -m644 "${sources.firefox}/catppuccin_${cfg.flavor}_${cfg.accent}.xpi" "$dst/${addonId}.xpi" | ||
| '') | ||
| ]; |
There was a problem hiding this comment.
I think an option for defining the profiles to install it would be nice, instead of just assuming the profile
There was a problem hiding this comment.
I have now added a profiles option that takes a list of profiles, defaulting with [ "default" ].
There was a problem hiding this comment.
I didn't see your comment here. I don't think there should be a default value, also, the config needs update to install in all profiles, it is currently just installing on the default profile still
There was a problem hiding this comment.
the config needs update to install in all profiles, it is currently just installing on the default profile still
My bad, thank you for catching that.
modules/home-manager/firefox.nix
Outdated
| (pkgs.runCommand "catppuccin-firefox" { } '' | ||
| dst="$out/share/mozilla/extensions/{ec8030f7-c20a-464f-9b0e-13a3a9e97384}" | ||
| mkdir -p "$dst" | ||
| install -v -m644 "${sources.firefox}/catppuccin_${cfg.flavor}_${cfg.accent}.xpi" "$dst/${addonId}.xpi" | ||
| '') |
There was a problem hiding this comment.
I think it would be better to move this to the package rather than here. Maybe we can use split outputs? Or a scope?
There was a problem hiding this comment.
I was able to move it to the package using split outputs. I have never needed to use split outputs or package scopes before, so I am not sure if the way I did it is the best.
I made an output for each addon in the form ${flavor}_${accent}. (e.g. mocha_mauve). I could not leave out out without causing an error, so I kept it. I was not sure what to put in it, so $out is just an empty directory.
modules/home-manager/firefox.nix
Outdated
|
|
||
| # Each Firefox addon has an ID used to refer to it in the config | ||
| # Unfortunately, these addons just use randomly generated ones | ||
| addonIds = { |
There was a problem hiding this comment.
Not a review, I'm just very impressed (and sorry) that you sorted through all of these. Thanks!
There was a problem hiding this comment.
My pleasure! I am more than happy to go to such lengths for declarative configuration.
Anomalocaridid
force-pushed
the
firefox
branch
3 times, most recently
from
275b6b3 to
48926e6
Compare
Anomalocaridid
force-pushed
the
firefox
branch
from
a21d38d to
cf0035c
Compare
pkgs/firefox/package.nix
Outdated
| fetchFromGitHub, | ||
| lib, | ||
| }: | ||
| buildCatppuccinPort (finalAttrs: rec { |
There was a problem hiding this comment.
Why make it a rec attribute set if there is finalAttrs?
There was a problem hiding this comment.
I needed to access addonIds to make outputs and referencing it through finalAttrs there caused infinite recursion. I also assumed it would be better to use finalAttrs where I could rather than just using rec for everything.
Is there a better way to accomplish this?
There was a problem hiding this comment.
Yeah, that makes sense. I think you can just put addonIds in a let in and then inherit it in passthru then you can remove rec and use it to make outputs using the variable. I think this is a better way
There was a problem hiding this comment.
Good idea. I also removed finalAttrs because that was the only thing it was used for.
Anomalocaridid
force-pushed
the
firefox
branch
2 times, most recently
from
d7be395 to
849183a
Compare
Anomalocaridid
force-pushed
the
firefox
branch
from
849183a to
2d97f2f
Compare
| // { | ||
| finalPackage = lib.mkOption { | ||
| type = lib.types.package; | ||
| readOnly = true; |
There was a problem hiding this comment.
perhpas we should mark this as visible = false such that the search won't have this come up?
| readOnly = true; | |
| readOnly = true; | |
| visible = false; |
This adds support for Firefox-based browsers that have settings implemented by home manager's internal
mkFirefoxModulefunction. This should hopefully also make it easy to add support for other Firefox-based browsers in the future, although changes may have to be made if any special cases arise.Currently, this only implements support for the three web browsers I saw that use
mkFireFoxModule, namely:I added a
finalPackageoption to each of their modules because I overrodeextraPrefsFilesto try to immutably set the theme addon.I was not sure how to best share the common logic between the browsers, so I put them all in one file,
firefox.nix. The common logic seemed like it would be too long to include incatppuccinLiband currently the only differences between the browsers' modules are the name of the browser itself.Also, manually enabling the theme is needed in some cases, such as when switching between Catppuccin themes and in Floorp's case, it is needed even on a clean setup for some reason. Unfortunately, I am not sure if there is a way to address this.
Furthermore, in case it is helpful, I had to use the
oldtag fromcatppuccin/firefoxbecause otherwise this would need to access the GitHub release artifacts, although those are the same files anyways. I assumed this would be fine because the GTK module appears to do something similar.