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

Sat Sep 20 11:04:22 UTC 2014

> On Sept. 20, 2014, 8:20 vorm., 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
> 
> René J.V. Bertin wrote:
>     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.

> But than I'll also keep from changing the Darwin references to APPLE in the CMake files.

This is indeed nothing that belongs into this patch, but may still be a required patch in general (to permit Darwin + X11 in future versions) that Apple users might be interested in.

- Thomas

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

On Sept. 20, 2014, 10:54 vorm., 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, 10:54 vorm.)
> 
> 
> 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).
> 
> 
> File Attachments
> ----------------
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20140920/0dcd137f/attachment-0001.html>