home-cursor: use mkRenamedOptionModule for xsession.pointerCursor

[ncfavier]

This hides the obsolete options from the manual, at the cost of displaying two warnings: the one from mkRenamedOptionModule and the generic migration instructions for xsession.pointerCursor.

Fixes (superficially) #3543, though it does not address the underlying issue with mkAliasOptionModule.

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.

[rycee]

Unfortunately, in this case mkAliasOptionModule was used for a reason. If you look at the list entry below the fold in GitHub (click the ⬇) you'll notice that there is actually already a more helpful warning printed. The main difference is that it includes a reference to home.pointerCursor.x11.enable, which would be missing if we use mkRenamedOptionModule.

That said, I think we could get away by changing the warning text below slightly. Something simple like

To enable X11 specific cursor configurations, please enable 'home.pointerCursor.x11.enable'.

may be sufficient. The other options will get their own warnings through mkRenamedOptionModule.

[rycee]

That said², I'm not certain why the home.pointerCursor.x11.enable option is there at all. Presumably the user would like to use the cursor settings whenever config.xsession.enable is set? @polykernel Can you recall why that is?

[ncfavier]

I did see that, that's why I said "at the cost of getting two warnings". I don't think it's too bad, although it might need a change in wording.

Probably a better solution would be to make mkRenamedOptionModule accept a "replacement instructions" message, just like mkRemovedOptionModule does.

But if we can just remove home.pointerCursor.x11.enable that's even simpler.

[ncfavier]

Or (assuming we do need home.pointerCursor.x11.enable) maybe we should indeed use mkRemovedOptionModule and fail instead of accepting a configuration that suddenly does not have X cursors anymore (although it's probably a bit late to be revisiting the migration path for this option).

[polykernel]

That said², I'm not certain why the home.pointerCursor.x11.enable option is there at all. Presumably the user would like to use the cursor settings whenever config.xsession.enable is set? @polykernel Can you recall why that is?

IIRC, the enable option was introduced as a toggle for each backend in case more options are added for the backend in the future.

After some thought, I think each home.pointerCursor.<backend> can also be a nullable submodule which achieves the same functionality as the enable option (e.g setting home.pointerCursor.x11 = {} would enable the x11 backend).

[ncfavier]

If you can submit a PR for that, that would be great! Similarly, I think the GTK backend can simply be enabled if gtk.enable is true. Then both enable options should get a mkRemovedOptionModule explaining why they've been removed.

[polykernel]

When drafting the PR, I noticed that it is not possible to maintain compatibility with the old xsession.pointerCursor module after the removal of x11.enable. Since the xsession.initExtra and xresources.properties settings become dependent on xsession.enable, this means it is not possible to enable the full X cursor configuration in isolation anymore which was supported by xsession.pointerCursor. With the gtk.enable option, the same type of issue arise because of coupled dependencies. For example, if a user wants to enable X cursor configurations via home.pointerCursor and has gtk.enable set to true only for gtk theme configuration, then the gtk cursor configuration also gets set by the home.pointerCursor unbeknownst to the user.

Is the incompatibility introduced acceptable for the module? In any case, I have submitted a draft PR as a work in progress.

[ncfavier]

Maybe it would be much simpler to keep the options, but make x11.enable default to config.xsession.enable and gtk.enable default to config.gtk.enable. That way we don't lose backwards compatibility (although those use cases seem questionable to me) and we don't have to bother with the whole "mkRemovedOptionModule in submodules" situation, and we don't have to tell the user to set anything else.

[polykernel]

I agree. Would you be able to append the default value changes to this PR or should I make another PR?

[ncfavier]

Done

[ncfavier]

I guess we can abandon this now that markdown descriptions are rendered fine.