Add configuration file support along with other features

NautiluX / slide

Simple slideshow showing random images from specified directory

MIT License

105 stars 19 forks source link

Add configuration file support along with other features #46

Closed alfred-reynolds closed 3 years ago

alfred-reynolds commented 3 years ago

This PR adds support for loading a JSON formatted configuration file.

Moved app configuration state into a custom AppConfig struct
Added scheduler logic to let you change between image paths displayed based on the time
Added ability to override display settings per folder when loading images (from previous PR)
Added on the fly config file reloading (read when a new image is scheduled to load)
Restructured object ownership out of the main() func to allow for on the fly reload and better encapsulation
Added Log() function to encapsulate debug logging logic so we can pull the debugMode member from most classes

NautiluX commented 3 years ago

looks like you went crazy over the past weeks! Thanks for this PR, on a first glance it looks like viable features, but it'll take me some time to go over it. For future, if you develop things in branches and keep them up-to-date with the main branch of this repo, you can separate features more easily in PRs, that'll make the review process easier (and quicker)

alfred-reynolds commented 3 years ago

I do actually have multiple branches, it just turned out each new feature built heavily on the last so they had to roll together :)

NautiluX commented 3 years ago

Hi @alfred-reynolds,

sorry to get back to you so late, can you please try to refactor the code a bit to keep the downloading part encapsulated and separated from the image loop logic? Also if you could somehow separate this PR by features that would make it so much easier for me to review. Like: PR 1: Adding folder configuration support PR 2: Adding reddit support PR 3: Bug fixes for image stretching issues

Unfortunately we don't have any tests in place for this project so merging this huge PR makes me a bit nervous.

alfred-reynolds commented 3 years ago

Hey @NautiluX , I've gone and removed the reddit/download code. It wasn't fit for purpose due to the complexity of parsing the rss feeds from reddit and other sites. The image stretching changes are only a couple of lines of changes, the pain from the git dance to apply them against the older branch and reapplying upstream seems like a bad time tradeoff.