Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
312 stars 134 forks source link

brewkenPersistentSettings.conf? #694

Closed mikfire closed 1 year ago

mikfire commented 1 year ago

I think a merge may have missed something.

ls -ltr ~/.config/brewtarget
total 356
drwxr-xr-x 2 me me        2 Apr 22  2021 obsolete/
-rw-r--r-- 1 me me        0 Apr 22  2021 brewtarget_log.txt
-rw-r--r-- 1 me me     4584 Mar 31  2022 brewtarget-debug.conf
-rw-r--r-- 1 me me   4584 Sep 23 15:20 brewtarget.conf
-rw-r----- 1 me me   714752 Dec 29 10:24 database.sqlite
-rw-r--r-- 1 me me 146680 Dec 29 10:26 brewtarget.log
-rw-r--r-- 1 me me   4088 Dec 29 10:26 brewkenPersistentSettings.conf
matty0ung commented 1 year ago

Took me a minute to twig this one. (Just finished a brew day, so not 100% on the ball!) It's a one-line fix. I'll include it in the Hop fix.

mikfire commented 1 year ago

Is there any plan for converting the current "brewtarget.conf" over to "brewtargetPersistentSettings.conf"?

As it works now, it just created a new configuration file and ignored all my previous settings for the database type, what formula I want to use to calculate IBU, which fields I want displayed in US customary and which in SI (yes, welcome to the USA; I prefer SI but my homebrew supply store only uses US), etc.

I would prefer if we did something like this:

  1. Test to see if brewtargetPersistentSettings.conf exists;
  2. If it does, open it and we are all happy;
  3. If it doesn't, test to see if brewtarget.conf exists;
  4. If it does, open it and then save it as brewtargetPersistentSettings.conf;
  5. If it doesn't, then create a default brewtargetPersistentSettings.conf Please notice the intentional lack of any statement about deleting the brewtarget.conf file. I do not like deleting user's files, as it always ends in heart ache.

Failing that, you at least need to pop a dialog and warn the user what you are doing and suggest how they can get their old settings back. Again, this would probably need to at least test for the existence of both files and you have done 40% of the better solution.

matty0ung commented 1 year ago

Ah, hum, yes, you are entirely right!

matty0ung commented 1 year ago

I think your #1 - #5 would actually be relatively small work. (Famous last words, I know.) I'll have a look next year. (Sorry, obligatory new year's eve joke there.)

matty0ung commented 1 year ago

This should be fixed in the 3.0.6 release.