Disabling of 'kdav-davitemfetchjob' test in KDav

[Ben Cooksley]

Hi everyone,

Currently we have an issue where the test 'kdav-davitemfetchjob' within
kdav causes kdeinit5 to be relaunched, as can be seen in the CI
run log below.

https://build.kde.org/job/Applications/job/kdav/job/kf5-qt5%20SUSEQt5.12/38/console

This is behaviour which is not permitted within the CI system, as the
newly spawned 'kdeinit5' processes are treated as being launched by
that test by CTest, meaning it will wait for eternity for them to exit
(which as resident daemon processes, they will not do without manual
intervention).

This in turn blocks the job from completing, and blocks CI resources
in the process.

I'd therefore like to propose disabling this test across all platforms
as it's behaviour is incompatible with the CI system.

Cheers,
Ben

D25296: [RFC] Fix Display Configuration icon margins

[Noah Davis]

ndavis added a comment.


  I figured it out. The 22px version needs the `22-22-` prefix in its ID.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #vdg, ndavis
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

Re: kconfig-elektra

[David Faure]

On mardi 12 novembre 2019 16:58:19 CET k...@markus-raab.org wrote:
> Dear KDE developers,
> 
> As discussed in Akademy 2018 [1] Felix Resch and Dardan Haxhimustafa are
> working on a patch for KConfig so that KConfig uses Elektra [2] instead
> of the KConfigINI backend.
> 
> We forked the KConfig repository [3] and currently try to:
> 1. successfully start KDE to use Elektra (Felix Resch)
> 2. implement a new plugin for Elektra that supports the KConfig INI
>files for a smooth transition to Elektra (Dardan Haxhimustafa)
> 3. improve Elektra's qt-gui, which will provide a low-level GUI for the
>whole (KDE) configuration (Dardan Haxhimustafa)

Cool.
 
> Two questions came up until now:
> 1. Is KConfigBackend supposed to be thread safe?

No, KSharedConfig::openConfig is thread safe and creates different KConfig 
instances in each thread. This way, KConfig and KConfigBackend don't need to 
be thread-safe.

> 2. If Elektra finds a merge conflict between the KConfig objects and the
>configuration files on the disc, should we then do a 3-way merge
>which prefers "ours" or should we ask the user with a KDialog how to
>proceed?

I'm a bit out of context (2018 was a long time ago), what's the scenario that 
might lead to this happening? It sounds to me like a dialog would bother the 
user with low-level technical things that they won't understand nor know how 
to resolve. If you can describe how this can happen, then we can conclude what 
should be happening automatically to fix the situation.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

D25265: Support adding a contact to a specific backend

[Jonah Brüchert]

jbbgameich added inline comments.

INLINE COMMENTS

> apol wrote in datasourcemodel.h:43
> Use DisplayRole for this?

I intend to add an actual display role later. Strictly speaking this is the id 
used by KPeople and not anything that should be shown to the user ideally.

REPOSITORY
  R307 KPeople

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

To: jbbgameich, apol, #plasma:_mobile
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

[Alexander Saoutkin]

feverfew added inline comments.

INLINE COMMENTS

> sitter wrote in krun.cpp:598
> So did the old code though. See old line 584.
> 
> It's iterating by-reference, so you can simply assign to it via
> 
> https://doc.qt.io/qt-5/qurl.html#operator-eq-1
> 
> to change the content of the QUrl object (not the instance of the object in 
> the list, but rather the content of that object).

That's within the same loop though?

I explicitly want to change the QUrl in that list, so that I can simply return 
the same list without having to do a copy of it all.

I can't get this to work. I need to change the definition of the struct to be:

`struct MountRequest {QDBusReply reply; QUrl &url;};`

This doesn't compile because "is implicitly deleted because the default 
definition would be ill-formed"...

REPOSITORY
  R241 KIO

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

To: feverfew, fvogt, davidedmundson, dfaure, ngraham
Cc: sitter, davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, 
michaelh, bruns

D25015: Update breeze theme shadows

[Niccolò Venerandi]

niccolove added a comment.


  In D25015#563147 , @ngraham wrote:
  
  > Do these shadows get cached or something? When I build the diff and restart 
plasmashell, the shadows I see are identical, pixel-for-pixel.
  
  
  I don't know. But you can manually change them by replacing the .svgz in 
/usr/share/plasma/desktopthemes/default/ dialogs/background.svgz and 
widgets/panel-background.svgz with the new svg.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg
Cc: cblack, ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, 
LeGast00n, GB_2, michaelh, bruns

D25015: Update breeze theme shadows

[Nathaniel Graham]

ngraham added a comment.


  Do these shadows get cached or something? When I build the diff and restart 
plasmashell, the shadows I see are identical, pixel-for-pixel.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg
Cc: cblack, ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, 
LeGast00n, GB_2, michaelh, bruns

D19498: loadLibrary: Use enum values to define what type of plugin we load

[Albert Astals Cid]

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

REPOSITORY
  R317 Kross

BRANCH
  enum-for-plugin-types

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

To: aspotashev, #frameworks, aacid
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D21235: Add handling of fuseiso filesystem type

[Stefan Brüns]

bruns added a comment.


  Moving it to a separate class will also make creating a unit test trivial.

REPOSITORY
  R245 Solid

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

To: hallas, bruns, ngraham
Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D21235: Add handling of fuseiso filesystem type

[Stefan Brüns]

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

INLINE COMMENTS

> fstabhandling.cpp:301
> +
> +void Solid::Backends::Fstab::FstabHandling::parseMtabFile(const QString& 
> mtabPath, QMultiHash& activeMounts, QHash QString>& activeFstype)
> +{

The API is totally awkward.

You end up with something significantly cleaner when you create a new class for 
it:

  QStringMultiHash m_mtabCache;
  QHash m_mtabFstypeCache;
  bool m_mtabCacheValid;

and make the parser a member function.

Then instantiate the class for the fuseiso case and for the globalFstabCache.

REPOSITORY
  R245 Solid

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

To: hallas, bruns, ngraham
Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25015: Update breeze theme shadows

[Niccolò Venerandi]

niccolove edited the test plan for this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg
Cc: cblack, ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, 
LeGast00n, GB_2, michaelh, bruns

D25015: Update breeze theme shadows

[Niccolò Venerandi]

niccolove updated this revision to Diff 69820.
niccolove added a comment.


  Updated to new shadows

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25015?vs=68912&id=69820

BRANCH
  breeze-shadows (branched from master)

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

AFFECTED FILES
  src/desktoptheme/breeze/dialogs/background.svg
  src/desktoptheme/breeze/widgets/panel-background.svg

To: niccolove, #vdg
Cc: cblack, ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, 
LeGast00n, GB_2, michaelh, bruns

kconfig-elektra

[kde5]

Dear KDE developers,

As discussed in Akademy 2018 [1] Felix Resch and Dardan Haxhimustafa are
working on a patch for KConfig so that KConfig uses Elektra [2] instead
of the KConfigINI backend.

We forked the KConfig repository [3] and currently try to:
1. successfully start KDE to use Elektra (Felix Resch)
2. implement a new plugin for Elektra that supports the KConfig INI
   files for a smooth transition to Elektra (Dardan Haxhimustafa)
3. improve Elektra's qt-gui, which will provide a low-level GUI for the
   whole (KDE) configuration (Dardan Haxhimustafa)

Two questions came up until now:
1. Is KConfigBackend supposed to be thread safe?
2. If Elektra finds a merge conflict between the KConfig objects and the
   configuration files on the disc, should we then do a 3-way merge
   which prefers "ours" or should we ask the user with a KDialog how to
   proceed?

We welcome any Feedback either here in the mailing list or as issues
in our KConfig repository [3].

best regards,
Markus Raab, Felix Resch and Dardan Haxhimustafa


[1] https://community.kde.org/Akademy/2018/Config_Workshop
Slides: http://www.complang.tuwien.ac.at/raab/akademy.pdf
[2] https://www.libelektra.org/
[3] https://github.com/ElektraInitiative/kconfig

-- 
Markus Raab  http://www.complang.tuwien.ac.at/raab/
TU Wien   markus.r...@complang.tuwien.ac.at
Compilers and Languages  Phone: (+431) 58801/185185
Argentinierstr. 8, 1040 Wien, Austria   DVR 0005886

D25236: Remove the last traces of KSslError from TCPSlaveBase

[Albert Astals Cid]

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

REPOSITORY
  R241 KIO

BRANCH
  next

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

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

D24568: Provide clang-format target with a KDE Frameworks style file

[David Edmundson]

davidedmundson added a comment.


  As an update on this from the Plasma POV.
  
  I added the macro to every repo and told every dev to do a final test before 
we commit the formatted results.
  
  I had some feedback and the result was that we can't proceed with in the 
current state [1].
  
  What's noteworthy is we were generally ok with the results from the first 
file we prepared in T11214 , so potentially 
we just need some settings tweaked. 
  I'll try and break that down into future diffs.
  
  1.https://mail.kde.org/pipermail/plasma-devel/2019-November/106186.html

REPOSITORY
  R240 Extra CMake Modules

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

To: cullmann, #frameworks, dfaure
Cc: zzag, sitter, mwolff, ochurlaud, nalvarez, kossebau, aacid, davidedmundson, 
dhaumann, apol, ognarb, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

KDE CI: Frameworks » kitemmodels » kf5-qt5 SUSEQt5.13 - Build # 39 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kitemmodels/job/kf5-qt5%20SUSEQt5.13/39/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Fri, 15 Nov 2019 18:07:37 +
 Build duration:
4 min 19 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5ItemModels-5.65.0.xmlcompat_reports/KF5ItemModels_compat_report.htmllogs/KF5ItemModels/5.65.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: 1 test(s), Passed: 11 test(s), Skipped: 0 test(s), Total: 12 test(s)Failed: projectroot.autotests.kconcatenaterows_qml
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report60%
(3/5)57%
(43/76)57%
(43/76)72%
(6504/8981)57%
(2922/5098)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(14/14)100%
(14/14)99%
(2416/2440)62%
(1051/1701)autotests.proxymodeltestsuite67%
(12/18)67%
(12/18)69%
(1759/2545)54%
(651/1211)src.core81%
(17/21)81%
(17/21)80%
(2329/2915)65%
(1220/1886)src.qml0%
(0/2)0%
(0/2)0%
(0/26)0%
(0/2)tests.proxymodeltestapp0%
(0/21)0%
(0/21)0%
(0/1055)0%
(0/298)

KDE CI: Frameworks » kitemmodels » kf5-qt5 SUSEQt5.12 - Build # 62 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kitemmodels/job/kf5-qt5%20SUSEQt5.12/62/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Fri, 15 Nov 2019 18:07:36 +
 Build duration:
3 min 50 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5ItemModels-5.65.0.xmlcompat_reports/KF5ItemModels_compat_report.htmllogs/KF5ItemModels/5.65.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: 1 test(s), Passed: 11 test(s), Skipped: 0 test(s), Total: 12 test(s)Failed: projectroot.autotests.kconcatenaterows_qml
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report60%
(3/5)57%
(43/76)57%
(43/76)72%
(6504/8985)57%
(2922/5098)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(14/14)100%
(14/14)99%
(2416/2440)62%
(1051/1701)autotests.proxymodeltestsuite67%
(12/18)67%
(12/18)69%
(1759/2545)54%
(651/1211)src.core81%
(17/21)81%
(17/21)80%
(2329/2916)65%
(1220/1886)src.qml0%
(0/2)0%
(0/2)0%
(0/26)0%
(0/2)tests.proxymodeltestapp0%
(0/21)0%
(0/21)0%
(0/1058)0%
(0/298)

KDE CI: Frameworks » kitemmodels » kf5-qt5 FreeBSDQt5.13 - Build # 36 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kitemmodels/job/kf5-qt5%20FreeBSDQt5.13/36/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Fri, 15 Nov 2019 18:07:36 +
 Build duration:
2 min 5 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 11 test(s), Skipped: 0 test(s), Total: 12 test(s)Failed: projectroot.autotests.kconcatenaterows_qml

D25330: Expose KNumberModel to QML

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R275:a1cccfa14d06: Expose KNumberModel to QML (authored by 
davidedmundson).

REPOSITORY
  R275 KItemModels

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25330?vs=69813&id=69818

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

AFFECTED FILES
  src/core/CMakeLists.txt
  src/qml/plugin.cpp

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

D25330: Expose KNumberModel to QML

[Volker Krause]

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

REPOSITORY
  R275 KItemModels

BRANCH
  master

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

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

D25308: when kioslave5 couldn't be found in libexec-ish locations try $PATH

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> slave.cpp:521
> +// isn't the same as applicationDirPath().
> +QString kioslaveExecutable = 
> QStandardPaths::findExecutable(QStringLiteral("kioslave5"));
> +}

Remove QString declaration before.

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: sitter, dfaure, apol
Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D25331: Duplicate QML installation for unit tests

[David Edmundson]

davidedmundson created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  We run tests pre-installation so have to do various hacks to make things
  findable.
  
  This copies the approach used in p-f.

TEST PLAN
  Deleted my install
  Ran test

REPOSITORY
  R275 KItemModels

BRANCH
  master

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

AFFECTED FILES
  src/qml/CMakeLists.txt

To: davidedmundson
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25296: [RFC] Fix Display Configuration icon margins

[Nathaniel Graham]

ngraham updated this revision to Diff 69815.
ngraham marked 4 inline comments as done.
ngraham added a comment.


  - Fix stylesheet and color-changing ability
  - Vertical lines still blurry; not sure why

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25296?vs=69716&id=69815

BRANCH
  fix-xrandr-icon (branched from master)

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

AFFECTED FILES
  src/desktoptheme/breeze/icons/preferences.svg

To: ngraham, #vdg, ndavis
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

KDE CI: Frameworks » kitemmodels » kf5-qt5 SUSEQt5.13 - Build # 38 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kitemmodels/job/kf5-qt5%20SUSEQt5.13/38/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Fri, 15 Nov 2019 15:40:40 +
 Build duration:
14 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5ItemModels-5.65.0.xmlcompat_reports/KF5ItemModels_compat_report.htmllogs/KF5ItemModels/5.65.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: 1 test(s), Passed: 11 test(s), Skipped: 0 test(s), Total: 12 test(s)Failed: projectroot.autotests.kconcatenaterows_qml
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report60%
(3/5)57%
(43/76)57%
(43/76)72%
(6504/8980)57%
(2922/5098)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(14/14)100%
(14/14)99%
(2416/2440)62%
(1051/1701)autotests.proxymodeltestsuite67%
(12/18)67%
(12/18)69%
(1759/2545)54%
(651/1211)src.core81%
(17/21)81%
(17/21)80%
(2329/2915)65%
(1220/1886)src.qml0%
(0/2)0%
(0/2)0%
(0/25)0%
(0/2)tests.proxymodeltestapp0%
(0/21)0%
(0/21)0%
(0/1055)0%
(0/298)

KDE CI: Frameworks » kitemmodels » kf5-qt5 SUSEQt5.12 - Build # 61 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kitemmodels/job/kf5-qt5%20SUSEQt5.12/61/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Fri, 15 Nov 2019 15:40:41 +
 Build duration:
11 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5ItemModels-5.65.0.xmlcompat_reports/KF5ItemModels_compat_report.htmllogs/KF5ItemModels/5.65.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: 1 test(s), Passed: 11 test(s), Skipped: 0 test(s), Total: 12 test(s)Failed: projectroot.autotests.kconcatenaterows_qml
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report60%
(3/5)57%
(43/76)57%
(43/76)72%
(6504/8984)57%
(2922/5098)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(14/14)100%
(14/14)99%
(2416/2440)62%
(1051/1701)autotests.proxymodeltestsuite67%
(12/18)67%
(12/18)69%
(1759/2545)54%
(651/1211)src.core81%
(17/21)81%
(17/21)80%
(2329/2916)65%
(1220/1886)src.qml0%
(0/2)0%
(0/2)0%
(0/25)0%
(0/2)tests.proxymodeltestapp0%
(0/21)0%
(0/21)0%
(0/1058)0%
(0/298)

Re: KDE CI: Frameworks » kitemmodels » kf5-qt5 SUSEQt5.13 - Build # 37 - Unstable!

[David Edmundson]

Urgh, my fault.
Will fix.

KDE CI: Frameworks » kitemmodels » kf5-qt5 FreeBSDQt5.13 - Build # 35 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kitemmodels/job/kf5-qt5%20FreeBSDQt5.13/35/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Fri, 15 Nov 2019 15:40:40 +
 Build duration:
8 min 10 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 11 test(s), Skipped: 0 test(s), Total: 12 test(s)Failed: projectroot.autotests.kconcatenaterows_qml

D25330: Expose KNumberModel to QML

[David Edmundson]

davidedmundson created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REPOSITORY
  R275 KItemModels

BRANCH
  master

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

AFFECTED FILES
  src/core/CMakeLists.txt
  src/qml/plugin.cpp

To: davidedmundson
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25270: Correctly set i18n arguments in one pass

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R304:0099d090be51: Correctly set i18n arguments in one pass 
(authored by davidedmundson).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25270?vs=69630&id=69814

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

AFFECTED FILES
  src/qtquick/qml/Button.qml

To: davidedmundson, leinir, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D13358: Add new class that is a model of numbers between two values

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R275:4b1e1b44ef9f: Add new class that is a model of numbers 
between two values (authored by davidedmundson).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D13358?vs=35681&id=69812#toc

REPOSITORY
  R275 KItemModels

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13358?vs=35681&id=69812

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/knumbermodeltest.cpp
  src/core/CMakeLists.txt
  src/core/knumbermodel.cpp
  src/core/knumbermodel.h

To: davidedmundson, vkrause
Cc: broulik, markg, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D25270: Correctly set i18n arguments in one pass

[Aleix Pol Gonzalez]

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

REPOSITORY
  R304 KNewStuff

BRANCH
  master

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

To: davidedmundson, leinir, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25308: when kioslave5 couldn't be found in libexec-ish locations try $PATH

[Aleix Pol Gonzalez]

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

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: sitter, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25265: Support adding a contact to a specific backend

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> datasourcemodel.h:43
> +enum RoleNames {
> +PluginIdRole = Qt::UserRole + 1,
> +};

Use DisplayRole for this?

> personpluginmanager.h:91
> + * Creates a contact with the specified @p properties
> + * in the requested backend
> + * @returns if it could be done successfully

include @param

> personpluginmanager.h:96
> + */
> +static bool addContact(const QString &pluginId, const QVariantMap 
> &properties);
>  };

I wonder if that's the best API.

Would it make sense to have separate setDefaultSource(QString) and 
addContact(QVariantMap properties).
The default source could even come from a setting in the system.

REPOSITORY
  R307 KPeople

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

To: jbbgameich, apol, #plasma:_mobile
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25311: Add KColumnHeadersProxyModel

[Volker Krause]

vkrause added a comment.


  In D25311#563011 , @ahiemstra 
wrote:
  
  > > I'm wondering if this is technically a proxy model, or rather a 
QAbstractListModel? Its content is not representing cells in the source model 
after all, which mapTo/FromSource assumes I think.
  >
  > Yeah I'm not entirely sure. If it was possible I would probably have based 
this on QTransposeProxyModel, but that is Qt 5.13+ stuff.
  
  
  I don't think that would help much either, as that assumes you are mapping 
cells to cells, not cells to header data.
  
  >> It works in the example as you implement the entire QAIM interface needed 
for it, so the mapping functions are probably not even called, until the source 
model changes, or you add a selection model on top.
  > 
  > Most of what I needed I needed to implement because QAbstractProxyModel 
does not reimplement them.
  
  Right. My point is that if you'd inherit from QAbstractListModel and drop 
parent, index, columnCount and mapTo/FromSource you should end up with the same 
result. The sourceModel property might need to be added explicitly in that case 
though.

REPOSITORY
  R275 KItemModels

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

To: ahiemstra
Cc: vkrause, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D25323: [text thumbnail] Force Syntax Highligthing when no definition for file was found

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D25323#562989 , @meven wrote:
  
  > In D25323#562908 , @kossebau 
wrote:
  >
  > > Thanks for looking at the issue. No time to look closer the next days, 
but curious about this partial change (which has been discussed before and 
discarded):
  > >  changing `QColor ( 245, 245, 245 ); // light-grey background ` to 
`highlightingTheme.editorColor(KSyntaxHighlighting::Theme::EditorColorRole::BackgroundColor)`
 implies, one cannot use KSyntaxHighlighting to render text highlighted e.g. 
for a print-out on a paper (or only for a PDF). Compare e.g. the example 
https://phabricator.kde.org/source/syntax-highlighting/browse/master/examples/codepdfprinter/.
  >
  >
  > I don't think it implies this, and I don't see the point here, textCreator 
makes thumbnails for text files only anyway.
  >  The change is just so that the background follows the user theme and 
avoids an hardcoded value that is not good practice.
  
  
  See below why here a hardcoded color can make sense, at least to some.
  
  >> Is this change for background needed to make that rehighlight approach 
working?
  > 
  > No, but IMO, this ought to be changed.
  
  So far people agreed that having a consistent paper-like background made 
sense, also when there were different proposals in the last year:
  a) thumbnails are kind of representing print-out (see also shape border), so 
like PDFs & other docs do not change "paper" background color on style change
  b) currently thumbnail pixmaps are cached, both in processes as well as on 
disc, so on any theme change the background will be outdated
  
  And I still think that holds. But you are free to challenge that, as young 
generations ought to do ;) Not a maintainer, just stated my 2 cent here.
  
  >> For the rest, a not existing definition might be something other users of 
KSyntaxHighlighting might run into as well, so that use-case should be ideally 
supported by concepts in their API already (or at least ve documented how one 
is supposed to deal with that case), The proposed work-around code here does 
not look long-term stable, calling `rehighlight` seems to just work by chance 
currently to gain whatever effect (which effect does it have actually?),
  > 
  > https://doc.qt.io/qt-5/qsyntaxhighlighter.html#rehighlight is part of 
QSyntaxHighlighter, it is pretty stable, and is documented as `Reapplies the 
highlighting to the whole document` precisely what is needed.
  
  What I mean is that it is strange this has to be done explicitly here only in 
this case, and that it still works, given we just checked that 
syntaxHighlighter has no definitions. This seems rather random.
  
  >> But as code for human readers it makes little sense. Having to have some 
non-code comment makes that even more clear something in KSyntaxHighlighting 
API is not supporting us here.
  > 
  > I totally agree the change might need to be in KSyntaxHighlighting instead, 
and this patch was meant as a discussion opener.
  >  I would be happy to offer a patch to KSyntaxHighlighting instead, I would 
just welcome some suggestion from KSyntaxHighlighting maintainer how to handle 
this.
  >  I opened D25328  as a naive proposal.
  
  Thanks. Sadly no detailed insight into syntax highlighting, so hoping on its 
maintainers :)

REPOSITORY
  R320 KIO Extras

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

To: meven, kossebau, cullmann, vkrause
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D25311: Add KColumnHeadersProxyModel

[Arjen Hiemstra]

ahiemstra marked an inline comment as done.
ahiemstra added a comment.


  > We should probably have a unit test, even if it just covers the basic case.
  
  Done.
  
  > This currently assumes the source model's columns are static. At a minimum 
that needs a comment.
  
  Oops. That was not the intention. Fixed.
  
  > I'm wondering if this is technically a proxy model, or rather a 
QAbstractListModel? Its content is not representing cells in the source model 
after all, which mapTo/FromSource assumes I think.
  
  Yeah I'm not entirely sure. If it was possible I would probably have based 
this on QTransposeProxyModel, but that is Qt 5.13+ stuff.
  
  > It works in the example as you implement the entire QAIM interface needed 
for it, so the mapping functions are probably not even called, until the source 
model changes, or you add a selection model on top.
  
  Most of what I needed I needed to implement because QAbstractProxyModel does 
not reimplement them.

REPOSITORY
  R275 KItemModels

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

To: ahiemstra
Cc: vkrause, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D25311: Add KColumnHeadersProxyModel

[Arjen Hiemstra]

ahiemstra updated this revision to Diff 69807.
ahiemstra added a comment.


  - Add a unit test for KColumnHeadersProxyModel

REPOSITORY
  R275 KItemModels

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25311?vs=69806&id=69807

BRANCH
  columnheadersmodel

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/kcolumnheadersproxymodeltest.cpp
  src/core/CMakeLists.txt
  src/core/kcolumnheadersproxymodel.cpp
  src/core/kcolumnheadersproxymodel.h
  src/qml/plugin.cpp
  tests/qml/columnheaders.qml

To: ahiemstra
Cc: vkrause, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D25311: Add KColumnHeadersProxyModel

[Arjen Hiemstra]

ahiemstra updated this revision to Diff 69806.
ahiemstra added a comment.


  - Forward column change singals as row change signals
  - Forward header data change signals as dataChanged
  - Add a unit test for KColumnHeadersProxyModel

REPOSITORY
  R275 KItemModels

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25311?vs=69765&id=69806

BRANCH
  columnheadersmodel

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/kcolumnheadersproxymodeltest.cpp
  src/core/CMakeLists.txt
  src/core/kcolumnheadersproxymodel.cpp
  src/core/kcolumnheadersproxymodel.h
  src/qml/plugin.cpp
  tests/qml/columnheaders.qml

To: ahiemstra
Cc: vkrause, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D25015: Update breeze theme shadows

[Nathaniel Graham]

ngraham added a comment.


  In D25015#562992 , @niccolove 
wrote:
  
  > What do you think of:
  >  F7765076: Screenshot_20191115_145942.png 

  
  
  That feels smoother.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg
Cc: cblack, ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, 
LeGast00n, GB_2, michaelh, bruns

D25015: Update breeze theme shadows

[Niccolò Venerandi]

niccolove added a comment.


  This is what you think ends too early (for comparison):
  F7762929: Screenshot_20191114_100519.png 

  What do you think of:
  F7765076: Screenshot_20191115_145942.png 


REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg
Cc: cblack, ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, 
LeGast00n, GB_2, michaelh, bruns

D25323: [text thumbnail] Force Syntax Highligthing when no definition for file was found

[Méven Car]

meven added a comment.


  In D25323#562908 , @kossebau wrote:
  
  > Thanks for looking at the issue. No time to look closer the next days, but 
curious about this partial change (which has been discussed before and 
discarded):
  >  changing `QColor ( 245, 245, 245 ); // light-grey background ` to 
`highlightingTheme.editorColor(KSyntaxHighlighting::Theme::EditorColorRole::BackgroundColor)`
 implies, one cannot use KSyntaxHighlighting to render text highlighted e.g. 
for a print-out on a paper (or only for a PDF). Compare e.g. the example 
https://phabricator.kde.org/source/syntax-highlighting/browse/master/examples/codepdfprinter/.
  
  
  I don't think it implies this, and I don't see the point here, textCreator 
makes thumbnails for text files only anyway.
  The change is just so that the background follows the user theme and avoids 
an hardcoded value that is not good practice.
  
  > Is this change for background needed to make that rehighlight approach 
working?
  
  No, but IMO, this ought to be changed.
  
  > For the rest, a not existing definition might be something other users of 
KSyntaxHighlighting might run into as well, so that use-case should be ideally 
supported by concepts in their API already (or at least ve documented how one 
is supposed to deal with that case), The proposed work-around code here does 
not look long-term stable, calling `rehighlight` seems to just work by chance 
currently to gain whatever effect (which effect does it have actually?),
  
  https://doc.qt.io/qt-5/qsyntaxhighlighter.html#rehighlight is part of 
QSyntaxHighlighter, it is pretty stable, and is documented as `Reapplies the 
highlighting to the whole document` precisely what is needed.
  
  > But as code for human readers it makes little sense. Having to have some 
non-code comment makes that even more clear something in KSyntaxHighlighting 
API is not supporting us here.
  
  I totally agree the change might need to be in KSyntaxHighlighting instead, 
and this patch was meant as a discussion opener.
  I would be happy to offer a patch to KSyntaxHighlighting instead, I would 
just welcome some suggestion from KSyntaxHighlighting maintainer how to handle 
this.
  I opened D25328  as a naive proposal.
  
  In D25323#562911 , @kossebau wrote:
  
  > Also am I wonfering how this relates to the bug report you referred to? Can 
you tell what effect your code change has on the symptoms reported in 
https://bugs.kde.org/show_bug.cgi?id=409380#c0 ?
  
  
  I misinterpreted the bug, I thought it was about kio-extras. My bad.
  The fix here is in fact a downstream workaround for this bug as you noticed.

REPOSITORY
  R320 KIO Extras

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

To: meven, kossebau, cullmann, vkrause
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D25323: [text thumbnail] Force Syntax Highligthing when no definition for file was found

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R320 KIO Extras

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

To: meven, kossebau, cullmann, vkrause
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D25328: Always rehiglhight() after definition was changed

[Méven Car]

meven edited the summary of this revision.
meven edited the test plan for this revision.

REPOSITORY
  R216 Syntax Highlighting

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

To: meven, kossebau, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25328: Always rehiglhight() after definition was changed

[Méven Car]

meven created this revision.
meven added reviewers: kossebau, cullmann, vkrause.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
meven requested review of this revision.

REVISION SUMMARY
  When setting definition, Rehighlight text even if the definition has not 
changed.
  Useful when the dummy default definition is set, so that highlight is run 
even in this case.

TEST PLAN
  ctest
  In dolphin, text files content is readable in thumbnails.
  
  BUG: 409380
  FIXED-IN: 5.65

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

AFFECTED FILES
  src/lib/syntaxhighlighter.cpp

To: meven, kossebau, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> feverfew wrote in krun.cpp:598
> Yes, but note later I need to then change the values in the `QList` outside 
> of the for loop, hence why I store the index in a struct associated with the 
> reply. How would I do that easily with a range based for loop?

So did the old code though. See old line 584.

It's iterating by-reference, so you can simply assign to it via

https://doc.qt.io/qt-5/qurl.html#operator-eq-1

to change the content of the QUrl object (not the instance of the object in the 
list, but rather the content of that object).

REPOSITORY
  R241 KIO

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

To: feverfew, fvogt, davidedmundson, dfaure, ngraham
Cc: sitter, davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, 
michaelh, bruns

D25326: Move Plasma's SortFilterProxyModel into KItemModel's QML plugin

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> sortfiltermodel.cpp:84
> +QQmlEngine *engine = QQmlEngine::contextForObject(this)->engine();
> +args << 
> engine->toScriptValue(idx.data(m_roleIds.value(m_filterRole)));
> +

Can we also just have it send `source_row` and `source_parent` since we have 
proper `QModelIndex` handling now?

> sortfiltermodel.cpp:97
> +}
> +QSortFilterProxyModel::setFilterRegExp(QRegExp(exp, 
> Qt::CaseInsensitive));
> +Q_EMIT filterRegExpChanged(exp);

`QRegExp` is deprecated, use `setFilterRegularExpression` taking a 
`QRegularExpression`

> sortfiltermodel.h:43
> + */
> +Q_PROPERTY(QString filterRegExp READ filterRegExp WRITE setFilterRegExp 
> NOTIFY filterRegExpChanged)
> +

Can this be a `QRegularExpression`?
I would assume a JS `RegExp` object (or regexp literal `/syntax/`) was 
supported (didn't test)

> sortfiltermodel.h:48
> + */
> +Q_PROPERTY(QString filterString READ filterString WRITE setFilterString 
> NOTIFY filterStringChanged REVISION 1)
> +

Given this is a new class/import, remove `REVISION`

> sortfiltermodel.h:56
> + * ignored. Attempts to write a non-callable to this property are 
> silently ignored, but you can set
> + * it to null.
> + */

Could this use a `RESET` method instead of telling people to assign `null`?

> sortfiltermodel.h:78
> + */
> +Q_PROPERTY(int count READ count NOTIFY countChanged)
> +

Is this needed?

> sortfiltermodel.h:80
> +
> +friend class DataModel;
> +

Remove

> sortfiltermodel.h:117
> + */
> +Q_INVOKABLE QVariantMap get(int i) const;
> +

I don't like this. We have proper `QModelIndex` handling in QML now, this is a 
hack imho

> sortfiltermodel.h:119
> +
> +Q_INVOKABLE int mapRowToSource(int i) const;
> +

Remove, `QAbstractProxyModel::mapToSource` (and `fromSource`) is `Q_INVOKABLE` 
and we have proper `QModelIndex` handling in QML now

> sortfiltermodel.h:138
> +private:
> +QString m_filterRole;
> +QString m_sortRole;

Does this need a `d` pointer or is it just for use in QML? Perhaps rename the 
header to `_p.h` then?

REPOSITORY
  R275 KItemModels

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

To: davidedmundson
Cc: broulik, ahiemstra, mart, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

[Alexander Saoutkin]

feverfew added inline comments.

INLINE COMMENTS

> sitter wrote in krun.cpp:583
> Coding style says we use curly braces even for single-line if statements I 
> think.

Yep you're right, will fix in a mo.

> sitter wrote in krun.cpp:598
> What I meant is you should literally iterate using
> 
>   for (QUrl &url : urls) {
> 
> which is what the code did before but you changed it for some reason.

Yes, but note later I need to then change the values in the `QList` outside of 
the for loop, hence why I store the index in a struct associated with the 
reply. How would I do that easily with a range based for loop?

REPOSITORY
  R241 KIO

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

To: feverfew, fvogt, davidedmundson, dfaure, ngraham
Cc: sitter, davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, 
michaelh, bruns

D25326: Move Plasma's SortFilterProxyModel into KItemModel's QML plugin

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> ahiemstra wrote in sortfiltermodel.h:63
> This (and sortRole below) hides the properties from QSortFilterProxy with the 
> same name. Maybe it is a better idea to use filterRoleName/sortRoleName?

Or potentially we could just kill our handling of it?

REPOSITORY
  R275 KItemModels

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

To: davidedmundson
Cc: ahiemstra, mart, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D25244: Implement ISpellChecker backend for Windows >= 8

[Christoph Cullmann]

cullmann added a comment.


  Ok, thank you!

REPOSITORY
  R246 Sonnet

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

To: cullmann, #frameworks, vonreth
Cc: mludwig, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24641: Collect more information from version control systems

[Harald Sitter]

sitter added a comment.


  This is starting to look really good. All functions will need documenting in 
the header of that file so they show up on api.kde.org, see other modules for 
examples.

INLINE COMMENTS

> ECMSourceVersionControl.cmake:61
> +
> +set(_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_NOT_VCS FALSE)
> +set(_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_MISSING_GIT_EXEC FALSE)

Do we really need this? It seems to me we could just spam it for every call, in 
the grand scheme of things it makes no difference but is less logic one has to 
worry about when extending this module.

> ECMSourceVersionControl.cmake:75
> +# Git
> +if(NOT _ECM_SOURCE_VERSION_GIT_EXECUTABLE)
> +find_program(_ECM_SOURCE_VERSION_GIT_EXECUTABLE

I'd move the exec detection and missing reporting into its own helper which 
gets called by all "public" functions. Currently the logic is duped in both 
functions.

> ECMSourceVersionControl.cmake:87
> +)
> +set(ECM_SOURCE_VERSION_CONTROL_REVISION 
> "${ECM_SOURCE_VERSION_CONTROL_REVISION}" PARENT_SCOPE)
> +else()

I think it's more idiomatic if you accept the variable name as a function 
argument instead of hardcoding one.

For example `find_program`, but really most helpers work like that off the top 
of my head.

REPOSITORY
  R240 Extra CMake Modules

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

To: thomasfischer, sitter, kossebau
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D25312: Document configuration file path on Android

[Jonah Brüchert]

This revision was automatically updated to reflect the committed changes.
Closed by commit R289:61cc70fd3cff: Document configuration file path on Android 
(authored by jbbgameich).

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25312?vs=69768&id=69801

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

AFFECTED FILES
  src/knotification.h

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

D25244: Implement ISpellChecker backend for Windows >= 8

[Michel Ludwig]

mludwig added a comment.


  In D25244#562817 , @cullmann wrote:
  
  > Some feedback?
  >  Else I would just try that.
  >  For me it worked fine (obviously).
  
  
  I will try it out over the weekend. It's something I planned to work on 
myself, but you were faster ;)

REPOSITORY
  R246 Sonnet

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

To: cullmann, #frameworks, vonreth
Cc: mludwig, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

[Harald Sitter]

sitter added a comment.


  some style complaints. Looks great other than that 👍

INLINE COMMENTS

> krun.cpp:583
> +const QUrl url = urls[i];
> +if (KIO::DesktopExecParser::isProtocolInSupportedList(url, 
> appSupportedProtocols))
> +continue;

Coding style says we use curly braces even for single-line if statements I 
think.

> sitter wrote in krun.cpp:598
> Is there a reason you are not using a range based for loop here? `for (const 
> QUrl &url : urls)`

What I meant is you should literally iterate using

  for (QUrl &url : urls) {

which is what the code did before but you changed it for some reason.

REPOSITORY
  R241 KIO

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

To: feverfew, fvogt, davidedmundson, dfaure, ngraham
Cc: sitter, davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, 
michaelh, bruns

D25326: Move Plasma's SortFilterProxyModel into KItemModel's QML plugin

[Arjen Hiemstra]

ahiemstra added inline comments.

INLINE COMMENTS

> sortfiltermodel.h:63
> + */
> +Q_PROPERTY(QString filterRole READ filterRole WRITE setFilterRole)
> +

This (and sortRole below) hides the properties from QSortFilterProxy with the 
same name. Maybe it is a better idea to use filterRoleName/sortRoleName?

> sortfiltermodel.h:74
> +Q_PROPERTY(Qt::SortOrder sortOrder READ sortOrder WRITE setSortOrder)
> +
> +/**

Having a sortColumn property would also be useful, since that is apparently 
missing from QSortFilterProxy. I seem to recall that was added to the Plasma 
version a while ago.

REPOSITORY
  R275 KItemModels

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

To: davidedmundson
Cc: ahiemstra, mart, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D25326: Move Plasma's SortFilterProxyModel into KItemModel's QML plugin

[David Edmundson]

davidedmundson edited the summary of this revision.

REPOSITORY
  R275 KItemModels

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

To: davidedmundson
Cc: mart, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25326: Move Plasma's SortFilterProxyModel into KItemModel's QML plugin

[Marco Martin]

mart added a comment.


  +1: i found myself many times wanting to use this but not being able because 
of plasma dep.
  the code is used a lot in plasmoids and seems quite robust

REPOSITORY
  R275 KItemModels

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

To: davidedmundson
Cc: mart, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25326: Move Plasma's SortFilterProxyModel into KItemModel's QML plugin

[David Edmundson]

davidedmundson created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  It exposes QSFPM in a usable way from QML, but also exposes a way to
  perform JS callbacks as an advanced filter method.
  
  This is mostly a 1-1 move from plasma-frameworks, but with the following
  change.
  
  - Removing a broken workaround for trying to handle Plasma's DataModel
  
  having dynamic role names.
  
  - port to the new connect syntax
  - removing the plasma namespace
  
  I don't know if we want to change the name to match the others having a
  K prefix?
  
  I can add a unit test if that

REPOSITORY
  R275 KItemModels

BRANCH
  master

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

AFFECTED FILES
  src/qml/CMakeLists.txt
  src/qml/plugin.cpp
  src/qml/sortfiltermodel.cpp
  src/qml/sortfiltermodel.h

To: davidedmundson
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25323: [text thumbnail] Force Syntax Highligthing when no definition for file was found

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Also am I wonfering how this relates to the bug report you referred to? Can 
you tell what effect your code change has on the symptoms reported in 
https://bugs.kde.org/show_bug.cgi?id=409380#c0 ?

REPOSITORY
  R320 KIO Extras

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

To: meven, kossebau, cullmann, vkrause
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D25323: [text thumbnail] Force Syntax Highligthing when no definition for file was found

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Thanks for looking at the issue. No time to look closer the next days, but 
curious about this partial change (which has been discussed before and 
discarded):
  changing `QColor ( 245, 245, 245 ); // light-grey background ` to 
`highlightingTheme.editorColor(KSyntaxHighlighting::Theme::EditorColorRole::BackgroundColor)`
 implies, one cannot use KSyntaxHighlighting to render text highlighted e.g. 
for a print-out on a paper (or only for a PDF). Compare e.g. the example 
https://phabricator.kde.org/source/syntax-highlighting/browse/master/examples/codepdfprinter/.
 
  Is this change for background needed to make that rehighlight approach 
working?
  
  For the rest, a not existing definition might be something other users of 
KSyntaxHighlighting might run into as well, so that use-case should be ideally 
supported by concepts in their API already (or at least ve documented how one 
is supposed to deal with that case), The proposed work-around code here does 
not look long-term stable, calling `rehighlight` seems to just work by chance 
currently to gain whatever effect (which effect does it have actually?), But as 
code for human readers it makes little sense. Having to have some non-code 
comment makes that even more clear something in KSyntaxHighlighting API is not 
supporting us here.

REPOSITORY
  R320 KIO Extras

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

To: meven, kossebau, cullmann, vkrause
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

KDE CI: Frameworks » kitemmodels » kf5-qt5 SUSEQt5.12 - Build # 60 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kitemmodels/job/kf5-qt5%20SUSEQt5.12/60/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Fri, 15 Nov 2019 11:02:45 +
 Build duration:
8 min 22 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5ItemModels-5.65.0.xmlcompat_reports/KF5ItemModels_compat_report.htmllogs/KF5ItemModels/5.65.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: 1 test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 11 test(s)Failed: projectroot.autotests.kconcatenaterows_qml
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report60%
(3/5)55%
(41/74)55%
(41/74)72%
(6428/8887)57%
(2903/5057)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(13/13)100%
(13/13)99%
(2382/2406)62%
(1041/1681)autotests.proxymodeltestsuite67%
(12/18)67%
(12/18)69%
(1759/2545)54%
(651/1211)src.core80%
(16/20)80%
(16/20)80%
(2287/2853)65%
(1211/1865)src.qml0%
(0/2)0%
(0/2)0%
(0/25)0%
(0/2)tests.proxymodeltestapp0%
(0/21)0%
(0/21)0%
(0/1058)0%
(0/298)

KDE CI: Frameworks » kitemmodels » kf5-qt5 WindowsMSVCQt5.13 - Build # 25 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kitemmodels/job/kf5-qt5%20WindowsMSVCQt5.13/25/
 Project:
kf5-qt5 WindowsMSVCQt5.13
 Date of build:
Fri, 15 Nov 2019 11:02:45 +
 Build duration:
8 min 6 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 11 test(s)Failed: projectroot.autotests.kconcatenaterows_qml

KDE CI: Frameworks » kitemmodels » kf5-qt5 FreeBSDQt5.13 - Build # 34 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kitemmodels/job/kf5-qt5%20FreeBSDQt5.13/34/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Fri, 15 Nov 2019 11:02:44 +
 Build duration:
3 min 34 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 11 test(s)Failed: projectroot.autotests.kconcatenaterows_qml

KDE CI: Frameworks » kitemmodels » kf5-qt5 SUSEQt5.13 - Build # 37 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kitemmodels/job/kf5-qt5%20SUSEQt5.13/37/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Fri, 15 Nov 2019 11:02:44 +
 Build duration:
2 min 55 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5ItemModels-5.65.0.xmlcompat_reports/KF5ItemModels_compat_report.htmllogs/KF5ItemModels/5.65.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: 1 test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 11 test(s)Failed: projectroot.autotests.kconcatenaterows_qml
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report60%
(3/5)55%
(41/74)55%
(41/74)72%
(6428/8883)57%
(2903/5057)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(13/13)100%
(13/13)99%
(2382/2406)62%
(1041/1681)autotests.proxymodeltestsuite67%
(12/18)67%
(12/18)69%
(1759/2545)54%
(651/1211)src.core80%
(16/20)80%
(16/20)80%
(2287/2852)65%
(1211/1865)src.qml0%
(0/2)0%
(0/2)0%
(0/25)0%
(0/2)tests.proxymodeltestapp0%
(0/21)0%
(0/21)0%
(0/1055)0%
(0/298)

D25313: Add autotest for QML KConcatenateRowsProxyModel

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R275:a820a2800f03: Add autotest for QML 
KConcatenateRowsProxyModel (authored by davidedmundson).

REPOSITORY
  R275 KItemModels

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25313?vs=69773&id=69796

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/concatenaterowstest.qml
  autotests/kconcatenaterows_qml.cpp
  tests/qml/concatenaterows.qml

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

D20766: Use appropriate background color for text previews

[Méven Car]

meven added a comment.


  I have opened https://phabricator.kde.org/D25323 to fix the missing 
highlighting in kio-extras.

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

To: eshalygin, kossebau, cfeck
Cc: dhaumann, cullmann, vkrause, cfeck, meven, broulik, kde-frameworks-devel, 
kfm-devel, pberestov, iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, 
alexde, GB_2, Codezela, feverfew, michaelh, spoorun, navarromorales, firef, 
ngraham, andrebarros, bruns, emmanuelp, mikesomov

D25323: [text thumbnail] Force Syntax Highligthing when no definition for file was found

[Méven Car]

meven created this revision.
meven added reviewers: kossebau, cullmann, vkrause.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
meven requested review of this revision.

REVISION SUMMARY
  By default KSyntaxHighlighter does not highlight text when it has not found a 
valid definition.
  In this case force KSyntaxHighlighter to highlight the text.
  
  BUG: 409380
  FIXED-IN: 20.04
  
  Alternative to D21295 

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

AFFECTED FILES
  thumbnail/textcreator.cpp

To: meven, kossebau, cullmann, vkrause
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

[Alexander Saoutkin]

feverfew updated this revision to Diff 69788.
feverfew added a comment.


  - Align arguments

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23384?vs=69774&id=69788

BRANCH
  arcpatch-D23384

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

AFFECTED FILES
  src/core/CMakeLists.txt
  src/core/desktopexecparser.cpp
  src/core/org.kde.KIOFuse.VFS.xml
  src/widgets/CMakeLists.txt
  src/widgets/krun.cpp
  src/widgets/org.kde.KIOFuse.VFS.xml

To: feverfew, fvogt, davidedmundson, dfaure, ngraham
Cc: sitter, davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, 
michaelh, bruns

D25244: Implement ISpellChecker backend for Windows >= 8

[Christoph Cullmann]

cullmann added a comment.


  Some feedback?
  Else I would just try that.
  For me it worked fine (obviously).

REPOSITORY
  R246 Sonnet

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

To: cullmann, #frameworks, vonreth
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25296: [RFC] Fix Display Configuration icon margins

[Noah Davis]

ndavis added inline comments.

INLINE COMMENTS

> preferences.svg:4
> +.ColorScheme-Text {
>  color:#7B7C7E;
>}

Wrong default colors. Just replace the whole stylesheet with this one: 
https://community.kde.org/Guidelines_and_HOWTOs/Icon_Workflow_Tips#Breeze

> preferences.svg:36
> +  }
> +

D25296: [RFC] Fix Display Configuration icon margins

[Noah Davis]

ndavis added a comment.


  It's blurry in your screenshot because you're scaling a 22px icon up to 32px. 
The solution is to make a 32px version named 
`32-32-preferences-desktop-display-randr`. BTW, the icon is not actually 
aligned with the grid. It's a little off here and there. You should be able to 
fix it easily with the node tool by snapping nodes to the grid.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #vdg, ndavis
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25311: Add KColumnHeadersProxyModel

[Volker Krause]

vkrause added a comment.


  I'm wondering if this is technically a proxy model, or rather a 
QAbstractListModel? Its content is not representing cells in the source model 
after all, which mapTo/FromSource assumes I think. It works in the example as 
you implement the entire QAIM interface needed for it, so the mapping functions 
are probably not even called, until the source model changes, or you add a 
selection model on top.

REPOSITORY
  R275 KItemModels

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

To: ahiemstra
Cc: vkrause, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns