Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make example scripts more portable #391

Merged
merged 3 commits into from
Mar 5, 2025

Conversation

rubiefawn
Copy link
Contributor

This is an attempt to make swww_randomize.sh and swww_randomize_multi.sh more portable by replacing the $RANDOM bash-ism with more portable way of randomizing the files.

This also removes swww_randomize_multi.sh's check to make sure it is not already running, since it uses $BASHPID. In my subjective opinion, such logic is not needed in the first place, since these scripts are likely to be run once by the compositor automatically rather than manually invoked multiple times. That said, if it is required, I will write a portable solution that doesn't use $BASHPID and apply that solution in swww_randomize.sh as well, which currently lacks this check.

The use of read(1) in the innermost loop wasn't properly capturing the
piped-in values, and would hang forever waiting for input. The syntax
used in the original swww_randomize_multi.sh does not have this
problem. I don't really understand how the other syntax does not also
have this problem, but I am reverting it so that it at least works.

I removed the $ in front of the $img variable when unsetting it, since
including it was expanding the variable. Oops!
Copy link
Contributor

@filip-rs filip-rs left a comment

Choose a reason for hiding this comment

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

LGTM, both scripts work fine on my machine (Arch + hyprland). Good change, could be merged.

@LGFae
Copy link
Owner

LGFae commented Mar 5, 2025

My bad, I had forgotten to merge this. Thanks!

@LGFae LGFae merged commit dcb5668 into LGFae:main Mar 5, 2025
10 checks passed
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.

3 participants