Skip to content

wayfire: remove optional toString#1658

Merged
trueNAHO merged 1 commit intonix-community:masterfrom
trueNAHO:wayfire-remove-optional-to-string
Jul 13, 2025
Merged

wayfire: remove optional toString#1658
trueNAHO merged 1 commit intonix-community:masterfrom
trueNAHO:wayfire-remove-optional-to-string

Conversation

@trueNAHO
Copy link
Copy Markdown
Member

@trueNAHO trueNAHO commented Jul 9, 2025

This PR should not be merged untested.

Things done

Notify maintainers

@0x5a4

@trueNAHO
Copy link
Copy Markdown
Member Author

This PR should not be merged untested.

If the current approach does not work, the toString conversion could be centralized as:

diff --git a/modules/wayfire/hm.nix b/modules/wayfire/hm.nix
index d11ad25..7b8b814 100644
--- a/modules/wayfire/hm.nix
+++ b/modules/wayfire/hm.nix
@@ -26,9 +26,11 @@ mkTarget {
         imageScalingMode,
       }:
       let
-        wayfireBackground = pkgs.runCommand "wayfire-background.png" { } ''
-          ${lib.getExe' pkgs.imagemagick "convert"} ${image} $out
-        '';
+        wayfireBackground = toString (
+          pkgs.runCommand "wayfire-background.png" { } ''
+            ${lib.getExe' pkgs.imagemagick "convert"} ${image} $out
+          ''
+        );
       in
       {
         wayland.windowManager.wayfire.wf-shell.settings = {

@einetuer
Copy link
Copy Markdown
Contributor

einetuer commented Jul 10, 2025

What about this might not work? Straight eval failure?

EDIT: yes.

@einetuer
Copy link
Copy Markdown
Contributor

einetuer commented Jul 10, 2025

Tested, and the toString call is required (the home-manager type is too restrictive, my bad).

EDIT: Created nix-community/home-manager#7427 to address this. If merged, I will create a follow up PR here to get rid of toString once and for all

@stylix-automation stylix-automation bot added the status: merge conflict Merge conflict label Jul 11, 2025
@trueNAHO trueNAHO force-pushed the wayfire-remove-optional-to-string branch from 6e44d19 to a36e8b1 Compare July 11, 2025 11:02
@trueNAHO trueNAHO marked this pull request as draft July 11, 2025 11:03
@trueNAHO
Copy link
Copy Markdown
Member Author

Created nix-community/home-manager#7427 to address this. If merged, I will create a follow up PR here to get rid of toString once and for all

Thanks :)

If the only additional change is updating the Home Manager input, I can also do it in this PR. I leave it up to you. Either way, I am converting this PR to a draft until nix-community/home-manager#7427 is merged.

@stylix-automation stylix-automation bot removed the status: merge conflict Merge conflict label Jul 11, 2025
@einetuer
Copy link
Copy Markdown
Contributor

The home-manager PR was merged, you can also make the update here. I'll test it again then

@trueNAHO trueNAHO force-pushed the wayfire-remove-optional-to-string branch from a36e8b1 to f3f2903 Compare July 12, 2025 16:39
@trueNAHO
Copy link
Copy Markdown
Member Author

The home-manager PR was merged, you can also make the update here. I'll test it again then

I have updated the home-manager input. Could you test this PR again?

@trueNAHO trueNAHO marked this pull request as ready for review July 12, 2025 19:22
Copy link
Copy Markdown
Contributor

@einetuer einetuer left a comment

Choose a reason for hiding this comment

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

Works!

@trueNAHO trueNAHO merged commit a950a3f into nix-community:master Jul 13, 2025
5 checks passed
@stylix-automation
Copy link
Copy Markdown
Contributor

Backport failed for release-25.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-25.05
git worktree add -d .worktree/backport-1658-to-release-25.05 origin/release-25.05
cd .worktree/backport-1658-to-release-25.05
git switch --create backport-1658-to-release-25.05
git cherry-pick -x a950a3f529e1952a7ddf2af138b90a99edf41fe5

@trueNAHO
Copy link
Copy Markdown
Member Author

Backport failed for release-25.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-25.05
git worktree add -d .worktree/backport-1658-to-release-25.05 origin/release-25.05
cd .worktree/backport-1658-to-release-25.05
git switch --create backport-1658-to-release-25.05
git cherry-pick -x a950a3f529e1952a7ddf2af138b90a99edf41fe5

The nix-community/home-manager#7427 PR has not been backported to the stable branch yet. Since this is a fairly low priority change, we can just not backport this for the time being.

LunaCOLON3 pushed a commit to LunaCOLON3/stylix that referenced this pull request Jul 14, 2025
Link: nix-community#1658

Reviewed-by: 1444 <54070204+0x5a4@users.noreply.github.com>
Tested-by: 1444 <54070204+0x5a4@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: home-manager Home Manager target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants