marguerite / linux-bing-wallpaper

set Bing Wallpaper of the Day as your Linux Desktop's wallpaper
109 stars 49 forks source link

Allow changing output directory #33

Closed petrmanek closed 3 years ago

petrmanek commented 5 years ago

Hello!

This PR includes two simple features:

  1. The directory, where wallpapers are saved, is editable using the -dir flag. Default behavior falls back to the same path that was hard-coded until now.
  2. New environment none was introduced, which does nothing when wallpapers are downloaded. This is useful if you have your own wallpaper selection logic outside the script and don't want the default feh behavior.
marguerite commented 5 years ago

@petrmanek

golang uses CamelCase variable, so “default_dir” should be defaultDir. or you can skip the variable definition, just putting those stuff in flag parameter

I saw you defined a “none” case in setWallpaper(), but the env it takes comes from desktopEnv(), which by default returns “”.

So you have no way to pass the “none” env into setWallpaper()

petrmanek commented 5 years ago

Thanks for the review. Since I'm a n00b in golang, it's always well-appreciated! :)

ad 1) the variable name was corrected in 1e406cdc3b79063ee9ae4606a96c27e56854fc18

ad 2) you are of course right, but as far as I can see that holds only if the -env flag is not provided at all (I'm assuming that line #461 will not overwrite env if it has been previously set)

My use case is that the I want the script to fill a directory with pictures. I've got my own logic that determines which picture is set later on. It is therefore useful to have the capability to suppress the wallpaper-setting part while preserving the picture-downloading part. That's also, why I've made an effort to preserve the default behavior if -env has not been provided at all.

marguerite commented 3 years ago

those features have been implemented.

  1. you can pass “—desktop=none” from cli
  2. you can pass “—dir=“ for wallpaper storage directory from cli

also there’s a config.yaml which you can play with and place in /home//.config/linux-bing-wallpaper/config.yaml