D16929: Remove dead search provider

2018-11-16 Thread Christophe Giboudeaux
cgiboudeaux added a comment.


  In D16929#360570 , @vkrause wrote:
  
  > Remove more dead search providers, just checking for non-resolving domains 
isn't enough.
  
  
  mmh.. http://www.cnrtl.fr/definition/$something works fine here

REPOSITORY
  R241 KIO

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

To: vkrause
Cc: cgiboudeaux, kde-frameworks-devel, michaelh, ngraham, bruns


D16927: KTextEditor : disconnect contextmenu from all aboutToXXContextMenu receivers

2018-11-16 Thread René J . V . Bertin
rjvbb added a comment.


  >   Btw., the 2 nullptr in the disconnect can be left out, or?
  
  Not to my understanding. From the docs:
  
Disconnect everything connected to a specific signal:
  disconnect(myObject, SIGNAL(mySignal()), 0, 0);

equivalent to the non-static overloaded function
  myObject->disconnect(SIGNAL(mySignal()));
  
  I don't think the 2nd form would match the rest of the code, do you?

REPOSITORY
  R39 KTextEditor

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

To: rjvbb, #ktexteditor, #frameworks, cullmann
Cc: cullmann, kde-frameworks-devel, kwrite-devel, michaelh, ngraham, bruns, 
demsking, head7, kfunk, sars, dhaumann


D16927: KTextEditor : disconnect contextmenu from all aboutToXXContextMenu receivers

2018-11-16 Thread Christoph Cullmann
cullmann added a comment.


  Btw., the 2 nullptr in the disconnect can be left out, or?
  + a comment for  the future would be nice in the code

REPOSITORY
  R39 KTextEditor

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

To: rjvbb, #ktexteditor, #frameworks, cullmann
Cc: cullmann, kde-frameworks-devel, kwrite-devel, michaelh, ngraham, bruns, 
demsking, head7, kfunk, sars, dhaumann


D16927: KTextEditor : disconnect contextmenu from all aboutToXXContextMenu receivers

2018-11-16 Thread Christoph Cullmann
cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.


  Sounds reasonable.

REPOSITORY
  R39 KTextEditor

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

To: rjvbb, #ktexteditor, #frameworks, cullmann
Cc: cullmann, kde-frameworks-devel, kwrite-devel, michaelh, ngraham, bruns, 
demsking, head7, kfunk, sars, dhaumann


D16934: Add help-browser symlink to 16px and 22px directories

2018-11-16 Thread TrickyRicky
trickyricky26 edited the summary of this revision.
trickyricky26 added a reviewer: Breeze.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #breeze
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D16934: Add help-browser symlink to 16px and 22px directories

2018-11-16 Thread TrickyRicky
trickyricky26 created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
trickyricky26 requested review of this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  fix-help-browser-symlink (branched from master)

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

AFFECTED FILES
  icons-dark/apps/16/help-browser.svg
  icons-dark/apps/22/help-browser.svg
  icons/apps/16/help-browser.svg
  icons/apps/22/help-browser.svg

To: trickyricky26
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread TrickyRicky
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-sort-options-icon (branched from master)

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: cfeck, emateli, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread TrickyRicky
trickyricky26 edited the summary of this revision.
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-sort-options-icon (branched from master)

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: cfeck, emateli, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread TrickyRicky
trickyricky26 updated this revision to Diff 45611.
trickyricky26 added a comment.
This revision is now accepted and ready to land.


  - Fix naming of ascending and descending versions; make ascending versions 
less confusing; fix symlink sort-name
  
  I have flipped the naming of the ascending and descending versions to be 
correct.
  The arrow in the ascending versions is now always at the top and does not cut 
off lines.
  The symbolic link `sort-name` was changed from `view-sort-ascending` to 
`view-sort-ascending-name`

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16905?vs=45599=45611

BRANCH
  add-sort-options-icon (branched from master)

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

AFFECTED FILES
  icons-dark/actions/16/sort-name.svg
  icons-dark/actions/16/sort.svg
  icons-dark/actions/16/view-sort-ascending-name.svg
  icons-dark/actions/16/view-sort-ascending.svg
  icons-dark/actions/16/view-sort-descending-name.svg
  icons-dark/actions/16/view-sort-descending.svg
  icons-dark/actions/16/view-sort.svg
  icons-dark/actions/22/sort-name.svg
  icons-dark/actions/22/sort.svg
  icons-dark/actions/22/view-sort-ascending-name.svg
  icons-dark/actions/22/view-sort-ascending.svg
  icons-dark/actions/22/view-sort-descending-name.svg
  icons-dark/actions/22/view-sort-descending.svg
  icons-dark/actions/22/view-sort.svg
  icons-dark/actions/24/sort-name.svg
  icons-dark/actions/24/view-sort-ascending-name.svg
  icons-dark/actions/24/view-sort-ascending.svg
  icons-dark/actions/24/view-sort-descending-name.svg
  icons-dark/actions/24/view-sort-descending.svg
  icons-dark/actions/24/view-sort.svg
  icons-dark/actions/32/view-sort-ascending-name.svg
  icons-dark/actions/32/view-sort-ascending.svg
  icons-dark/actions/32/view-sort-descending-name.svg
  icons-dark/actions/32/view-sort-descending.svg
  icons-dark/actions/32/view-sort.svg
  icons/actions/16/sort-name.svg
  icons/actions/16/sort.svg
  icons/actions/16/view-sort-ascending-name.svg
  icons/actions/16/view-sort-ascending.svg
  icons/actions/16/view-sort-descending-name.svg
  icons/actions/16/view-sort-descending.svg
  icons/actions/16/view-sort.svg
  icons/actions/22/sort-name.svg
  icons/actions/22/sort.svg
  icons/actions/22/view-sort-ascending-name.svg
  icons/actions/22/view-sort-ascending.svg
  icons/actions/22/view-sort-descending-name.svg
  icons/actions/22/view-sort-descending.svg
  icons/actions/22/view-sort.svg
  icons/actions/24/sort-name.svg
  icons/actions/24/view-sort-ascending-name.svg
  icons/actions/24/view-sort-ascending.svg
  icons/actions/24/view-sort-descending-name.svg
  icons/actions/24/view-sort-descending.svg
  icons/actions/24/view-sort.svg
  icons/actions/32/view-sort-ascending-name.svg
  icons/actions/32/view-sort-ascending.svg
  icons/actions/32/view-sort-descending-name.svg
  icons/actions/32/view-sort-descending.svg
  icons/actions/32/view-sort.svg

To: trickyricky26, #vdg, #breeze, ndavis
Cc: cfeck, emateli, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16929: Remove dead search provider

2018-11-16 Thread Volker Krause
vkrause retitled this revision from "Remove dmoz search provider" to "Remove 
dead search provider".
vkrause edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: vkrause
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D16929: Remove dmoz search provider

2018-11-16 Thread Volker Krause
vkrause updated this revision to Diff 45609.
vkrause added a comment.


  Remove more dead search providers, just checking for non-resolving domains 
isn't enough.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16929?vs=45605=45609

BRANCH
  master

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

AFFECTED FILES
  src/urifilters/ikws/searchproviders/dictfr.desktop
  src/urifilters/ikws/searchproviders/dmoz.desktop
  src/urifilters/ikws/searchproviders/ibl.desktop

To: vkrause
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D16931: Extract more tags from exif metadata

2018-11-16 Thread Alexander Stippich
astippich created this revision.
astippich added a reviewer: bruns.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
astippich requested review of this revision.

REVISION SUMMARY
  Extract more metadata from exif that can
  be easily mapped to the available properties.
  This includes description, copyright, software
  and artist.
  
  BUG: 341162

REPOSITORY
  R286 KFileMetaData

BRANCH
  exif_image_description

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

AFFECTED FILES
  autotests/exiv2extractortest.cpp
  src/extractors/exiv2extractor.cpp

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams


D16931: Extract more tags from exif metadata

2018-11-16 Thread Alexander Stippich
astippich added a dependency: D16617: fix extraction of GPS altitude for exif 
data.

REPOSITORY
  R286 KFileMetaData

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

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams


D16617: fix extraction of GPS altitude for exif data

2018-11-16 Thread Alexander Stippich
astippich added a dependent revision: D16931: Extract more tags from exif 
metadata.

REPOSITORY
  R286 KFileMetaData

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

To: astippich, #frameworks, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams


D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread TrickyRicky
trickyricky26 added a comment.


  So you're saying the direction of the arrows is consisten with the current 
way, as long as the ascending and descending names are switched, correct?

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: cfeck, emateli, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16930: Android: improve clang support

2018-11-16 Thread Volker Krause
vkrause accepted this revision.
vkrause added a comment.
This revision is now accepted and ready to land.


  Looks ok to me, less stuff to set manually is always good.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: apol, #build_system, #android, vkrause
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns


D12114: Revive 'Description' property for DublinCore metadata

2018-11-16 Thread Alexander Stippich
This revision was automatically updated to reflect the committed changes.
Closed by commit R286:103f86746b8a: Revive Description property for 
DublinCore metadata (authored by astippich).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12114?vs=45234=45607

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

AFFECTED FILES
  autotests/epubextractortest.cpp
  autotests/odfextractortest.cpp
  autotests/office2007extractortest.cpp
  autotests/samplefiles/test.odt
  src/extractors/dublincoreextractor.cpp
  src/extractors/epubextractor.cpp
  src/extractors/odfextractor.cpp
  src/extractors/office2007extractor.cpp

To: astippich, #baloo, #frameworks, bruns
Cc: kde-frameworks-devel, bruns, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, abrahams


D16930: Android: improve clang support

2018-11-16 Thread Aleix Pol Gonzalez
apol created this revision.
apol added reviewers: Build System, Android, vkrause.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
apol requested review of this revision.

REVISION SUMMARY
  Infer the toolchain we are using through cmake's detection of ranlib
  instead of asking the user to fill it.
  It's only used in androiddeployqt and cmake expects it to be version
  "clang" for clang builds while androiddeployqt expects it to be "4.9".

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  toolchain/Android.cmake
  toolchain/ECMAndroidDeployQt.cmake
  toolchain/deployment-file.json.in
  toolchain/specifydependencies.cmake

To: apol, #build_system, #android, vkrause
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns


D16929: Remove dmoz search provider

2018-11-16 Thread Volker Krause
vkrause created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
vkrause requested review of this revision.

REVISION SUMMARY
  Service is dead.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/urifilters/ikws/searchproviders/dmoz.desktop

To: vkrause
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D16196: add a description property to KFileMetaData

2018-11-16 Thread Alexander Stippich
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R286:32c1e88ed922: add a description property to KFileMetaData 
(authored by astippich).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16196?vs=44832=45604

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

AFFECTED FILES
  src/properties.h
  src/propertyinfo.cpp

To: astippich, mgallien, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams


Re: Regression in Frameworks - DBus Hangs

2018-11-16 Thread Ben Cooksley
On Sat, Nov 17, 2018 at 4:30 AM David Edmundson
 wrote:
>
>
> Don't thank me yet, I locked up one of the build jobs, sorry.
> https://build.kde.org/job/Applications/job/konsole/job/kf5-qt5%20SUSEQt5.9/380/console

Diagnostic output available from D-Bus:

jenkins@6934489e2599:~> qdbus org.kde.konsole-2448 /MainApplication
org.qtproject.Qt.QCoreApplication.quitLockEnabled
true
jenkins@6934489e2599:~> qdbus org.kde.konsole-2448 /MainApplication
org.qtproject.Qt.QGuiApplication.quitOnLastWindowClosed
true

(Because Jenkins started the container, running gdb to get a backtrace
isn't possible)

Based on this i'd say your theory we have a QEventLoopLocker in play
is probably the right one here.
It's a bit of a workaround, but could the test change to just asking
Konsole to exit rather than close it's window?

>
> >Pipeline kf5-qt5 SUSEQt5.9
>
> Output isn't too helpful, though I am surprised to see
>  kf5.kinit.klauncher in the logs when I'm supposedly invoking the process 
> directly.

That happens whenever it sees a new name on the D-Bus server, so is
unrelated (you'll see a few towards the start of the DBusTest output -
that was me doing the above and asking it to quit nicely)

>
> David

Cheers,
Ben


D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread Christoph Feck
cfeck added a comment.


  On lists that are sorted alphabetically A-Z (i.e. ascending), the first item 
is at the top, the last at the bottom. That's why user interfaces show an arrow 
pointing down; they show the direction of item "flow".

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: cfeck, emateli, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread Emirald Mateli
emateli added a comment.


  @trickyricky26 I think we should stick to whatever conventions the KDE 
community has. I checked with Dolphin and the arrow goes up for descending and 
up for ascending, however the VDG makes these calls in the end.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: emateli, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread TrickyRicky
trickyricky26 planned changes to this revision.
trickyricky26 added a comment.


  @emateli
  
  1. You're absolutely right about that, don't know how I missed this. With the 
correct naming, should the arrows still point to the end with longer lines?
  2. They are sorted, but some lines were shortened to make room for the arrow 
so it could stay in the bottom right. Earlier I had a design with the arrows 
moved to the top in the ascending version (I have edited the comment to have 
correct naming):
  
  In D16905#360495 , @trickyricky26 
wrote:
  
  > This is how the set looks with the arrows above the lines in the ascending 
versions:
  >
  > | size  | `view-sort` | `view-sort-descending` | 
`view-sort-ascending` |
  > | - | --- | -- | 
- |
  > | `16px`| F6427148: view-sort-16.svg.png 
   | F6427145: 
view-sort-16-ascending.svg.png    
   | F6427205: view-sort-16-descending-alt.svg.png 
 |
  > | `22px` and `24px` | F6427153: view-sort-22.svg.png 
   | F6427155: 
view-sort-22-ascending.svg.png    
   | F6427208: view-sort-22-descending-alt.svg.png 
 |
  > | `32px`| F6427177: view-sort-32.svg.png 
   | F6427162: 
view-sort-32-ascending.svg.png    
   | F6427210: view-sort-32-descending-alt.svg.png 
 |
  > |
  
  
  Do you feel these are less confusing?

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: emateli, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread Emirald Mateli
emateli added a comment.


  1. Aren't the names in reverse? `view-sort-ascending` shows a list in 
descending order
  2. In `view-sort-descending` the items are not even sorted. Not sure how i 
feel about that.

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-sort-options-icon (branched from master)

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: emateli, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add new generic sorting icons and rename existing alphabetic sorting icons to -name scheme

2018-11-16 Thread TrickyRicky
trickyricky26 retitled this revision from "Add a generic sorting options icon" 
to "Add new generic sorting icons and rename existing alphabetic sorting icons 
to -name scheme".
trickyricky26 edited the summary of this revision.
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-sort-options-icon (branched from master)

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add a generic sorting options icon

2018-11-16 Thread TrickyRicky
trickyricky26 updated this revision to Diff 45599.
trickyricky26 edited the test plan for this revision.
trickyricky26 added a comment.
This revision is now accepted and ready to land.


  - Add new generic sorting icons, rename existing ordered sorting icons to 
-name scheme
  
  (see updated revision description)

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16905?vs=45534=45599

BRANCH
  add-sort-options-icon (branched from master)

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

AFFECTED FILES
  icons-dark/actions/16/sort.svg
  icons-dark/actions/16/view-sort-ascending-name.svg
  icons-dark/actions/16/view-sort-ascending.svg
  icons-dark/actions/16/view-sort-descending-name.svg
  icons-dark/actions/16/view-sort-descending.svg
  icons-dark/actions/16/view-sort.svg
  icons-dark/actions/22/sort.svg
  icons-dark/actions/22/view-sort-ascending-name.svg
  icons-dark/actions/22/view-sort-ascending.svg
  icons-dark/actions/22/view-sort-descending-name.svg
  icons-dark/actions/22/view-sort-descending.svg
  icons-dark/actions/22/view-sort.svg
  icons-dark/actions/24/view-sort-ascending-name.svg
  icons-dark/actions/24/view-sort-ascending.svg
  icons-dark/actions/24/view-sort-descending-name.svg
  icons-dark/actions/24/view-sort-descending.svg
  icons-dark/actions/24/view-sort.svg
  icons-dark/actions/32/view-sort-ascending-name.svg
  icons-dark/actions/32/view-sort-ascending.svg
  icons-dark/actions/32/view-sort-descending-name.svg
  icons-dark/actions/32/view-sort-descending.svg
  icons-dark/actions/32/view-sort.svg
  icons/actions/16/sort.svg
  icons/actions/16/view-sort-ascending-name.svg
  icons/actions/16/view-sort-ascending.svg
  icons/actions/16/view-sort-descending-name.svg
  icons/actions/16/view-sort-descending.svg
  icons/actions/16/view-sort.svg
  icons/actions/22/sort.svg
  icons/actions/22/view-sort-ascending-name.svg
  icons/actions/22/view-sort-ascending.svg
  icons/actions/22/view-sort-descending-name.svg
  icons/actions/22/view-sort-descending.svg
  icons/actions/22/view-sort.svg
  icons/actions/24/view-sort-ascending-name.svg
  icons/actions/24/view-sort-ascending.svg
  icons/actions/24/view-sort-descending-name.svg
  icons/actions/24/view-sort-descending.svg
  icons/actions/24/view-sort.svg
  icons/actions/32/view-sort-ascending-name.svg
  icons/actions/32/view-sort-ascending.svg
  icons/actions/32/view-sort-descending-name.svg
  icons/actions/32/view-sort-descending.svg
  icons/actions/32/view-sort.svg

To: trickyricky26, #vdg, #breeze, ndavis
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add a generic sorting options icon

2018-11-16 Thread Nathaniel Graham
ngraham added a comment.


  All right, let's go with that.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add a generic sorting options icon

2018-11-16 Thread TrickyRicky
trickyricky26 added a comment.


  | size  | `view-sort` | `view-sort-ascending` | 
`view-sort-descending` |
  | - | --- | - | 
-- |
  | `16px`| F6427148: view-sort-16.svg.png 
   | F6427145: 
view-sort-16-ascending.svg.png    
  | F6427205: view-sort-16-descending-alt.svg.png 
  |
  | `22px` and `24px` | F6427153: view-sort-22.svg.png 
   | F6427155: 
view-sort-22-ascending.svg.png    
  | F6427157: view-sort-22-descending.svg.png 
  |
  | `32px`| F6427177: view-sort-32.svg.png 
   | F6427162: 
view-sort-32-ascending.svg.png    
  | F6427167: view-sort-32-descending.svg.png 
  |
  |
  
  I think this version looks best, as you said, the direction is clear enough 
with `22px` and `32px` icons and I prefer the consistency with the placement of 
the arrow.
  The change is necessary for the `16px` icon, though.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add a generic sorting options icon

2018-11-16 Thread Nathaniel Graham
ngraham added a comment.


  I prefer that version, thanks!

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add a generic sorting options icon

2018-11-16 Thread TrickyRicky
trickyricky26 added a comment.


  This is how the set looks with the arrows above the lines in the descending 
versions:
  
  | size  | `view-sort` | `view-sort-ascending` | 
`view-sort-descending` |
  | - | --- | - | 
-- |
  | `16px`| F6427148: view-sort-16.svg.png 
   | F6427145: 
view-sort-16-ascending.svg.png    
  | F6427205: view-sort-16-descending-alt.svg.png 
  |
  | `22px` and `24px` | F6427153: view-sort-22.svg.png 
   | F6427155: 
view-sort-22-ascending.svg.png    
  | F6427208: view-sort-22-descending-alt.svg.png 
  |
  | `32px`| F6427177: view-sort-32.svg.png 
   | F6427162: 
view-sort-32-ascending.svg.png    
  | F6427210: view-sort-32-descending-alt.svg.png 
  |
  |
  
  Even if the arrows are at the top instead of the bottom like the other two 
icons, which is a bit inconsistent, this change makes the direction of sorting 
clearer at 100% scaling, .

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16927: KTextEditor : disconnect contextmenu from all aboutToXXContextMenu receivers

2018-11-16 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R39 KTextEditor

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

To: rjvbb, #ktexteditor, #frameworks
Cc: kde-frameworks-devel, kwrite-devel, michaelh, ngraham, bruns, demsking, 
head7, cullmann, kfunk, sars, dhaumann


D16905: Add a generic sorting options icon

2018-11-16 Thread Nathaniel Graham
ngraham added a comment.


  For the `-descending` versions, could we consider moving the arrows up so 
they don't obscure some of the horizontal lines? The effect is okay for the 
larger versions, but I think it doesn't work for the 16px version because the 
descending nature of the lines is lost.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16905: Add a generic sorting options icon

2018-11-16 Thread Noah Davis
ndavis added a comment.


  In D16905#360449 , @trickyricky26 
wrote:
  
  > I wasn't quite sure if I should use 2px strokes in the `32px` versions, so 
I used 1px for now as that is more consistent with the rest.
  >  If there are no more suggestions, I will update this revision with these 
icons.
  
  
  1px is correct. Looks good to me!

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16925: Use HTTPS for all search providers that support it

2018-11-16 Thread Volker Krause
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:ce74ff959524: Use HTTPS for all search providers that 
support it (authored by vkrause).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16925?vs=45594=45595

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

AFFECTED FILES
  src/urifilters/ikws/searchproviders/7digital.desktop
  src/urifilters/ikws/searchproviders/acronym.desktop
  src/urifilters/ikws/searchproviders/amazon.desktop
  src/urifilters/ikws/searchproviders/amazon_mp3.desktop
  src/urifilters/ikws/searchproviders/amg.desktop
  src/urifilters/ikws/searchproviders/archpkg.desktop
  src/urifilters/ikws/searchproviders/backports.desktop
  src/urifilters/ikws/searchproviders/baidu.desktop
  src/urifilters/ikws/searchproviders/beolingus.desktop
  src/urifilters/ikws/searchproviders/bing.desktop
  src/urifilters/ikws/searchproviders/call.desktop
  src/urifilters/ikws/searchproviders/citeseer.desktop
  src/urifilters/ikws/searchproviders/cpan.desktop
  src/urifilters/ikws/searchproviders/ctan.desktop
  src/urifilters/ikws/searchproviders/dbug.desktop
  src/urifilters/ikws/searchproviders/de2en.desktop
  src/urifilters/ikws/searchproviders/de2fr.desktop
  src/urifilters/ikws/searchproviders/deb.desktop
  src/urifilters/ikws/searchproviders/docbook.desktop
  src/urifilters/ikws/searchproviders/doi.desktop
  src/urifilters/ikws/searchproviders/ecosia.desktop
  src/urifilters/ikws/searchproviders/en2de.desktop
  src/urifilters/ikws/searchproviders/en2es.desktop
  src/urifilters/ikws/searchproviders/en2fr.desktop
  src/urifilters/ikws/searchproviders/en2it.desktop
  src/urifilters/ikws/searchproviders/es2en.desktop
  src/urifilters/ikws/searchproviders/feedster.desktop
  src/urifilters/ikws/searchproviders/flickr.desktop
  src/urifilters/ikws/searchproviders/foldoc.desktop
  src/urifilters/ikws/searchproviders/fr2de.desktop
  src/urifilters/ikws/searchproviders/fr2en.desktop
  src/urifilters/ikws/searchproviders/fsd.desktop
  src/urifilters/ikws/searchproviders/google.desktop
  src/urifilters/ikws/searchproviders/google_movie.desktop
  src/urifilters/ikws/searchproviders/imdb.desktop
  src/urifilters/ikws/searchproviders/it2en.desktop
  src/urifilters/ikws/searchproviders/jamendo.desktop
  src/urifilters/ikws/searchproviders/jeeves.desktop
  src/urifilters/ikws/searchproviders/kde.desktop
  src/urifilters/ikws/searchproviders/kde_apps.desktop
  src/urifilters/ikws/searchproviders/kde_look.desktop
  src/urifilters/ikws/searchproviders/kde_techbase.desktop
  src/urifilters/ikws/searchproviders/kde_userbase.desktop
  src/urifilters/ikws/searchproviders/leo.desktop
  src/urifilters/ikws/searchproviders/multitran-deru.desktop
  src/urifilters/ikws/searchproviders/multitran-enru.desktop
  src/urifilters/ikws/searchproviders/multitran-esru.desktop
  src/urifilters/ikws/searchproviders/multitran-frru.desktop
  src/urifilters/ikws/searchproviders/multitran-itru.desktop
  src/urifilters/ikws/searchproviders/multitran-nlru.desktop
  src/urifilters/ikws/searchproviders/netcraft.desktop
  src/urifilters/ikws/searchproviders/nl-telephone.desktop
  src/urifilters/ikws/searchproviders/opendesktop.desktop
  src/urifilters/ikws/searchproviders/rfc.desktop
  src/urifilters/ikws/searchproviders/rpmfind.desktop
  src/urifilters/ikws/searchproviders/ruby_application_archive.desktop
  src/urifilters/ikws/searchproviders/sourceforge.desktop
  src/urifilters/ikws/searchproviders/technorati.desktop
  src/urifilters/ikws/searchproviders/technoratitags.desktop
  src/urifilters/ikws/searchproviders/tvtome.desktop
  src/urifilters/ikws/searchproviders/urbandictionary.desktop
  src/urifilters/ikws/searchproviders/vimeo.desktop
  src/urifilters/ikws/searchproviders/wikia.desktop
  src/urifilters/ikws/searchproviders/wordref.desktop
  src/urifilters/ikws/searchproviders/yahoo.desktop
  src/urifilters/ikws/searchproviders/yahoo_image.desktop
  src/urifilters/ikws/searchproviders/yahoo_local.desktop
  src/urifilters/ikws/searchproviders/yahoo_shopping.desktop
  src/urifilters/ikws/searchproviders/yahoo_video.desktop

To: vkrause, #frameworks, cfeck, elvisangelaccio, pino, apol
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns


D16905: Add a generic sorting options icon

2018-11-16 Thread TrickyRicky
trickyricky26 added a comment.


  I made all necessary versions of these icons (`24px` not shown since they're 
the same as `22px`):
  
  | size   | `view-sort` | `view-sort-ascending` | `view-sort-descending` |
  | -- | --- | - | -- |
  | `16px` | F6427148: view-sort-16.svg.png 
   | F6427145: 
view-sort-16-ascending.svg.png    
  | F6427151: view-sort-16-descending.svg.png 
  |
  | `22px` | F6427153: view-sort-22.svg.png 
   | F6427155: 
view-sort-22-ascending.svg.png    
  | F6427157: view-sort-22-descending.svg.png 
  |
  | `32px` | F6427160: view-sort-32.svg.png 
   | F6427162: 
view-sort-32-ascending.svg.png    
  | F6427167: view-sort-32-descending.svg.png 
  |
  |
  
  I wasn't quite sure if I should use 2px strokes in the `32px` versions, so I 
used 1px for now as that is more consistent with the rest. If there are no more 
suggestions, I will update this revision with these icons.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16925: Use HTTPS for all search providers that support it

2018-11-16 Thread Aleix Pol Gonzalez
apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.


  Makes sense

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: vkrause, #frameworks, cfeck, elvisangelaccio, pino, apol
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns


D16925: Use HTTPS for all search providers that support it

2018-11-16 Thread Nathaniel Graham
ngraham added reviewers: Frameworks, cfeck, elvisangelaccio, pino.

REPOSITORY
  R241 KIO

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

To: vkrause, #frameworks, cfeck, elvisangelaccio, pino
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D16919: Re-add DBus test

2018-11-16 Thread Kurt Hindenburg
hindenburg added a comment.


  yea - feel free to commit any changes related to this yourself if you want.

REPOSITORY
  R319 Konsole

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

To: davidedmundson, #konsole, #frameworks, hindenburg
Cc: hindenburg, konsole-devel, ngraham, maximilianocuria


Re: Regression in Frameworks - DBus Hangs

2018-11-16 Thread David Edmundson
Don't thank me yet, I locked up one of the build jobs, sorry.
https://build.kde.org/job/Applications/job/konsole/job/kf5-qt5%20SUSEQt5.9/380/console

>Pipeline kf5-qt5 SUSEQt5.9

Output isn't too helpful, though I am surprised to see
 kf5.kinit.klauncher in the logs when I'm supposedly invoking the process
directly.

David


D16925: Use HTTPS for all search providers that support it

2018-11-16 Thread Volker Krause
vkrause created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
vkrause requested review of this revision.

REVISION SUMMARY
  This still has to be propagated through all translations to take
  effect though.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/urifilters/ikws/searchproviders/7digital.desktop
  src/urifilters/ikws/searchproviders/acronym.desktop
  src/urifilters/ikws/searchproviders/amazon.desktop
  src/urifilters/ikws/searchproviders/amazon_mp3.desktop
  src/urifilters/ikws/searchproviders/amg.desktop
  src/urifilters/ikws/searchproviders/archpkg.desktop
  src/urifilters/ikws/searchproviders/backports.desktop
  src/urifilters/ikws/searchproviders/baidu.desktop
  src/urifilters/ikws/searchproviders/beolingus.desktop
  src/urifilters/ikws/searchproviders/bing.desktop
  src/urifilters/ikws/searchproviders/call.desktop
  src/urifilters/ikws/searchproviders/citeseer.desktop
  src/urifilters/ikws/searchproviders/cpan.desktop
  src/urifilters/ikws/searchproviders/ctan.desktop
  src/urifilters/ikws/searchproviders/dbug.desktop
  src/urifilters/ikws/searchproviders/de2en.desktop
  src/urifilters/ikws/searchproviders/de2fr.desktop
  src/urifilters/ikws/searchproviders/deb.desktop
  src/urifilters/ikws/searchproviders/docbook.desktop
  src/urifilters/ikws/searchproviders/doi.desktop
  src/urifilters/ikws/searchproviders/ecosia.desktop
  src/urifilters/ikws/searchproviders/en2de.desktop
  src/urifilters/ikws/searchproviders/en2es.desktop
  src/urifilters/ikws/searchproviders/en2fr.desktop
  src/urifilters/ikws/searchproviders/en2it.desktop
  src/urifilters/ikws/searchproviders/es2en.desktop
  src/urifilters/ikws/searchproviders/feedster.desktop
  src/urifilters/ikws/searchproviders/flickr.desktop
  src/urifilters/ikws/searchproviders/foldoc.desktop
  src/urifilters/ikws/searchproviders/fr2de.desktop
  src/urifilters/ikws/searchproviders/fr2en.desktop
  src/urifilters/ikws/searchproviders/fsd.desktop
  src/urifilters/ikws/searchproviders/google.desktop
  src/urifilters/ikws/searchproviders/google_movie.desktop
  src/urifilters/ikws/searchproviders/imdb.desktop
  src/urifilters/ikws/searchproviders/it2en.desktop
  src/urifilters/ikws/searchproviders/jamendo.desktop
  src/urifilters/ikws/searchproviders/jeeves.desktop
  src/urifilters/ikws/searchproviders/kde.desktop
  src/urifilters/ikws/searchproviders/kde_apps.desktop
  src/urifilters/ikws/searchproviders/kde_look.desktop
  src/urifilters/ikws/searchproviders/kde_techbase.desktop
  src/urifilters/ikws/searchproviders/kde_userbase.desktop
  src/urifilters/ikws/searchproviders/leo.desktop
  src/urifilters/ikws/searchproviders/multitran-deru.desktop
  src/urifilters/ikws/searchproviders/multitran-enru.desktop
  src/urifilters/ikws/searchproviders/multitran-esru.desktop
  src/urifilters/ikws/searchproviders/multitran-frru.desktop
  src/urifilters/ikws/searchproviders/multitran-itru.desktop
  src/urifilters/ikws/searchproviders/multitran-nlru.desktop
  src/urifilters/ikws/searchproviders/netcraft.desktop
  src/urifilters/ikws/searchproviders/nl-telephone.desktop
  src/urifilters/ikws/searchproviders/opendesktop.desktop
  src/urifilters/ikws/searchproviders/rfc.desktop
  src/urifilters/ikws/searchproviders/rpmfind.desktop
  src/urifilters/ikws/searchproviders/ruby_application_archive.desktop
  src/urifilters/ikws/searchproviders/sourceforge.desktop
  src/urifilters/ikws/searchproviders/technorati.desktop
  src/urifilters/ikws/searchproviders/technoratitags.desktop
  src/urifilters/ikws/searchproviders/tvtome.desktop
  src/urifilters/ikws/searchproviders/urbandictionary.desktop
  src/urifilters/ikws/searchproviders/vimeo.desktop
  src/urifilters/ikws/searchproviders/wikia.desktop
  src/urifilters/ikws/searchproviders/wordref.desktop
  src/urifilters/ikws/searchproviders/yahoo.desktop
  src/urifilters/ikws/searchproviders/yahoo_image.desktop
  src/urifilters/ikws/searchproviders/yahoo_local.desktop
  src/urifilters/ikws/searchproviders/yahoo_shopping.desktop
  src/urifilters/ikws/searchproviders/yahoo_video.desktop

To: vkrause
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D16919: Re-add DBus test

2018-11-16 Thread David Edmundson
davidedmundson added a comment.


  CI seems to be frozen again. :(

REPOSITORY
  R319 Konsole

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

To: davidedmundson, #konsole, #frameworks, hindenburg
Cc: hindenburg, konsole-devel, ngraham, maximilianocuria


D16919: Re-add DBus test

2018-11-16 Thread Kurt Hindenburg
This revision was automatically updated to reflect the committed changes.
Closed by commit R319:3eb9a34f62c8: Re-add DBus test (authored by 
davidedmundson, committed by hindenburg).

REPOSITORY
  R319 Konsole

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16919?vs=45568=45591

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

AFFECTED FILES
  src/autotests/CMakeLists.txt
  src/autotests/DBusTest.cpp
  src/autotests/DBusTest.h

To: davidedmundson, #konsole, #frameworks, hindenburg
Cc: hindenburg, konsole-devel, ngraham, maximilianocuria


D16919: Re-add DBus test

2018-11-16 Thread Kurt Hindenburg
hindenburg accepted this revision.
hindenburg added a comment.
This revision is now accepted and ready to land.


  Thanks so much for looking into this.

REPOSITORY
  R319 Konsole

BRANCH
  master

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

To: davidedmundson, #konsole, #frameworks, hindenburg
Cc: hindenburg, konsole-devel, ngraham, maximilianocuria


D16894: [ECM] use a macro to add compiler flags conditionally

2018-11-16 Thread René J . V . Bertin
rjvbb added a comment.


  Something side-ways related: I went down this hole because cmake's 
`generate_export_header` failed because of an unsupported flag that was added.
  Regardless of how we implement things here, shouldn't there be something like 
`ecm_generate_export_header` which empties CMAKE_CXX_FLAGS temporarily because 
calling CMake's version and then restores the variable? There's no feedback at 
all in this function, the generated export header just contains dummy EXPORT 
macros, leaving the user to wonder why the linker fails. Or should the 
visibility flags also be set conditionally, after setting all other compiler 
options?
  
  > 1. add_compile_flag_if_supported( [CXX_ONLY])
  
  So that is basically just a wrapper around `compile_check_cxx_compiler_flag` 
with a bit of icing (consistent cache variables, which my approach also has, 
and a (cripped!) C++-only option).
  Issues I see:
  
  1- these will fail if someone adds an unsupported flag for which the compiler 
emits a warning, potentially breaking a build that would otherwise succeed. You 
could argue "just don't add unsupported options then". That may still be 
acceptable for individual projects, but I think it's not a good idea 
nonetheless. Build systems (and thus the ECM) should be as fool-proof as 
possible, and using ID+version checks can help here.
  2- (citing): "The macro name name suggests it does the compile check on all 
compilers."
  3- this assumes that the C++ compiler accepts all options accepted by all 
compilers (without emitting warnings), which is not true in practice
  
  Here's a counter proposal that should be clear in its intention and 
implementation (clear though a bit long and complex because covering all 
{b,c}ases):
  
include(CMakeParseArguments)

# ECM_ADD_CXX_COMPILER_FLAGS_IF_SUPPORTED(FLAGS 
# [SUPPORTED_IF ]
# )
# add  or  to CMAKE_CXX_FLAGS is the compiler supports them.
# Support is determined by the SUPPORTED_IF expression if provided or by
# querying the compiler directly otherwise. The compiler is also queried
# when using a Clang C++ compiler on APPLE.
# examples:
# Check 
# ecm_add_cxx_compiler_flags_if_supported(FLAGS -a -b -c SUPPORTED_IF 
CMAKE_C_COMPILER_ID STREQUAL "GNU" OR CMAKE_C_COMPILER_ID MATCHES "Clang")
# ecm_add_cxx_compiler_flags_if_supported(FLAGS -d -e -f)

function (ECM_ADD_CXX_COMPILER_FLAGS_IF_SUPPORTED)
set(_OPTIONS_ARGS)
set(_ONE_VALUE_ARGS)
set(_MULTI_VALUE_ARGS FLAGS SUPPORTED_IF)

cmake_parse_arguments(EASCXXFLAGS "${_OPTIONS_ARGS}" 
"${_ONE_VALUE_ARGS}" "${_MULTI_VALUE_ARGS}" ${ARGN} )
if(NOT EASCXXFLAGS_FLAGS)
message( FATAL_ERROR "ecm_add_cxx_compiler_flags_if_supported: 
'FLAGS' is a required argument." )
endif()
# if the user provided supported_if conditions, evaluate them now:
if (NOT EASCXXFLAGS_SUPPORTED_IF OR (${EASCXXFLAGS_SUPPORTED_IF}))
# without conditions, or with Clang on APPLE we'll need to ask the 
compiler directly.
# (Clang on APPLE needs verification because of Apple's llvm 
versions which cannot be
# (matched easily to stock llvm versions.
if(NOT EASCXXFLAGS_SUPPORTED_IF OR (APPLE AND CMAKE_CXX_COMPILER_ID 
MATCHES "Clang"))
# one flag at a time:
foreach(flag IN ITEMS ${EASCXXFLAGS_FLAGS})
# use a standardised and informative cached test variable
set(HASFLAG "${CMAKE_CXX_COMPILER_ID}++_ACCEPTS${flag}")
check_cxx_compiler_flag(${flag} ${HASFLAG})
if ({${HASFLAG}})
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${flag}" 
PARENT_SCOPE)
endif()
endforeach()
else()
# all flags can be appended at once
string(REPLACE ";" " " FLAGS "${EASCXXFLAGS_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${FLAGS}" PARENT_SCOPE)
endif()
endif()
endfunction()
  
  Using a function here to keep temp. variables local. And using the ECM 
"namespace" because there's nothing KDE specific in this.

REPOSITORY
  R240 Extra CMake Modules

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

To: rjvbb, #build_system, kfunk
Cc: kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system, 
michaelh, ngraham, bruns


D16894: [ECM] use a macro to add compiler flags conditionally

2018-11-16 Thread René J . V . Bertin
rjvbb added a comment.


  >   Thus these places need to be turned into:
  >   
  > ...
  > if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND 
CMAKE_CXX_COMPILER_VERSION VERSION_LESS "3.8")
  > elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" AND 
CMAKE_CXX_COMPILER_VERSION VERSION_LESS "8.1.0")
  
  As you know this doesn't work reliably: whether it works or not depends on 
cmake version and on CMP0025 - and thus requires all projects to set that 
policy before calling `project`. I don't think that's an option, nor is 
expecting Mac users to patch all toplevel CMake files of projects they want to 
work with (or patch CMake so it sets the policy which is basically what I do on 
my end). I also vaguely recall that cmake used the dotless AppleClang version 
in earlier versions (= 810 instead of 8.1.0).
  
  Besides, all those repeated separate ifs and elses do not make the code 
easier to read or parse. Isn't it the whole purpose of having macros to reduce 
code duplication and to hide complexity in a single location so that the 
intended behaviour becomes easier to follow?
  
  This makes Aleix's suggestion just to use `compiler_check_flag` much 
more appealing, despite the cost and the fact it's so easy to break.
  
  > Statements like `CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT 
CMAKE_CXX_COMPILER_VERSION VERSION_LESS 3.5` do not make sense.
  
  That's a bit too strong. They make sense everywhere except on APPLE (and even 
there they can be reliable).
  
  > If you're unsure in which version of AppleClang a certain feature was 
introduced then:
  
  This is the gist of what I'm doing, except that I do not want to rely on 
Apple clang being reported as AppleClang (= CMP0025 set to NEW).
  
  > This "let's abuse ${ARGN} as code to be evaluated later" is pretty ugly.
  
  It comes from a cmake ML post by a cmake dev (Brad Kind IIRC) so I guess it's 
sanctioned ugliness (as so much in cmake syntax). The boolean operators are 
only available in the IF functions so there aren't much options if you don't 
want to use CmakeParseArguments. I'm already looking at doing just that for a 
version that will look like `kde_add_supported_compiler_flags(FLAGS  
SUPPORTED_IF )`. The names (macro and keywords) are open for 
discussion of course, and so is the addition of additional options.
  
  I'll look at kdevelop's versions too. I have no real objection to favour my 
own code over them if they work reliably. But now that you mention kdevelop: 
I've long been forced to use stock compilers to build KDevelop (on Mac). I 
always put that off to my AppleClang being too old but with this new knowledge 
I suspect it's simply because something goes wrong in the compiler detection.
  
  > I don't really understand why this branch is needed. The macro name name 
suggests it does the compile check on all compilers. Again very misleading.
  
  Erm, no?! The macro name suggests it runs a check, which is true. It doesn't 
say which check...
  (FWIW, `cmake__compiler_check_flag` is equally misleading: it doesn't 
check `${flag}` but `"${CMAKE_CXX_FLAGS} ${flag}"`)

REPOSITORY
  R240 Extra CMake Modules

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

To: rjvbb, #build_system, kfunk
Cc: kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system, 
michaelh, ngraham, bruns


D16905: Add a generic sorting options icon

2018-11-16 Thread TrickyRicky
trickyricky26 edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, #breeze, ndavis
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns


D16894: [ECM] use a macro to add compiler flags conditionally

2018-11-16 Thread Kevin Funk
kfunk requested changes to this revision.
kfunk added a comment.
This revision now requires changes to proceed.


  I don't like the hiding of the if-branches as argument to macro. We shouldn't 
to this as it makes the code harder to understand.
  
  The general issue with the existing code here is: Statements like 
`CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT CMAKE_CXX_COMPILER_VERSION 
VERSION_LESS 3.5` do not make sense. If we check the compiler version, then we 
need to differentiate between Clang and AppleClang (cf. something like 
https://github.com/Microsoft/LightGBM/blob/master/CMakeLists.txt#L20).
  
  Thus these places need to be turned into:
  
...
if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_VERSION 
VERSION_LESS "3.8")
  ...
endif()
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" AND 
CMAKE_CXX_COMPILER_VERSION VERSION_LESS "8.1.0")
  ...
endif()
  
  If you're unsure in which version of AppleClang a certain feature was 
introduced then:
  
...
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" AND 
CMAKE_CXX_COMPILER_VERSION VERSION_LESS "8.1.0")
  add_compile_flag_if_supported(...)
endif()
  
  Please check out the functions provided in: 
kdevelop.git:cmake/modules/KDevelopMacrosInternal.cmake
  
#   add_compile_flag_if_supported( [CXX_ONLY])
#   add_target_compile_flag_if_supported( 
 )
  
  They are more clean than your implementation, I think it would make sense to 
actually add them to KDECompilerSettings.cmake and prefix them with `kde_`.

INLINE COMMENTS

> KDECompilerSettings.cmake:202
> +# is expected to support _FLAG.
> +if (${ARGN})
> +if(APPLE AND CMAKE_CXX_COMPILER_ID MATCHES "Clang")

This "let's abuse ${ARGN} as code to be evaluated later" is pretty ugly. The 
previous version (with the if-statements at  the caller side) is way more clean 
and understandable.

> KDECompilerSettings.cmake:203
> +if (${ARGN})
> +if(APPLE AND CMAKE_CXX_COMPILER_ID MATCHES "Clang")
> +# Clang on APPLE needs verification because of Apple's

I don't really understand why this branch is needed. The macro name name 
suggests it does the compile check on all compilers. Again very misleading.

REPOSITORY
  R240 Extra CMake Modules

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

To: rjvbb, #build_system, kfunk
Cc: kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system, 
michaelh, ngraham, bruns


Re: Regression in Frameworks - DBus Hangs

2018-11-16 Thread Ben Cooksley
On Fri, Nov 16, 2018 at 11:04 PM David Edmundson
 wrote:
>
> A running KJob would not show in a backtrace.
>
> Update at https://phabricator.kde.org/D16919

Ah I see. Thanks for taking a look into that.

Cheers,
Ben


D16919: Re-add DBus test

2018-11-16 Thread David Edmundson
davidedmundson created this revision.
davidedmundson added reviewers: Konsole, Frameworks.
Herald added a project: Konsole.
Herald added a subscriber: konsole-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  Ported from KProcess::startDetached to a QProcess we can kill should the
  window not exit when closed.
  
  On the KDE CI where the window isn't quitting on close the test might
  fail, but it will give us output where we can investigate.

TEST PLAN
  Passes locally

REPOSITORY
  R319 Konsole

BRANCH
  master

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

AFFECTED FILES
  src/autotests/CMakeLists.txt
  src/autotests/DBusTest.cpp
  src/autotests/DBusTest.h

To: davidedmundson, #konsole, #frameworks
Cc: konsole-devel, ngraham, maximilianocuria, hindenburg


Re: Regression in Frameworks - DBus Hangs

2018-11-16 Thread David Edmundson
A running KJob would not show in a backtrace.

Update at https://phabricator.kde.org/D16919


D16894: [ECM] use a macro to add compiler flags conditionally

2018-11-16 Thread René J . V . Bertin
rjvbb edited the test plan for this revision.

REPOSITORY
  R240 Extra CMake Modules

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

To: rjvbb, #build_system
Cc: apol, kde-frameworks-devel, kde-buildsystem, #build_system, michaelh, 
ngraham, bruns


D16894: [ECM] use a macro to add compiler flags conditionally

2018-11-16 Thread René J . V . Bertin
rjvbb retitled this revision from "[ECM] use a macro to test compiler flag 
support" to "[ECM] use a macro to add compiler flags conditionally".
rjvbb set the repository for this revision to R240 Extra CMake Modules.

REPOSITORY
  R240 Extra CMake Modules

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

To: rjvbb, #build_system
Cc: apol, kde-frameworks-devel, kde-buildsystem, #build_system, michaelh, 
ngraham, bruns


D16894: [ECM] use a macro to test compiler flag support

2018-11-16 Thread René J . V . Bertin
rjvbb updated this revision to Diff 45567.
rjvbb added a comment.


  A simpler version, setting CMAKE__FLAGS directly (also fixes a 
persistence error in my previous implementation).
  
  I've thought some more about "why not just use check__compiler_flag 
everywhere". I think that the risk with using a built-in function is that 
future developers or maintainers may think to be smart and replace them with 
built-in ID+version checking. Because we know since what version which compiler 
supports a given flag, right? Providing a dedicated function should reduce that 
risk, if not only because of the comments it comes with.
  
  Ideally there should probably also be a version that accepts multiple flags; 
would that be possible with cmake's language?

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16894?vs=45513=45567

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

AFFECTED FILES
  kde-modules/KDECompilerSettings.cmake
  kde-modules/KDEFrameworkCompilerSettings.cmake

To: rjvbb, #build_system
Cc: apol, kde-frameworks-devel, kde-buildsystem, #build_system, michaelh, 
ngraham, bruns


Re: Regression in Frameworks - DBus Hangs

2018-11-16 Thread Ben Cooksley
On Wed, Nov 14, 2018 at 8:17 AM Ben Cooksley  wrote:
>
> On Tue, Nov 13, 2018 at 2:25 PM David Edmundson
>  wrote:
> >
> > I don't think application CI jobs run with workspace available. I also 
> > can't think of anything that's been merged in workspace.
>
> That's correct. CI Jobs are only provided with the things they depend on.
>
> >
> > ---
> > quitOnLastWindowClosed is also blocked if a QEventLoopLocker is placed on 
> > the main application.
> >
> > All KJobs do this and a Kjob existing and hanging seems plausible, but 
> > that's a lot harder to grep for.
>
> Would it show in a backtrace perhaps?
>
> >
> > All stdout/stderr of our newly spawned konsole should be forwarded to the 
> > test.
> > @Ben is that available?
> > Ideally with QT_LOGGING_RULES=*.debug=true
>
> The CI system sets QT_LOGGING_RULES=*.debug=true globally prior to
> executing tests [1], so that should definitely be the case, so i'd say
> the test is silencing the messages :(
>
> Kurt, any ideas here on what would be the best way to get at the
> output from the Konsole process launched by the test?

Ping?

>
> >
> >
> >
> >
> >
> >
>
> [1]: 
> https://cgit.kde.org/sysadmin/ci-tooling.git/tree/helpers/run-tests.py#n94
>
> Thanks,
> Ben

Cheers,
Ben


D16643: Correct the accept flag of the event object on DragMove

2018-11-16 Thread Eike Hein
hein added a comment.


  @mart ping

REPOSITORY
  R296 KDeclarative

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

To: trmdi, mart, broulik, #plasma, hein, davidedmundson
Cc: anthonyfieroni, bruns, dkorth, ngraham, kde-frameworks-devel, michaelh


D16196: add a description property to KFileMetaData

2018-11-16 Thread Alexander Stippich
astippich added a comment.


  I'm gonna merge this tonight to get this in before the string freeze

REPOSITORY
  R286 KFileMetaData

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

To: astippich, mgallien, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams