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

Jump to bottom

nixos/polybar: init #81510

Closed
wants to merge 1 commit into from
Closed

nixos/polybar: init #81510

wants to merge 1 commit into from

Conversation

ehmry
Copy link
Contributor

@ehmry ehmry commented Mar 2, 2020 •
edited

Motivation for this change

It is a bit difficult to get polybar working. It looks strange when a font is missing, and might be missing compile-time support for some use-cases. Adding a NixOS module takes care of the font and makes package configuration more discoverable to the user.

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

Polybar maintainers: @afldcr @filalex77

@ehmry ehmry requested a review from infinisil as a code owner March 2, 2020 11:59
Br1ght0ne
Br1ght0ne approved these changes Mar 2, 2020
View reviewed changes
Copy link
Member

@Br1ght0ne Br1ght0ne left a comment

Choose a reason for hiding this comment

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

Good idea for making defaults work. Thanks, LGTM!

nixos/modules/programs/polybar.nix Outdated Show resolved Hide resolved
@aaronjanse
Copy link
Member

Since we're adding a polybar module to nixpkgs, would we want to merge some of home-manager's polybar code?

@ehmry
Copy link
Contributor Author

ehmry commented Mar 2, 2020

@aaronjanse probably? I don't use home-manager but the more I think about, its must be more of a home-manager problem than a nixos problem. Is time better spent on improving home-manager rather than expanding nixos/modules/programs?

@aaronjanse
Copy link
Member

I personally would like to see as much as home-manager as nixpkgs as possible. Otherwise, external modules have to juggle duplicated options between home-manager and nixpkgs, which could get ugly.

@infinisil
Copy link
Member

I personally would like to see as much as home-manager as nixpkgs as possible

I can't parse that sentence

@aaronjanse
Copy link
Member

aaronjanse commented Mar 3, 2020 •
edited

I personally would like to see as much as home-manager as nixpkgs as possible

I can't parse that sentence

Oops, I forgot a few words!
"I personally would like to see as much of home-manager merged into nixpkgs as possible."

In other words, if something in home-manager can be merged into nixpkgs, I would personally want it to be merged ;-)

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/why-isnt-more-of-home-manager-merged-into-nixpkgs/6096/5

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/why-isnt-more-of-home-manager-merged-into-nixpkgs/6096/6

@bennofs
Copy link
Contributor

bennofs commented Mar 4, 2020 •
edited

To be honest, this looks too trivial for a module to include in nixpkgs. It doesn't do anything except add a font to the system fonts. Is that even necessary in all cases? If I use another font for my polybar, I won't need the siji font, will I? Of course you need the fonts that you use in configs.

@ehmry
Copy link
Contributor Author

ehmry commented Mar 5, 2020

I'm closing this issue, as polybar support belongs in something like home-manager.

@ehmry ehmry closed this Mar 5, 2020
@worldofpeace
Copy link
Contributor

To be honest, this looks too trivial for a module to include in nixpkgs. It doesn't do anything except add a font to the system fonts. Is that even necessary in all cases? If I use another font for my polybar, I won't need the siji font, will I? Of course you need the fonts that you use in configs.

I simple solution could be to just add documentation for using the polybar package. I believe there's a section for that in the nixpkgs manual.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worldofpeace worldofpeace worldofpeace left review comments

@Br1ght0ne Br1ght0ne Br1ght0ne approved these changes

@infinisil infinisil Awaiting requested review from infinisil

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@ehmry @aaronjanse @infinisil @nixos-discourse @bennofs @worldofpeace @Br1ght0ne