anufrievroman / waypaper

GUI wallpaper manager for Wayland and Xorg Linux systems
https://anufrievroman.gitbook.io/waypaper
GNU General Public License v3.0
277 stars 11 forks source link

fix: Allowed use of --random without --restore #29

Closed badloop closed 4 months ago

badloop commented 4 months ago

--random now assigns a random wallpaper and saves the configuration based on the new assignment

anufrievroman commented 4 months ago

Thank you for PR. A couple of requests: 1) Could you please submit only changes to the main? I mean, in other files it's just autoformatting, so it sort of complicates the history, plus I don't really use such strict formatting. 2) Why do you use enumerate if later you never use i ?

badloop commented 4 months ago

Yes, I will clean those up. Apologies, the enumerate was during an attempt to build the wallpaper array within main rather than execute save() on every loop. I will fix that as well.

badloop commented 4 months ago

Done

anufrievroman commented 4 months ago

I also don't like saving on each iteration, it often causes troubles with cloud sync etc when the config file is changed very often. How about trying something like this :

if args.random:
    w = get_random_file(cf.backend, cf.image_folder, cf.include_subfolders)
    cf.wallpaper[cf.monitors.index(monitor)] = w # find the index (instead of enumerate) and replace value in the list

and then cf.save after the loop?

p.s. here I use monitors to find the index because their names are unique.

badloop commented 4 months ago

I also don't like saving on each iteration, it often causes troubles with cloud sync etc when the config file is changed very often. How about trying something like this :

if args.random:
    w = get_random_file(cf.backend, cf.image_folder, cf.include_subfolders)
    cf.wallpaper[cf.monitors.index(monitor)] = w # find the index (instead of enumerate) and replace value in the list

and then cf.save after the loop?

p.s. here I use monitors to find the index because their names are unique.

This doesn't work because of the way cf.save() adjusts the cf.wallpaper value based on the cf.monitors value like I said in the issue thread.

https://github.com/anufrievroman/waypaper/issues/6#issuecomment-1946211542

anufrievroman commented 4 months ago

Hmm 🤔 I'll need to check it deeper then. I don't have a moment right now, but I'll check it...

anufrievroman commented 4 months ago

Also, another little thing, let's put

            cf.selected_wallpaper = w
            cf.save()

inside the if arg.random because otherwise, we don't need to save anything

badloop commented 4 months ago

Done

anufrievroman commented 4 months ago

Thank you for the correction! I'll check tomorrow.