Skip to content

home.pointerCursor: init#2891

Merged
berbiche merged 2 commits intonix-community:masterfrom
polykernel:home-pointercursor/init
May 3, 2022
Merged

home.pointerCursor: init#2891
berbiche merged 2 commits intonix-community:masterfrom
polykernel:home-pointercursor/init

Conversation

@polykernel
Copy link
Copy Markdown
Contributor

@polykernel polykernel commented Apr 13, 2022

Description

The current architecture for cursor configurations is composed of individual
options for different backends. For example, X specific settings are managed under
xsession.pointerCursor and gtk specific settings are managed under gtk.cursorTheme.
While this architecture is modular, it causes duplication of similar structures for
each component. In theory, this provides flexibility because the components are independent
of each other which can be arranged in arbitrary ways to achieve the desired result.
However in practice, users wish to have one cursor theme applied to their entire system
The duplication of options correspond to duplication of settings on the user side and it
becomes a burden to keep track of all necessary settings.

This commit is an attempt to unify cursor configurations for different window systems and
GUI toolkits based on #2481 (comment). home.pointerCursor is introduced as the interface
for all cursor configurations. It contain all options relevant to cursor themes with eneral options delcared under home.pointerCursor.* and backend specific options declared under home.pointerCursor.<backend>.*.
By default, a backend independent configuration is generated. Backend specific configurations can be toggled
via the home.pointerCursor.<backend>.enable option for each backend. This was decided over using a
list of enums because it allows easy access to the state of the backend. Note generating different
cursor configurations for different backends is still possible by defining only home.pointerCursor
and managing the respective options manually.

I am not sure whether options such as gtk.cursorTheme and xsession.pointerCursor should be merged into home.pointerCursor or not. Any feedback would be greatly appreciated!

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.

@polykernel polykernel requested review from league and rycee as code owners April 13, 2022 23:49
@polykernel
Copy link
Copy Markdown
Contributor Author

@league I would like to know your opinion on whether xsession.pointerCursor should remain as is or be merged into home.pointerCursor.

@league
Copy link
Copy Markdown
Collaborator

league commented Apr 14, 2022

I think the merging seems sensible, but with the proviso that I don't use any of these settings currently and wouldn't notice if it subtly breaks something for others. I think I'm approaching the point of view that much of the value for a module like this is in helping people figure out what are the ways to set these sorts of things (especially just for X where you're more "on your own"). If the "happy path" you've blazed here is helpful, then fine. And if someone needs it to work differently, well, it's not really that much harder to set xsession.initExtra or xresources.properties one's self, rather than use these higher-level properties.

@league
Copy link
Copy Markdown
Collaborator

league commented Apr 14, 2022

I guess at a glance I'm not certain why the CI is complaining about:

error: The option `home.pointerCursor.package' is used but not defined.

But assuming that's resolved, it's okay with me.

@polykernel
Copy link
Copy Markdown
Contributor Author

Thank you for your input!

I think the merging seems sensible, but with the proviso that I don't use any of these settings currently and wouldn't notice if it subtly breaks something for others. I think I'm approaching the point of view that much of the value for a module like this is in helping people figure out what are the ways to set these sorts of things (especially just for X where you're more "on your own"). If the "happy path" you've blazed here is helpful, then fine. And if someone needs it to work differently, well, it's not really that much harder to set xsession.initExtra or xresources.properties one's self, rather than use these higher-level properties.

My original idea was to keep xsession.pointerCursor separate and make home.pointerCursor a high level entrypoint for the configurations. This allows the lower-level settings to remain modular while providing ease of use to the user. As you suggested, users can also override home.pointerCursor to change the different lower-level setting used(arguably more easily as it is only an indirection layer). However, this model introduces ambiguity in terms of dependencies(e.g which option should be depended on for a module that needs to use the X default cursor?) and result in substantial code duplication with more backends. I think going with the merging is simpler for users. We can always add more complexity incrementally if needed.

Copy link
Copy Markdown
Member

@berbiche berbiche left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Comment thread modules/config/home-cursor.nix Outdated
@EHfive
Copy link
Copy Markdown

EHfive commented Apr 18, 2022

Hi, could you also address the cursor problem on generic Linux, i.e. also suffix XCURSOR_PATH with /usr/share/icons:/usr/share/pixmaps if config.targets.genericLinux.enable == true .

libXcursor by default fallback XCURSOR_PATH to ~/.local/share/icons:~/.icons:${prefix}/share/icons:${prefix}/share/pixmaps, which resolves to ~/.local/share/icons:~/.icons:/nix/store/<hash>-libxcursor-x.x.x/share/icons:/nix/store/<hash>-libxcursor-x.x.x/share/pixmaps, causing it failed to found cursor icons installed in /usr/share/icons:/usr/share/pixmaps (as well as paths in home-manager profile).

NixOS has set XCURSOR_PATH to system profile paths to address the problem. See https://github.com/NixOS/nixpkgs/blob/634141959076a8ab69ca2cca0f266852256d79ee/nixos/modules/config/xdg/icons.nix#L31-L40

@polykernel
Copy link
Copy Markdown
Contributor Author

Hi, could you also address the cursor problem on generic Linux, i.e. also suffix XCURSOR_PATH with /usr/share/icons:/usr/share/pixmaps if config.targets.genericLinux.enable == true .

libXcursor by default fallback XCURSOR_PATH to ~/.local/share/icons:~/.icons:${prefix}/share/icons:${prefix}/share/pixmaps, which resolves to ~/.local/share/icons:~/.icons:/nix/store/<hash>-libxcursor-x.x.x/share/icons:/nix/store/<hash>-libxcursor-x.x.x/share/pixmaps, causing it failed to found cursor icons installed in /usr/share/icons:/usr/share/pixmaps (as well as paths in home-manager profile).

NixOS has set XCURSOR_PATH to system profile paths to address the problem. See https://github.com/NixOS/nixpkgs/blob/634141959076a8ab69ca2cca0f266852256d79ee/nixos/modules/config/xdg/icons.nix#L31-L40

Thanks for the suggestion. I have applied the changes you requested but unfortunately I don't have a non-NixOS box at the moment where I can test the changes. Please let me know if any issues/regressions arise.

@EHfive
Copy link
Copy Markdown

EHfive commented Apr 18, 2022

Thanks.
I have tested the patch branch in my flake setup, I can confirm /usr/share/icons:/usr/share/pixmaps has added to XCURSOR_PATH, and one of the previously broken app can now locates the cursor icon successfully.

I have to set both targets.genericLinux.enable = true and home.pointerCursor = { /* ... */ } to got /usr/share/icons:/usr/share/pixmaps appended to XCURSOR_PATH. But what I originally want is to have non-home-manager defined cursor shows in nix installed apps, so in that case setting home.pointerCursor to something non-null should not be required.

So I guess a more appropriate place to put this logic is ./modules/targets/generic-linux.nix, but that might exceeds the scope of this PR? (edit: If that is the case, I can make a PR if you have less interest in this.)

# home.nix
{ pkgs, ... }: {
  targets.genericLinux.enable = true;
  home.pointerCursor = {
    package = pkgs.vanilla-dmz;
    name = "Vanilla-DMZ";
  };
}

@berbiche
Copy link
Copy Markdown
Member

So I guess a more appropriate place to put this logic is ./modules/targets/generic-linux.nix

Yes, it would be more appropriate IMO.

@polykernel
Copy link
Copy Markdown
Contributor Author

I have to set both targets.genericLinux.enable = true and home.pointerCursor = { /* ... */ } to got /usr/share/icons:/usr/share/pixmaps appended to XCURSOR_PATH. But what I originally want is to have non-home-manager defined cursor shows in nix installed apps, so in that case setting home.pointerCursor to something non-null should not be required.

So I guess a more appropriate place to put this logic is ./modules/targets/generic-linux.nix, but that might exceeds the scope of this PR? (edit: If that is the case, I can make a PR if you have less interest in this.)

I think it is better to set XCURSOR_PATH separately in /modules/targets/generic-linux.nix with a higher priority in this case because moving the definition out of the mkIf block will break evaluation for users other platforms due to the assertion. Although it is possible to define an explicit enable flag to guard the assertion, it would still force the user to define options under home.pointerCursor due to strict evaluation of the submodule and in my opinion it is more complex than necessary.

In any case, I think another dedicated PR will suit this better.

Comment thread modules/config/home-cursor.nix Outdated
# https://wiki.archlinux.org/title/Cursor_themes#Environment_variable
home.sessionVariables = {
XCURSOR_PATH = mkDefault (concatStringsSep ":" [
"\${XCURSOR_PATH}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See my comment in the other PR #2902 (comment)

The current architecture for cursor configurations is composed of individual
options for different backends. For example, X specific settings are managed under
`xsession.pointerCursor` and gtk specific settings are managed under `gtk.cursorTheme`.
While this architecture is modular, it causes duplication of similar structures for
each component. In theory, this provides flexibility because the components are independent
of each other which can be arranged in arbitrary ways to achieve the desired result.
However in practice, users wish to have one cursor theme applied to their entire system
The duplication of options correspond to duplication of settings on the user side and it
becomes a burden to keep track of all necessary settings.

This commit is an attempt to unify cursor configurations for different window systems and
GUI toolkits based on #2481 (comment).
`home.pointerCursor` is introduced as the interface for all cursor configurations.
It contain all options relevant to cursor themes with eneral options delcared under `home.pointerCursor.*`
and backend specific options declared under `home.pointerCursor.<backend>.*`. By default, a backend
independent configuration is generated. Backend specific configurations can be toggled via the
`home.pointerCursor.<backend>.enable` option for each backend. This was decided over using a
list of enums because it allows easy access to the state of the backend. Note generating different
cursor configurations for different backends is still possible by defining only `home.pointerCursor`
and managing the respective options manually.
- Removed `xession.pointerCursor` as x11 cursor configurations are now handled in `home.pointerCursor.x11`.
- Updated `meta.maintainer` field in `home.pointerCursor` and CODEOWNERS.
berbiche pushed a commit that referenced this pull request Apr 20, 2022
…2902)

This commit appends system-wide icon and pixmap directory and the icon
directory in the home-manager profile to the XCURSOR_PATH session variable
for the generic linux target. This is necessary because the default prefix
for libXcursor resolves to the Nix store which excludes the aforementioned
directories from being searched for cursor themes. [1]

[1] - #2891 (comment).
Copy link
Copy Markdown
Member

@berbiche berbiche left a comment

Choose a reason for hiding this comment

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

LGTM!

I'll wait a few days to see if there are other reviews before merging.

teto pushed a commit to teto/home-manager that referenced this pull request Apr 28, 2022
…ix-community#2902)

This commit appends system-wide icon and pixmap directory and the icon
directory in the home-manager profile to the XCURSOR_PATH session variable
for the generic linux target. This is necessary because the default prefix
for libXcursor resolves to the Nix store which excludes the aforementioned
directories from being searched for cursor themes. [1]

[1] - nix-community#2891 (comment).
@berbiche berbiche merged commit c13ffa3 into nix-community:master May 3, 2022
@berbiche
Copy link
Copy Markdown
Member

berbiche commented May 3, 2022

I forgot that we need to add a news entry to the 22.05 release notes (docs/release-notes/rl-2205.adoc).

@polykernel polykernel deleted the home-pointercursor/init branch May 7, 2022 19:55
jevy pushed a commit to jevy/home-manager that referenced this pull request Jul 1, 2022
* home.pointerCursor: init

The current architecture for cursor configurations is composed of individual
options for different backends. For example, X specific settings are managed under
`xsession.pointerCursor` and gtk specific settings are managed under `gtk.cursorTheme`.
While this architecture is modular, it causes duplication of similar structures for
each component. In theory, this provides flexibility because the components are independent
of each other which can be arranged in arbitrary ways to achieve the desired result.
However in practice, users wish to have one cursor theme applied to their entire system
The duplication of options correspond to duplication of settings on the user side and it
becomes a burden to keep track of all necessary settings.

This commit is an attempt to unify cursor configurations for different window systems and
GUI toolkits based on nix-community#2481 (comment).
`home.pointerCursor` is introduced as the interface for all cursor configurations.
It contain all options relevant to cursor themes with eneral options delcared under `home.pointerCursor.*`
and backend specific options declared under `home.pointerCursor.<backend>.*`. By default, a backend
independent configuration is generated. Backend specific configurations can be toggled via the
`home.pointerCursor.<backend>.enable` option for each backend. This was decided over using a
list of enums because it allows easy access to the state of the backend. Note generating different
cursor configurations for different backends is still possible by defining only `home.pointerCursor`
and managing the respective options manually.

* xcursor: migrate options to home.pointerCursor

- Removed `xession.pointerCursor` as x11 cursor configurations are now handled in `home.pointerCursor.x11`.
- Updated `meta.maintainer` field in `home.pointerCursor` and CODEOWNERS.
@teto teto mentioned this pull request Aug 22, 2022
7 tasks
teto pushed a commit to teto/home-manager that referenced this pull request Aug 22, 2022
…ix-community#2902)

This commit appends system-wide icon and pixmap directory and the icon
directory in the home-manager profile to the XCURSOR_PATH session variable
for the generic linux target. This is necessary because the default prefix
for libXcursor resolves to the Nix store which excludes the aforementioned
directories from being searched for cursor themes. [1]

[1] - nix-community#2891 (comment).
teto pushed a commit to teto/home-manager that referenced this pull request Aug 22, 2022
* home.pointerCursor: init

The current architecture for cursor configurations is composed of individual
options for different backends. For example, X specific settings are managed under
`xsession.pointerCursor` and gtk specific settings are managed under `gtk.cursorTheme`.
While this architecture is modular, it causes duplication of similar structures for
each component. In theory, this provides flexibility because the components are independent
of each other which can be arranged in arbitrary ways to achieve the desired result.
However in practice, users wish to have one cursor theme applied to their entire system
The duplication of options correspond to duplication of settings on the user side and it
becomes a burden to keep track of all necessary settings.

This commit is an attempt to unify cursor configurations for different window systems and
GUI toolkits based on nix-community#2481 (comment).
`home.pointerCursor` is introduced as the interface for all cursor configurations.
It contain all options relevant to cursor themes with eneral options delcared under `home.pointerCursor.*`
and backend specific options declared under `home.pointerCursor.<backend>.*`. By default, a backend
independent configuration is generated. Backend specific configurations can be toggled via the
`home.pointerCursor.<backend>.enable` option for each backend. This was decided over using a
list of enums because it allows easy access to the state of the backend. Note generating different
cursor configurations for different backends is still possible by defining only `home.pointerCursor`
and managing the respective options manually.

* xcursor: migrate options to home.pointerCursor

- Removed `xession.pointerCursor` as x11 cursor configurations are now handled in `home.pointerCursor.x11`.
- Updated `meta.maintainer` field in `home.pointerCursor` and CODEOWNERS.
spacekookie pushed a commit to spacekookie/home-manager that referenced this pull request Feb 10, 2023
…ix-community#2902)

This commit appends system-wide icon and pixmap directory and the icon
directory in the home-manager profile to the XCURSOR_PATH session variable
for the generic linux target. This is necessary because the default prefix
for libXcursor resolves to the Nix store which excludes the aforementioned
directories from being searched for cursor themes. [1]

[1] - nix-community#2891 (comment).
spacekookie pushed a commit to spacekookie/home-manager that referenced this pull request Feb 10, 2023
* home.pointerCursor: init

The current architecture for cursor configurations is composed of individual
options for different backends. For example, X specific settings are managed under
`xsession.pointerCursor` and gtk specific settings are managed under `gtk.cursorTheme`.
While this architecture is modular, it causes duplication of similar structures for
each component. In theory, this provides flexibility because the components are independent
of each other which can be arranged in arbitrary ways to achieve the desired result.
However in practice, users wish to have one cursor theme applied to their entire system
The duplication of options correspond to duplication of settings on the user side and it
becomes a burden to keep track of all necessary settings.

This commit is an attempt to unify cursor configurations for different window systems and
GUI toolkits based on nix-community#2481 (comment).
`home.pointerCursor` is introduced as the interface for all cursor configurations.
It contain all options relevant to cursor themes with eneral options delcared under `home.pointerCursor.*`
and backend specific options declared under `home.pointerCursor.<backend>.*`. By default, a backend
independent configuration is generated. Backend specific configurations can be toggled via the
`home.pointerCursor.<backend>.enable` option for each backend. This was decided over using a
list of enums because it allows easy access to the state of the backend. Note generating different
cursor configurations for different backends is still possible by defining only `home.pointerCursor`
and managing the respective options manually.

* xcursor: migrate options to home.pointerCursor

- Removed `xession.pointerCursor` as x11 cursor configurations are now handled in `home.pointerCursor.x11`.
- Updated `meta.maintainer` field in `home.pointerCursor` and CODEOWNERS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants