Review Request 129363: reliability fixes (WIP)

[René J.V. Bertin]

> On Nov. 14, 2016, 5:56 a.m., Yichao Yu wrote:
> > LGTM in general other than the few comments (and the copy of them in the other files). IIUC, the conditions can be simply deleted instead of commenting out too.

Yeah, I didn't mention that, but evidently the idea was always to remove all the commented code. I've just kept it for now so I don't have to study the patchfile to see what I changed.


> On Nov. 14, 2016, 5:56 a.m., Yichao Yu wrote:
> > qt4/config/qtcurveconfig.cpp, line 3459
> > <https://git.reviewboard.kde.org/r/129363/diff/3/?file=484406#file484406line3459>
> >
> >     Won't this set `titlebarButtons_*` twice?

I'm not sure what you're afraid of here and why only for the Qt4 version. As far as I can see each of those `titlebarButtons_` members have 3 "declinations": the button colour (e.g. `titlebarButtons_colorClose`), the colour of the active symbol shown in the button (e.g. `titlebarButtons_colorCloseIcon`) and the colour of the inactive symbol (e.g. `titlebarButtons_colorCloseInactiveIcon`). As far as I can tell each is set only once in that function. Of course every setting is set at least twice, once to the built-in default and once to the user's choice, but that's not new.

The lines on which you commented start blocks that introduce a new UI feature: the colour pickers are made inactive when the theme is set to use its global active/inactive colours for the button symbols. It's a pity that the picker buttons don't show the colour that will be used in that case, but I still find it preferably to being able to change those colours and not seeing the change reflected.

Sorry if leaving the commented code confused you here!


- René J.V.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129363/#review100825
-----------------------------------------------------------


On Nov. 8, 2016, 7:56 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129363/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2016, 7:56 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and Yichao Yu.
> 
> 
> Repository: qtcurve
> 
> 
> Description
> -------
> 
> I've been making a number of fixes to QtCurve recently, mostly to improve the reliability of its settings management. The current set of fixes is getting a bit complex and thus something I'd rather put up for review while I work on it.
> 
> The leitmotiv is: read and write all settings all the time, even if you don't use everything. This prevents settings getting lost.
> 
> This approach is all the more important since I've restored the support for a system-wide stylerc file. For now that's a MacPorts patch, but I'd be happy to include it here if it's considered of general use.
> 
> 
> Diffs
> -----
> 
>   .reviewboardrc PRE-CREATION 
>   qt4/common/config_file.cpp 7a0a6bc 
>   qt4/config/qtcurveconfig.cpp 740150f 
>   qt4/style/qtcurve.cpp 5447e07 
>   qt5/common/config_file.cpp 7630f4e 
>   qt5/config/qtcurveconfig.cpp df8ce89 
>   qt5/style/qtcurve.cpp 4457def 
> 
> Diff: https://git.reviewboard.kde.org/r/129363/diff/
> 
> 
> Testing
> -------
> 
> Being tested on Mac and Linux with QtCurve the main style.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20161114/ffda6ab3/attachment.html>