Skip to content

All: Deprecate legacy configuration setting (breaking changes)

HostGrady requested to merge HostGrady/fix/replace-legacy-boilerplate into master

Fixing dmscripts biggest pain point

Prelude

This is a MAJOR change but it is breaking.

Features

Now, I say "breaking" but really the entire thing was broken to begin with. What this change does is replace all of the old boilerplate with the new boilerplate. Among the boilerplate changes includes a simple notification instead of completely breaking the scripts by forcing users to read diff files.

All important configuration error messages are in the error log, as defined in DM_CONFIG_DIFF_LOGFILE (default is stderr) and in a notification with notify-send. You can use DM_SHUTUP="any nonempty value" to toggle notifications off, the log file is always written to.

Breaking changes

Due to the new boilerplate, an unintended side affect is that any program depending on a list doesn't work. This is because the way the configuration was sourced earlier was by getting the file name and then sourcing it that way where as the way the new syntax works is that it sources.

Note that this only affects things created with declare. This is a super strange quirk of bash programming and apparently it is a feature and not a bug. Adding the 'g' option fixes any issues but of course we didn't do that. As a safety measure, any programs which depend on lists have an additional check to see if the list is defined and non-empty and if it's non-empty it will display an error message to the user explaining what they need to do.

In practice, this change negatively affects like 5 script total with the biggest one being dm-confedit. The only difference you will find for the vast majority of scripts is you get a notification and it sleeps for a second before running dmenu which I think is significantly less intrusive and ultimately worth it. The change also probably reduced the overall size of the project quite considerably.

Next steps

Currently none of the functions in _dm-helper.sh were removed or changed in any way. This is because some scripts still rely on legacy features from the boilerplate. In practice I think it's 1 or 2 but it's still significant. In addition, some of the scripts suck because they still don't have proper "main" methods. I am willing to address these concerns but on a different day.

Additional tidbits

I haven't tested rofi but like, fzf is super broken. Using the default fzf in dmscripts is almost unusable for some scripts (such as dm-weather). It also completely breaks some scripts for some reason. This has been partially addressed but a full revaluation of fzf needs to be done (an issue will be made following the merge request).

In addition, for some ungodly reason, dm-hub just does not work with dm-templates. I have no idea why but it crashes every time. Reviewing all the scripts definitely has revealed some subtle bugs and some more refactoring probably needs to be done.

UPDATE:

Unfortunately, after doing some digging, it appears as though the security measures I tried to implement haven't worked in the way that I thought. Because /etc/dmscripts/config always gets written to, it means that there will always be a list with the -g option as a result my warnings never trigger. This means that if the user's arrays in ~/.config/dmscripts/config do not include the -g option it will just silently ignore any entries in that file which obviously is not the desired result.

The notification will still be sent and people should still have the clue to diff the files but the actual breaking behaviour is now significantly more subtle. I don't have write access but I recommend adding a post_install hook to the dmscripts-git package and mention that arrays now need to include the -g flag. The code should probably be removed in a future dmscripts release

Edited by HostGrady

Merge request reports