[Feature Request] Consolidation of runtime variables inside JSON config. file

[RogueScholar]

First of all, congratulations on sustaining this great project that I've been happily using cross-platform for...something like two years already, I think. (Is that even possible? This getting older really is for the birds.) This is a magnificent example of the original UNIX philosophy made manifest: doing one tightly confined behavior quickly and reliably. That said, I have for awhile now been quietly hoping for a little more refinement in the mechanisms for users to modify the program's default behaviors that I'd like to review with you, if I may. Some of this is based off of implicit assumptions I've made due to small gaps in the documentation and if those are in error, please do correct me.

• From what I see in the README and believe is borne out in the codebase (insert grain of salt disclaimer here; I've worked with many programming languages over the years but never Rust), the only pathway to set the durations of the activity timers is through arguments added to the invocation command at runtime. Would you be open to considering allowing those to also be read from the JSON config file used to store the credentials for Wallhaven? Just now I had the impulse to tinker with mine in a Windows environment and ended up on something of a (small) wild goose chase. I started at the config file, then (having forgotten the mechanism at work for running at startup) went looking for a shortcut in the "%APPDATA%\Microsoft\Windows\Start Menu\Programs\Startup\", and only when coming up empty there made my way to the registry in HKCU\SOFTWARE\Microsoft\Windows\CurrentVersion\Run, found the Value in question there and modified its data to match my new desired behavior. I think my initial expectation that such choices would be stored in the config file was essentially reasonable for a user to make and adding the possibility for the values to be read from there would save others this confusion and make the program more convenient to manage.
• As regards the configuration file itself, would it be possible to define a path for it using an environment variable? %WPC_CONFIG% (Windows)/$WPC_CONFIG (Unix-like), perhaps? I keep the wpc.exe binary in a catch-all folder added to %PATH% for various and sundry executables that don't install to their own preferred location in the filesystem and in the interests of keeping things orderly would prefer not to also have the JSON config file floating around there in a directory that otherwise only contains binaries.
• I'd like to echo the request for clarity about the functions of the two activity timers that was made in Clarify meaning of timers #6, since the Usage output on the command line and likewise the README are both still rather vague on the subject. I'd deduced their respective functions back during the initial setup years ago but not thought to make a note of it anywhere at the time, leading to a duplication of that effort again today. Perhaps the choice of more explicit names such as --fetch or --retrieve for the timer controlling the retrieval of new images from sources and --change or --switch for the one controlling the updating of the wallpaper on the desktop itself (--update would work too once vacated by the other timer, though that would introduce a breaking change in behavior for current users)? Doing that and adding a few words to their description in the Usage printout/README would likely eliminate any future confusion for anyone.
• Could you include a brief explanation of the behind-the-scenes machinations of the -S/--startup flag somewhere? In its current state it feels ominously permanent since there's no clear companion mechanism to deactivate it once it has been triggered. I don't know that such a mechanism is called for here (though some might make use of it), but it does seem reasonable to leave something of a "sign" for users who may change their mind after having set it. (This one is just me wearing my SE hat and not anything that I personally have a need for; I'm quite happy with WPC and it being started in the background by the system for me.)

Everything else I could mention just iterates on these four (the ambiguity in the --maxage flag's function, for instance) so I think this is a good place to end. As mentioned above, I don't have any familiarity with Rust or I would naturally present this in the form of a PR for you to review. However, if you'd like to collaborate on any of these, I'd be amenable to making the updates to the README as well as the Usage printout so long as you could supply a brief primer on the Rust rubrics to be mindful of and which file(s) required modification. Thanks again for all the work you put in on this to date; wpc is always my go-to answer when someone asks me for a desktop wallpaper changer suggestion.

  —Peter

[jkotra]

Thanks for your kind words and useful suggestions. I will try to implement these suggestion when i get some time.

[RogueScholar]

Just wanted to say that I've been following your progress and it's coming along splendidly; you've already managed to knock nearly all the items off my list.

I did want to pass along one minor glitch that I observed, though. After you added the --wallhaven-config flag, rather than go through the usual workflow of disabling the startup entry via the command line and re-enabling it, instead I manually edited the invocation command in the registry to include it, passing the original config file that WPC had generated as the value—shame on me for always looking for a shortcut. Peculiarly, that caused the program to error out. When I duplicated the circumstances by hand in the terminal, it responded with messages that it wasn't finding the key names that it was looking for in the JSON file. The key names from the file it had generated all had wh_ prefixed to them, however it reported that it was looking for the same names without the prefix, i.e. username and not wh_username. I just edited the config file by hand as well and after stripping the prefixes everything was sorted. I have no idea if that was simply a matter of the config file having been generated before you'd done any of the recent work, or if there is in fact a mismatch in the code between what is written to the file and what it reads at runtime. I skimmed the codebase and couldn't tell for sure one way or the other (I did see some lingering references to the key names with the wh_ prefix, but they might've been testcases or no-ops. I wasn't joking when I said I was Rust-illiterate, LOL…), so I figured I'd just apprise you of the behavior and leave the matter in your capable hands.

Lastly, I'm ashamed to admit, there is one last item on my wishlist that I wish I'd included originally. I only use WPC with Wallhaven as that's the community I'm active in, and I confess that everytime I scroll through the README file here, I feel a tiny pang of jealousy when I see the config flags for the minimum width and height dimensions that are available to users that go the other direction and pull their wallpapers from Reddit. If you felt inclined to add duplicating those options to also work with Wallhaven (ideally as just a couple more key/value pairs in the config file), that would truly be aces. It feels silly to gripe about how prolific the portrait orientation backgrounds have become with mobile devices having truly changed the world in recent years, and looking back I wish that I'd separated my collections in Wallhaven to contain only a single orientation within each collection, but I didn't and now I have collections with 1k+ backgrounds all mixed together. Hopefully this isn't a huge ask as I'm pretty sure Wallhaven does provide the XY dimensions through the API, but if not, feel free to disregard this. I'm a very happy camper just with what's been done so far; it's as reliable as ever and finally my wallpapers and the config file are all nicely tucked away where I expect to find them. Everything "Just Works.™"

Best wishes for a delightful and prosperous new year to you and yours. 😺

[jkotra]

I guess there's a case to be made that dimension filter should be global, just like grayscale filter.

will look into it, thanks for your valuable suggestions.

[jkotra]

in progress: #22