Skip to content

Fix saving configurations after deleting some entries#2109

Merged
f4exb merged 1 commit intof4exb:masterfrom
dforsi:fix/settings-save
May 11, 2024
Merged

Fix saving configurations after deleting some entries#2109
f4exb merged 1 commit intof4exb:masterfrom
dforsi:fix/settings-save

Conversation

@dforsi
Copy link
Contributor

@dforsi dforsi commented May 11, 2024

This PR always deletes all configurations except "General" from the configuration file, otherwise when deleting at least one entry from a group the previous last entry wouldn't be deleted (at least on Linux which uses a .conf file with the ini format). For example if you had configuration-1 and configuration-2 and you deleted one of them (it doesn't matter which one) the old configuration-2 wouldn't get deleted.

At this time the only group that doesn't end with a number is "General" and those which end with a number are:
$ grep startsWith sdrbase/settings/mainsettings.cpp
if (groups[i].startsWith("preset"))
else if (groups[i].startsWith("command"))
else if (groups[i].startsWith("featureset"))
else if (groups[i].startsWith("pluginpreset"))
else if (groups[i].startsWith("configuration"))

The simplest patch is to keep only "General" and delete everything else.
The pros are:

  • we don't forget to add groups to delete
  • if entries gets deprecated in future, they will be automatically deleted

The cons are:

  • if entries gets deprecated in future, they will be automatically deleted :) possibly without informing the user
  • it prevents external programs to add their own entries (not sure if it's a valid use case, though)

A more complicated approach would be storing a counter of how many entries were read from the configuration and delete only those exceeding the counter.

@f4exb f4exb merged commit 75afe41 into f4exb:master May 11, 2024
@dforsi dforsi deleted the fix/settings-save branch May 16, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants