Review Request 120624: add gtkbreeze, kconf_update tool to set gtk settings on first login

[Sebastian Kügler]

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


Found a bunch of issues around string handling, error handling and compiler definitions.


misc/gtkbreeze/CMakeLists.txt
<https://git.reviewboard.kde.org/r/120624/#comment48061>

    This (and a few other includes here) don't seem necessary. Could be cleaned up a tad.



misc/gtkbreeze/CMakeLists.txt
<https://git.reviewboard.kde.org/r/120624/#comment48049>

    Is this really needed here? I think we can just make the code work with these flags set, and be happier about it.
    
    Why did you add those?



misc/gtkbreeze/CMakeLists.txt
<https://git.reviewboard.kde.org/r/120624/#comment48062>

    Might aswell just put main.cpp here and remove the additional temporary var gtkbreeze_SRCS. It's only one source file, anyway.



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48050>

    foreach (
    
    missing space



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48048>

    "/" should be a QDir::Separator, this will make it much faster since it can skip the more expensive QString ctor. Also, inconsistent spacing on that line.



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48051>

    QDir::separator() instead of "/"



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48052>

    QStringLiteral("oxygen-gtk") is faster



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48053>

    exists (grammar)



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48054>

    Use QStringLiteral here and on the following line for faster string creation



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48055>

    no spacing inside parentheses



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48056>

    Error handling when opening the file fails?



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48057>

    Wrapping all the strings on this and the following lines in QStringLiterals will make creation of all those strings much faster. (And removes the need for non-standard compiler flags.)



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48058>

    Superfluous, the QFile object goes out of scope here anyway, and will be closed in the dtor.



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48046>

    const



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48047>

    const



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48059>

    const



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48045>

    I don't understand this. Does our startkde set the Ubuntu font? (I'm pretty sure that it's not.)
    
    Can you clarify, or perhaps explain how the Oxygen font is set here?



misc/gtkbreeze/main.cpp
<https://git.reviewboard.kde.org/r/120624/#comment48060>

    Error handling here could be improved. I don't now exactly what makes sense here (return !0 if no settings have changed, if settings files aren't writable, perhaps?).


- Sebastian Kügler


On Oct. 19, 2014, 3:15 p.m., Jonathan Riddell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120624/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2014, 3:15 p.m.)
> 
> 
> Review request for Plasma and Hugo Pereira Da Costa.
> 
> 
> Repository: breeze
> 
> 
> Description
> -------
> 
> add gtkbreeze, kconf_update tool to set gtk settings on first login
> this checks if gtk settings are already set, if they are not or are set to oxygen they update them, else it quits
> it does this for both gtk 2 and 3
> it sets gtk to the orion theme because it's available for both gtk 2 and 3 and it looks similar to breeze
> it sets the icons to oxygen because I could not get breeze icons to work with gtk 2 or 3
> I also could not get icons to show on buttons or in menus in gtk 3
> 
> 
> Diffs
> -----
> 
>   misc/CMakeLists.txt ff891a9 
>   misc/gtkbreeze/CMakeLists.txt PRE-CREATION 
>   misc/gtkbreeze/gtkbreeze.upd PRE-CREATION 
>   misc/gtkbreeze/main.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/120624/diff/
> 
> 
> Testing
> -------
> 
> run it and run gtk-demo and gtk3-demo
> 
> 
> Thanks,
> 
> Jonathan Riddell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20141020/b58687be/attachment-0001.html>