Review Request 120287: [OS X] make kde-workspace build

[René J.V. Bertin]

> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > kcontrol/krdb/krdb.cpp, line 56
> > <https://git.reviewboard.kde.org/r/120287/diff/3/?file=313628#file313628line56>
> >
> >     added newline
> 
> René J.V. Bertin wrote:
>     Ah, the famous German rigor ... is this really an issue, shouldn't that empty line have been there in the 1st place?!
> 
> Martin Gräßlin wrote:
>     yes of course. It's a change which has nothing to do with the overall change. It's not atomic any more.
> 
> Thomas Lübking wrote:
>     > shouldn't that empty line have been there in the 1st place?!
>     
>     c++98 newline @ EOF, later versions don't, ie. it's not required to add that line despite you may get warnings about it (and actually kate has an option to automatically add it on saving)
>     
>     Do you use xcode?
> 
> Thomas Lübking wrote:
>     c++98 *required* newline @ EOF ...
> 
> René J.V. Bertin wrote:
>     But this is not @EOF, it's a newline inserted to detach the 1st function from the last #include statement (or rather, the last conditional #ifdef Q_WS_X11 include).
>     
>     Given that that's an X11 conditional include, I don't think that cosmetic change (adding a newline) is completely irrelevant either.
>     
>     @Thomas: yes, I use Xcode, but not here. CMake can generate Xcode projects, but that only works for relatively simple ones, so it's not even feasible to use Xcode. Most of the time I use KDevelop (git/kde4-legacy) because of the access to the documentation it gives. I can give less guarantee that I didn't edit the file in vi at some point, though ;)
>     
>     [OT]: there's a Google plugin for Xcode that removes white space from empty lines and I think also the final newline. And I do use Xcode for my OS X Keychain work from time to time, because KDevelop can't access the native SDK documentation.[/OT]
> 
> Thomas Lübking wrote:
>     Indeed - then this change is completely unrelated and pointless.
>     
>     Yes, there probably *should* have been a newline in the first place, but it's actually irrelevant whether there is and such changes don't belong into this kind of patch.
>     
>     > I don't think that cosmetic change (adding a newline) is completely irrelevant either.
>     
>     The point is that this is a purely cosmetic change, unrelated to the rest of your patch.
>     You are right, that one would demand a newline in *new code* here, but it doesn't belong in your patch.
>     
>     If you wish to tidy up aged code for KF5, you're very welcome =)
>     
>     Here's for vim:
>     https://techbase.kde.org/Policies/Kdelibs_Coding_Style#Vim
>     
>     For kate, "configure editor..."
>     /"Appearance"
>        o) Highlight Tabs
>        o) Highlight trailign spaces
>        o) Show indention lines
>     /"editing"/"indention"
>        o) Spaces
>        Tab width: 4 chars
>        Indention width: 4 chars
>     /"Open/Save"
>        Remove Trailing space: on modified lines
>         ) Append newline at end of file

Well, if said German rigor has it come to that, I'll remove the newline. But than I'll also keep from changing the Darwin references to APPLE in the CMake files.


- René J.V.


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


On Sept. 20, 2014, 12:05 a.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> -----------------------------------------------------------
> 
> (Updated Sept. 20, 2014, 12:05 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> A few rather straightforward patches to make the relevant bits of KDE4's kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the various relevant KDE settings among which desktop search, fonts, colours and even style.
> The oxygen style builds and looks good but shows some updating glitches due to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140920/270b9b1a/attachment.htm>