stylix icon indicating copy to clipboard operation
stylix copied to clipboard

doc: remove redundant documentation dummy values

Open trueNAHO opened this issue 7 months ago • 2 comments

The following long-standing unresolved question finally seems be resolved:

Can docs/settings.nix be removed due to the new stylix.enable option?

Not yet: although the option is not required any more, the default value for stylix.image is still of an incorrect type, so it needs to be overridden. This will be fixed in #407, after which it should be possible to remove the file.

-- https://github.com/nix-community/stylix/pull/244#issuecomment-2156998617

The removal of /doc/settings.nix resolves the following subtle regression:

This is a very subtle regression making dummy stylix values mandatory inside pkgs.nixosOptionsDoc. Although Stylix works around this limitation with /docs/settings.nix, I am unsure how I was able to use pkgs.nixosOptionsDoc without stylix dummy values in my Home Manager setup until now. However, since /docs/settings.nix may be removed in the future, this regression should be fine and can be ignored in this PR:

Due to some targets not being enabled by default with stylix.mkEnableTarget, the documentation incorrectly generates Default: false in some cases with https://github.com/danth/stylix/pull/399. Resolving this involves manually extending docs/settings.nix or enabling all targets by default with stylix.mkEnableTarget.

-- https://github.com/danth/stylix/issues/400#issue-2319041678

For reference, I resolved this regression on my end by adding the "${inputs.stylix}/docs/settings.nix module to my pkgs.nixosOptionsDoc environment.

-- https://github.com/nix-community/stylix/pull/717#pullrequestreview-2619444260

Entirely removing /doc/settings.nix does not cause nix build .#docs to fail and diff --recursive reports no changes with master (43a652de771aabd206ad9ce137171d0da0966198) after applying the following debugging patch on top of this PR:

diff --git a/doc/default.nix b/doc/default.nix
index 5740392..5791f66 100644
--- a/doc/default.nix
+++ b/doc/default.nix
@@ -336,7 +336,7 @@ let

   # Permalink to view a source file on GitHub. If the commit isn't known,
   # then fall back to the latest commit.
-  declarationCommit = inputs.self.rev or "master";
+  declarationCommit = "43a652de771aabd206ad9ce137171d0da0966198";
   declarationPermalink = "https://github.com/nix-community/stylix/blob/${declarationCommit}";

   # Renders a single option declaration. Example output:

Things done

Notify maintainers

@danth

trueNAHO avatar May 20 '25 18:05 trueNAHO

This feels like a duplicate of https://github.com/nix-community/stylix/pull/1212 ?

Or I guess more of a subset of it.

MattSturgeon avatar May 21 '25 00:05 MattSturgeon

This feels like a duplicate of #1212 ?

Or I guess more of a subset of it.

I did not thoroughly review that PR yet, but when I skimmed over it, I wondered if /doc/settings.nix could already be removed even without the additional simplifications from that PR. And that seems to be the case. Considering this PR works on its own and for how long this /doc/settings.nix workaround has existed, it seems worth merging this PR separately.

Just waiting on @danth's approval in case something non-obvious has been missed, which is the reason this has not already been removed.

trueNAHO avatar May 21 '25 18:05 trueNAHO

Just waiting on [danth]'s approval in case something non-obvious has been missed, which is the reason this has not already been removed.

Let's just optimistically merge this for now.

trueNAHO avatar Jun 12 '25 22:06 trueNAHO

Successfully created backport PR for release-25.05:

  • #1491

stylix-automation[bot] avatar Jun 12 '25 22:06 stylix-automation[bot]