Install script not cross-platform compatible

[gjgress]

In the install script, it checks if python3-pyqt5 (among other things) is on the system, and requests to install them if they are not.

Of course, on non-Debian platforms, apt does not work, so it cannot possibly install on the operating system.

There is another issue, which is that on Arch Linux, python3-pyqt5 actually goes by python-pyqt5-- so even if the dependency is met, the installer can't find it.

If you respond with 'n', the install script stops, which means that a non-Debian user won't be able to install the program. I argue that the install script should allow the user to continue the installation even if dependencies aren't met-- let the python program throw an error if issues arise.

Of course, the easy workaround for non-Debian users is just to respond with 'y'. The script attempts to install with apt, fails, but carries on (which I would guess is unintended behavior). If you do meet the dependency, the rest of the install script won't have any issues.

[gjgress]

I was cleaning up the scripts and stuff from messing around with this and... I noticed that you use sudo for things in the install script (this meant I could not remove it as a user)? I'm no expert but, this seems like bad form... If your install directory is in ~/.local/bin , then you do not need sudo to copy over things, and it will really screw with permissions. If there is a reason for it I don't understand, let me know, otherwise I can create a PR to make those changes.

In fact, this will also fix the issues other users have had where the install script fails to uninstall. Because the rm in the uninstall script uses rm, it will fail because it doesn't have sufficient permissions. So this change will also fix the uninstall script.

[gjgress]

One last thing-- if the user runs the install script as a user, perhaps it would be better if the config was written to ~/.config/video-wallpaper/settings.conf instead of inside the scriptdir folder. If the user runs it with sudo, then the config could be written to /etc/video-wallpaper/settings.conf. This conforms to Filesystem Hierarchy Standard conventions (kinda).

Once again, happy to open a PR if that behavior is desired :)

[SwallowYourDreams]

Hey there,

thanks a lot for your input! I've used it to include several fixes and additions to my fork (started a pull request for this one just now). The script will now

• no longer continue after a failed apt install attempt.
• no longer do the local install with superuser privileges (this was a remnant from earlier system-wide installs)

As for unfulfilled dependencies: I don't think it's wise to have the installer continue by default even though dependencies are not fulfilled. This might frustrate users unfamiliar with the concept of dependencies who'd then try to run the script and not understand why it crashes.

Instead, I've added a "--distro-agnostic" flag to the installer. This will allow you to skip dependency checking for all distro-specific packages and thus avoid all issues arising from different package managers or package names.

As for the config location, I'll look into it once I find the time. If you wish to solve this yourself and open a pull request, I'll be happy to merge it.

[gjgress]

Awesome. Glad my feedback was useful.

I think the --distro-agnostic flag should be sufficient. My thought process was to prompt the user (with default No), tell them the script thinks dependencies are not met, and ask if they want to continue knowing the risks. Then again, I suppose new users often skim warning messages like these...

Glad the sudo changes were made. Config stuff is not too important, certainly a QoL change. But in general putting very general config files (like "settings.conf") in a directory like /bin where many things can be, can be very confusing and at its worst cause conflicts. I guess I don't know any other program that does that but, still.

With PR #14 now existing and adding these changes, I'll close this thread with comment.

[SwallowYourDreams]

The feedback was useful indeed!

I've looked into your suggestion of moving settings.conf from /home/$USER/.local/bin to /home/$USER/.config/video-wallpaper. It's much more logical that way. Thanks once more!