D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

2018-07-31 Thread Alex Richardson 
arichardson added a comment.


  +1 LGTM. Thanks!

REVISION DETAIL
  https://phabricator.kde.org/D14440

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

2018-07-28 Thread Alex Richardson 
arichardson added a comment.


  Does this mean for Qt < 5.12 (or .13?) pressing open in kwrite will select 
the cwd again?
  
  I now have almost all my source directories NFS mounted rather than having to 
use sftp so it is no longer such a big issue for me. However, I do wonder if it 
makes sense to keep the current behaviour for older Qt versions? I.e. something 
like `!m_directorySet || QT_VERSION < 5.12`?

REVISION DETAIL
  https://phabricator.kde.org/D14440

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2018-04-08 Thread Alex Richardson 
arichardson added a comment.


  In D4193#242444 , @fvogt wrote:
  
  > What about `Plasma/5.12`? It has a minimum of Qt 5.9 as well.
  
  
  Good point. Any objections against me cherry-picking it to the 5.12 branch?

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D4193

To: arichardson, #plasma, elvisangelaccio
Cc: fvogt, ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2018-04-08 Thread Alex Richardson 
This revision was automatically updated to reflect the committed changes.
Closed by commit R135:bfd41a95530f: KDEPlatformFileDialog: Fix initial 
directory selection for remote files (authored by arichardson).

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4193?vs=31284=31656

REVISION DETAIL
  https://phabricator.kde.org/D4193

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp

To: arichardson, #plasma, elvisangelaccio
Cc: ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, plasma-devel, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2018-04-04 Thread Alex Richardson 
arichardson updated this revision to Diff 31284.
arichardson edited the summary of this revision.
arichardson added a comment.


  - Removed version check since we now depend on Qt 5.9
  - Updated commit message

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4193?vs=21216=31284

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D4193

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp

To: arichardson, #plasma, elvisangelaccio
Cc: ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, plasma-devel, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2018-03-31 Thread Alex Richardson 
arichardson added a comment.


  I'll update and test this again when I get back to my work computer on 
Tuesday. Should I just remove the code or add a comment that since qt 5.8 it is 
no longer necessary to set the directory as well?

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D4193

To: arichardson, #plasma, elvisangelaccio
Cc: ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, plasma-devel, 
ragreen, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2017-10-24 Thread Alex Richardson 
arichardson updated this revision to Diff 21216.
arichardson added a comment.


  - rebased
  - removed runtime check

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4193?vs=13383=21216

BRANCH
  arcpatch-D4193

REVISION DETAIL
  https://phabricator.kde.org/D4193

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp

To: arichardson, #plasma
Cc: ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2017-04-13 Thread Alex Richardson 
arichardson added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kdeplatformfiledialoghelper.cpp:365-370
> http://doc.qt.io/qt-5/qversionnumber.html

I don't think we can depend on that yet, can we? Also I'm not sure we really 
need that runtime check. How likely is it that someone compiles plasma 
integration against Qt 5.7 and runs it with 5.8? Is that even supported? Aren't 
we using private APIs?

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D4193

To: arichardson, #plasma
Cc: anthonyfieroni, elvisangelaccio, graesslin, plasma-devel, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2017-04-13 Thread Alex Richardson 
arichardson updated this revision to Diff 13383.

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4193?vs=10338=13383

BRANCH
  arcpatch-D4193

REVISION DETAIL
  https://phabricator.kde.org/D4193

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp

To: arichardson, #plasma
Cc: elvisangelaccio, graesslin, plasma-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2017-04-13 Thread Alex Richardson 
arichardson marked an inline comment as done.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D4193

To: arichardson, #plasma
Cc: elvisangelaccio, graesslin, plasma-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

[Differential] [Commented On] D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2017-02-22 Thread Alex Richardson 
arichardson added a comment.


  Ping?
  
  I work a lot with remote projects over sftp:// so this would be very 
important for me.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D4193

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: arichardson, #plasma
Cc: elvisangelaccio, graesslin, plasma-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

[Differential] [Commented On] D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2017-02-01 Thread Alex Richardson 
arichardson added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in kdeplatformfiledialoghelper.cpp:365-370
> > what is the easy way to do that?
> 
> There is KCoreAddons::version()

I want the version of Qt not KCoreAddons so that won't work.
Do we really need a runtime Qt version check? We link to Qt5PlatformSupport, is 
that API stable or do we need to recompile for new Qt versions anyway?

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D4193

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: arichardson, #plasma
Cc: elvisangelaccio, graesslin, plasma-devel, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas

[Differential] [Commented On] D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2017-01-23 Thread Alex Richardson 
arichardson added inline comments.

INLINE COMMENTS

> graesslin wrote in kdeplatformfiledialoghelper.cpp:365-370
> Shouldn't that be a runtime check?

Yes it should be but what is the easy way to do that? Do I have to parse the 
result of qVersion()? CMakeLists.txt says the minimum version is 5.5 so I can't 
use QVersionNumber?

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D4193

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: arichardson, #plasma
Cc: graesslin, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Request, 4 lines] D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files

2017-01-18 Thread Alex Richardson 
arichardson created this revision.
arichardson added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  We currently get the following sequence of calls:
  
  KDEPlatformFileDialogHelper::setDirectory 
QUrl("sftp://server/home/alr48/cheri/build_sdk.sh;)
  KDEPlatformFileDialogHelper::setDirectory 
QUrl("sftp://server/home/alr48/cheri/build_sdk.sh;)
  KDEPlatformFileDialogHelper::selectFile QUrl("file:///home/alex/build_sdk.sh")
  KDEPlatformFileDialogHelper::setDirectory QUrl("file:///home/alex/)
  
  Previously KDEPlatformFileDialogHelper::selectFile() would change
  options()->initialDirectory() unconditionally even if it was already
  set by the QFileDialog code. The final setDirectory() call is actually a call
  to setDirectory(options->initialDirectory()) which was set in the selectFile()
  call. It no longer seems to be required to derive initialDirectory from the
  selectFile() call and this will now to override the correct initial directory
  that was set by Qt.
  Qt should not be passing a local URL when the actual directory URL is remote
  but the code in QFileDialogPrivate::init() unconditionally sets a local URL
  until https://codereview.qt-project.org/#/c/182661/ or another fix is 
submitted.
  
  BUG: 374913

TEST PLAN
  Remote directory is now opened correctly

REPOSITORY
  R135 Integration for Qt applications in Plasma

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D4193

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: arichardson, #plasma
Cc: plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

[Differential] [Accepted] D851: Port kded plugin to json metadata

2016-01-23 Thread arichardson (Alex Richardson) 
arichardson accepted this revision.
arichardson added a reviewer: arichardson.
arichardson added a comment.
This revision is now accepted and ready to land.

Looks good to me once the JSON has been changed to use bool


INLINE COMMENTS
  kwrited.json:154 This and the following should be a boolean not a string.
  `desktoptojson -s kdedmodule.desktop -i $in -o $out` should correctly do that

REPOSITORY
  rKWRITED KWriteD

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D851

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: heikobecker, plasma-devel, arichardson
Cc: arichardson
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126740: Add a script for optimizing svgs

2016-01-21 Thread Alex Richardson 

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




optimize.svg.sh (line 1)
<https://git.reviewboard.kde.org/r/126740/#comment62489>

won't work on BSD (/usr/local/bin/bash)

/usr/bin/env bash should work everywhere



optimize.svg.sh (line 4)
<https://git.reviewboard.kde.org/r/126740/#comment62487>

if command -v svgo >/dev/null; then



optimize.svg.sh (line 14)
<https://git.reviewboard.kde.org/r/126740/#comment62488>

Paths with spaces will break this.

```
find . -name "*.svg" -size 4k -print0 | while IFS= read -r -d '' file; do
svgo -i $file -o $file $ARGS
done
```

(http://mywiki.wooledge.org/BashFAQ/001)



optimize.svg.sh (line 16)
<https://git.reviewboard.kde.org/r/126740/#comment62491>

Here an everywhere else
$v -> "$v" https://github.com/koalaman/shellcheck/wiki/SC2086



optimize.svg.sh (line 20)
<https://git.reviewboard.kde.org/r/126740/#comment62490>

    spaces in a parent dir will break this loop


- Alex Richardson


On Jan. 19, 2016, 10:54 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
> 
> (Updated Jan. 19, 2016, 10:54 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: breeze-icons
> 
> 
> Description
> ---
> 
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea 
> that right now we're serving right away the svg's from inkscape and there's 
> room for improvement, potentially.
> 
> This patch just introduces a script that optimizes the svg's using `svgo`.
> 
> More could be done, like using gzip files, we can look into that if anyone's 
> interested. In fact, we used to use svgz for the icons, I wonder why that 
> changed. 
> 
> This will change the files in-place rather than as a build step, which is 
> what I considered first. The process to run svgo on every file was about 30 
> minutes to 1h on my system, so I doubt it's really desirable.
> 
> A reduced file size is important because it will greatly reduce disk IO, 
> which is a bottle-neck we have.
> 
> 
> Diffs
> -
> 
>   optimize.svg.sh PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
> 
> 
> Testing
> ---
> 
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
> 
> #run the script
> 
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

[Differential] [Changed Subscribers] D851: Port kded plugin to json metadata

2016-01-21 Thread arichardson (Alex Richardson) 
arichardson added a subscriber: arichardson.

INLINE COMMENTS
  CMakeLists.txt:78 Add SERVICE_TYPES kdedmodule.desktop here to ensure that 
all keys have the right type
  CMakeLists.txt:81 Could be simplified to use 
`kcoreaddons_add_plugin(kded_kwrite SOURCES kwrited.cpp JSON "kwrited.json" 
INSTALL_NAMESPACE "kf5/kded")`

REPOSITORY
  rKWRITED KWriteD

REVISION DETAIL
  https://phabricator.kde.org/D851

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: heikobecker, plasma-devel
Cc: arichardson
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126348: Make it possible to provide the metadata in json

2015-12-15 Thread Alex Richardson 

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



src/kpackage/package.cpp (line 24)
<https://git.reviewboard.kde.org/r/126348/#comment61280>

no longer required, right?



src/kpackage/package.cpp (line 916)
<https://git.reviewboard.kde.org/r/126348/#comment61281>

`path.endsWith("/metadata.desktop") || path.endsWith("/metadata.json")`? 
QRegExp seems like overkill here.


- Alex Richardson


On Dec. 15, 2015, 3:24 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126348/
> ---
> 
> (Updated Dec. 15, 2015, 3:24 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> Due to KCoreAddons transition from using desktop files to json files, 
> KPackage ended in a weird state where desktop files were allowed to use 
> desktop files but not json.
> 
> This patch makes sure that manifest.json files are also acceptable.
> 
> 
> Diffs
> -
> 
>   autotests/data/testjsonmetadatapackage/contents/ui/main.qml PRE-CREATION 
>   autotests/data/testjsonmetadatapackage/metadata.json PRE-CREATION 
>   autotests/querytest.cpp 0a2f11a 
>   src/kpackage/package.cpp 8416054 
>   src/kpackage/packageloader.cpp 1e1e382 
>   src/kpackage/private/packagejobthread.cpp 444dacc 
>   src/kpackage/private/packages.cpp 2f6bbcf 
> 
> Diff: https://git.reviewboard.kde.org/r/126348/diff/
> 
> 
> Testing
> ---
> 
> Tests still pass (except for 
> https://build.kde.org/job/kpackage%20master%20stable-kf5-qt5/37/PLATFORM=Linux,compiler=gcc/testReport/,
>  that is not passing in master).
> Adds a test plugin with a json file in it.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126320: Read KPluginMetada's property X-Plasma-ComponentTypes as a stringlist

2015-12-14 Thread Alex Richardson 

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

Ship it!



src/plasma/scripting/scriptengine.cpp (line 89)
<https://git.reviewboard.kde.org/r/126320/#comment61223>

readStringList() is static so KPluginMetaData:: is probably the better way 
to call it


- Alex Richardson


On Dec. 11, 2015, 6:48 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126320/
> ---
> 
> (Updated Dec. 11, 2015, 6:48 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Alex Richardson.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> plasma-scriptengine.desktop defines the property "X-Plasma-ComponentTypes"
> as Type=QStringList. When reading it using KPluginMetaData::value(..) it
> expects a QString back. This used to work but regressed in kcoreaddons in
> commit cfd18cf09b559a050fd6a2680ad4e71eeb950383. Now I'm not sure if calling
> KPluginMetaData::value(..) on a property that is known to be a stringlist
> should actually return a QString (Alex?), but making it read the property
> as a stringlist works and is correct and also fixes Plasma startup for me.
> 
> 
> Diffs
> -
> 
>   src/plasma/scripting/scriptengine.cpp 1b132de 
> 
> Diff: https://git.reviewboard.kde.org/r/126320/diff/
> 
> 
> Testing
> ---
> 
> Plasma would get stuck on startup, now I can run Plasma again.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126320: Read KPluginMetada's property X-Plasma-ComponentTypes as a stringlist

2015-12-14 Thread Alex Richardson 

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



src/plasma/scripting/scriptengine.cpp (line 67)
<https://git.reviewboard.kde.org/r/126320/#comment61224>

This should probably also be changed to QStringList?


- Alex Richardson


On Dec. 11, 2015, 6:48 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126320/
> ---
> 
> (Updated Dec. 11, 2015, 6:48 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Alex Richardson.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> plasma-scriptengine.desktop defines the property "X-Plasma-ComponentTypes"
> as Type=QStringList. When reading it using KPluginMetaData::value(..) it
> expects a QString back. This used to work but regressed in kcoreaddons in
> commit cfd18cf09b559a050fd6a2680ad4e71eeb950383. Now I'm not sure if calling
> KPluginMetaData::value(..) on a property that is known to be a stringlist
> should actually return a QString (Alex?), but making it read the property
> as a stringlist works and is correct and also fixes Plasma startup for me.
> 
> 
> Diffs
> -
> 
>   src/plasma/scripting/scriptengine.cpp 1b132de 
> 
> Diff: https://git.reviewboard.kde.org/r/126320/diff/
> 
> 
> Testing
> ---
> 
> Plasma would get stuck on startup, now I can run Plasma again.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126348: Make it possible to provide the metadata in json

2015-12-14 Thread Alex Richardson 

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


Can't really comment on KPackage internals, but looks good to me other than 
this one issue


src/kpackage/private/packagejobthread.cpp (line 143)
<https://git.reviewboard.kde.org/r/126348/#comment61242>

I think it would be better if this code was added to the KPluginMetaData 
ctor (in an `if (path.endswith(".json"))` branch.


- Alex Richardson


On Dec. 14, 2015, 5:11 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126348/
> ---
> 
> (Updated Dec. 14, 2015, 5:11 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> Due to KCoreAddons transition from using desktop files to json files, 
> KPackage ended in a weird state where desktop files were allowed to use 
> desktop files but not json.
> 
> This patch makes sure that manifest.json files are also acceptable.
> 
> 
> Diffs
> -
> 
>   autotests/querytest.cpp 0a2f11a 
>   src/kpackage/package.cpp 8416054 
>   src/kpackage/packageloader.cpp 1e1e382 
>   src/kpackage/private/packagejobthread.cpp 444dacc 
>   src/kpackage/private/packagejobthread_p.h 1babf0b 
>   src/kpackage/private/packages.cpp 2f6bbcf 
> 
> Diff: https://git.reviewboard.kde.org/r/126348/diff/
> 
> 
> Testing
> ---
> 
> Tests still pass (except for 
> https://build.kde.org/job/kpackage%20master%20stable-kf5-qt5/37/PLATFORM=Linux,compiler=gcc/testReport/,
>  that is not passing in master).
> Adds a test plugin with a json file in it.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126268: Move shutdown scripts into ksmserver cleanup

2015-12-07 Thread Alex Richardson 

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



ksmserver/server.cpp (line 1105)
<https://git.reviewboard.kde.org/r/126268/#comment61038>

continue?


- Alex Richardson


On Dec. 7, 2015, 12:26 a.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126268/
> ---
> 
> (Updated Dec. 7, 2015, 12:26 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Shutdown scripts are done by startkde after ksmserver quits. Which never
> happens because we've told systemd to shutdown.
> 
> Old systems worked because they used to communicate with the display
> manager which then closed us before shutting down, this is no longer the
> case.
> 
> This moves handling the shutdown scripts into ksmserver (which already
> handles startup scripts) and runs them before asking logind to shutdown.
> 
> BUG: 356190
> 
> 
> Diffs
> -
> 
>   ksmserver/server.h 1a2f0810c53d508d7d721fb4be1c25f0585c0602 
>   ksmserver/server.cpp 9477e542399ad65f993e581fc4212883f282b70e 
>   startkde/startkde.cmake 41a8975cce1fb2a4e7a034e697ce6e2cc59d5b1e 
>   startkde/startplasma.cmake 8360a636d3f68c957a15158484360a611cfe3ff8 
> 
> Diff: https://git.reviewboard.kde.org/r/126268/diff/
> 
> 
> Testing
> ---
> 
> had a shutdown script to touch a file in my home, worked for shutdown and 
> logout.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126210: [phonon] Do not set RPATH

2015-12-03 Thread Alex Richardson 

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


+1

I think RPATH stuff is already set up correctly by extra-cmake-modules so there 
should be no need for that code

- Alex Richardson


On Nov. 30, 2015, 7:16 p.m., Andreas Sturmlechner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126210/
> ---
> 
> (Updated Nov. 30, 2015, 7:16 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> This very old piece of code triggers a scanelf warning during build. I don't 
> know why it is there, hopefully someone knows.
> 
> 
> Diffs
> -
> 
>   phonon/platform_kde/CMakeLists.txt 281425fb6f6c1a41f14399ac17f8a94cd99507ad 
> 
> Diff: https://git.reviewboard.kde.org/r/126210/diff/
> 
> 
> Testing
> ---
> 
> Builds, and runs (unfortunately can't run the tests), phonon test playback 
> included.
> 
> 
> Thanks,
> 
> Andreas Sturmlechner
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126115: Unset environment variables before starting kwin_wayland

2015-11-19 Thread Alex Richardson 


> On Nov. 19, 2015, 2:08 p.m., Matthias Klumpp wrote:
> > Did you consider running the whole script with `env -i`, or (likely the 
> > better idea) run KWin with `env -i`?
> > That should sanitize the environment (unset all env vars, except for 
> > shell-defaults). You could then set exactly the variables you need, to the 
> > exact values you want, so we don't miss unsetting anything.
> 
> Martin Gräßlin wrote:
> No I didn't consider that, because I wasn't aware that this exists.
> 
> Martin Gräßlin wrote:
> Just tried with changing directly in the wayland session file. That 
> doesn't work at all. I think the main problem is that I lose important env 
> variables related to the logind session/dbus, etc.
> 
> So only way would be for the command to start kwin_wayland. But that as 
> well would require to set quite an amount of env variables, but worth a try.
> 
> Martin Gräßlin wrote:
> Just gave a try - the command looks horrible, but I got the session 
> started and env variables are properly filtered.
> 
> Command looks like this now:
> /usr/bin/env -i KDE_FULL_SESSION=true KDE_SESSION_VERSION=5 
> KDE_SESSION_UID=${KDE_SESSION_UID} XDG_CURRENT_DESKTOP=KDE 
> QT_QPA_PLATFORM=wayland PAM_KWALLET5_LOGIN=${PAM_KWALLET5_LOGIN} USER=${USER} 
> LANGUAGE=${LANGUAGE} XDG_SEAT=${XDG_SEAT} 
> XDG_SESSION_TYPE=${XDG_SESSION_TYPE} XCURSOR_SIZE=${XCURSOR_SIZE} 
> HOME=${HOME} DESKTOP_SESSION=${DESKTOP_SESSION} 
> XDG_SEAT_PATH=${XDG_SEAT_PATH} 
> DBUS_SESSION_BUS_ADDRESS=${DBUS_SESSION_BUS_ADDRESS} LOGNAME=${LOGNAME} 
> XDG_SESSION_CLASS=${XDG_SESSION_CLASS} XDG_SESSION_ID=${XDG_SESSION_ID} 
> PATH=${PATH} XDG_SESSION_PATH=${XDG_SESSION_PATH} 
> XDG_RUNTIME_DIR=${XDG_RUNTIME_DIR} XCURSOR_THEME=${XCURSOR_THEME} 
> LANG=${LANG} XDG_SESSION_DESKTOP=${XDG_SESSION_DESKTOP} 
> XCURSOR_PATH=${XCURSOR_PATH} XDG_VTNR=${XDG_VTNR} PWD=${PWD} 
> XDG_DATA_DIRS=${XDG_DATA_DIRS} XDG_CONFIG_DIRS=${XDG_CONFIG_DIRS} 
> @KWIN_WAYLAND_BIN_PATH@ --xwayland --libinput 
> --exit-with-session=@CMAKE_INSTALL_FULL_LIBEXECDIR@/startplasma
> 
> Matthias Klumpp wrote:
> Urgh, terrible! But I think this might just be the best workaround we can 
> come up with, given the circumstances. It at least protects against someone 
> adding new env vars which load bad code into the compositor. It might be an 
> issue as soon as kwin starts to require another env var which isn't in the 
> list, but that's an issue we can solve.
> I wonder if we can simplify that command somehow, though, e.g. by placing 
> the allowed variables in a file or new variable, to make this easier to read.
> Apart from that, +1 from me (I'll take a look at other DEs though, maybe 
> someone has come up with a bettr solution to this issue).
> 
> Xuetian Weng wrote:
> Woundn't this break user's workflow? Since startplasma is started by 
> kwin, the environment variable for the desktop will be derived from that.
> 
> If distribution has some global configuration under /etc/profile.d (which 
> is really common, e.g. openjdk, mozilla plugin path), they will not be set.
> 
> Martin Gräßlin wrote:
> > Woundn't this break user's workflow?
> 
> Yes, but what do you prefer? Breaking user's workflows or a secure system?
> 
> There is nothing wrong of course with sourcing the env scripts in 
> startplasma again and distributions using things like /etc/profile.d would be 
> advised to do exactly that.

I don't see how this adds any security if you still keep $PATH. what if the 
user prepends `$HOME/my-evil-binaries/` to $PATH?
Maybe it makes more sense to restrict which scripts are executed before kwin 
launches?


- Alex


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


On Nov. 19, 2015, 12:22 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126115/
> ---
> 
> (Updated Nov. 19, 2015, 12:22 p.m.)
> 
> 
> Review request for Plasma, David Edmundson and Matthias Klumpp.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Any environment variable which can be used to specify a path to a
> binary object to be loaded in the KWin process bears the risk of
> being abused to add code to KWin to perform as a key logger.
> 
> E.g. an env variable pointing QT_PLUGIN_PATH to a location in $HOME
> and adjusting QT_STYLE_OVERRIDE to load a specific QStyle plugin from
> that location would allow to easily log all keys without KWin noticing.
> 
> As env variables can be specified in scripts sourced before the session
> starts there is not much KWin can do about that to protect itself.
> 
> This affects all the LD_* variables and

Re: Review Request 126104: Make networkmanager-qt an optional dependency

2015-11-19 Thread Alex Richardson 

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

(Updated Nov. 19, 2015, 4:10 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit de5ed6c4cdded1f2ba2ce43022be1d1a9b475bf0 by Alex 
Richardson to branch master.


Repository: plasma-workspace


Description
---

I can't compile networkmanager-qt on my current system because the version
of NetworkManager seems to be too old.

Also not every system has NetworkManager (FreeBSD?) so it should be
optional.


Diffs
-

  CMakeLists.txt d21a2b7a7774ac712f905f1db1ee478dca10b45c 
  dataengines/CMakeLists.txt 014da10aac5629be202bd5ad96012f817bf846ef 

Diff: https://git.reviewboard.kde.org/r/126104/diff/


Testing
---

compiles


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126115: Unset environment variables before starting kwin_wayland

2015-11-19 Thread Alex Richardson 


> On Nov. 19, 2015, 2:08 p.m., Matthias Klumpp wrote:
> > Did you consider running the whole script with `env -i`, or (likely the 
> > better idea) run KWin with `env -i`?
> > That should sanitize the environment (unset all env vars, except for 
> > shell-defaults). You could then set exactly the variables you need, to the 
> > exact values you want, so we don't miss unsetting anything.
> 
> Martin Gräßlin wrote:
> No I didn't consider that, because I wasn't aware that this exists.
> 
> Martin Gräßlin wrote:
> Just tried with changing directly in the wayland session file. That 
> doesn't work at all. I think the main problem is that I lose important env 
> variables related to the logind session/dbus, etc.
> 
> So only way would be for the command to start kwin_wayland. But that as 
> well would require to set quite an amount of env variables, but worth a try.
> 
> Martin Gräßlin wrote:
> Just gave a try - the command looks horrible, but I got the session 
> started and env variables are properly filtered.
> 
> Command looks like this now:
> /usr/bin/env -i KDE_FULL_SESSION=true KDE_SESSION_VERSION=5 
> KDE_SESSION_UID=${KDE_SESSION_UID} XDG_CURRENT_DESKTOP=KDE 
> QT_QPA_PLATFORM=wayland PAM_KWALLET5_LOGIN=${PAM_KWALLET5_LOGIN} USER=${USER} 
> LANGUAGE=${LANGUAGE} XDG_SEAT=${XDG_SEAT} 
> XDG_SESSION_TYPE=${XDG_SESSION_TYPE} XCURSOR_SIZE=${XCURSOR_SIZE} 
> HOME=${HOME} DESKTOP_SESSION=${DESKTOP_SESSION} 
> XDG_SEAT_PATH=${XDG_SEAT_PATH} 
> DBUS_SESSION_BUS_ADDRESS=${DBUS_SESSION_BUS_ADDRESS} LOGNAME=${LOGNAME} 
> XDG_SESSION_CLASS=${XDG_SESSION_CLASS} XDG_SESSION_ID=${XDG_SESSION_ID} 
> PATH=${PATH} XDG_SESSION_PATH=${XDG_SESSION_PATH} 
> XDG_RUNTIME_DIR=${XDG_RUNTIME_DIR} XCURSOR_THEME=${XCURSOR_THEME} 
> LANG=${LANG} XDG_SESSION_DESKTOP=${XDG_SESSION_DESKTOP} 
> XCURSOR_PATH=${XCURSOR_PATH} XDG_VTNR=${XDG_VTNR} PWD=${PWD} 
> XDG_DATA_DIRS=${XDG_DATA_DIRS} XDG_CONFIG_DIRS=${XDG_CONFIG_DIRS} 
> @KWIN_WAYLAND_BIN_PATH@ --xwayland --libinput 
> --exit-with-session=@CMAKE_INSTALL_FULL_LIBEXECDIR@/startplasma
> 
> Matthias Klumpp wrote:
> Urgh, terrible! But I think this might just be the best workaround we can 
> come up with, given the circumstances. It at least protects against someone 
> adding new env vars which load bad code into the compositor. It might be an 
> issue as soon as kwin starts to require another env var which isn't in the 
> list, but that's an issue we can solve.
> I wonder if we can simplify that command somehow, though, e.g. by placing 
> the allowed variables in a file or new variable, to make this easier to read.
> Apart from that, +1 from me (I'll take a look at other DEs though, maybe 
> someone has come up with a bettr solution to this issue).
> 
> Xuetian Weng wrote:
> Woundn't this break user's workflow? Since startplasma is started by 
> kwin, the environment variable for the desktop will be derived from that.
> 
> If distribution has some global configuration under /etc/profile.d (which 
> is really common, e.g. openjdk, mozilla plugin path), they will not be set.
> 
> Martin Gräßlin wrote:
> > Woundn't this break user's workflow?
> 
> Yes, but what do you prefer? Breaking user's workflows or a secure system?
>     
> There is nothing wrong of course with sourcing the env scripts in 
> startplasma again and distributions using things like /etc/profile.d would be 
> advised to do exactly that.
> 
> Alex Richardson wrote:
> I don't see how this adds any security if you still keep $PATH. what if 
> the user prepends `$HOME/my-evil-binaries/` to $PATH?
> Maybe it makes more sense to restrict which scripts are executed before 
> kwin launches?
> 
> Matthias Klumpp wrote:
> Maybe starting a session manager as the first thing makes sense. That one 
> can then spawn KWin and KWin can tell the service to start plasmashell when 
> it's ready.
> Reminds me that I wanted to redesign the KDE startup process a while ago, 
> getting rid of most of the scripts. A `systemd --user` based startup could do 
> the exact same, btw.
> Unsetting most vars will break assumptions, but since this affects only 
> Wayland, it's probably okay for a little bit of breakage to exist, although 
> if we can avoid that we should try to avoid breaking things.
> Generally I agree with Martin: Security >> Breaking Workflows >> 
> Backwards-Compatibility
> 
> Matthias Klumpp wrote:
> @Alex: KWin is started with an absolute path now, which makes $PATH and 
> aliases irrelevant. So that particular issue is solved :)

I know it doesn't make a difference for kwin. But I thought kwin will then run 
startplasma which wi

Review Request 126104: Make networkmanager-qt an optional dependency

2015-11-18 Thread Alex Richardson 

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

Review request for Plasma.


Repository: plasma-workspace


Description
---

I can't compile networkmanager-qt on my current system because the version
of NetworkManager seems to be too old.

Also not every system has NetworkManager (FreeBSD?) so it should be
optional.


Diffs
-

  CMakeLists.txt d21a2b7a7774ac712f905f1db1ee478dca10b45c 
  dataengines/CMakeLists.txt 014da10aac5629be202bd5ad96012f817bf846ef 

Diff: https://git.reviewboard.kde.org/r/126104/diff/


Testing
---

compiles


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 125028: Monitor for the clock changes from the kernel

2015-09-02 Thread Alex Richardson 

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


Looks good to me


dataengines/time/timeengine.cpp (line 32)
<https://git.reviewboard.kde.org/r/125028/#comment58665>

#ifdef Q_OS_LINUX



dataengines/time/timeengine.cpp (line 66)
<https://git.reviewboard.kde.org/r/125028/#comment58666>

#ifdef Q_OS_LINUX



dataengines/time/timeengine.cpp (line 67)
<https://git.reviewboard.kde.org/r/125028/#comment58663>

We should probably add `TFD_CLOEXEC` but glibc headers don't seem to have 
it: Linux source has `#define TFD_CLOEXEC O_CLOEXEC` so we can use `O_CLOEXEC`



dataengines/time/timeengine.cpp (line 70)
<https://git.reviewboard.kde.org/r/125028/#comment58664>

Maybe add a comment that glibc doesn't provide the 
`TFD_TIMER_CANCEL_ON_SET` constant and that's why you are using 3 instead of 
the constants.

Also shouldn't there be an error check here?

It is unlikely that someone runs with Linux < 3.0  and we therefore get a 
`EINVAL`, but it can still return `ENOMEN` or `EBADF` if the call to 
`timerfd_create` failed.


- Alex Richardson


On Sept. 2, 2015, 11:29 a.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125028/
> ---
> 
> (Updated Sept. 2, 2015, 11:29 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> At the moment we update the clock when we resume from suspend or from
> the time KCM.
> 
> However the signal from the clock KCM isn't very good as it signals just
> after it requests an NTP update or clock change, not necessarily when
> that finishes.
> 
> Also we miss any external NTP updates which might occur at other times
> throughout the day, such as a big change when we connect to the network
> 
> This method also avoids needing solid to tell us when we're resuming
> from suspend, which is deprecated API and also seems slightly slower.
> 
> BUG: 344870
> 
> 
> Diffs
> -
> 
>   dataengines/time/timeengine.cpp 2fd9792209ff5e78bd3dee1ed938eb0b1173de8d 
> 
> Diff: https://git.reviewboard.kde.org/r/125028/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124877: fix corruption of startupconfig(files) in Qt5.6

2015-09-02 Thread Alex Richardson 

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



startkde/kstartupconfig/kdostartupconfig.cpp (line 86)
<https://git.reviewboard.kde.org/r/124877/#comment58667>

`QByteArray buf = keys.readLine()`

I don't see the point of preallocation 1024 bytes (I doubt this does any 
good anyway) and using the less safe C-style API


- Alex Richardson


On Aug. 31, 2015, 5:33 p.m., Takahiro Hashimoto wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124877/
> ---
> 
> (Updated Aug. 31, 2015, 5:33 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 351609
> https://bugs.kde.org/show_bug.cgi?id=351609
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Building Plasma5 with Qt 5.6 branch, kstartupconfig5/kdostartupconfig5 
> generates corrupted files.
> 
> $HOME/.config/startupconfigfiles
> $HOME/.config/startupconfig
> 
> 
> Diffs
> -
> 
>   startkde/kstartupconfig/kdostartupconfig.cpp 3944c06 
> 
> Diff: https://git.reviewboard.kde.org/r/124877/diff/
> 
> 
> Testing
> ---
> 
> works for me. 
> 
> 1. execute kstartupconfig5
> 2. check if both files are not corrupted.
> 
> 
> Thanks,
> 
> Takahiro Hashimoto
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124877: fix corruption of startupconfig(files) in Qt5.6

2015-09-02 Thread Alex Richardson 


> On Sept. 2, 2015, 3:14 p.m., Alex Richardson wrote:
> > startkde/kstartupconfig/kdostartupconfig.cpp, line 86
> > <https://git.reviewboard.kde.org/r/124877/diff/1/?file=397209#file397209line86>
> >
> > `QByteArray buf = keys.readLine()`
> > 
> > I don't see the point of preallocation 1024 bytes (I doubt this does 
> > any good anyway) and using the less safe C-style API

Or even better just `QString line = QString::fromLocal8Bit(keys.readLine());`


- Alex


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


On Aug. 31, 2015, 5:33 p.m., Takahiro Hashimoto wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124877/
> ---
> 
> (Updated Aug. 31, 2015, 5:33 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 351609
> https://bugs.kde.org/show_bug.cgi?id=351609
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Building Plasma5 with Qt 5.6 branch, kstartupconfig5/kdostartupconfig5 
> generates corrupted files.
> 
> $HOME/.config/startupconfigfiles
> $HOME/.config/startupconfig
> 
> 
> Diffs
> -
> 
>   startkde/kstartupconfig/kdostartupconfig.cpp 3944c06 
> 
> Diff: https://git.reviewboard.kde.org/r/124877/diff/
> 
> 
> Testing
> ---
> 
> works for me. 
> 
> 1. execute kstartupconfig5
> 2. check if both files are not corrupted.
> 
> 
> Thanks,
> 
> Takahiro Hashimoto
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 123556: KPackage::findPackages with a filter callback

2015-04-29 Thread Alex Richardson 

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



src/kpackage/packageloader.cpp (line 273)
https://git.reviewboard.kde.org/r/123556/#comment54542

This will crash if there is no filter.

`if (!filter || filter(plugin))` should work.


- Alex Richardson


On April 29, 2015, 1:12 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123556/
 ---
 
 (Updated April 29, 2015, 1:12 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kpackage
 
 
 Description
 ---
 
 Add a KPackage::findPackages function similar to KPluginLoader::findPlugins
 
 
 Diffs
 -
 
   src/kpackage/packageloader.h 2761d98 
   src/kpackage/packageloader.cpp 00defbd 
 
 Diff: https://git.reviewboard.kde.org/r/123556/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 123131: Port kio_man away from kdelibs4support

2015-04-04 Thread Alex Richardson 


 On March 25, 2015, 9:54 p.m., Lukáš Tinkl wrote:
  man/kio_man.cpp, line 242
  https://git.reviewboard.kde.org/r/123131/diff/1/?file=356605#file356605line242
 
  Not correct, this returns only user configured list of languages, 
  whereas it previously returned the full list.
 
 Nick Shaforostoff wrote:
 please see 
 https://projects.kde.org/projects/kde/kdesdk/lokalize/repository/revisions/master/entry/src/common/languagelistmodel.cpp
  for the ways to get a list of all languages (there is a KDE-way and Qt-way)
 
 Alex Richardson wrote:
 So if I parse this correctly the KDE way is: 
 `KConfig(QLatin1String(locale/kf5_all_languages), KConfig::NoGlobals, 
 QStandardPaths::GenericDataLocation).groupList()`?
 Should there be something in KI18n or is this sufficiently rare that we 
 can just add @deprecated use 
 `KConfig(QLatin1String(locale/kf5_all_languages), KConfig::NoGlobals, 
 QStandardPaths::GenericDataLocation).groupList()` instead to 
 KLocale::languageList()?
 
 David Faure wrote:
 Sounds to me like it should be a method in ki18n, it's a bit too arcane 
 to be maintainable.

Seems like it's not the right replacement:

No special env vars set:
```
KLocale::global()-languageList()  (de, en_US)
KLocalizedString::availableApplicationTranslations() = (pt_BR, nds, kk, 
ga, mr, km, ko, en_US, ca, gl, is, it, el, nb, tr, 
ro, pl, es, et, en_GB, eu, ja, ru, nl, cs, nn, pt, 
ar, ug, hi, lt, da, lv, uk, zh_CN, zh_TW, fi, de, sk, 
hr, sl, hu, bg, fr, ca@valencia, sv)
QLocale().uiLanguages() = (de-DE)
KConfig(kf5_all_languages).groupsList() = (ca@valencia, ln, de, tw, 
lo, ty, lt, lv, ug, uk, pt_BR, dz, mg, mh, mi, ur, 
mk, ml, mn, mo, uz, mr, ms, mt, el, en, crh, eo, my, 
be@latin, es, et, eu, vi, na, nb, nd, ne, vo, ng, fa, 
nl, nn, csb, nr, fi, fj, sr@ijekavian, nv, wa, fo, ny, 
fr, oc, fy, wo, ga, sr@latin, hne, om, gd, or, os, gl, 
uz@cyrillic, gn, gu, xh, gv, zh_HK, pa, nso, pi, ha, pl, 
he, hi, mai, ps, pt, ho, hr, hu, yi, hy, hz, yo, ia, 
id, ie, aa, ik, ab, qu, ae, za, io, af, nds, is, ven, 
it, iu, zh, am, ar, as, ja, sr@ijekavianlatin, x-test, ay, 
zu, az, rn, ro, en_US, ba, ru, be, rw, bg, bh, bi, 
jv, bn, sa, bo, sc, sd, br, se, bs, sg, si, ka, sk, 
sl, zh_CN, sm, ast, sn, so, sq, ki, sr, bn_IN
 , ca, ss, kk, st, kl, su, km, sv, kn, ce, sw, ko, 
ch, ks, rom, ku, kv, kw, ta, co, ky, zh_TW, te, cs, 
tg, cu, th, cv, ti, hsb, la, lb, tk, cy, tn, to, 
en_GB, li, tr, da, ts, tt, dsb)
```

With LANGUAGE=de_DE

```
KLocale::global()-languageList()  (de, en_US)
KLocalizedString::availableApplicationTranslations() = (sv, pt_BR, 
ca@valencia, kk, ga, mr, km, ko, en_US, nds, ca, gl, is, 
it, el, nb, tr, ro, en_GB, pl, es, et, eu, ja, ru, 
nl, cs, nn, pt, ar, ug, hi, lt, da, zh_CN, zh_TW, lv, 
uk, fi, de, sk, hr, sl, hu, bg, fr)
QLocale().uiLanguages() = (de-DE)
KConfig(kf5_all_languages).groupsList() = (...)
```

With LANGUAGE=en_GB:de_DE
```
KLocale::global()-languageList()  (en_GB, de, en_US)
KLocalizedString::availableApplicationTranslations() = (ga, nds, kk, 
mr, km, ko, ca, gl, pt_BR, is, it, el, nb, tr, ro, 
en_US, pl, es, et, eu, ja, ru, nl, cs, nn, pt, ar, 
en_GB, ug, hi, lt, da, lv, uk, fi, de, sk, hr, sl, 
hu, bg, fr, ca@valencia, zh_CN, zh_TW, sv)
QLocale().uiLanguages() = (en-GB, de-DE)
```

With LANGUAGE=de_DE:en_GB
```
KLocale::global()-languageList()  (en_GB, de, en_US)
KLocalizedString::availableApplicationTranslations() = (mr, km, ko, 
nds, en_GB, ca, gl, is, it, el, nb, tr, ro, pl, es, 
et, eu, ja, ru, nl, cs, nn, zh_CN, zh_TW, pt, ar, 
ca@valencia, ug, hi, lt, da, lv, uk, fi, de, sk, hr, 
sl, hu, bg, pt_BR, fr, sv, en_US, ga, kk)
QLocale().uiLanguages() = (de-DE, en-GB)
KConfig(kf5_all_languages).groupsList() = (...)
```
This one is very confusing, why is en_GB still the highest priority according 
to KLocale::global()-languageList()? Maybe it is reading some config file 
where I set that?

With LANGUAGE=fr_FR

```
KLocale::global()-languageList()  (de, en_US)
KLocalizedString::availableApplicationTranslations() = (et, eu, ja, ru, 
nl, cs, nn, pt, en_GB, ar, ca@valencia, ug, hi, lt, da, 
lv, uk, fi, de, sk, hr, sl, zh_CN, zh_TW, hu, bg, fr, 
sv, nds, ga, kk, mr, km, ko, pt_BR, ca, gl, en_US, is, 
it, el, nb, tr, ro, pl, es)
QLocale().uiLanguages() = (fr-FR)
```

Even more confusion, LANGUAGE now has no effect on 
KLocale::global()-languageList()

With LANG=fr_FR
```
KLocale::global()-languageList()  (fr, en_US)
KLocalizedString::availableApplicationTranslations() = (sk, hr, sl, hu, 
bg, pt_BR, fr, sv, en_US, kk, ga, km, mr, ko, en_GB, 
ca, gl, is, it, el, nb, tr, ro, pl, es, et, eu, ja, 
ru, nl, cs, nn, zh_CN, zh_TW, pt, ar, ca@valencia, nds, 
ug, hi, lt, da, lv, uk, fi, de)
QLocale().uiLanguages() = (fr-FR)
```

LANG seems to work where LANGUAGE doesn't.


I have no idea what's going on here, I need some feedback from someone with 
more I18N experience.

However, QLocale().uiLanguages() seems to be almost the right replacement if we 
append en_US and convert it from

Re: Review Request 123142: Fixes plasma-desktop compilation with boost 1.57

2015-03-26 Thread Alex Richardson 

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


+1, should fix the build for me.
Not sure what the preferred style in plasma is (`static_castbool(foo)` or 
`bool(foo)`) so I can't give the ship it.

- Alex Richardson


On March 26, 2015, 7:47 p.m., Tomaz  Canabrava wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123142/
 ---
 
 (Updated March 26, 2015, 7:47 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-desktop
 
 
 Description
 ---
 
 Plasma-desktop uses Boost::optional on a few places, but it stored the value 
 as boolean ( bool thisValid  = currentValue; for instance ), the issue is 
 that boost::optional conversion to bool must be explicit, thus a static_cast 
 is needed.
 
 
 Diffs
 -
 
   lib/kactivities-stats/src/lib/stats/resultset_iterator.cpp 664b399 
 
 Diff: https://git.reviewboard.kde.org/r/123142/diff/
 
 
 Testing
 ---
 
 compiles.
 
 
 Thanks,
 
 Tomaz  Canabrava
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 123088: Adding libkactivities-stats to the build

2015-03-26 Thread Alex Richardson 


 On March 26, 2015, 3:34 p.m., Hrvoje Senjan wrote:
  the library doesn't seem to like boost 1.56:
  
  ```
  In file included from 
  ../lib/kactivities-stats/src/lib/stats/resultset.cpp:425:
  ../lib/kactivities-stats/src/lib/stats/resultset_iterator.cpp:78:14: error: 
  no viable conversion from 'const boost::optionalResult' to 'bool'
  bool thisValid  = currentValue;
   ^
  ../lib/kactivities-stats/src/lib/stats/resultset_iterator.cpp:79:14: error: 
  no viable conversion from 'const boost::optionalResult' to 'bool'
  bool otherValid = other.currentValue;
   ^~~
  ../lib/kactivities-stats/src/lib/stats/resultset_iterator.cpp:96:16: error: 
  no viable conversion from 'const boost::optionalResult' to 'bool'
  return currentValue;
 ^~~~
  1 warning and 3 errors generated.
  ```
  or with gcc:
  ```
  [  232s] In file included from 
  /home/abuild/rpmbuild/BUILD/plasma-desktop-5.2.91git~20150326T143831~cc30975/lib/kactivities-stats/src/lib/stats/resultset.cpp:425:0:
  [  232s] 
  /home/abuild/rpmbuild/BUILD/plasma-desktop-5.2.91git~20150326T143831~cc30975/lib/kactivities-stats/src/lib/stats/resultset_iterator.cpp:
   In member function 'bool 
  KActivities::Experimental::Stats::ResultSet::const_iterator::Private::operator==(const
   KActivities::Experimental::Stats::ResultSet::const_iterator::Private) 
  const':
  [  232s] 
  /home/abuild/rpmbuild/BUILD/plasma-desktop-5.2.91git~20150326T143831~cc30975/lib/kactivities-stats/src/lib/stats/resultset_iterator.cpp:76:27:
   error: cannot convert 'const 
  boost::optionalKActivities::Experimental::Stats::ResultSet::Result' to 
  'bool' in initialization
  [  232s]  bool thisValid  = currentValue;
  [  232s]^
  [  232s] 
  /home/abuild/rpmbuild/BUILD/plasma-desktop-5.2.91git~20150326T143831~cc30975/lib/kactivities-stats/src/lib/stats/resultset_iterator.cpp:77:33:
   error: cannot convert 'const 
  boost::optionalKActivities::Experimental::Stats::ResultSet::Result' to 
  'bool' in initialization
  [  232s]  bool otherValid = other.currentValue;
  [  232s]  ^
  [  232s] 
  /home/abuild/rpmbuild/BUILD/plasma-desktop-5.2.91git~20150326T143831~cc30975/lib/kactivities-stats/src/lib/stats/resultset_iterator.cpp:
   In member function 'bool 
  KActivities::Experimental::Stats::ResultSet::const_iterator::Private::isValid()
   const':
  [  232s] 
  /home/abuild/rpmbuild/BUILD/plasma-desktop-5.2.91git~20150326T143831~cc30975/lib/kactivities-stats/src/lib/stats/resultset_iterator.cpp:94:16:
   error: cannot convert 'const 
  boost::optionalKActivities::Experimental::Stats::ResultSet::Result' to 
  'bool' in return
  [  232s]  return currentValue;
  [  232s] ^
  ```
 
 Ivan Čukić wrote:
 Is it fixed if you replace with this (seems this is a nice 1.56 bug):
 
 bool thisValid  = !!currentValue;
 bool otherValid = !!other.currentValue;

Seems they are using c++11 `explicit operator bool()`, so I wouldn't call it a 
bug but intentional.


- Alex


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


On March 26, 2015, 1:47 p.m., Ivan Čukić wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123088/
 ---
 
 (Updated March 26, 2015, 1:47 p.m.)
 
 
 Review request for Plasma, Eike Hein and Marco Martin.
 
 
 Repository: plasma-desktop
 
 
 Description
 ---
 
 The experimental library can not land in the kactivities framework just yet. 
 Since it is to be used by plasma-desktop in the next release, we need to 
 include it (for the time being) in p-d.
 
 
 The idea came from Eike. While it has downsides, it also has a few benefits:
 - no need to have a monthly release cycle, so we can actually do work without 
 much bureaucracy surrounding API and ABI compatibility;
 - when the library becomes stable, it will move to KActivities and lose the 
 'experimental' part of the name. This means that the distributions will not 
 have issues of two packages providing the same files;
 - no need to sync the releases and think about which KActivities version 
 plasma 5.x will be used against;
 - I guess I'm forgetting more benefits. :)
 
 The patch includes update.sh script that syncs this code with that in 
 KActivities. (the development should still be in the KActivities 
 ivan/libkactivities-experimental-stats branch)
 
 
 Diffs
 -
 
   CMakeLists.txt 8714f89 
   lib/kactivities-stats/CMakeLists.txt PRE-CREATION 
   lib/kactivities-stats/KF5ActivitiesExperimentalStatsConfig.cmake.in 
 PRE-CREATION

Review Request 123131: Port kio_man away from kdelibs4support

2015-03-25 Thread Alex Richardson 

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

Review request for KDE Frameworks and Plasma.


Repository: kio-extras


Description
---

Port kio_man away from kdelibs4support


Diffs
-

  CMakeLists.txt 02ae89f2524324f758450ad368f64849e39b2f7d 
  man/CMakeLists.txt cb4585a289d3f69b8a16429ce87bfce115d1cc27 
  man/kio_man.cpp e8cf2d70c50c4c20adbb5a81a6213175d397b76e 
  man/kmanpart.cpp 3af7fc182806e8b89297e8de884ce611c827e881 
  man/man2html.cpp 3c493ba334bce890b450d7b11ab00c6e783708d4 

Diff: https://git.reviewboard.kde.org/r/123131/diff/


Testing
---

man view in kdevelop5 works

Not sure about the `KLocale::global()-languageList();` - 
`QLocale().uiLanguages();` change though so I would like some feedback.


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 123131: Port kio_man away from kdelibs4support

2015-03-25 Thread Alex Richardson 


 On März 25, 2015, 9:54 nachm., Lukáš Tinkl wrote:
  man/kio_man.cpp, line 242
  https://git.reviewboard.kde.org/r/123131/diff/1/?file=356605#file356605line242
 
  Not correct, this returns only user configured list of languages, 
  whereas it previously returned the full list.
 
 Nick Shaforostoff wrote:
 please see 
 https://projects.kde.org/projects/kde/kdesdk/lokalize/repository/revisions/master/entry/src/common/languagelistmodel.cpp
  for the ways to get a list of all languages (there is a KDE-way and Qt-way)

So if I parse this correctly the KDE way is: 
`KConfig(QLatin1String(locale/kf5_all_languages), KConfig::NoGlobals, 
QStandardPaths::GenericDataLocation).groupList()`?
Should there be something in KI18n or is this sufficiently rare that we can 
just add @deprecated use `KConfig(QLatin1String(locale/kf5_all_languages), 
KConfig::NoGlobals, QStandardPaths::GenericDataLocation).groupList()` instead 
to KLocale::languageList()?


- Alex


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


On März 25, 2015, 9:17 nachm., Alex Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123131/
 ---
 
 (Updated März 25, 2015, 9:17 nachm.)
 
 
 Review request for KDE Frameworks, Plasma and Martin Koller.
 
 
 Repository: kio-extras
 
 
 Description
 ---
 
 Port kio_man away from kdelibs4support
 
 
 Diffs
 -
 
   CMakeLists.txt 02ae89f2524324f758450ad368f64849e39b2f7d 
   man/CMakeLists.txt cb4585a289d3f69b8a16429ce87bfce115d1cc27 
   man/kio_man.cpp e8cf2d70c50c4c20adbb5a81a6213175d397b76e 
   man/kmanpart.cpp 3af7fc182806e8b89297e8de884ce611c827e881 
   man/man2html.cpp 3c493ba334bce890b450d7b11ab00c6e783708d4 
 
 Diff: https://git.reviewboard.kde.org/r/123131/diff/
 
 
 Testing
 ---
 
 man view in kdevelop5 works
 
 Not sure about the `KLocale::global()-languageList();` - 
 `QLocale().uiLanguages();` change though so I would like some feedback.
 
 
 Thanks,
 
 Alex Richardson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122733: Fix path traversal checks in KPackage

2015-03-04 Thread Alex Richardson 


 On March 4, 2015, 7:23 p.m., Hrvoje Senjan wrote:
  this has broken wallpaper loading here...
  there's loads of Attempting to read file from invalid package! file type: 
  metadata file name:  package path: /usr/share/wallpapers/Aghi/ ...
  warnings...
 
 Marco Martin wrote:
 right, now an autotest fails :/

I'll look into this. The Attempting to read file from invalid package should 
probably only be printed if d-fallbackFilePath() returns an empty string. But 
that only prints a message and doesn't change the behaviour so it can't be the 
reason.

Are there any Path traversal attempt detected: messages?


- Alex


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


On March 3, 2015, 5:53 p.m., Alex Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122733/
 ---
 
 (Updated March 3, 2015, 5:53 p.m.)
 
 
 Review request for KDE Frameworks, Plasma and Marco Martin.
 
 
 Repository: kpackage
 
 
 Description
 ---
 
 They did not canonicalize the package base directory path so it would
 always fail when the package base path contained symlinks
 
 
 Diffs
 -
 
   src/kpackage/package.cpp eb4a09b987970e89f28587426b21d63731634087 
   src/kpackage/private/package_p.h e451412fa02c88113aa4c7bbca2dcda3432b2b02 
 
 Diff: https://git.reviewboard.kde.org/r/122733/diff/
 
 
 Testing
 ---
 
 Files inside the package are now found although the install location contains 
 a symlink
 
 
 Thanks,
 
 Alex Richardson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122733: Fix path traversal checks in KPackage

2015-03-03 Thread Alex Richardson 

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

(Updated March 3, 2015, 5:53 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Plasma and Marco Martin.


Repository: kpackage


Description
---

They did not canonicalize the package base directory path so it would
always fail when the package base path contained symlinks


Diffs
-

  src/kpackage/package.cpp eb4a09b987970e89f28587426b21d63731634087 
  src/kpackage/private/package_p.h e451412fa02c88113aa4c7bbca2dcda3432b2b02 

Diff: https://git.reviewboard.kde.org/r/122733/diff/


Testing
---

Files inside the package are now found although the install location contains a 
symlink


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 122795: Fix crashes when attempting to load invalid package

2015-03-03 Thread Alex Richardson 

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

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

AppletQuickItem: don't crash if d-qmlObject-mainComponent() is null

The if branch is entered if d-qmlObject-mainComponent() is null, but then
it is unconditionally dereferenced causing a potential crash




Don't crash if mainscript is missing


Better error message if applet could not be loaded


Diffs
-

  src/plasmaquick/appletquickitem.cpp 1abedf0d9089c69d12b981d4597363cb1c6fc443 
  src/scriptengines/qml/plasmoid/wallpaperinterface.cpp 
a406d452cadbe6afa6a51a127b6bc98ce137ddae 

Diff: https://git.reviewboard.kde.org/r/122795/diff/


Testing
---

no longer crashes, prints an error message instead


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122795: Fix crashes when attempting to load invalid package

2015-03-03 Thread Alex Richardson 


 On March 3, 2015, 11:39 p.m., Aleix Pol Gonzalez wrote:
  Would it maybe be possible to get a unit test for the applet loading?

I think all the crashes I got were due to 
https://git.reviewboard.kde.org/r/122733/. Now that that is fixed, the 
mainscript is always found. I'll try and create a unit test simulating that 
case.


- Alex


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


On March 3, 2015, 7:32 p.m., Alex Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122795/
 ---
 
 (Updated March 3, 2015, 7:32 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 AppletQuickItem: don't crash if d-qmlObject-mainComponent() is null
 
 The if branch is entered if d-qmlObject-mainComponent() is null, but then
 it is unconditionally dereferenced causing a potential crash
 
 
 
 
 Don't crash if mainscript is missing
 
 
 Better error message if applet could not be loaded
 
 
 Diffs
 -
 
   src/plasmaquick/appletquickitem.cpp 
 1abedf0d9089c69d12b981d4597363cb1c6fc443 
   src/scriptengines/qml/plasmoid/wallpaperinterface.cpp 
 a406d452cadbe6afa6a51a127b6bc98ce137ddae 
 
 Diff: https://git.reviewboard.kde.org/r/122795/diff/
 
 
 Testing
 ---
 
 no longer crashes, prints an error message instead
 
 
 Thanks,
 
 Alex Richardson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 122733: Fix path traversal checks in KPackage

2015-02-26 Thread Alex Richardson 

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

Review request for KDE Frameworks, Plasma and Marco Martin.


Repository: kpackage


Description
---

They did not canonicalize the package base directory path so it would
always fail when the package base path contained symlinks


Diffs
-

  src/kpackage/package.cpp eb4a09b987970e89f28587426b21d63731634087 
  src/kpackage/private/package_p.h e451412fa02c88113aa4c7bbca2dcda3432b2b02 

Diff: https://git.reviewboard.kde.org/r/122733/diff/


Testing
---

Files inside the package are now found although the install location contains a 
symlink


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122312: Use sysconf() instead of relying on UT_NAMESIZE

2015-02-26 Thread Alex Richardson 

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

(Updated Feb. 26, 2015, 6:53 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Raphael Kubo da Costa.


Repository: user-manager


Description
---

Use sysconf() instead of relying on UT_NAMESIZE

This will work on any UNIX system even those that don't have utmp.h such
as FreeBSD.


Diffs
-

  src/accountinfo.cpp 68fc865 

Diff: https://git.reviewboard.kde.org/r/122312/diff/


Testing
---

compiles, runs and shows user name too long tooltip when I enter a really long 
name


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122619: [KPluginMetaData] Add support for Hidden key

2015-02-19 Thread Alex Richardson 

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

Ship it!


Now that there is a use case for it, it makes sense to add it.

- Alex Richardson


On Feb. 18, 2015, 9:44 a.m., Sebastian Kügler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122619/
 ---
 
 (Updated Feb. 18, 2015, 9:44 a.m.)
 
 
 Review request for KDE Frameworks, Plasma, Alex Richardson, David Faure, and 
 Marco Martin.
 
 
 Repository: kcoreaddons
 
 
 Description
 ---
 
 Add support for Hidden key
 
 This key does not make a lot of sense for binary plugins (those loaded
 from .so files which retrieve their metadata from the json-embedded
 data), but is useful for plugins loaded from .desktop files.
 This allows us to override the visibility of installed plugins by the
 user.
 
 I'm only considering the Hidden key here. I am not sure how the
 NoDisplay key (which seems to be used here and there), so maybe we
 should also consider NoDisplay. This is the most basic patch we can
 achieve the desired results with, however.
 
 
 Diffs
 -
 
   autotests/data/hiddenplugin.desktop PRE-CREATION 
   autotests/kpluginmetadatatest.cpp 8a14f57dd0a8650f41185290da966f036cb66fb6 
   src/lib/plugin/desktopfileparser.cpp 
 b1b5440b48e4fd412932a7d7e794d641b1406699 
   src/lib/plugin/kpluginmetadata.h 7ca6747c766d92be828fa3350d088a2f8b1b12f4 
   src/lib/plugin/kpluginmetadata.cpp cf90ebc204186f26a3199753fb8c97922a278bd1 
 
 Diff: https://git.reviewboard.kde.org/r/122619/diff/
 
 
 Testing
 ---
 
 * Added autotest to make sure both cases work
 * Tried to hide a KPackage by adding a Hidden=true to the metadata.desktop 
 file
 
 Both work as expected with this patch.
 
 
 Thanks,
 
 Sebastian Kügler
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121738: Let kcmkded also handle kded modules that use JSON metadata

2015-02-03 Thread Alex Richardson 

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

(Updated Feb. 3, 2015, 11:32 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-desktop


Description
---

Let kcmkded also handle kded modules that use JSON metadata


Diffs
-

  kcms/kded/kcmkded.h 9afa1881f56d2f913b7e4f7eba5e556795ac5551 
  kcms/kded/kcmkded.cpp 6902f3a8c30af1ae43761c53b3b235e12f1fba9c 

Diff: https://git.reviewboard.kde.org/r/121738/diff/


Testing
---


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 122312: Use sysconf() instead of relying on UT_NAMESIZE

2015-01-29 Thread Alex Richardson 

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

Review request for Plasma and Raphael Kubo da Costa.


Repository: user-manager


Description
---

Use sysconf() instead of relying on UT_NAMESIZE

This will work on any UNIX system even those that don't have utmp.h such
as FreeBSD.


Diffs
-

  src/accountinfo.cpp 68fc865 

Diff: https://git.reviewboard.kde.org/r/122312/diff/


Testing
---

compiles, runs and shows user name too long tooltip when I enter a really long 
name


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121762: Fix build on FreeBSD

2015-01-18 Thread Alex Richardson 

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

(Updated Jan. 18, 2015, 8 p.m.)


Review request for Plasma and Raphael Kubo da Costa.


Repository: plasma-desktop


Description
---

5 Commits:

---

Fix compilation of geometry_parser.cpp with clang 3.4

Apparently the default instantiation depth of 256 (with clang 3.4) is not
enough to compile geometry_parser.cpp. It produces the following error:

/usr/local/include/boost/type_traits/is_reference.hpp:38:62: fatal error: 
recursive template instantiation exceeded maximum depth of 256
BOOST_TT_AUX_BOOL_TRAIT_DEF1(is_reference,T,::boost::detail::is_reference_implT::value)
 ^
/usr/local/include/boost/type_traits/detail/bool_trait_def.hpp:69:30: note: 
expanded from macro 'BOOST_TT_AUX_BOOL_TRAIT_DEF1'
BOOST_TT_AUX_BOOL_C_BASE(C) \
 ^
/usr/local/include/boost/type_traits/detail/bool_trait_def.hpp:63:81: note: 
expanded from macro 'BOOST_TT_AUX_BOOL_C_BASE'

And after that a few hundred lines of template instantiation details.
Rasing the limit to 512 allows successfully compiling these files.

---


Fix build of kbpreviewframe.cpp with clang

Clang doesn't allow variable length arrays of non-POD type

---

Add missing includes of cmath and cstdlib

---

Fix build of kapplymousetheme if X11 isn't installed to /usr

---

Use a proper CMake find module for libcanberra instead of pkg_check_modules

This fixes the build on e.g. FreeBSD where libcanberra is not in /usr/lib
but /usr/local/lib so that the linker complains that it cannot find
-lcanberra since the CMake variable from pkg_check_modules does not contain
an absolute path.

The CMake module was copied from the KMix repository. If there are any more
users of libcanberra it might make sense to add this to extra-cmake-modules


Diffs (updated)
-

  kcms/keyboard/CMakeLists.txt 3db5dc3f938bbe25259067783bda4fdd882f60f6 
  kcms/keyboard/kcmmisc.cpp 9aed7de2a8a63aa546dc9484eea0edadcc64ec66 
  kcms/keyboard/preview/kbpreviewframe.cpp 
9735eb02ac6875bb3905fb0bf8183a9d97e0e3e4 
  kcms/keyboard/tests/CMakeLists.txt bd62c7d281766bc5c04b0b1e933861d258241cf7 
  kcms/phonon/CMakeLists.txt 8f964e2041cc812bf70f6a48e2915427f088b990 
  cmake/modules/FindCanberra.cmake PRE-CREATION 
  containments/folder/plugin/positioner.cpp 
c24827197bf20b4cb55a6b885f023074f179159c 
  kcms/input/CMakeLists.txt a2b72a45a3b694b7fa367abf33d0752b0dc9734b 

Diff: https://git.reviewboard.kde.org/r/121762/diff/


Testing
---

compiles


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122133: Fix compilation error by using std::vector rather than a conventional array.

2015-01-18 Thread Alex Richardson 

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


Fixed by https://git.reviewboard.kde.org/r/121762 which I just pushed

- Alex Richardson


On Jan. 18, 2015, 7:32 p.m., Roney Gomes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122133/
 ---
 
 (Updated Jan. 18, 2015, 7:32 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-desktop
 
 
 Description
 ---
 
 I couldn't build plasma-desktop from master since the compiler was throwing 
 the following error in keyboard/preview/kbpreviewframe.cpp, starting on line 
 184: error: variable length array of non-POD element type 'QPoint'. 
 
 The main source of the problem was the allocation of QPoint temp[cordi_count] 
 bound to a non constant variable.
 
 This patch makes use of some C++11 features, is that a problem?
 
 
 Diffs
 -
 
   kcms/keyboard/preview/kbpreviewframe.cpp 9735eb0 
 
 Diff: https://git.reviewboard.kde.org/r/122133/diff/
 
 
 Testing
 ---
 
 plasma-desktop recompiled with success.
 
 
 Thanks,
 
 Roney Gomes
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121762: Fix build on FreeBSD

2015-01-18 Thread Alex Richardson 

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

(Updated Jan. 18, 2015, 9:56 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Raphael Kubo da Costa.


Repository: plasma-desktop


Description
---

5 Commits:

---

Fix compilation of geometry_parser.cpp with clang 3.4

Apparently the default instantiation depth of 256 (with clang 3.4) is not
enough to compile geometry_parser.cpp. It produces the following error:

/usr/local/include/boost/type_traits/is_reference.hpp:38:62: fatal error: 
recursive template instantiation exceeded maximum depth of 256
BOOST_TT_AUX_BOOL_TRAIT_DEF1(is_reference,T,::boost::detail::is_reference_implT::value)
 ^
/usr/local/include/boost/type_traits/detail/bool_trait_def.hpp:69:30: note: 
expanded from macro 'BOOST_TT_AUX_BOOL_TRAIT_DEF1'
BOOST_TT_AUX_BOOL_C_BASE(C) \
 ^
/usr/local/include/boost/type_traits/detail/bool_trait_def.hpp:63:81: note: 
expanded from macro 'BOOST_TT_AUX_BOOL_C_BASE'

And after that a few hundred lines of template instantiation details.
Rasing the limit to 512 allows successfully compiling these files.

---


Fix build of kbpreviewframe.cpp with clang

Clang doesn't allow variable length arrays of non-POD type

---

Add missing includes of cmath and cstdlib

---

Fix build of kapplymousetheme if X11 isn't installed to /usr

---

Use a proper CMake find module for libcanberra instead of pkg_check_modules

This fixes the build on e.g. FreeBSD where libcanberra is not in /usr/lib
but /usr/local/lib so that the linker complains that it cannot find
-lcanberra since the CMake variable from pkg_check_modules does not contain
an absolute path.

The CMake module was copied from the KMix repository. If there are any more
users of libcanberra it might make sense to add this to extra-cmake-modules


Diffs
-

  kcms/keyboard/CMakeLists.txt 3db5dc3f938bbe25259067783bda4fdd882f60f6 
  kcms/keyboard/kcmmisc.cpp 9aed7de2a8a63aa546dc9484eea0edadcc64ec66 
  kcms/keyboard/preview/kbpreviewframe.cpp 
9735eb02ac6875bb3905fb0bf8183a9d97e0e3e4 
  kcms/keyboard/tests/CMakeLists.txt bd62c7d281766bc5c04b0b1e933861d258241cf7 
  kcms/phonon/CMakeLists.txt 8f964e2041cc812bf70f6a48e2915427f088b990 
  cmake/modules/FindCanberra.cmake PRE-CREATION 
  containments/folder/plugin/positioner.cpp 
c24827197bf20b4cb55a6b885f023074f179159c 
  kcms/input/CMakeLists.txt a2b72a45a3b694b7fa367abf33d0752b0dc9734b 

Diff: https://git.reviewboard.kde.org/r/121762/diff/


Testing
---

compiles


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121761: Fix build on FreeBSD

2015-01-16 Thread Alex Richardson 

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

(Updated Jan. 17, 2015, 12:01 vorm.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Raphael Kubo da Costa.


Repository: plasma-workspace


Description
---

Add missing errno.h include

Wrap Linux-only signals in #ifdef Q_OS_LINUX

Fix build with X11 not installed to /usr


Diffs
-

  drkonqi/detachedprocessmonitor.cpp 85a87874c6e9ce47856a01195b42aef9f3a4991a 
  drkonqi/systeminformation.cpp 8f5fc7fe789a4bbe23f178e7e940ba1d1a1b59db 
  ksmserver/CMakeLists.txt 84a8aa393dfb6ed4671094d1fccbb3c79c53f9af 
  ksmserver/screenlocker/greeter/authenticator.cpp 
ad60f0bd0076cd9c8c3875f13dc92b5da253bb1a 
  ksmserver/screenlocker/greeter/autotests/killtest.cpp 
6f2ef114459b5d641e357f3a817b82e7af2e72a3 
  libkworkspace/CMakeLists.txt 53ce6108bd91f87108206fca02ac303dadf069e1 

Diff: https://git.reviewboard.kde.org/r/121761/diff/


Testing
---

compiles, still compiles on linux


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121763: Fix build on FreeBSD

2015-01-16 Thread Alex Richardson 

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

(Updated Jan. 17, 2015, 12:05 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Raphael Kubo da Costa.


Repository: kinfocenter


Description
---

We can't call the KCM devinfo there since we need to link against -ldevinfo
and CMake would try to link to the KCM instead of the library in that case.


Diffs
-

  Modules/devinfo/CMakeLists.txt 2395ce3dc83080e959cbfa9f97724218cdff6bd9 
  Modules/devinfo/devinfo.desktop 1bc98a06b9a567ee45c51c7f25ee5ad6b43750d7 
  Modules/info/CMakeLists.txt 7b0e0affd13d6b556749f4a350012f27fb43ae0b 
  Modules/pci/CMakeLists.txt 5b2b30a0c3791a8add00a380e61469a96cd66ae1 

Diff: https://git.reviewboard.kde.org/r/121763/diff/


Testing
---


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 121761: Fix build on FreeBSD

2014-12-30 Thread Alex Richardson 

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

Review request for Plasma and Raphael Kubo da Costa.


Repository: plasma-workspace


Description
---

Add missing errno.h include

Wrap Linux-only signals in #ifdef Q_OS_LINUX

Fix build with X11 not installed to /usr


Diffs
-

  drkonqi/detachedprocessmonitor.cpp 85a87874c6e9ce47856a01195b42aef9f3a4991a 
  drkonqi/systeminformation.cpp 8f5fc7fe789a4bbe23f178e7e940ba1d1a1b59db 
  ksmserver/CMakeLists.txt 84a8aa393dfb6ed4671094d1fccbb3c79c53f9af 
  ksmserver/screenlocker/greeter/authenticator.cpp 
ad60f0bd0076cd9c8c3875f13dc92b5da253bb1a 
  ksmserver/screenlocker/greeter/autotests/killtest.cpp 
6f2ef114459b5d641e357f3a817b82e7af2e72a3 
  libkworkspace/CMakeLists.txt 53ce6108bd91f87108206fca02ac303dadf069e1 

Diff: https://git.reviewboard.kde.org/r/121761/diff/


Testing
---

compiles, still compiles on linux


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 121762: Fix build on FreeBSD

2014-12-30 Thread Alex Richardson 

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

Review request for Plasma and Raphael Kubo da Costa.


Repository: plasma-desktop


Description
---

Fix build with clang 3.4

Clang doesn't allow variable length arrays of non-POD data types
Also it seems that the default template instantion depth is not
enough to compile geometry_parser.cpp

--

Add missing includes of cmath and cstdlib

-

Fix clang warning -Wmismatched-tags

-


Fix build of geometry_parser test with clang

We need to increase the template instantiation depth

-

Fix more -Wmismatched-tag warnings



Fix build of kapplymousetheme if X11 isn't installed to /usr




Fix build of phonon KCM if libcanberra is not in /usr


Diffs
-

  kcms/keyboard/bindings.h d59790752ced2ec8fd77269f75012e74ccda4cc4 
  kcms/keyboard/flags.h 3957a983f06044e8f5ed130afc4774f8c644ed8c 
  kcms/keyboard/kcm_add_layout_dialog.h 
a2c0ac5ebc264d69555063668cb68be5255b3030 
  kcms/keyboard/kcm_keyboard.h c88fe3dd8540df07eb886b3119dfbe30fab2844b 
  kcms/keyboard/kcm_keyboard_widget.h 5994ea4815d6398afaf6765b77a2bc8cd349f780 
  kcms/keyboard/kcm_view_models.h 872e26185248c6dfb8d8808115865ae18596cbea 
  kcms/keyboard/kcmmisc.cpp 9aed7de2a8a63aa546dc9484eea0edadcc64ec66 
  containments/folder/plugin/positioner.cpp 
c24827197bf20b4cb55a6b885f023074f179159c 
  kcms/input/CMakeLists.txt a2b72a45a3b694b7fa367abf33d0752b0dc9734b 
  kcms/keyboard/CMakeLists.txt 3db5dc3f938bbe25259067783bda4fdd882f60f6 
  kcms/keyboard/keyboard_daemon.h 4d7587f9113edea31dd07f23d658818941731df2 
  kcms/keyboard/layout_tray_icon.h 8338bf26307627747fb434ae8e0903050142aef4 
  kcms/keyboard/layouts_menu.h db2f3ff5844e16340ad3ca6102e6b1c4866ad8db 
  kcms/keyboard/preview/kbpreviewframe.cpp 
9735eb02ac6875bb3905fb0bf8183a9d97e0e3e4 
  kcms/keyboard/tests/CMakeLists.txt bd62c7d281766bc5c04b0b1e933861d258241cf7 
  kcms/keyboard/xkb_helper.h d3dab1d9b05784ae463db44ab941119e5b214986 
  kcms/phonon/CMakeLists.txt 8f964e2041cc812bf70f6a48e2915427f088b990 

Diff: https://git.reviewboard.kde.org/r/121762/diff/


Testing
---

compiles


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121761: Fix build on FreeBSD

2014-12-30 Thread Alex Richardson 


 On Dez. 30, 2014, 6:42 nachm., Raphael Kubo da Costa wrote:
  I'd keep the `errno.h` includes alphabetically sorted, and commit each of 
  those changes (errno, then X11 dependencies etc) separately. Other than 
  that, looks good to me.

Those are three separate commits. I thought having 3 RR just for those small 
issues is too much


- Alex


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


On Dez. 30, 2014, 6:33 nachm., Alex Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/121761/
 ---
 
 (Updated Dez. 30, 2014, 6:33 nachm.)
 
 
 Review request for Plasma and Raphael Kubo da Costa.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 Add missing errno.h include
 
 Wrap Linux-only signals in #ifdef Q_OS_LINUX
 
 Fix build with X11 not installed to /usr
 
 
 Diffs
 -
 
   drkonqi/detachedprocessmonitor.cpp 85a87874c6e9ce47856a01195b42aef9f3a4991a 
   drkonqi/systeminformation.cpp 8f5fc7fe789a4bbe23f178e7e940ba1d1a1b59db 
   ksmserver/CMakeLists.txt 84a8aa393dfb6ed4671094d1fccbb3c79c53f9af 
   ksmserver/screenlocker/greeter/authenticator.cpp 
 ad60f0bd0076cd9c8c3875f13dc92b5da253bb1a 
   ksmserver/screenlocker/greeter/autotests/killtest.cpp 
 6f2ef114459b5d641e357f3a817b82e7af2e72a3 
   libkworkspace/CMakeLists.txt 53ce6108bd91f87108206fca02ac303dadf069e1 
 
 Diff: https://git.reviewboard.kde.org/r/121761/diff/
 
 
 Testing
 ---
 
 compiles, still compiles on linux
 
 
 Thanks,
 
 Alex Richardson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 121763: Fix build on FreeBSD

2014-12-30 Thread Alex Richardson 

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

Review request for Plasma and Raphael Kubo da Costa.


Repository: kinfocenter


Description
---

We can't call the KCM devinfo there since we need to link against -ldevinfo
and CMake would try to link to the KCM instead of the library in that case.


Diffs
-

  Modules/devinfo/CMakeLists.txt 2395ce3dc83080e959cbfa9f97724218cdff6bd9 
  Modules/devinfo/devinfo.desktop 1bc98a06b9a567ee45c51c7f25ee5ad6b43750d7 
  Modules/info/CMakeLists.txt 7b0e0affd13d6b556749f4a350012f27fb43ae0b 
  Modules/pci/CMakeLists.txt 5b2b30a0c3791a8add00a380e61469a96cd66ae1 

Diff: https://git.reviewboard.kde.org/r/121763/diff/


Testing
---


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121762: Fix build on FreeBSD

2014-12-30 Thread Alex Richardson 


 On Dez. 30, 2014, 7:05 nachm., Raphael Kubo da Costa wrote:
  kcms/keyboard/CMakeLists.txt, line 127
  https://git.reviewboard.kde.org/r/121762/diff/1/?file=337446#file337446line127
 
  `-fexceptions` is already set above.

AFAIK set_source_files_properties doesn't append, it replaces. Fixed in the new 
patch by using set_property(SOURCE ... APPEND)


- Alex


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


On Dez. 30, 2014, 6:35 nachm., Alex Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/121762/
 ---
 
 (Updated Dez. 30, 2014, 6:35 nachm.)
 
 
 Review request for Plasma and Raphael Kubo da Costa.
 
 
 Repository: plasma-desktop
 
 
 Description
 ---
 
 Fix build with clang 3.4
 
 Clang doesn't allow variable length arrays of non-POD data types
 Also it seems that the default template instantion depth is not
 enough to compile geometry_parser.cpp
 
 --
 
 Add missing includes of cmath and cstdlib
 
 -
 
 Fix clang warning -Wmismatched-tags
 
 -
 
 
 Fix build of geometry_parser test with clang
 
 We need to increase the template instantiation depth
 
 -
 
 Fix more -Wmismatched-tag warnings
 
 
 
 Fix build of kapplymousetheme if X11 isn't installed to /usr
 
 
 
 
 Fix build of phonon KCM if libcanberra is not in /usr
 
 
 Diffs
 -
 
   kcms/keyboard/bindings.h d59790752ced2ec8fd77269f75012e74ccda4cc4 
   kcms/keyboard/flags.h 3957a983f06044e8f5ed130afc4774f8c644ed8c 
   kcms/keyboard/kcm_add_layout_dialog.h 
 a2c0ac5ebc264d69555063668cb68be5255b3030 
   kcms/keyboard/kcm_keyboard.h c88fe3dd8540df07eb886b3119dfbe30fab2844b 
   kcms/keyboard/kcm_keyboard_widget.h 
 5994ea4815d6398afaf6765b77a2bc8cd349f780 
   kcms/keyboard/kcm_view_models.h 872e26185248c6dfb8d8808115865ae18596cbea 
   kcms/keyboard/kcmmisc.cpp 9aed7de2a8a63aa546dc9484eea0edadcc64ec66 
   containments/folder/plugin/positioner.cpp 
 c24827197bf20b4cb55a6b885f023074f179159c 
   kcms/input/CMakeLists.txt a2b72a45a3b694b7fa367abf33d0752b0dc9734b 
   kcms/keyboard/CMakeLists.txt 3db5dc3f938bbe25259067783bda4fdd882f60f6 
   kcms/keyboard/keyboard_daemon.h 4d7587f9113edea31dd07f23d658818941731df2 
   kcms/keyboard/layout_tray_icon.h 8338bf26307627747fb434ae8e0903050142aef4 
   kcms/keyboard/layouts_menu.h db2f3ff5844e16340ad3ca6102e6b1c4866ad8db 
   kcms/keyboard/preview/kbpreviewframe.cpp 
 9735eb02ac6875bb3905fb0bf8183a9d97e0e3e4 
   kcms/keyboard/tests/CMakeLists.txt bd62c7d281766bc5c04b0b1e933861d258241cf7 
   kcms/keyboard/xkb_helper.h d3dab1d9b05784ae463db44ab941119e5b214986 
   kcms/phonon/CMakeLists.txt 8f964e2041cc812bf70f6a48e2915427f088b990 
 
 Diff: https://git.reviewboard.kde.org/r/121762/diff/
 
 
 Testing
 ---
 
 compiles
 
 
 Thanks,
 
 Alex Richardson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121762: Fix build on FreeBSD

2014-12-30 Thread Alex Richardson 

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

(Updated Dez. 30, 2014, 9:37 nachm.)


Review request for Plasma and Raphael Kubo da Costa.


Changes
---

Fixed issues and removed -Wmismatched-tags fixes from this RR since they are 
unrelated


Repository: plasma-desktop


Description (updated)
---

5 Commits:

---

Fix compilation of geometry_parser.cpp with clang 3.4

Apparently the default instantiation depth of 256 (with clang 3.4) is not
enough to compile geometry_parser.cpp. It produces the following error:

/usr/local/include/boost/type_traits/is_reference.hpp:38:62: fatal error: 
recursive template instantiation exceeded maximum depth of 256
BOOST_TT_AUX_BOOL_TRAIT_DEF1(is_reference,T,::boost::detail::is_reference_implT::value)
 ^
/usr/local/include/boost/type_traits/detail/bool_trait_def.hpp:69:30: note: 
expanded from macro 'BOOST_TT_AUX_BOOL_TRAIT_DEF1'
BOOST_TT_AUX_BOOL_C_BASE(C) \
 ^
/usr/local/include/boost/type_traits/detail/bool_trait_def.hpp:63:81: note: 
expanded from macro 'BOOST_TT_AUX_BOOL_C_BASE'

And after that a few hundred lines of template instantiation details.
Rasing the limit to 512 allows successfully compiling these files.

---


Fix build of kbpreviewframe.cpp with clang

Clang doesn't allow variable length arrays of non-POD type

---

Add missing includes of cmath and cstdlib

---

Fix build of kapplymousetheme if X11 isn't installed to /usr

---

Use a proper CMake find module for libcanberra instead of pkg_check_modules

This fixes the build on e.g. FreeBSD where libcanberra is not in /usr/lib
but /usr/local/lib so that the linker complains that it cannot find
-lcanberra since the CMake variable from pkg_check_modules does not contain
an absolute path.

The CMake module was copied from the KMix repository. If there are any more
users of libcanberra it might make sense to add this to extra-cmake-modules


Diffs (updated)
-

  cmake/modules/FindCanberra.cmake PRE-CREATION 
  containments/folder/plugin/positioner.cpp 
c24827197bf20b4cb55a6b885f023074f179159c 
  kcms/input/CMakeLists.txt a2b72a45a3b694b7fa367abf33d0752b0dc9734b 
  kcms/keyboard/CMakeLists.txt 3db5dc3f938bbe25259067783bda4fdd882f60f6 
  kcms/keyboard/kcmmisc.cpp 9aed7de2a8a63aa546dc9484eea0edadcc64ec66 
  kcms/keyboard/preview/kbpreviewframe.cpp 
9735eb02ac6875bb3905fb0bf8183a9d97e0e3e4 
  kcms/keyboard/tests/CMakeLists.txt bd62c7d281766bc5c04b0b1e933861d258241cf7 
  kcms/phonon/CMakeLists.txt 8f964e2041cc812bf70f6a48e2915427f088b990 

Diff: https://git.reviewboard.kde.org/r/121762/diff/


Testing
---

compiles


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121732: Port kcm_kded away from kdelibs4support

2014-12-29 Thread Alex Richardson 

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

(Updated Dec. 29, 2014, 2:41 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-desktop


Description
---

Port kcm_kded away from kdelibs4support


Diffs
-

  kcms/kded/CMakeLists.txt 07217e0cfe5490aab8099940de2ea26139fc52fe 
  kcms/kded/kcmkded.h 272a6191a6c668dbc486673c724d5da02bbc5001 
  kcms/kded/kcmkded.cpp 34a1acf6c6105d3ef7202663417603b47478e359 

Diff: https://git.reviewboard.kde.org/r/121732/diff/


Testing
---

Compiles


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 121738: Let kcmkded also handle kded modules that use JSON metadata

2014-12-29 Thread Alex Richardson 

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

Review request for Plasma.


Repository: plasma-desktop


Description
---

Let kcmkded also handle kded modules that use JSON metadata


Diffs
-

  kcms/kded/kcmkded.h 9afa1881f56d2f913b7e4f7eba5e556795ac5551 
  kcms/kded/kcmkded.cpp 6902f3a8c30af1ae43761c53b3b235e12f1fba9c 

Diff: https://git.reviewboard.kde.org/r/121738/diff/


Testing
---


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 121732: Port kcm_kded away from kdelibs4support

2014-12-28 Thread Alex Richardson 

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

Review request for Plasma.


Repository: plasma-desktop


Description
---

Port kcm_kded away from kdelibs4support


Diffs
-

  kcms/kded/CMakeLists.txt 07217e0cfe5490aab8099940de2ea26139fc52fe 
  kcms/kded/kcmkded.h 272a6191a6c668dbc486673c724d5da02bbc5001 
  kcms/kded/kcmkded.cpp 34a1acf6c6105d3ef7202663417603b47478e359 

Diff: https://git.reviewboard.kde.org/r/121732/diff/


Testing
---

Compiles


Thanks,

Alex Richardson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel