Review Request: Only save HomeUrl config value when it actually changed (alias included)

[Frank Reininghaus]

> On Oct. 31, 2012, 12:53 p.m., Frank Reininghaus wrote:
> > Thanks for the patch! Unfortunately, it fixes only a part of the problem. Consider the case that you change the Home URL to something different from ~, click OK and close Dolphin (such that a HomeUrl entry is generated), and then change the URL back to ~. In that case, KConfig will still see that the Home URL is different from the default value (QDir::homePath()), and still generate a HomeUrl entry with the user's URL. It would actually be expected that the HomeUrl entry is removed from dolphinrc.
> > 
> > I think the best way to achieve this is to just change the code for the default value in dolphin_generalsettings.kcfg in such a way that a Home URL which is reverted to ~ is recognised as the default value. I.e., replace QDir::homePath() by KUrl(QDir::homePath()).prettyUrl(). I'll take care of that.
> > 
> > > I would expect the cancel button to allow me to go back to the state it was before i pressed apply. But lets not discuss that in here. If you want to discuss that, please do open a thread > on kfm-devel.
> > 
> > Hm, as far as I can see, *you* are the one who started a discussion about this issue. Please don't ask others to open threads on a mailing list if *you* want to make Dolphin to behave different from every other KDE app. We obviously want to be consistent with other parts of KDE.
> 
> Mark Gaiser wrote:
>     Are you sure about that?
>     I just tried your steps. Just to make sure, this is what i did:
>     1. Open dolphinrc and remove the HomeUrl
>     2. Open dolphin and set the "Location:" to something that is not my home url
>     3. dolphinrc now has that value in HomeUrl
>     4. Open dolphinrc and change the HomeUrl to ~
>     5. Start dolphin, go to settings and press OK (without changing the "Location:")
>     6. Open dolphinrc and observe the HomeUrl. It's still ~, not file:///home/whatever
>     
>     To me that looks like the case where a ~ was translated to file:///home/whatever is fixed and isn't translated anymore if not needed.
>     The only thing i can still find is when you change your "Location:" to anything that's not the current location and then change it back to "~" through the "Location:" field. Then it is translated to file:///home/whatever, but that is to be expected if you ask me.. Or do you expect that if you put in a "~" in that field that you end up with exactly that in the HomeUrl as well? If that's the case then i can fix that as well.
>     
>     As for the cancel button. I mentioned that because it influences how i created the patch.
> 
> Frank Reininghaus wrote:
>     > 4. Open dolphinrc and change the HomeUrl to ~
>     
>     No, that's not what I did, see below.
>     
>     > The only thing i can still find is when you change your "Location:" to anything that's not the current location and then change it back to "~" through the "Location:" field.
>     
>     Yes, that's exactly what I tried.
>     
>     > Or do you expect that if you put in a "~" in that field that you end up with exactly that in the HomeUrl as well?
>     
>     No. As I said: I expect that if the 'location' is equal to the default value (i.e., QDir::homePath()), no matter if you don't change the default, or you enter '~', or you enter the absolute path, there should be no HomeUrl in dolphinrc. This is how it's usually done: you only store those settings that are different from the default.
> 
> Mark Gaiser wrote:
>     Ahh gotcha :)
>     Working on fixing that.
> 
> Mark Gaiser wrote:
>     Ehm right..
>     What i'm "trying" to do now is figure out how i can delete a config line if a default one is entered. Detecting is the default one is entered isn't difficult and works, but how do i delete a config line? IT all seems to be done through the "GeneralSettings" class, but i can't find that class anywhere! What is going on with that class? Where is it?

I said already in my first comment here that I will take care of it (when I'm sitting in front of my home computer with some spare time) and how I am planning to fix it. Sometimes, reading carefully what other people write really does save you some time - you end up reinventing the wheel way to many times if you don't ;-)


- Frank


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107142/#review21220
-----------------------------------------------------------


On Oct. 31, 2012, 12:10 a.m., Mark Gaiser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107142/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2012, 12:10 a.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Check if the provided HomeUrl from dolphinrc is different from the url the location field in the dolphin settings window.
> I'm simply putting both values in a KUrl and only updating HomeUrl if the value is actually different. KUrl translates ~ to the users home folder so "~" and "file:///home/user" will actually match. This allows the tilde sign to be used as HomeUrl in dolphinrc. Previously it would be translated to "file:///home/user" as soon as you opened the dolphin settings window.
> 
> What i wanted to do was a bit more complicated. I wanted make sure that only changed values where saved and written to the config file. However, that's not how dolphin currently works. For instance, if you change a setting (pick twin view as an example) and press the apply button it is actually written to the file and makes the cancel button completely useless! Then the dolphin components that need updating (the view container in this case) are updated on the fly based on the new config. I can't change this without changing a lot more files in dolphin that go way outside the scope of this bug. So i left it the way it is and just made an additional check to fix this bug.
> 
> 
> This addresses bug 308569.
>     http://bugs.kde.org/show_bug.cgi?id=308569
> 
> 
> Diffs
> -----
> 
>   dolphin/src/settings/startup/startupsettingspage.cpp 633cdac 
> 
> Diff: http://git.reviewboard.kde.org/r/107142/diff/
> 
> 
> Testing
> -------
> 
> Set the HomeUrl in dolphinrc to "~" and open the dolphin settings. Press OK. HomeUrl still has "~" which is as it should be.
> Also tried other settings and they all applied just fine. The HomeUrl only gets changed when it really changed.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20121031/038ed6a0/attachment.htm>