htop: replace individual option with 'settings'

[bjpbakker]

Description

Add some meters supported in htop but missing from the configuration.

Checklist

• Change is backwards compatible.

• Code formatted with ./format.

• Code tested through nix-shell --pure tests -A run.all.

• Test cases updated/added. See example.

• Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

  See CONTRIBUTING for more information and recent commit messages for examples.

• If this PR adds a new module

  • Added myself as module maintainer. See example.

  • Added myself and the module files to .github/CODEOWNERS.

[berbiche]

Hi, thanks for the contribution.

How can I verify these values?

[bjpbakker]

These values come from htops builtin setup that I used to tweak the left and right meters.

You could do a cross check with the htop. The individual *Meter.c files define the meter names used in config. Some grepping lead to me the following possible meters. I haven't checked all of them for being supported, but happy to do that and add any missing ones to this PR if you like.

AllCPUs
AllCPUs2
AllCPUs4
AllCPUs8
Battery
Blank
Clock
CPU
Date
DateTime
DiskIO
Hostname
HugePages
LeftCPUs
LeftCPUs2
LeftCPUs4
LeftCPUs8
Load
LoadAverage
Memory
NetworkIO
PressureStallCPUSome
PressureStallIOFull
PressureStallIOSome
PressureStallMemoryFull
PressureStallMemorySome
RightCPUs
RightCPUs2
RightCPUs4
RightCPUs8
SELinux
Swap
System
Systemd
Tasks
Uptime
ZFSARC
ZFSCARC
Zram

[teto]

It looks overkill to me to provide this level of detail for configuration (a losing battle) but now that we've started doing it, maybe you should add all of them ^^

[rycee]

I would suggest deprecating pretty much all the options in the htop module and instead add a plain settings option. Maybe adding the fields as constants in config.lib.htop.fields so that people can refer to names instead of numbers. I.e.,

programs.htop.settings.fields = with config.lib.htop.fields; [ PID COMM ];

[bjpbakker]

It looks overkill to me to provide this level of detail for configuration (a losing battle)

I would suggest deprecating pretty much all the options in the htop module and instead add a plain settings option.

Agreed, that should be the way to go on the long run.

Either way, I've added the missing ones for now (checked against htop@2328e52).

[teto]

would be nice to do the depreciation before the nixos 21.05 release.

[bjpbakker]

I can definitely deprecate all options and replace them with a single settings that you basically paste the htoprc contents into.

One of the benefits of doing that, is that people can use setup in htop itself to build their configuration and then paste it in their home-manager config. That works if all specific settings are dropped from nix configuration.

I aim to open a new PR with those changes somewhere next week.

[teto]

wouldn't it be possible to update this one instead ?

[bjpbakker]

wouldn't it be possible to update this one instead ?

Sure, it's just that the scope of this has changed a bit, but I can also push it to this PR.

[bjpbakker]

I've deprecated all options and introduced settings for setting htop configuration values from Nix config. Some lib.htop predefined fields and modes, and functions for meters are provided for convenience.

Hope this helps.

@teto sorry it took a while, was quite busy the last couple weeks. Let me know any feedback, happy to address it :)

[teto]

This looks like a dubious check, maybe in the default settings' prefix cfg.X with lib.warn "warning" cfg.X isntead. Also maybe add a release note. I dont know when the deprecation will be removed. Maybe just after the release of nixos.

[bjpbakker]

Agreed, this one is ugly.

A few days ago I already pushed an added release note. Now also a commit to redo the configuration a bit. Moved defaults into the settings attrset and show a deprecation warning on any (deprecated) option that is explicitly specified in user config. Messages are much more clear now and should properly inform what to do, without relying on reading releases notes too much :)

If this PR is good to go, I'll draft up another one to remove all deprecated things (to merge after 21.05 release, or when suitable).

[bjpbakker]

So, is this one good to go now, with release notes updated and better deprecation warnings?

/cc: @teto

[teto]

can you fix the tests please ?

[bjpbakker]

can you fix the tests please ?

@teto sorry about that, unfortunate timing of picking previous master. I did run the htop tests which were fine, didn't run the entire suite myself again. Fixed by rebasing on top of current master and checking the full suite locally.

@rycee since you are the current code owner, would you like me to put my name down as new code owner of this module?

[teto]

I think @rycee would be happy to cover less ground so I would favor you becming the new owner :)

[bjpbakker]

@teto I now see more issues in the install target of the CI build; fixing them. should be all fixed now.

[rvl]

With the config.lib.htop.leftMeters function, how would a user control the ordering of meters in the column?