D18114: FakeInput: add support for pointer move with absolute coordinates

[Jan Grulich]

jgrulich added a comment.


  Hi, can I get please this re-approved? It's now just again about the 
additional mouse support.

REPOSITORY
  R127 KWayland

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

To: jgrulich, davidedmundson, graesslin, zzag
Cc: graesslin, zzag, kde-frameworks-devel, michaelh, ngraham, bruns

D18010: Allow zero altitude/longitude/latitude exiv gps data

[Alexander Stippich]

astippich added inline comments.

INLINE COMMENTS

> bruns wrote in exiv2extractor.cpp:298
> when you rewrite this as
> 
>   if (it != data.end()) {
>   auto ratio = it->value().toRational();
>   it = data.findKey(Exiv2::ExifKey("Exif.GPSInfo.GPSAltitudeRef"));
>   if ((ratio.second != 0) && (it != data.end())) {
>   auto altRef = it->value().toLong();
>   if (altRef) {
>   alt = -1.0 * ratio.first / ratio.second;
>   } else {
>   alt = 1.0 * ratio.first / ratio.second;
>   }
>   }
>   }
> 
> does the unit test pass without special handling of the altitude fuzz?

Yes, it does

REPOSITORY
  R286 KFileMetaData

BRANCH
  fix_zero_gps

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

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

D18308: Ignore all non-storage deviceAdded signals from Solid

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, Frameworks, ngraham.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  createCacheEntry is also called from SolidDeviceNotifier::deviceAdded,
  which issues signals for all kinds of devices.
  
  Ignore any events but for Solid::StorageAccess devices. This also avoids
  trying to connect() to a nullptr.

REPOSITORY
  R293 Baloo

BRANCH
  submit

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

AFFECTED FILES
  src/file/storagedevices.cpp

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

D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames

[Christoph Feck]

cfeck added a comment.


  Thanks for testing, btw.

REPOSITORY
  R241 KIO

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

To: cfeck, #frameworks, #dolphin, dfaure
Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns

D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames

[Christoph Feck]

cfeck added a comment.


  It's possible that Kate doesn't use KIO for local files, but falls back to 
QFile. It _would_ be possible somehow (e.g. via the QPA plugins) to inject the 
hack into all Qt applications, but I suggest to improve Dolphin in a way that 
it shows an error message or even a rename dialog when trying to open or 
execute such a file.

REPOSITORY
  R241 KIO

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

To: cfeck, #frameworks, #dolphin, dfaure
Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns

D17816: Support for xattrs on kio copy/move

[Christoph Feck]

cfeck added a comment.


  I am still not fond of having a local event loop in KIO. The whole point of 
KIO is that it should work asynchronously.
  
  If the tests fail, could they be adapted? Or is the reason why the tests fail 
unfixable?

REPOSITORY
  R241 KIO

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

To: cochise, dfaure
Cc: cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, 
ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, michaelh

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  In D15999#394818 , @filipf wrote:
  
  > +1 in regards to at least getting the new switch user icon into master, 
it's great
  
  
  @pstefan, any chance we can cudgel you into doing this? 

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: rooty, ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, 
bruns

D13316: manually set format in jpeg thumbnail creator

[Nathaniel Graham]

ngraham added a comment.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.


  Is there any reason not to commit this now?

REPOSITORY
  R320 KIO Extras

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

To: miklosm, dfaure, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, kfm-devel, ngraham, alexde, feverfew, michaelh, 
spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames

[Nerdopolis Turfwalker]

nerdopolist added a comment.


  Awesome! For what I can input, base file operations work. I can copy, delete, 
rename files, and testing, and diffing the trees with my kiocopy test script, 
it copies them perfectly. However, for example, some of the files I cannot open 
with kate

REPOSITORY
  R241 KIO

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

To: cfeck, #frameworks, #dolphin, dfaure
Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns

D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames

[Christoph Feck]

cfeck updated this revision to Diff 49684.
cfeck added a comment.


  Fix worst case estimate. We need up to two UTF-16 code units for each byte.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18161?vs=49176=49684

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

AFFECTED FILES
  src/ioslaves/file/CMakeLists.txt
  src/ioslaves/file/file.cpp
  src/ioslaves/file/legacycodec.cpp
  src/ioslaves/file/legacycodec.h

To: cfeck, #frameworks, #dolphin, dfaure
Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns

D11320: Fix QVariantMapList operator >> implementation

[Albert Astals Cid]

This revision was automatically updated to reflect the committed changes.
Closed by commit R281:66362ec67b8f: Fix QVariantMapList operator  
implementation (authored by aleksanderm, committed by aacid).
Herald added a subscriber: kde-frameworks-devel.

REPOSITORY
  R281 ModemManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11320?vs=29478=49683

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

AFFECTED FILES
  src/generictypes.cpp

To: aleksanderm, #frameworks, jgrulich, broulik
Cc: kde-frameworks-devel, broulik, michaelh, ngraham, bruns

D13416: image thumbnailer: instruct Qt to look at the file contents when detecting its format

[Albert Astals Cid]

aacid closed this revision.

REPOSITORY
  R320 KIO Extras

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

To: miklosm, #frameworks, #dolphin, dfaure, elvisangelaccio
Cc: kde-frameworks-devel, kfm-devel, aacid, alexde, feverfew, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D13416: image thumbnailer: instruct Qt to look at the file contents when detecting its format

[Albert Astals Cid]

aacid added a comment.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.


  I pushed this at 
https://commits.kde.org/kio-extras/0be80392f01e0ec01174990c430dcd8f1ed35d9e

REPOSITORY
  R320 KIO Extras

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

To: miklosm, #frameworks, #dolphin, dfaure, elvisangelaccio
Cc: kde-frameworks-devel, kfm-devel, aacid, alexde, feverfew, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D15999: Add icons with added background to system.svgz

[Filip Fila]

filipf added a comment.


  They only work for me when using a dark theme:
  
  F6552425: image.png 
  
  F6552428: image.png 
  
  We could just do QML circles. Pro is they assure legibility with many 
non-Breeze icons, con is they might be unnecessary for circular icons. It 
however remains to be seen if there are even people using different icons.
  
  +1 in regards to at least getting the new switch user icon into master, it's 
great

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: rooty, ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, 
bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a subscriber: rooty.
ngraham added a comment.
This revision now requires changes to proceed.


  I still haven't managed to get these icons to actually work, per recent 
discussions. The background is always light instead of dark. Also the patch 
this was in support of (D16031 ) was 
abandoned. @filipf and/or @rooty, did you guys want to use these icons for your 
5.16 login screen work? Or were we going with QML circles behind the icons?
  
  Regardless, @pstefan, I'd really love the new switch user icon in another 
patch so we could at least get that in!

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: rooty, ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, 
bruns

D17664: fix for macOS

[Albert Astals Cid]

This revision was automatically updated to reflect the committed changes.
Closed by commit R6:74f3eeb69ead: fix for macOS (authored by yurikoles, 
committed by aacid).
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D17664?vs=47793=49678#toc

REPOSITORY
  R6 KActivities

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17664?vs=47793=49678

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

AFFECTED FILES
  CMakeLists.txt
  src/cli/CMakeLists.txt

To: yurikoles, ivan, kfunk
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.12 - Build # 17 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.12/17/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Wed, 16 Jan 2019 23:07:00 +
 Build duration:
5 min 31 sec and counting
   JUnit Tests
  Name: projectroot Failed: 4 test(s), Passed: 48 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_dropjobtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: projectroot.src.ioslaves.trash.tests.testtrashName: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

D17726: Add helper function to disable redirections (useful for kde-open)

[Albert Astals Cid]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:8c3f000b72ed: Add helper function to disable redirections 
(useful for kde-open) (authored by achauvel, committed by aacid).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D17726?vs=48028=49677#toc

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17726?vs=48028=49677

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

AFFECTED FILES
  src/widgets/krun.cpp
  src/widgets/krun.h
  src/widgets/krun_p.h

To: achauvel, dfaure, #frameworks, cfeck
Cc: yurchor, kde-frameworks-devel, michaelh, ngraham, bruns

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

[Albert Astals Cid]

aacid added a comment.


  @bruns waiting for your comment on whether your issues were addressed or not

REPOSITORY
  R296 KDeclarative

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

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

D18237: Fix ResultIterator

[Nathaniel Graham]

ngraham added reviewers: Baloo, bruns, poboiko.

REPOSITORY
  R293 Baloo

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

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

D18237: Fix ResultIterator

[Albert Astals Cid]

aacid set the repository for this revision to R293 Baloo.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.

REPOSITORY
  R293 Baloo

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

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

Power Management and Inhibition by Applications

[Matthieu Gallien]

Hello,

I am trying to work on a feature request to add the ability to suppress laptop 
sleep when playing music.

Currently, Juk is doing it via dbus calls to "org.kde.Solid.PowerManagement".

I also had a look to Solid frameworks and the power classes that add an 
asynchronous way to do that.
Currently, the Solid framework only conditionally provides those features and 
it seems distributions (at least Debian) do not always build it.

I am also unsure about the current recommended solution that would work for 
most desktop environments.

What would be the best way forward ?

I can also see some other features that could make sense to provide in 
libraries that music players could use (lyrics fetching, album art fetching, 
...). This could be in a dedicated framework to help build media players.

Best regards

--
Matthieu Gallien

Power Management and Inhibition by Applications

[Matthieu Gallien]

Hello,

I am trying to work on a feature request to add the ability to suppress laptop 
sleep when playing music.

Currently, Juk is doing it via dbus calls to "org.kde.Solid.PowerManagement".

I also had a look to Solid frameworks and the power classes that add an 
asynchronous way to do that.
Currently, the Solid framework only conditionally provides those features and 
it seems distributions (at least Debian) do not always build it.

I am also unsure about the current recommended solution that would work for 
most desktop environments.

What would be the best way forward ?

I can also see some other features that could make sense to provide in 
libraries that music players could use (lyrics fetching, album art fetching, 
...). This could be in a dedicated framework to help build media players.

Best regards

--
Matthieu Gallien

D18164: Review KateGotoBar

[loh tar]

loh.tar updated this revision to Diff 49665.
loh.tar added a comment.


  - Connect/Disconnect
  - Remove m_gotoRange->setFocus
  
  Like this? :-/
  
  I noticed bad behavior and have remove these setFocus, test it if its OK.
  On edit was always the focus set to the spinbox.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18164?vs=49653=49665

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/katedialogs.h

To: loh.tar, #ktexteditor, cullmann
Cc: dhaumann, cullmann, anthonyfieroni, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, ngraham, bruns, demsking, sars

D18302: Add Android notification channel support

[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 is required for API level 28 or higher. This actually matches the
  notfication events KNotification defines quite nicely, so those can be
  individually configured now, like we are used to on other platforms.

REPOSITORY
  R289 KNotifications

BRANCH
  master

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

AFFECTED FILES
  src/android/org/kde/knotifications/KNotification.java
  src/android/org/kde/knotifications/NotifyByAndroid.java
  src/notifybyandroid.cpp
  src/notifybyandroid.h

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

D18288: Make the clock svg's shadows more logically correct and visually appropriate

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:d755013c7421: Make the clock svgs shadows more 
logically correct and visually appropriate (authored by ngraham).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18288?vs=49592=49658

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

AFFECTED FILES
  src/desktoptheme/breeze/widgets/clock.svgz

To: ngraham, #vdg, #plasma, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

KDE CI: Frameworks » plasma-framework » kf5-qt5 SUSEQt5.11 - Build # 16 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/plasma-framework/job/kf5-qt5%20SUSEQt5.11/16/
 Project:
kf5-qt5 SUSEQt5.11
 Date of build:
Wed, 16 Jan 2019 16:44:14 +
 Build duration:
20 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlcompat_reports/KF5Plasma_compat_report.htmllogs/KF5Plasma/5.54.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 6 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: projectroot.autotests.dialognativetestFailed: projectroot.autotests.plasma_configmodeltestFailed: projectroot.autotests.plasma_fallbackpackagetestFailed: projectroot.autotests.plasma_iconitemtestFailed: projectroot.autotests.plasma_packagestructuretestFailed: projectroot.autotests.plasma_storagetest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report33%
(6/18)36%
(45/126)36%
(45/126)27%
(3602/13319)18%
(1814/9840)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests86%
(12/14)86%
(12/14)55%
(610/1117)29%
(315/1086)src.declarativeimports.calendar0%
(0/6)0%
(0/6)0%
(0/464)0%
(0/243)src.declarativeimports.core31%
(5/16)31%
(5/16)13%
(299/2248)7%
(96/1456)src.declarativeimports.plasmacomponents0%
(0/6)0%
(0/6)0%
(0/518)0%
(0/207)src.declarativeimports.plasmaextracomponents0%
(0/3)0%
(0/3)0%
(0/42)0%
(0/22)src.declarativeimports.platformcomponents0%
(0/3)0%
(0/3)0%
(0/58)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/14)0%
(0/2)src.plasma64%
(14/22)64%
(14/22)40%
(1410/3491)28%
(787/2817)src.plasma.packagestructure0%
(0/7)0%
(0/7)0%
(0/134)0%
(0/12)src.plasma.private50%
(9/18)50%
(9/18)43%
(674/1574)29%
(301/1034)src.plasma.scripting0%
(0/3)0%
(0/3)0%
(0/162)0%
(0/128)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick33%
(4/12)33%
(4/12)29%
(578/2013)18%
(310/1713)src.plasmaquick.private50%
(1/2)50%
(1/2)29%
(31/106)36%
(5/14)src.scriptengines.qml.plasmoid0%
(0/6)0%
(0/6)0%
(0/1178)0%
(0/1028)tests.dpi0%
   

KDE CI: Frameworks » plasma-framework » kf5-qt5 SUSEQt5.10 - Build # 6 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/plasma-framework/job/kf5-qt5%20SUSEQt5.10/6/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Wed, 16 Jan 2019 16:44:14 +
 Build duration:
16 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlcompat_reports/KF5Plasma_compat_report.htmllogs/KF5Plasma/5.54.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 6 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: projectroot.autotests.dialognativetestFailed: projectroot.autotests.plasma_configmodeltestFailed: projectroot.autotests.plasma_fallbackpackagetestFailed: projectroot.autotests.plasma_iconitemtestFailed: projectroot.autotests.plasma_packagestructuretestFailed: projectroot.autotests.plasma_storagetest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report33%
(6/18)36%
(45/126)36%
(45/126)27%
(3604/13319)18%
(1814/9840)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests86%
(12/14)86%
(12/14)55%
(612/1117)29%
(315/1086)src.declarativeimports.calendar0%
(0/6)0%
(0/6)0%
(0/464)0%
(0/243)src.declarativeimports.core31%
(5/16)31%
(5/16)13%
(299/2248)7%
(96/1456)src.declarativeimports.plasmacomponents0%
(0/6)0%
(0/6)0%
(0/518)0%
(0/207)src.declarativeimports.plasmaextracomponents0%
(0/3)0%
(0/3)0%
(0/42)0%
(0/22)src.declarativeimports.platformcomponents0%
(0/3)0%
(0/3)0%
(0/58)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/14)0%
(0/2)src.plasma64%
(14/22)64%
(14/22)40%
(1410/3491)28%
(787/2817)src.plasma.packagestructure0%
(0/7)0%
(0/7)0%
(0/134)0%
(0/12)src.plasma.private50%
(9/18)50%
(9/18)43%
(674/1574)29%
(301/1034)src.plasma.scripting0%
(0/3)0%
(0/3)0%
(0/162)0%
(0/128)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick33%
(4/12)33%
(4/12)29%
(578/2013)18%
(310/1713)src.plasmaquick.private50%
(1/2)50%
(1/2)29%
(31/106)36%
(5/14)src.scriptengines.qml.plasmoid0%
(0/6)0%
(0/6)0%
(0/1178)0%
(0/1028)tests.dpi0%

D18164: Review KateGotoBar

[Christoph Cullmann]

cullmann added a comment.


  I would disconnect the textChanged in ::closed() to avoid having updates 
there even if the bar is hidden again (and reconnect on show).
  
  Otherwise I think this is ready to go in.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, cullmann
Cc: dhaumann, cullmann, anthonyfieroni, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, ngraham, bruns, demsking, sars

KDE CI: Frameworks » knotifications » kf5-qt5 AndroidQt5.11 - Build # 12 - Still Failing!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks/job/knotifications/job/kf5-qt5%20AndroidQt5.11/12/
 Project:
kf5-qt5 AndroidQt5.11
 Date of build:
Wed, 16 Jan 2019 16:51:14 +
 Build duration:
49 sec and counting
   CONSOLE OUTPUT
  [...truncated 211 lines...] * Qt5TextToSpeech, Qt text to speech module   Required to build text to speech notification support * PkgConfig * Qt5Test (required version >= 5.10.0)-- The following REQUIRED packages have been found: * ECM (required version >= 5.54.0), Extra CMake Modules.,  * Qt5Gui (required version >= 5.11.3) * Qt5Widgets * Qt5AndroidExtras * Gradle * Qt5 (required version >= 5.10.0) * Qt5Core * KF5WindowSystem (required version >= 5.54.0) * KF5Config (required version >= 5.54.0) * KF5Codecs (required version >= 5.54.0) * KF5CoreAddons (required version >= 5.54.0)-- The following features have been disabled: * QCH, API documentation in QCH format (for e.g. Qt Assistant, Qt Creator & KDevelop)-- The following OPTIONAL packages have not been found: * X11 * Canberra, Library for generating event sounds,Needed to build audio notification support * Phonon4Qt5 (required version >= 4.6.60), Qt-based audio library   Needed to build audio notification support when Canberra isn't available * dbusmenu-qt5, DBusMenuQt,Support for notification area menus via the DBusMenu protocol-- Configuring done-- Generating done-- Build files have been written to: /home/user/workspace/Frameworks/knotifications/kf5-qt5 AndroidQt5.11/build[Pipeline] }[Pipeline] // stage[Pipeline] stage[Pipeline] { (Compiling)[Pipeline] sh+ python3 -u ci-tooling/helpers/compile-build.py --product Frameworks --project knotifications --branchGroup kf5-qt5 --platform AndroidQt5.11 --usingInstall /home/user/install-prefix/Scanning dependencies of target KF5Notifications_autogenScanning dependencies of target gradle[  5%] Automatic MOC for target KF5Notifications[ 10%] Generating gradle/gradlew[ 10%] Built target gradleScanning dependencies of target knotifications_aar[ 15%] Generating gradle_build/KF5Notifications/build/outputs/aar/KF5Notifications-debug.aar[ 15%] Built target KF5Notifications_autogen[ 20%] Automatic RCC for knotifications.qrcScanning dependencies of target KF5Notifications[ 25%] Building CXX object src/CMakeFiles/KF5Notifications.dir/knotification.cpp.o[ 30%] Building CXX object src/CMakeFiles/KF5Notifications.dir/knotificationmanager.cpp.o[ 35%] Building CXX object src/CMakeFiles/KF5Notifications.dir/kpassivepopup.cpp.o[ 40%] Building CXX object src/CMakeFiles/KF5Notifications.dir/knotifyconfig.cpp.o[ 45%] Building CXX object src/CMakeFiles/KF5Notifications.dir/knotificationplugin.cpp.o[ 50%] Building CXX object src/CMakeFiles/KF5Notifications.dir/notifybypopupgrowl.cpp.o[ 55%] Building CXX object src/CMakeFiles/KF5Notifications.dir/notifybylogfile.cpp.o[ 60%] Building CXX object src/CMakeFiles/KF5Notifications.dir/notifybyexecute.cpp.oStarting a Gradle Daemon, 1 incompatible and 1 stopped Daemons could not be reused, use --status for details[ 65%] Building CXX object src/CMakeFiles/KF5Notifications.dir/notifybytaskbar.cpp.o[ 70%] Building CXX object src/CMakeFiles/KF5Notifications.dir/ECMQmLoader-knotifications5_qt.cpp.o[ 75%] Building CXX object src/CMakeFiles/KF5Notifications.dir/notifybyandroid.cpp.o[ 80%] Building CXX object src/CMakeFiles/KF5Notifications.dir/debug_p.cpp.o[ 85%] Building CXX object src/CMakeFiles/KF5Notifications.dir/notifybytts.cpp.o[ 90%] Building CXX object src/CMakeFiles/KF5Notifications.dir/KF5Notifications_autogen/mocs_compilation.cpp.o[ 95%] Building CXX object src/CMakeFiles/KF5Notifications.dir/KF5Notifications_autogen/EWIEGA46WW/qrc_knotifications.cpp.o[100%] Linking CXX shared library ../bin/libKF5Notifications.so[100%] Built target KF5NotificationsFAILURE: Build failed with an exception.* Where:Build file '/home/user/workspace/Frameworks/knotifications/kf5-qt5 AndroidQt5.11/build/src/android/gradle_build/KF5Notifications/build.gradle' line: 3* What went wrong:A problem occurred evaluating root project 'KF5Notifications'.> Could not find method google() for arguments [] on repository container of type org.gradle.api.internal.artifacts.dsl.DefaultRepositoryHandler.* Try:Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.BUILD FAILEDTotal time: 3.478 secssrc/android/CMakeFiles/knotifications_aar.dir/build.make:65: recipe for target 'src/android/gradle_build/KF5Notifications/build/outputs/aar/KF5Notifications-debug.aar' failedmake[2]: *** [src/android/gradle_build/KF5Notifications/build/outputs/aar/KF5Notifications-debug.aar] Error 1CMakeFiles/Makefile2:276: recipe for target 'src/android/CMakeFiles/knotifications_aar.dir/all' failedmake[1]: *** [src/android/CMakeFiles/knotifications_aar.dir/all] Error 2Makefile:140: recipe for target 'all' failedmake: *** [all] Error 2[Pipeline] }[Pipeline] // stage[Pipeline] }ERROR: script returned exit 

D18274: Make notifications work on Android with API level < 23

[Volker Krause]

This revision was automatically updated to reflect the committed changes.
Closed by commit R289:60dc31f81b4f: Make notifications work on Android with API 
level  23 (authored by vkrause).

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18274?vs=49551=49655

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

AFFECTED FILES
  src/android/org/kde/knotifications/NotifyByAndroid.java

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

KDE CI: Frameworks » plasma-framework » kf5-qt5 FreeBSDQt5.12 - Build # 10 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/plasma-framework/job/kf5-qt5%20FreeBSDQt5.12/10/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Wed, 16 Jan 2019 16:44:14 +
 Build duration:
3 min 55 sec and counting
   JUnit Tests
  Name: projectroot Failed: 6 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: projectroot.autotests.dialognativetestFailed: projectroot.autotests.plasma_configmodeltestFailed: projectroot.autotests.plasma_fallbackpackagetestFailed: projectroot.autotests.plasma_iconitemtestFailed: projectroot.autotests.plasma_packagestructuretestFailed: projectroot.autotests.plasma_storagetest

D11235: [DrKonqi] Request change of ptrace scope from KCrash

[Oswald Buddenhagen]

ossi added a comment.


  will also need to wait for commit message update, like the other change.

INLINE COMMENTS

> ptracer.cpp:92
> +
> +qCWarning(DRKONQI_LOG) << "debugged process did not acknowledge setting 
> ptracer to" << debuggerpid;
> +close(sfd);

not wrong per se, but mildly misleading, as it suggests the problem is on the 
other end, which is not necessarily true, this being a catch-all. "cannot set 
ptracer" would cover all bases (while still not being particularly helpful).

REPOSITORY
  R871 DrKonqi

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

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D18164: Review KateGotoBar

[loh tar]

loh.tar updated this revision to Diff 49653.
loh.tar edited the summary of this revision.
loh.tar set the repository for this revision to R39 KTextEditor.
loh.tar added a comment.


  - Morph updateData() into a slot
  - connect to Document::textChanged signal

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18164?vs=49636=49653

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/katedialogs.h

To: loh.tar, #ktexteditor, cullmann
Cc: dhaumann, cullmann, anthonyfieroni, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, ngraham, bruns, demsking, sars

D11235: [DrKonqi] Request change of ptrace scope from KCrash

[Christoph Roick]

croick updated this revision to Diff 49651.
croick marked 5 inline comments as done.
croick added a comment.


  - emit warning if no socket is available

REPOSITORY
  R871 DrKonqi

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11235?vs=49649=49651

BRANCH
  ptracer

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

AFFECTED FILES
  src/CMakeLists.txt
  src/debuggerlaunchers.cpp
  src/debuggerlaunchers.h
  src/ptracer.cpp
  src/ptracer.h
  src/tests/crashtest/crashtest.cpp

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11235: [DrKonqi] Request change of ptrace scope from KCrash

[Christoph Roick]

croick added inline comments.

INLINE COMMENTS

> ossi wrote in ptracer.cpp:65
> you really could just use != here ... ^^

wow...

REPOSITORY
  R871 DrKonqi

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

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11236: [KCrash] Establish socket to allow change of ptracer

[Christoph Roick]

croick marked 2 inline comments as done.
croick added a comment.


  > that sounds suspicious. i don't think the kernel's behavior did changed, 
and the process hierarchy presumably didn't, either. the right is handed down 
the ancestry, and that's irrespective of whether the tracer is a "natural" 
ancestor or the tracee, or was set via prctl(). maybe your sysctl settings 
simply changed?
  
  At least I didn't do that knowingly ... I will check what could be the reason.

INLINE COMMENTS

> ossi wrote in kcrash.cpp:657
> ok, so now it's claimed to apply to the entire block. but how can that be? 
> thepollDrKonqiSocket() couldn't possibly be called then ...

True, actually DrKonqi puts the debugger in a block between SIGCONT and SIGSTOP.

REPOSITORY
  R285 KCrash

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

To: croick, #frameworks, ossi
Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, 
ngraham, bruns

D18164: Review KateGotoBar

[Christoph Cullmann]

cullmann added a comment.


  In D18164#394498 , @loh.tar wrote:
  
  > - Fix typo of member vars
  > - Add missing setView(m_view)
  > - Remove FIXME hint about timer, however would a comment for me nice
  > - Use QString()
  >
  >   Things which could be improved or not
  > - The up/down buttons are a little bit small, have tried to give them the 
hight (and width) of the hight of the other buttons, but withoutsuccess. 
Ideas?
  > - There is no tooltip on the spinbox and Goto button
  
  
  I can live with that ;=)
  
  > - The spinbox will not adjust its size and value boundaries when you paste 
text while its shown. Try:
  >   - New file->show bar
  >   - Paste 10+ lines
  >   - Spinbox keep size of one digit and can't be set to an other value as 1
  
  Perhaps one wants to connect to the textChanged signal and update the range.

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

To: loh.tar, #ktexteditor, cullmann
Cc: dhaumann, cullmann, anthonyfieroni, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, ngraham, bruns, demsking, sars

D11236: [KCrash] Establish socket to allow change of ptracer

[Christoph Roick]

croick updated this revision to Diff 49650.
croick marked 2 inline comments as done.
croick added a comment.


  - modify comment and remove check for EAGAIN

REPOSITORY
  R285 KCrash

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11236?vs=49617=49650

BRANCH
  ptracer

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

AFFECTED FILES
  src/kcrash.cpp

To: croick, #frameworks, ossi
Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, 
ngraham, bruns

D11235: [DrKonqi] Request change of ptrace scope from KCrash

[Christoph Roick]

croick updated this revision to Diff 49649.
croick marked an inline comment as done.
croick added a comment.


  - change order and add comment to kdeinit option in crashtest
  - add warning if debuggee does not respond as expected

REPOSITORY
  R871 DrKonqi

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11235?vs=49618=49649

BRANCH
  ptracer

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

AFFECTED FILES
  src/CMakeLists.txt
  src/debuggerlaunchers.cpp
  src/debuggerlaunchers.h
  src/ptracer.cpp
  src/ptracer.h
  src/tests/crashtest/crashtest.cpp

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D18125: KateStatusBar: Add dictionary button

[loh tar]

loh.tar added a subscriber: cullmann.
loh.tar added a comment.


  @cullmann @dhaumann A short not how to progress would be nice.
  
  - Keep codes as they are
  - Shorten codes as suggested
  - Replace codes by flags/ensign (no idea how)
  - Add (unneeded) label
  - Show (unneeded long) full name
  
  The latter two consume to much space for my taste and give only the very new 
user a benefit, once seen/clicked it's clear.
  
  Here a shot from an attempt to address some issues:
  
  - No "Preferred Dictionaries" header at the separator to not confuse when 
user has a non preferred dict in use (add dict to menu is todo)
  - Change order of entries with the intend to make more often used stuff 
accessible more easy (still experimental)
  - Removed "Spellcheck Selection...". I would like remove that from the main 
menu too and modify the normal "Spelling..." action to behave "smart" as many 
other actions
  
  F6551960: 1547653694.png 

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

To: loh.tar, #ktexteditor
Cc: cullmann, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, bruns, demsking, sars

D11235: [DrKonqi] Request change of ptrace scope from KCrash

[Oswald Buddenhagen]

ossi requested changes to this revision.
ossi added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> ptracer.cpp:65
> +bytes += r;
> +else if (r == -1 && !(errno == EINTR))
> +break;

you really could just use != here ... ^^

> ptracer.cpp:78
> +bytes += r;
> +else if (r == -1 && !(errno == EINTR))
> +break;

ditto

> ptracer.cpp:82
> +if (bytes == msize && memcmp(msg, rmsg, msize) == 0)
> +qCInfo(DRKONQI_LOG) << "ptracer set to" << debuggerpid 
> << "by debugged process";
> +}

a complementary warning if any of the steps goes wrong would seem appropriate.

> crashtest.cpp:139
>  
> -  //start drkonqi directly so that drkonqi's output goes to the console
> +  //start drkonqi directly by default so that drkonqi's output goes to the 
> console
>KCrash::CrashFlags flags = KCrash::AlwaysDirectly;

add comment: this can be disabled to be able to test kcrash's real default 
behavior.

> crashtest.cpp:143
>  flags |= KCrash::AutoRestart;
> +  if (parser.isSet(QStringLiteral("kdeinit")))
> +flags &= ~KCrash::AlwaysDirectly;

move that up, so the AlwaysDirectly stuff is in one block.

REPOSITORY
  R871 DrKonqi

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

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11236: [KCrash] Establish socket to allow change of ptracer

[Oswald Buddenhagen]

ossi added a comment.


  In D11236#394363 , @croick wrote:
  
  > Actually the point I removed does not seem to be true any longer. I'm 
almost certain, that the `prctrl(PR_SET_PTRACER, ...)` was not required until 
some time ago when starting DrKonqi as a fork. But now the internal backtrace 
is not working without. I cannot tell, when that started (currently using 
kernel 4.20).
  
  
  that sounds suspicious. i don't think the kernel's behavior did changed, and 
the process hierarchy presumably didn't, either. the right is handed down the 
ancestry, and that's irrespective of whether the tracer is a "natural" ancestor 
or the tracee, or was set via prctl(). maybe your sysctl settings simply 
changed?

INLINE COMMENTS

> kcrash.cpp:657
>  // Wait forever until the started process exits. This code path is 
> executed
> -// when launching drkonqi. Note that drkonqi will stop this process 
> in the meantime.
> -if (directly) {
> -//if the process was started directly, use waitpid(), as it's a 
> child...
> -while (waitpid(-1, nullptr, 0) != pid) {}
> -} else {
> +// when launching drkonqi. Note that DrKonqi will SIGSTOP this 
> process in the meantime
> +// and a backtrace will end here.

ok, so now it's claimed to apply to the entire block. but how can that be? 
thepollDrKonqiSocket() couldn't possibly be called then ...

> kcrash.cpp:907
> +r = poll(, 1, 1000); // wait for 1 second for a request by DrKonqi
> +} while (r == -1 && (errno == EINTR || errno == EAGAIN));
> +// only continue if POLLIN event returned

remove eagain here as well.

REPOSITORY
  R285 KCrash

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

To: croick, #frameworks, ossi
Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, 
ngraham, bruns

D18299: ComboBox: fix default delegate

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> ComboBox.qml:58
> +controlRoot.currentIndex = index;
> +controlRoot.popup.visible = false;
> +}

Are you sure this is needed? Qt docs say for that using `ItemDelegate` for a 
`ComboBox` is recommended as:
"This ensures that the interaction works as expected, and the popup will 
automatically close when appropriate. "
The `currentIndex` change makes sense, though

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: apol, #frameworks, davidedmundson
Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18299: ComboBox: fix default delegate

[Aleix Pol Gonzalez]

This revision was automatically updated to reflect the committed changes.
Closed by commit R858:cb49b355f946: ComboBox: fix default delegate (authored by 
apol).

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18299?vs=49646=49648

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

AFFECTED FILES
  org.kde.desktop/ComboBox.qml

To: apol, #frameworks, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18299: ComboBox: fix default delegate

[David Edmundson]

davidedmundson accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

BRANCH
  master

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

To: apol, #frameworks, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18299: ComboBox: fix default delegate

[Aleix Pol Gonzalez]

apol created this revision.
apol added a reviewer: Frameworks.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
apol requested review of this revision.

REVISION SUMMARY
  Should use the selected item when it's interacted with using the mouse.

TEST PLAN
  Selecting an item from the test combo box works now

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

BRANCH
  master

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

AFFECTED FILES
  org.kde.desktop/ComboBox.qml

To: apol, #frameworks
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18274: Make notifications work on Android with API level < 23

[Aleix Pol Gonzalez]

apol accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R289 KNotifications

BRANCH
  master

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

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

D18164: Review KateGotoBar

[loh tar]

loh.tar updated this revision to Diff 49636.
loh.tar added a comment.


  - Fix typo of member vars
  - Add missing setView(m_view)
  - Remove FIXME hint about timer, however would a comment for me nice
  - Use QString()
  
  Things which could be improved or not
  
  - The up/down buttons are a little bit small, have tried to give them the 
hight (and width) of the hight of the other buttons, but withoutsuccess. 
Ideas?
  - There is no tooltip on the spinbox and Goto button
  - The spinbox will not adjust its size and value boundaries when you paste 
text while its shown. Try:
- New file->show bar
- Paste 10+ lines
- Spinbox keep size of one digit and can't be set to an other value as 1

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18164?vs=49243=49636

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/katedialogs.h

To: loh.tar, #ktexteditor, cullmann
Cc: dhaumann, cullmann, anthonyfieroni, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, ngraham, bruns, demsking, sars

D18249: [datamodel] Rework items insert/remove

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 49630.

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18249?vs=49469=49630

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

AFFECTED FILES
  src/declarativeimports/core/datamodel.cpp

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

D18249: [datamodel] Rework items insert/remove

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in datamodel.cpp:379
> Optimization is you have [5, 6, 8] then [5, 6, 8, 9] so it make only 
> beginInsertRows({}, 3, 3);
> full replace (m_items[sourceName] = list.toVector();)
> endInsertRows()
> Same if you [5, 6]
> beginRemoveRows
> full replace (m_items[sourceName] = list.toVector();)
> endRemoveRows
> 
> It does not touch either m_roleNames size, which are changed.
> 
> I try to remove all first, then re-add with m_roleNames changes.

The worst case is when you have different values [5, 6, 8] and [5, 8, 9, 10] it 
will add only 10 and internally replace other, is QML model expect that? So why 
i first make full remove, then full insert.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

D18288: Make the clock svg's shadows more logically correct and visually appropriate

[David Edmundson]

davidedmundson accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  clock-spindle-shadow (branched from master)

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

To: ngraham, #vdg, #plasma, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D11235: [DrKonqi] Request change of ptrace scope from KCrash

[Christoph Roick]

croick marked 11 inline comments as done.
croick added a comment.


  `read()` and `write()` were assumed to work right away, I added a loop around.

INLINE COMMENTS

> ossi wrote in crashtest.cpp:133
> this addition isn't used or explained anywhere for all i can tell.

It's used to unset the `AlwaysDirectly` flag. It allows testing using with 
direct start and kdeinit start.

> ossi wrote in crashtest.cpp:138
> only if kdeinit was started from the same console, which you cannot assume.
> 
> also, this change wouldn't belong into this patch anyway.

Agreed, I added an option instead and would commit that separately.

REPOSITORY
  R871 DrKonqi

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

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11236: [KCrash] Establish socket to allow change of ptracer

[Christoph Roick]

croick marked 10 inline comments as done.
croick added a comment.


  > please explicitly mark all handled issues as done - you'll notice on the 
way that you didn't address some of them.
  
  Sorry, you were just responding too quickly.
  
  > you adjusted the summary kinda the wrong way around ... but come to think 
of it, i was actually kinda wrong - you indeed need to list all three cases to 
illustrate that neither applies. the ancestry case is special only in the sense 
that it automatically makes the "internal" debugger work (i'd mention that in 
parentheses of that case's bullet point).
  
  Actually the point I removed does not seem to be true any longer. I'm almost 
certain, that the `prctrl(PR_SET_PTRACER, ...)` was not required until some 
time ago when starting DrKonqi as a fork. But now the internal backtrace is not 
working without. I cannot tell, when that started (currently using kernel 4.20).

INLINE COMMENTS

> ossi wrote in kcrash.cpp:674
> please consistently put a space after the // marker. also, stick to the 
> file's capitalization style.

That comment was just copied from the old code.

> ossi wrote in kcrash.cpp:686
> hmm, what about the last sentence? it seems to me that some adjustment 
> (possibly just additional comments) is required.

see line 656

REPOSITORY
  R285 KCrash

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

To: croick, #frameworks, ossi
Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, 
ngraham, bruns

D11235: [DrKonqi] Request change of ptrace scope from KCrash

[Christoph Roick]

croick updated this revision to Diff 49618.
croick added a comment.


  - use debuggee pid instead of DrKonqi pid for socket name
  - make sure to read/write all bytes from/to socket

REPOSITORY
  R871 DrKonqi

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11235?vs=49607=49618

BRANCH
  ptracer

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

AFFECTED FILES
  src/CMakeLists.txt
  src/debuggerlaunchers.cpp
  src/debuggerlaunchers.h
  src/ptracer.cpp
  src/ptracer.h
  src/tests/crashtest/crashtest.cpp

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11236: [KCrash] Establish socket to allow change of ptracer

[Christoph Roick]

croick updated this revision to Diff 49617.
croick marked 2 inline comments as done.
croick added a comment.


  - unlink old sockets before binding
  - remove superfluous EAGAIN check
  - use debuggee pid instead of DrKonqi pid for socket name
  - fix comments

REPOSITORY
  R285 KCrash

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11236?vs=49609=49617

BRANCH
  ptracer

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

AFFECTED FILES
  src/kcrash.cpp

To: croick, #frameworks, ossi
Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, 
ngraham, bruns

D11235: [DrKonqi] Request change of ptrace scope from KCrash

[Oswald Buddenhagen]

ossi requested changes to this revision.
ossi added a comment.
This revision now requires changes to proceed.


  mark handled issues as done here as well.

INLINE COMMENTS

> ptracer.cpp:47
> +
> QStringLiteral("%1/kcrash_%2").arg(QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation))
> +  
> .arg(QCoreApplication::applicationPid());
> +

see complementary patch

> ptracer.cpp:60
> +if (write(sfd, msg, 22) > 0) {
> +fd_set set;
> +FD_ZERO();

cruft

> ptracer.cpp:68
> +if (poll(, 1, 1000) > 0 && (fd.revents & POLLIN) &&
> +read(sfd, rmsg, 22) > 0 &&
> +strncmp(msg, rmsg, 22) == 0) {

i'd compare to 22, because we know that the other side sends that.

> ptracer.cpp:69
> +read(sfd, rmsg, 22) > 0 &&
> +strncmp(msg, rmsg, 22) == 0) {
> +qCInfo(DRKONQI_LOG) << "ptracer set to" << pid << "by 
> debugged process";

i'd use memcmp(), as it's a fixed-size data block.

> ptracer.h:22
> +
> +/** On Linux, tell the process to allow the debugger to attach to itr */
> +void setPtracer(qint64 pid);

-r

> crashtest.cpp:133
>parser.addOption(QCommandLineOption(QStringLiteral("autorestart"), 
> i18n("Automatically restart")));
> +  parser.addOption(QCommandLineOption(QStringLiteral("kdeinit"), i18n("Start 
> DrKonqi using kdeinit")));
>parser.addPositionalArgument(QStringLiteral("type"), i18n("Type of 
> crash."), QStringLiteral("crash|malloc|div0|assert|threads"));

this addition isn't used or explained anywhere for all i can tell.

REPOSITORY
  R871 DrKonqi

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

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11236: [KCrash] Establish socket to allow change of ptracer

[Oswald Buddenhagen]

ossi requested changes to this revision.
ossi added a comment.
This revision now requires changes to proceed.


  mostly minor issues left.
  
  please explicitly mark all handled issues as done - you'll notice on the way 
that you didn't address some of them.
  
  you adjusted the summary kinda the wrong way around ... but come to think of 
it, i was actually kinda wrong - you indeed need to list all three cases to 
illustrate that neither applies. the ancestry case is special only in the sense 
that it automatically makes the "internal" debugger work (i'd mention that in 
parentheses of that case's bullet point).

INLINE COMMENTS

> kcrash.cpp:668
> +
> QStringLiteral("%1/kcrash_%2").arg(QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation))
> +  .arg(pid));
> +int sockfd = openDrKonqiSocket(socketpath);

not really significant, but imo it's backwards that you let the drkonqi pid 
rather than the kcrash pid determine the socket name.

> kcrash.cpp:674
> +if (directly) {
> +//if the process was started directly, use waitpid(), as 
> it's a child...
> +while ((running = waitpid(pid, nullptr, WNOHANG) != pid) && 
> pollDrKonqiSocket(pid, sockfd) >= 0) {}

please consistently put a space after the // marker. also, stick to the file's 
capitalization style.

> kcrash.cpp:686
> +// Wait forever until the started process exits. This code path is 
> executed
> +// when launching drkonqi. Note that drkonqi will SIGSTOP this 
> process in the meantime.
> +if (running) {

hmm, what about the last sentence? it seems to me that some adjustment 
(possibly just additional comments) is required.

> kcrash.cpp:870
> +{
> +struct sockaddr_un drkonqi_server;
> +

i'd move that down, before the member init.

REPOSITORY
  R285 KCrash

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

To: croick, #frameworks, ossi
Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, 
ngraham, bruns

D17691: Add rows info to the plasma virtual desktop protocol

[Marco Martin]

mart updated this revision to Diff 49610.
mart added a comment.


  - remove dead symbol

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17691?vs=49425=49610

BRANCH
  phab/virtualdesktoprows

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

AFFECTED FILES
  autotests/client/test_plasma_virtual_desktop.cpp
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/protocols/plasma-virtual-desktop.xml
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h

To: mart, #plasma, #kwin
Cc: zzag, davidedmundson, hein, kde-frameworks-devel, michaelh, ngraham, bruns

D11236: [KCrash] Establish socket to allow change of ptracer

[Christoph Roick]

croick updated this revision to Diff 49609.
croick retitled this revision from "[KCrash] Establish socket to allow change 
of ptrace scope" to "[KCrash] Establish socket to allow change of ptracer".
croick edited the summary of this revision.
croick added a comment.


  - use poll instead of select
  - set ptracer pid and poll for request for the directly started DrKonqi as 
well and make the ifdef-section more readable
  - replace bogus perror

REPOSITORY
  R285 KCrash

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11236?vs=43560=49609

BRANCH
  ptracer

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

AFFECTED FILES
  src/kcrash.cpp

To: croick, #frameworks, ossi
Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, 
ngraham, bruns

D11235: [DrKonqi] Request change of ptrace scope from KCrash

[Christoph Roick]

croick updated this revision to Diff 49607.
croick edited the summary of this revision.
croick edited the test plan for this revision.
croick added a comment.


  - rename queryPtrace to setPtracer
  - add a kdeinit option in crashtest

REPOSITORY
  R871 DrKonqi

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11235?vs=34671=49607

BRANCH
  ptracer

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

AFFECTED FILES
  src/CMakeLists.txt
  src/debuggerlaunchers.cpp
  src/debuggerlaunchers.h
  src/ptracer.cpp
  src/ptracer.h
  src/tests/crashtest/crashtest.cpp

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D18249: [datamodel] Rework items insert/remove

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> davidedmundson wrote in datamodel.cpp:379
> I don't understand what you're changing here, can you provide a bit more 
> detail on the exact problem.

Optimization is you have [5, 6, 8] then [5, 6, 8, 9] so it make only 
beginInsertRows({}, 3, 3);
full replace (m_items[sourceName] = list.toVector();)
endInsertRows()
Same if you [5, 6]
beginRemoveRows
full replace (m_items[sourceName] = list.toVector();)
endRemoveRows

It does not touch either m_roleNames size, which are changed.

I try to remove all first, then re-add with m_roleNames changes.

> davidedmundson wrote in datamodel.cpp:444
> this isn't equivalent.
> 
> I could have an entry with an empty count
> 
> Now this gets left behind

Got'cha

REPOSITORY
  R242 Plasma Framework (Library)

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

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

D18174: Ruby: fix RegExp after ": " and fix/improve detection of HEREDOC

[Nibaldo González]

nibags closed this revision.

REPOSITORY
  R216 Syntax Highlighting

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

To: nibags, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D18174: Ruby: fix RegExp after ": " and fix/improve detection of HEREDOC

[Nibaldo González]

nibags edited the summary of this revision.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  fix-ruby

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

To: nibags, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D18174: Ruby: fix RegExp after ": " and fix/improve detection of HEREDOC

[Nibaldo González]

nibags updated this revision to Diff 49600.
nibags edited the summary of this revision.
nibags added a comment.


  - Small correction in comment

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18174?vs=49212=49600

BRANCH
  fix-ruby

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

AFFECTED FILES
  autotests/folding/highlight.rb.fold
  autotests/html/highlight.rb.html
  autotests/input/highlight.rb
  autotests/reference/highlight.rb.ref
  data/syntax/ruby.xml

To: nibags, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D18164: Review KateGotoBar

[Christoph Cullmann]

cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.


  I think it is a nice enhancement over the current state.
  
  I give it an accept, but wait for the "Regarding code comments, will fix it 
when it got in general an "OK"" before I apply it, thanks!

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

To: loh.tar, #ktexteditor, cullmann
Cc: dhaumann, cullmann, anthonyfieroni, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, ngraham, bruns, demsking, sars