[haiku-commits] Re: haiku: hrev46671 - src/apps/webpositive

  • From: Adrien Destugues <pulkomandy@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Tue, 14 Jan 2014 09:50:15 +0100

> --- a/src/apps/webpositive/SettingsWindow.cpp
> +++ b/src/apps/webpositive/SettingsWindow.cpp
> @@ -245,8 +245,14 @@ SettingsWindow::_CreateGeneralPage(float spacing)
>          new BMessage(MSG_SEARCH_PAGE_CHANGED));
>      fSearchPageControl->SetModificationMessage(
>          new BMessage(MSG_SEARCH_PAGE_CHANGED));
> -    fSearchPageControl->SetText(
> -        fSettings->GetValue(kSettingsKeySearchPageURL, 
> kDefaultSearchPageURL));
> +    BString searchURL = fSettings->GetValue(kSettingsKeySearchPageURL,
> +        kDefaultSearchPageURL);
> +    if (searchURL == "http://www.google.com";) {
> +        // Migrate old settings files.
> +        searchURL = kDefaultSearchPageURL;
> +        fSettings->SetValue(kSettingsKeySearchPageURL, 
> kDefaultSearchPageURL);
> +    }
> +    fSearchPageControl->SetText(searchURL);

This way of migrating the settings isn't optimal.

I think we should do this:
- If the setting is the same as the default value, don't save it. This will 
need an UnsetValue in SettingsMessage.
- This way, the setting is reset to the default value thanks to the existing 
support for that in GetValue.

To avoid manual checking, we could add a default value parameter to SetValue, 
and have the check happen there (if the new value is the same as the default 
value, remove the entry from the settings message instead of setting it). Or 
we could have a way to filter the SettingsMessage against default values on 
saving. This would require it to store or get a "default state" message, 
which could be useful for implementing the "Default" button, anyway.

WebPositive uses a custom SettingsMessage for this. Don't we have a shared 
class for this anyway?

-- 
Adrien.

Other related posts: