D28112: [WIP] Expose application menu via KWindowInfo

[Carson Black]

cblack created this revision.
cblack added reviewers: Plasma, broulik.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
cblack requested review of this revision.

REVISION SUMMARY
  Application menu information is now available
  via KWindowInfo's APIs on X11.
  
  Note that this is dysfunctional, currently. Written tests
  are failing, and I've identified that the blocks at
  src/platforms/xcb/netwm.cpp lines 4677 and 4687 cause their
  respective tests to fail. However, I can't identify where
  I dun goofed.

REPOSITORY
  R278 KWindowSystem

BRANCH
  appmenu-x11 (branched from master)

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

AFFECTED FILES
  autotests/netwininfotestclient.cpp
  src/kwindowinfo.cpp
  src/kwindowinfo.h
  src/kwindowinfo_p.h
  src/netwm_def.h
  src/platforms/xcb/atoms_p.h
  src/platforms/xcb/kwindowinfo.cpp
  src/platforms/xcb/kwindowinfo_p_x11.h
  src/platforms/xcb/netwm.cpp
  src/platforms/xcb/netwm.h
  src/platforms/xcb/netwm_p.h

To: cblack, #plasma, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.12 - Build # 281 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.12/281/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Wed, 18 Mar 2020 01:22:54 +
 Build duration:
3 min 10 sec and counting
   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: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

KDE CI: Frameworks » breeze-icons » kf5-qt5 FreeBSDQt5.13 - Build # 167 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20FreeBSDQt5.13/167/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Wed, 18 Mar 2020 01:22:54 +
 Build duration:
2 min 9 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.13 - Build # 174 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.13/174/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Wed, 18 Mar 2020 01:22:54 +
 Build duration:
2 min 26 sec and counting
   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: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

D25815: Transmission-Qt tray icon added

[Noah Davis]

ndavis abandoned this revision.
ndavis added a comment.


  This icon has been added to breeze-icons

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

To: ndavis, #vdg, ngraham, vinzenzv
Cc: broulik, ndavis, ngraham, iasensio, kde-frameworks-devel, LeGast00n, 
cblack, fbampaloukas, GB_2, trickyricky26, michaelh, bruns

D28111: Add transmission-tray-icon

[Noah Davis]

This revision was automatically updated to reflect the committed changes.
Closed by commit R266:9895e572d730: Add transmission-tray-icon (authored by 
ndavis).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28111?vs=77872=77873

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

AFFECTED FILES
  icons-dark/status/22/transmission-tray-icon.svg
  icons/status/22/transmission-tray-icon.svg

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

D28111: Add transmission-tray-icon

[Carson Black]

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


  LGTM.

REPOSITORY
  R266 Breeze Icons

BRANCH
  transmission (branched from master)

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

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

D28111: Add transmission-tray-icon

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D28111: Add transmission-tray-icon

[Noah Davis]

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

REPOSITORY
  R266 Breeze Icons

BRANCH
  transmission (branched from master)

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

AFFECTED FILES
  icons-dark/status/22/transmission-tray-icon.svg
  icons/status/22/transmission-tray-icon.svg

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

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.13 - Build # 173 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.13/173/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Wed, 18 Mar 2020 00:37:23 +
 Build duration:
3 min 36 sec and counting
   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: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.12 - Build # 280 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.12/280/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Wed, 18 Mar 2020 00:37:23 +
 Build duration:
3 min 27 sec and counting
   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: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

KDE CI: Frameworks » breeze-icons » kf5-qt5 FreeBSDQt5.13 - Build # 166 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20FreeBSDQt5.13/166/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Wed, 18 Mar 2020 00:37:23 +
 Build duration:
3 min 3 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink

KDE CI: Frameworks » kpeople » kf5-qt5 WindowsMSVCQt5.14 - Build # 13 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kpeople/job/kf5-qt5%20WindowsMSVCQt5.14/13/
 Project:
kf5-qt5 WindowsMSVCQt5.14
 Date of build:
Wed, 18 Mar 2020 00:31:39 +
 Build duration:
2 min 56 sec and counting
   JUnit Tests
  Name: projectroot Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)

KDE CI: Frameworks » kpeople » kf5-qt5 SUSEQt5.12 - Build # 93 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kpeople/job/kf5-qt5%20SUSEQt5.12/93/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Wed, 18 Mar 2020 00:31:40 +
 Build duration:
2 min 41 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5People-5.69.0.xmlcompat_reports/KF5People_compat_report.htmllogs/KF5People/5.69.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: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report43%
(3/7)45%
(22/49)45%
(22/49)46%
(762/1656)30%
(255/852)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(4/4)100%
(4/4)99%
(212/214)45%
(101/222)examples0%
(0/5)0%
(0/5)0%
(0/143)0%
(0/42)src71%
(10/14)71%
(10/14)57%
(461/812)32%
(142/437)src.backends73%
(8/11)73%
(8/11)89%
(89/100)55%
(12/22)src.declarative0%
(0/4)0%
(0/4)0%
(0/93)0%
(0/27)src.widgets0%
(0/10)0%
(0/10)0%
(0/279)0%
(0/98)src.widgets.plugins0%
(0/1)0%
(0/1)0%
(0/15)0%
(0/4)

KDE CI: Frameworks » kpeople » kf5-qt5 SUSEQt5.13 - Build # 72 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kpeople/job/kf5-qt5%20SUSEQt5.13/72/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Wed, 18 Mar 2020 00:31:40 +
 Build duration:
3 min 6 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5People-5.69.0.xmlcompat_reports/KF5People_compat_report.htmllogs/KF5People/5.69.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: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report43%
(3/7)45%
(22/49)45%
(22/49)46%
(762/1656)30%
(255/852)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(4/4)100%
(4/4)99%
(212/214)45%
(101/222)examples0%
(0/5)0%
(0/5)0%
(0/143)0%
(0/42)src71%
(10/14)71%
(10/14)57%
(461/812)32%
(142/437)src.backends73%
(8/11)73%
(8/11)89%
(89/100)55%
(12/22)src.declarative0%
(0/4)0%
(0/4)0%
(0/93)0%
(0/27)src.widgets0%
(0/10)0%
(0/10)0%
(0/279)0%
(0/98)src.widgets.plugins0%
(0/1)0%
(0/1)0%
(0/15)0%
(0/4)

KDE CI: Frameworks » kpeople » kf5-qt5 FreeBSDQt5.13 - Build # 67 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kpeople/job/kf5-qt5%20FreeBSDQt5.13/67/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Wed, 18 Mar 2020 00:31:39 +
 Build duration:
1 min 33 sec and counting
   JUnit Tests
  Name: projectroot Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)

D28108: Handle busybox's sed like GNU sed

[Aleix Pol Gonzalez]

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

REPOSITORY
  R266 Breeze Icons

BRANCH
  master

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

To: heikobecker, #frameworks, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D17816: Support for xattrs on kio copy/move

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> file_unix.cpp:146
> +#elif HAVE_SYS_EXTATTR
> +ssize_t listlen = extattr_list_file(src_fd, EXTATTR_NAMESPACE_USER, 
> nullptr, 0);
> +#endif

extattr_list_**fd**, here and everywhere else

> file_unix.cpp:164
> +#endif
> +QList m_keyList = keylist.split('\0');
> +if (m_keyList.last().isEmpty()) m_keyList.removeLast(); // the last item 
> may be empty

This is wrong for the BSD implementation:

> extattr_list_file() returns a listof attributes present in the requested
>  namespace.Each list entry consists of a **single byte containing the
>  length of the attribute name**, followed by the attribute name.  The attri-
>  bute name is **not terminated by ASCII 0 (nul).**

> dfaure wrote in file_unix.cpp:164
> Can this *ever* return an empty list, because keylist was empty?
> 
> (Then last() will assert on the next line)

Why a temporary list at all?

  off_t offset = 0; size_t keyLen;
  while (offset < keylist.size()) {
  #if BSD
 keyLen = static_cast(keylist[offset]);
 offset++;
  #elif LINUX
  keyLen = strlen(keylist[offset]
  #endif
  key = keylist.mid(offset, keyLen);
  /* copy */
  ...
  #if BSD
  offset += keyLen;
  #elif LINUX
  offset += keyLen + 1;
  #endif
  }

> file_unix.cpp:180
> +}
> +QByteArray value(valuelen + 1, Qt::Uninitialized);
> +// get the value for key

Allocate outside the loop

> file_unix.cpp:185
> +#elif defined(Q_OS_MAC)
> +vallen = fgetxattr(src_fd, key.constData(), value.data(), valuelen, 
> 0, 0);
> +#elif HAVE_SYS_EXTATTR

valuelen

> file_unix.cpp:187
> +#elif HAVE_SYS_EXTATTR
> +vallen = extattr_get_file(src_fd, EXTATTR_NAMESPACE_USER, 
> key.constData(), value.data(), valuelen);
> +#endif

valuelen

> file_unix.cpp:199
> +if (errno == ENOTSUP) {
> +qCWarning(KIO_FILE) << "destination filesystem don't support 
> xattrs";
> +} else {

you can return here, no value in trying again and again ...

REPOSITORY
  R241 KIO

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

To: cochise, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta
Cc: usta, scheirle, anthonyfieroni, tmarshall, arrowd, cfeck, bruns, phidrho, 
dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, 
spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh

KDE CI: Frameworks » kpeople » kf5-qt5 SUSEQt5.12 - Build # 92 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kpeople/job/kf5-qt5%20SUSEQt5.12/92/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Tue, 17 Mar 2020 23:31:42 +
 Build duration:
2 min 9 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5People-5.69.0.xmlcompat_reports/KF5People_compat_report.htmllogs/KF5People/5.69.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: 2 test(s), Skipped: 0 test(s), Total: 3 test(s)Failed: projectroot.autotests.personsproxymodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report43%
(3/7)37%
(18/49)37%
(18/49)43%
(704/1653)27%
(232/852)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests75%
(3/4)75%
(3/4)89%
(189/213)41%
(90/222)examples0%
(0/5)0%
(0/5)0%
(0/143)0%
(0/42)src57%
(8/14)57%
(8/14)53%
(427/810)30%
(132/437)src.backends64%
(7/11)64%
(7/11)88%
(88/100)45%
(10/22)src.declarative0%
(0/4)0%
(0/4)0%
(0/93)0%
(0/27)src.widgets0%
(0/10)0%
(0/10)0%
(0/279)0%
(0/98)src.widgets.plugins0%
(0/1)0%
(0/1)0%
(0/15)0%
(0/4)

KDE CI: Frameworks » kpeople » kf5-qt5 SUSEQt5.13 - Build # 71 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kpeople/job/kf5-qt5%20SUSEQt5.13/71/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Tue, 17 Mar 2020 23:31:42 +
 Build duration:
2 min 12 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5People-5.69.0.xmlcompat_reports/KF5People_compat_report.htmllogs/KF5People/5.69.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: 2 test(s), Skipped: 0 test(s), Total: 3 test(s)Failed: projectroot.autotests.personsproxymodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report43%
(3/7)37%
(18/49)37%
(18/49)43%
(704/1653)27%
(232/852)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests75%
(3/4)75%
(3/4)89%
(189/213)41%
(90/222)examples0%
(0/5)0%
(0/5)0%
(0/143)0%
(0/42)src57%
(8/14)57%
(8/14)53%
(427/810)30%
(132/437)src.backends64%
(7/11)64%
(7/11)88%
(88/100)45%
(10/22)src.declarative0%
(0/4)0%
(0/4)0%
(0/93)0%
(0/27)src.widgets0%
(0/10)0%
(0/10)0%
(0/279)0%
(0/98)src.widgets.plugins0%
(0/1)0%
(0/1)0%
(0/15)0%
(0/4)

KDE CI: Frameworks » kpeople » kf5-qt5 WindowsMSVCQt5.14 - Build # 12 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kpeople/job/kf5-qt5%20WindowsMSVCQt5.14/12/
 Project:
kf5-qt5 WindowsMSVCQt5.14
 Date of build:
Tue, 17 Mar 2020 23:31:42 +
 Build duration:
2 min 19 sec and counting
   JUnit Tests
  Name: projectroot Failed: 2 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 3 test(s)Failed: projectroot.autotests.personsmodeltestFailed: projectroot.autotests.personsproxymodeltest

KDE CI: Frameworks » kpeople » kf5-qt5 FreeBSDQt5.13 - Build # 66 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kpeople/job/kf5-qt5%20FreeBSDQt5.13/66/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Tue, 17 Mar 2020 23:31:42 +
 Build duration:
1 min 8 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 3 test(s)Failed: projectroot.autotests.personsproxymodeltest

D28110: PersonsModel: Fix PersonVCardRole

[Jonah Brüchert]

This revision was automatically updated to reflect the committed changes.
Closed by commit R307:b6b86f158254: PersonsModel: Fix PersonVCardRole (authored 
by jbbgameich).

REPOSITORY
  R307 KPeople

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28110?vs=77869=77870

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

AFFECTED FILES
  src/personsmodel.cpp

To: jbbgameich, apol, nicolasfella
Cc: nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D28110: PersonsModel: Fix PersonVCardRole

[Nicolas Fella]

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

REPOSITORY
  R307 KPeople

BRANCH
  fix-personsmodel-vcard (branched from master)

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

To: jbbgameich, apol, nicolasfella
Cc: nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D28110: PersonsModel: Fix PersonVCardRole

[Jonah Brüchert]

jbbgameich updated this revision to Diff 77869.
jbbgameich added a comment.


  Use AbstractContact::VCardProperty

REPOSITORY
  R307 KPeople

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28110?vs=77868=77869

BRANCH
  fix-personsmodel-vcard (branched from master)

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

AFFECTED FILES
  src/personsmodel.cpp

To: jbbgameich, apol
Cc: nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D28094: [breeze-icons] fix shadow in some user (applets/128) icons

[Noah Davis]

ndavis added a comment.


  BTW, you can use this website to search for text in the code of all KDE 
software: https://lxr.kde.org/search

REPOSITORY
  R266 Breeze Icons

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

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

D17816: Support for xattrs on kio copy/move

[David Faure]

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

INLINE COMMENTS

> jobtest.cpp:118
> +} else {
> +qWarning() << "Xatr command not foud.";
> +}

typo in Xatr, and in "foud". I suggest:

  qWarning() << "Neither getfattr, getextattr nor xattr was found";

Although I have to wonder why this looks for all three, to then only use 
getfattr and skip tests in all other cases

> jobtest.cpp:480
> +attrs["user.name with space"] = "value with spaces";
> + return attrs;
> +}

this method has weird indentation

> jobtest.cpp:736
> +}
> +setXattr(filePath);
>  copyLocalFile(filePath, dest);

I fail to understand the code here.
This is the same call as two lines above, line 734, which is inside an if().
This one should be removed?

> jobtest.cpp:793
>  copyLocalSymlink(filePath, dest, QStringLiteral("relative"));
> +
>  QFile::remove(filePath);

please revert no-op changes

> jobtest.cpp:805
>  const QString dest = otherTmpDir() + "testlink_copied";
> +
>  createTestSymlink(filePath, QFile::encodeName(homeTmpDir()));

no-op (just empty lines)

> jobtest.cpp:808
>  copyLocalSymlink(filePath, dest, homeTmpDir());
> +
>  QFile::remove(filePath);

empty line

> file_unix.cpp:164
> +#endif
> +QList m_keyList = keylist.split('\0');
> +if (m_keyList.last().isEmpty()) m_keyList.removeLast(); // the last item 
> may be empty

Can this *ever* return an empty list, because keylist was empty?

(Then last() will assert on the next line)

REPOSITORY
  R241 KIO

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

To: cochise, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta
Cc: usta, scheirle, anthonyfieroni, tmarshall, arrowd, cfeck, bruns, phidrho, 
dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, 
spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh

D28110: PersonsModel: Fix PersonVCardRole

[Nicolas Fella]

nicolasfella added inline comments.

INLINE COMMENTS

> personsmodel.cpp:186
>  case PersonsModel::PersonVCardRole:
> -return QVariant::fromValue(person);
> +return person->customProperty(QStringLiteral("vcard"));
>  case PersonsModel::ContactsVCardRole:

Use AbstractContact::VCardProperty

REPOSITORY
  R307 KPeople

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

To: jbbgameich, apol
Cc: nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D28110: PersonsModel: Fix PersonVCardRole

[Jonah Brüchert]

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

REPOSITORY
  R307 KPeople

BRANCH
  fix-personsmodel-vcard (branched from master)

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

AFFECTED FILES
  src/personsmodel.cpp

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

KDE CI: Frameworks » breeze-icons » kf5-qt5 FreeBSDQt5.13 - Build # 165 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20FreeBSDQt5.13/165/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Tue, 17 Mar 2020 21:33:00 +
 Build duration:
3 min 9 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.12 - Build # 279 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.12/279/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Tue, 17 Mar 2020 21:33:01 +
 Build duration:
2 min 33 sec and counting
   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: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.13 - Build # 172 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.13/172/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Tue, 17 Mar 2020 21:33:00 +
 Build duration:
1 min 52 sec and counting
   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: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

D27617: [breeze-icons] Add telegram-desktop tray icons

[Noah Davis]

ndavis added a comment.


  Unfortunately, it looks like I'll have to revert this patch because when 
people use a light colorscheme with a dark plasma theme, the icons turn black 
because the icon is rendered as a pixmap when it has the red counter badge.

REPOSITORY
  R266 Breeze Icons

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

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

D27855: [Debug] Improve readability of positioninfo debug format

[Stefan Brüns]

bruns added a comment.


  In D27855#629330 , @kossebau wrote:
  
  > For Qt <5.14 `Qt::hex` needs to be instead unnamespaced `hex`, same with 
`Qt::dec` -> `dec`.
  >  Fixed with 28a534ccee9ffa22b2abb7188ab8abb98a099f99 

  
  
  Thx

REPOSITORY
  R293 Baloo

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

To: bruns, #baloo, ngraham
Cc: kossebau, bcooksley, kde-frameworks-devel, hurikhan77, lots0logs, 
LeGast00n, cblack, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D28108: Handle busybox's sed like GNU sed

[Heiko Becker]

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

REVISION SUMMARY
  Otherwise it uses the POSIX-style and fails with "sed: : No such file
  or directory".

TEST PLAN
  Built fine with busybox's sed

REPOSITORY
  R266 Breeze Icons

BRANCH
  master

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

AFFECTED FILES
  generate-24px-versions.sh

To: heikobecker, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27617: [breeze-icons] Add telegram-desktop tray icons

[Noah Davis]

ndavis added a comment.


  In D27617#629023 , @IlyaBizyaev 
wrote:
  
  > This breaks for people using Breeze for apps and Breeze Dark for Plasma:
  >  F8180783: image.png 
  >
  > > pass environemnt variable TDESKTOP_DISABLE_TRAY_COUNTER=1 to 
telegram-desktop also make it follows KDE color scheme
  >
  > This is not a solution; we need to make sure things work by default and not 
in terminal-hackish way. So for a novice user with default Kubuntu settings and 
an official Telegram build, "the icon is broken in KDE" now.
  >  Tbh I'd rather we didn't interfere with 3rd-party apps at all.
  
  
  Why does it need that environment variable? This seems to be a problem with 
Telegram, so maybe it should be fixed upstream?

REPOSITORY
  R266 Breeze Icons

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

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

D28094: [breeze-icons] fix shadow in some user (applets/128) icons

[Noah Davis]

ndavis added a comment.


  In D28094#629427 , @rocka wrote:
  
  > Weird, I edited them in Inkscape …
  >
  > Aren't those icons used as default avatar in user manager?
  
  
  Nope. Those were actually kept in the user-manager repo until they were 
removed in June 2019. These don't seem to be used for anything at all.

REPOSITORY
  R266 Breeze Icons

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

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

D26858: Provide an implementation for the tablet interface

[Aleix Pol Gonzalez]

apol updated this revision to Diff 77861.
apol added a comment.


  Fix tablet removal

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26858?vs=77839=77861

BRANCH
  arcpatch-D26858

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

AFFECTED FILES
  CMakeLists.txt
  autotests/server/CMakeLists.txt
  autotests/server/test_tablet_interface.cpp
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/tablet_interface.cpp
  src/server/tablet_interface.h

To: apol, #kwin, #frameworks, zzag
Cc: davidedmundson, zzag, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D28094: [breeze-icons] fix shadow in some user (applets/128) icons

[Rocket Aaron]

rocka added a comment.


  Weird, I edited them in Inkscape …
  
  Aren't those icons used as default avatar in user manager?

REPOSITORY
  R266 Breeze Icons

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

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

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.12 - Build # 278 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.12/278/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Tue, 17 Mar 2020 18:11:30 +
 Build duration:
2 min 26 sec and counting
   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: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.13 - Build # 171 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.13/171/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Tue, 17 Mar 2020 18:11:30 +
 Build duration:
1 min 55 sec and counting
   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: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

KDE CI: Frameworks » breeze-icons » kf5-qt5 FreeBSDQt5.13 - Build # 164 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20FreeBSDQt5.13/164/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Tue, 17 Mar 2020 18:11:30 +
 Build duration:
1 min 42 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink

D27416: Add small icons for KeePassXC

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R266:036ba91a2bc3: Add small icons for KeePassXC (authored by 
wsdfhjxc, committed by ngraham).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27416?vs=77809=77854

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

AFFECTED FILES
  icons-dark/status/22/keepassxc-locked.svg
  icons-dark/status/22/keepassxc-unlocked.svg
  icons/status/22/keepassxc-locked.svg
  icons/status/22/keepassxc-unlocked.svg

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

D27416: Add small icons for KeePassXC

[Carson Black]

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


  LGTM now. Good icon 

REPOSITORY
  R266 Breeze Icons

BRANCH
  keepassxc-small

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

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

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.13 - Build # 170 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.13/170/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Tue, 17 Mar 2020 17:48:50 +
 Build duration:
1 min 55 sec and counting
   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: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

KDE CI: Frameworks » breeze-icons » kf5-qt5 FreeBSDQt5.13 - Build # 163 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20FreeBSDQt5.13/163/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Tue, 17 Mar 2020 17:48:50 +
 Build duration:
1 min 45 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.12 - Build # 277 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.12/277/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Tue, 17 Mar 2020 17:48:50 +
 Build duration:
1 min 58 sec and counting
   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: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

D28093: [breeze-icons] add TeamViewer tray icons

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R266:cff8c406dd71: [breeze-icons] add TeamViewer tray icons 
(authored by rocka, committed by ngraham).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28093?vs=77810=77851

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

AFFECTED FILES
  icons-dark/status/22/TeamViewer.svg
  icons/status/22/TeamViewer.svg

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

D28094: [breeze-icons] fix shadow in some user (applets/128) icons

[Noah Davis]

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


  Can you fix the files for inkscape?

REPOSITORY
  R266 Breeze Icons

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

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

D28094: [breeze-icons] fix shadow in some user (applets/128) icons

[Noah Davis]

ndavis added a comment.


  Hmm. The shadows no longer work correctly in Inkscape, but they work fine in 
the Qt SVG renderer, which is unusual since usually the Qt SVG renderer is the 
picky one.

REPOSITORY
  R266 Breeze Icons

BRANCH
  fix-user-icon-shadow (branched from master)

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

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

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[Kevin Ottens]

ervin added a comment.


  In D27540#629362 , @ndavis wrote:
  
  > Is it possible to align all of the reset buttons like a column?
  
  
  Why I'm not surprised. ;-)
  
  Honestly with enough code it might be possible, but that'd be expensive in 
term of effort... I'll need to keep track of all widgets in the page. I'll give 
it a shot but don't hold your breath.

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

D28094: [breeze-icons] fix shadow in some user (applets/128) icons

[Noah Davis]

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


  I'm not sure if we actually use these anywhere, but the patch looks good to 
me.

REPOSITORY
  R266 Breeze Icons

BRANCH
  fix-user-icon-shadow (branched from master)

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

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

D27416: Add small icons for KeePassXC

[Noah Davis]

ndavis accepted this revision.
ndavis added a comment.


  LGTM

REPOSITORY
  R266 Breeze Icons

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

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

D28093: [breeze-icons] add TeamViewer tray icons

[Noah Davis]

ndavis accepted this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  teamviewer-tray (branched from master)

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

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

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[Noah Davis]

ndavis added a comment.


  Is it possible to align all of the reset buttons like a column?

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

D27840: Introduce SettingState* elements to ease KCM writing

[Kevin Ottens]

ervin updated this revision to Diff 77848.
ervin added a comment.


  Take feedback about the GUI into account

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27840?vs=76952=77848

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

AFFECTED FILES
  src/qmlcontrols/kcmcontrols/CMakeLists.txt
  src/qmlcontrols/kcmcontrols/kcmcontrolsplugin.cpp
  src/qmlcontrols/kcmcontrols/qml/SettingStateBinding.qml
  src/qmlcontrols/kcmcontrols/qml/SettingStateIndicator.qml
  src/qmlcontrols/kcmcontrols/qml/qmldir
  src/qmlcontrols/kcmcontrols/settingstateproxy.cpp
  src/qmlcontrols/kcmcontrols/settingstateproxy.h

To: ervin, crossi, hchain, meven, bport, davidedmundson, mart, ngraham, 
#frameworks, #plasma
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[Kevin Ottens]

ervin edited the test plan for this revision.

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[Kevin Ottens]

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

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[Kevin Ottens]

ervin updated this revision to Diff 77847.
ervin added a comment.


  Take feedback about the GUI into account

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27540?vs=76362=77847

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kconfigdialogmanager.cpp
  src/kconfigdialogmanager.h
  src/kconfigdialogmanager_p.h
  src/settingsstatusindicator.cpp
  src/settingsstatusindicator_p.h

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

KDE CI: Frameworks » baloo » kf5-qt5 SUSEQt5.12 - Build # 215 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/baloo/job/kf5-qt5%20SUSEQt5.12/215/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Tue, 17 Mar 2020 16:50:33 +
 Build duration:
4 min 25 sec and counting
   BUILD ARTIFACTS
  acc/KF5Baloo-5.69.0.xmllogs/KF5Baloo/5.69.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.autotests Failed: 0 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 4 test(s)Name: projectroot.autotests.unit Failed: 0 test(s), Passed: 34 test(s), Skipped: 0 test(s), Total: 34 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report43%
(10/23)68%
(116/171)68%
(116/171)60%
(6166/10355)42%
(2321/5585)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.benchmarks100%
(2/2)100%
(2/2)100%
(58/58)100%
(22/22)autotests.integration100%
(3/3)100%
(3/3)93%
(466/500)78%
(206/264)autotests.unit.codecs100%
(3/3)100%
(3/3)100%
(70/70)64%
(23/36)autotests.unit.engine100%
(17/17)100%
(17/17)100%
(799/799)56%
(236/424)autotests.unit.file100%
(12/12)100%
(12/12)98%
(1076/1097)53%
(338/632)autotests.unit.lib100%
(6/6)100%
(6/6)99%
(408/414)60%
(96/160)src.codecs100%
(5/5)100%
(5/5)89%
(130/146)76%
(35/46)src.engine92%
(35/38)92%
(35/38)77%
(1763/2298)53%
(695/1315)src.file69%
(27/39)69%
(27/39)53%
(1003/1882)42%
(458/1083)src.file.extractor0%
(0/6)0%
(0/6)0%
(0/181)0%
(0/70)src.kioslaves.kded0%
(0/1)0%
(0/1)0%
(0/35)0%
(0/30)src.kioslaves.search0%
(0/1)0%
(0/1)0%
(0/88)0%
(0/18)src.kioslaves.tags0%
(0/1)0%
(0/1)0%
(0/274)0%
(0/231)src.kioslaves.timeline0%
(0/2)0%
(0/2)0%
(0/256)0%
(0/131)src.lib55%
(6/11)55%
(6/11)41%
(393/965)37%
(212/577)src.qml0%
(0/2)0%
(0/2)0%
(0/69)0%
(0/20)src.qml.experimental0%
(0/3)0%
(0/3)0%

KDE CI: Frameworks » baloo » kf5-qt5 FreeBSDQt5.13 - Build # 91 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/baloo/job/kf5-qt5%20FreeBSDQt5.13/91/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Tue, 17 Mar 2020 16:50:33 +
 Build duration:
3 min 11 sec and counting
   JUnit Tests
  Name: projectroot.autotests Failed: 0 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 4 test(s)Name: projectroot.autotests.unit Failed: 0 test(s), Passed: 33 test(s), Skipped: 0 test(s), Total: 33 test(s)

KDE CI: Frameworks » baloo » kf5-qt5 SUSEQt5.13 - Build # 96 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/baloo/job/kf5-qt5%20SUSEQt5.13/96/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Tue, 17 Mar 2020 16:50:33 +
 Build duration:
2 min 20 sec and counting
   BUILD ARTIFACTS
  acc/KF5Baloo-5.69.0.xmllogs/KF5Baloo/5.69.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.autotests Failed: 0 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 4 test(s)Name: projectroot.autotests.unit Failed: 0 test(s), Passed: 34 test(s), Skipped: 0 test(s), Total: 34 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report43%
(10/23)68%
(116/171)68%
(116/171)60%
(6162/10355)42%
(2319/5585)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.benchmarks100%
(2/2)100%
(2/2)100%
(58/58)100%
(22/22)autotests.integration100%
(3/3)100%
(3/3)93%
(466/500)78%
(206/264)autotests.unit.codecs100%
(3/3)100%
(3/3)100%
(70/70)64%
(23/36)autotests.unit.engine100%
(17/17)100%
(17/17)100%
(799/799)56%
(236/424)autotests.unit.file100%
(12/12)100%
(12/12)98%
(1076/1097)53%
(338/632)autotests.unit.lib100%
(6/6)100%
(6/6)99%
(408/414)60%
(96/160)src.codecs100%
(5/5)100%
(5/5)89%
(130/146)76%
(35/46)src.engine92%
(35/38)92%
(35/38)77%
(1763/2298)53%
(695/1315)src.file69%
(27/39)69%
(27/39)53%
(999/1882)42%
(456/1083)src.file.extractor0%
(0/6)0%
(0/6)0%
(0/181)0%
(0/70)src.kioslaves.kded0%
(0/1)0%
(0/1)0%
(0/35)0%
(0/30)src.kioslaves.search0%
(0/1)0%
(0/1)0%
(0/88)0%
(0/18)src.kioslaves.tags0%
(0/1)0%
(0/1)0%
(0/274)0%
(0/231)src.kioslaves.timeline0%
(0/2)0%
(0/2)0%
(0/256)0%
(0/131)src.lib55%
(6/11)55%
(6/11)41%
(393/965)37%
(212/577)src.qml0%
(0/2)0%
(0/2)0%
(0/69)0%
(0/20)src.qml.experimental0%
(0/3)0%
(0/3)0%
  

D27855: [Debug] Improve readability of positioninfo debug format

[Friedrich W. H. Kossebau]

kossebau added a comment.


  For Qt <5.14 `Qt::hex` needs to be instead unnamespaced `hex`, same with 
`Qt::dec` -> `dec`.
  Fixed with 28a534ccee9ffa22b2abb7188ab8abb98a099f99 


REPOSITORY
  R293 Baloo

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

To: bruns, #baloo, ngraham
Cc: kossebau, bcooksley, kde-frameworks-devel, hurikhan77, lots0logs, 
LeGast00n, cblack, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D28093: [breeze-icons] add TeamViewer tray icons

[Nathaniel Graham]

ngraham added subscribers: ndavis, ngraham.
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  LGTM. @ndavis?

REPOSITORY
  R266 Breeze Icons

BRANCH
  teamviewer-tray (branched from master)

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

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

D27540: KCModule: Indicate when a setting has been changed from the default or previous value

[Kevin Ottens]

ervin added a comment.


  In D27540#627618 , @ndavis wrote:
  
  > Some extra rules I thought of:
  >
  > - With the checkable label example in the mockup above, it should reset 
both the label and the checkbox.
  
  
  Just for the record, this will unlikely to be enforceable on the framework 
side of the code, likely to be taken care of on a case by case basis when we 
encounter those in the modules themselves. Just managing expectations on what 
the code can easily do at that level of abstraction. So don't look for it in 
the patches I submit around the topic. ;-)
  
  The rest should be doable I'll update my patches shortly.

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg, ndavis
Cc: alexde, ndavis, iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

D26858: Provide an implementation for the tablet interface

[Vlad Zahorodnii]

zzag added inline comments.

INLINE COMMENTS

> test_tablet_interface.cpp:1-19
> +/
> +Copyright 2020 Aleix Pol Gonzalez 
> +
> +This library is free software; you can redistribute it and/or
> +modify it under the terms of the GNU Lesser General Public
> +License as published by the Free Software Foundation; either
> +version 2.1 of the License, or (at your option) version 3, or any

Note that we've switched to SPDX markers.

REPOSITORY
  R127 KWayland

BRANCH
  arcpatch-D26858

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

To: apol, #kwin, #frameworks, zzag
Cc: davidedmundson, zzag, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D26858: Provide an implementation for the tablet interface

[Aleix Pol Gonzalez]

apol updated this revision to Diff 77839.
apol added a comment.


  Address comment by David

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26858?vs=75564=77839

BRANCH
  arcpatch-D26858

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

AFFECTED FILES
  CMakeLists.txt
  autotests/server/CMakeLists.txt
  autotests/server/test_tablet_interface.cpp
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/tablet_interface.cpp
  src/server/tablet_interface.h

To: apol, #kwin, #frameworks, zzag
Cc: davidedmundson, zzag, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

Re: KDE CI: Applications » kreversi » kf5-qt5 FreeBSDQt5.13 - Build # 26 - Still Failing!

[Friedrich W. H. Kossebau]

Am Dienstag, 17. März 2020, 07:01:54 CET schrieb laurent Montel:
> Le lundi 16 mars 2020, 15:43:29 CET Friedrich W. H. Kossebau a écrit :
> > Laurent,
> > could you do the favour to change all the usages of
> > *DISABLE_DEPRECATED_BEFORE_AND_AT you introduced to a version matching to
> > the latest released version you are sure works fine? (or organize such a
> > change by people working on those repos). And this should be done for
> > stable as well (sadly 20.04 was just branched :/ ).
> > The "if (EXISTS "${CMAKE_SOURCE_DIR}/.git")" should be removed as well
> > then, no longer being needed, and to also ensure released tarballs are
> > built with same flags as developer builds (might e.g. affect overloaded
> > methods).
> I vote indeed for removing all theses lines in CmakeLists.txt, as it's the
> method used at the moment for fixing compile error.
> 
> I liked the Volker's work about deprecated method, he ported all code before
> to deprecate API. It was a good method as he knew how to port code, so he
> ported and after deprecated.

Yes, that would be the perfect world.
Given the amount of KDE software that does not really scale though. And Qt 
developers deprecating API will not do us such favour in any case.

> For other devs they deprecated API but no port KDE code and didn't write
> info how to port it,

No info how to port should though be less a case these days, no? Do you have 
some examples where you would think there should be more info?
We should improve our KF policies then to make sure proper porting info is 
present, if that is not yet happening.

At least where I was involved, some "Deprecated since x.y, use foo() instead" 
was done, which usually was enough to do porting code (too bad we do not have 
a way to automate such simple porting yet, in a perfect world we could have 
fixit snippets for clazy/clang-tidy).

> and it's too sad because for sure we will port this
> code later perhaps when we will port to Qt6, all this port will we do
> quickly perhaps with bugs (ok I already made some errors but I make the
> porting on several release so we can see problem on a long time.).

I think we all agree that doing work as early as possible is good. But forcing 
people into it, there we do not agree :)

> As Friedrich wrote the people/maintainer will port code when they will see
> compile warning, I will trust him but when I look at git commit I don't see
> a lot of commit about it,

Most will not port immediately, but when they get to it. Often bugs & features 
are more important. I would rather see builds failing as long as there are 
bugs reports with data-loss crashes open, to ensure people are forced to care 
for those ;)

> but I see some commit which removed this
> DISABLE_DEPRECATED_BEFORE_AND_AT macro.

The latter might have happened with macros set to 0x06 versions, no? Which 
might have happened as people were just upset of getting their builds broken 
every now and then by this, when they actually had no time to care for 
deprecated API at that time. And did not understand things enough to know that 
just setting a proper limit would have been better.

> To conclude I will remove theses lines in all modules (but in pim* and some
> apps as I maintains them) and "wait and see" how devs will make porting.

Thanks for handling that so quickly.
Though, instead of removing all lines completely, I would have hoped just the 
numbers would be adapted and the in-git-checkout-only wrapper removed.
So we would have a compiler-supported guard to not add new usages of already 
deprecated API. As by the recommendation given in
https://marc.info/?l=kde-devel=157190321318565=2
the ideal setup would be like this:
--- 8< ---
add_definitions(
# hide deprecated API of Qt <= 5.9
-DQT_DISABLE_DEPRECATED_BEFORE=0x050900 
# enable warnings for API up to Qt 6.0
-DQT_DEPRECATED_WARNINGS_SINCE=0x06
# hide deprecated API of KF <= 5.48
-DKF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x053000
# enable warnings for API up to KF 6.0
-DKF_DEPRECATED_WARNINGS_SINCE=0x06
)
--- 8< ---
((The *_DEPRECATED_WARNINGS_SINCE variants are needed, as explicitly set 
*_DISABLE_DEPRECATED_BEFORE* triggers a default of no warnings for 
deprecations in newer API, so this has to be overwritten.
Yes, in KDE Frameworks you do not see those, as they are done centrally in 
KDEFrameworkCompilerSettings.cmake))

Having none of those flags set results in warnings for all deprecated API, so 
still a setup that should allow developers to know what to work on.

Cheers
Friedrich

D26858: Provide an implementation for the tablet interface

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> tablet_interface.cpp:346
> +
> +TabletToolInterface *TabletSeatInterface::toolByHardwareId(quint64 serialId) 
> const
> +{

We're mixing up hardwareId and hardwareSerial

this method is called toolByHardwareId calls an internal method called 
hardwareSerial

Yet ultimately does check:
m_hardwareIdHigh

comparing it against a serial

REPOSITORY
  R127 KWayland

BRANCH
  arcpatch-D26858

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

To: apol, #kwin, #frameworks, zzag
Cc: davidedmundson, zzag, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27544: Fix update scenarios with no explicit downloadlink selected

[Dan Leinir Turthra Jensen]

leinir updated this revision to Diff 77818.
leinir marked 4 inline comments as done.
leinir added a comment.


  Address @ahiemstra's comments - thanks!
  
  - Turn on C++14 support
  - Fix some whitespace issues, a leak, and add a warning
  - Less magic numbers, with the power of enums

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27544?vs=77494=77818

BRANCH
  fix-update (branched from master)

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

AFFECTED FILES
  CMakeLists.txt
  src/core/engine.cpp
  src/core/jobs/httpworker.cpp
  src/core/security.cpp
  src/qtquick/qml/EntryDetails.qml
  src/qtquick/qml/NewStuffItem.qml
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/ThumbDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml
  src/qtquick/quickitemsmodel.cpp
  src/qtquick/quickitemsmodel.h

To: leinir, #knewstuff, #frameworks, #plasma, ngraham, apol, 
#discover_software_store
Cc: ahiemstra, alexde, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27544: Fix update scenarios with no explicit downloadlink selected

[Dan Leinir Turthra Jensen]

leinir marked 5 inline comments as done.
leinir added a comment.


  Thanks for those, @ahiemstra, good stuff there :)

INLINE COMMENTS

> ahiemstra wrote in engine.cpp:614
> Code style: & attaches to the name, not the type. (Yes I hate it too).
> 
> There's a few instances of this around.

so nasty... right, thanks for pointing those out, it feels so wrong to type :P

> ahiemstra wrote in engine.cpp:631
> This object will linger until the engine is destroyed, which seems like a 
> suboptimal thing. Maybe better to do:
> 
>   auto question = 
> std::make_unique(Question::SelectFromListQuestion);
> 
> then it will be automatically cleaned up once you exit the scope.

Hmm... That's something that'll want doing in a fair few places i think 
(including the documentation for Question) - but yup, might as well start 
somewhere :) It does mean turning on C++14 for KNewStuff, which... should be 
fine?

> ahiemstra wrote in engine.cpp:644
> You may want to log something in the else here, at least then it will be 
> possible to figure out why an update is not happening.

That's probably a good idea, yes... It'll commonly be due to the user 
cancelling the dialog, but if there's no good questionasker registered, it may 
happen anyway - plus i guess it's just good style anyway :)

> ahiemstra wrote in EntryDetails.qml:101
> That "1" here is a bit mysterious, what does it actually mean?

It's that whole one-indexed list thing that OCS has... But yup, your idea below 
sounds pretty good.

> ahiemstra wrote in quickitemsmodel.h:154
> Rather than using -1, you could add an enum that defines these values a bit 
> more, like:
> 
>   enum LinkId {
>   AutoDetectLink = -1,
>   FirstItem = 1
>   }
> 
> You can then also expose that to QML to avoid the magical 1 up there.

A good call, less magic numbers are always good. Did feel a little... 
unpleasant to add those.

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-update (branched from master)

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

To: leinir, #knewstuff, #frameworks, #plasma, ngraham, apol, 
#discover_software_store
Cc: ahiemstra, alexde, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

Re: Problems in KWayland causes by API and ABI compatibility promises

[Aleix Pol]

On Tue, Mar 17, 2020 at 11:27 AM David Edmundson
 wrote:
>
> There's definitely a problem, and I think it's a lot more than just
> discussing location and protocol versions.
>
> My typical workflow is reading the wayland protocol in detail, and
> kwayland is in that awkward position where I absolutely still need to
> read those underlying docs and the abstraction kwayland provides
> frankly only doubles my workload.
>
> Taking a step backwards, there's a lot of parts of kwayland that are
> very inconsistent.
>
> It looks like it's been written by 4 different devs all exploring
> different ideas of how things should work...which isn't too surprising
> given that's exactly what happened!
>
> IMHO we're lacking a "what actually is kwayland?"  and an accurate
> definition of what's the added value compared to just using the auto
> generated classes directly.
>
> 
>
> To bootstrap this I've started an initial list of discrete yes/no
> questions to help serve as a starting point of what kwayland's
> direction specifically should be.

I'd say the right question to ask is not what kwayland is but what we
need it to be.
When working on it, I sometimes feel it's about implementing wayland
interfaces so they can easily be adopted by another Qt-based, not
QtWaylandCompositor-based, Wayland compositor.

>  - Is it kwayland's job to abstract different versions of the same protocol?
>
>  - Is it kwayland's job to abstract different protocols that are
> semantically similar?
> (including xdgshellv6 and wm_base)
>
> - Is it kwayland's job to turn an event-driven API into a property-driven API?
> (i.e turning the request set_minimum_size into an API where you can
> query what was last received)
>
>  - Is it kwayland's job to abstract receiving double buffered stuff?
> (i.e the getter above only gets the value once committed)
>
>  - Is it kwayland's job to abstract sending double buffered stuff?
> (i.e implicitly send "wl_output.done() after Output::setSize())
>
>  - Is it kwayland's job to be a multiplexer?
> (i.e updating xdgoutput or plasmawindowmanagement forwards events to
> all listening resources)
> If so should we express this difference in the API or have them as
> methods on the global? Same for the sending to only the focussed
> window?
>
>  - Is it kwayland's job to convert basic types into Qt friendly types?

On all the above, I'd say the answer to all these we should set some
expectations. In the end we should be creating the KWayland that is
most useful for us.

As a rule of thumb, I'd say it doesn't make sense to implement on
KWayland things that won't be used outside of KWin, e.g. D26858.

Aleix

D27416: Add small icons for KeePassXC

[Wojciech Stachurski]

wsdfhjxc added a comment.


  Is it any better now? @cblack @ndavis

REPOSITORY
  R266 Breeze Icons

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

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

D27416: Add small icons for KeePassXC

[Wojciech Stachurski]

wsdfhjxc edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D28097: GIT_SILENT clang-format kpasswdserver.* plus some manual changes

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, dfaure, apol.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  Braces around if blocks.

TEST PLAN
  make && ctest

REPOSITORY
  R241 KIO

BRANCH
  l-kpasswdserver-clang-format (branched from master)

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

AFFECTED FILES
  src/kpasswdserver/kiod_kpasswdserver.cpp
  src/kpasswdserver/kpasswdserver.cpp
  src/kpasswdserver/kpasswdserver.h

To: ahmadsamir, #frameworks, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

Re: Problems in KWayland causes by API and ABI compatibility promises

[David Edmundson]

There's definitely a problem, and I think it's a lot more than just
discussing location and protocol versions.

My typical workflow is reading the wayland protocol in detail, and
kwayland is in that awkward position where I absolutely still need to
read those underlying docs and the abstraction kwayland provides
frankly only doubles my workload.

Taking a step backwards, there's a lot of parts of kwayland that are
very inconsistent.

It looks like it's been written by 4 different devs all exploring
different ideas of how things should work...which isn't too surprising
given that's exactly what happened!

IMHO we're lacking a "what actually is kwayland?"  and an accurate
definition of what's the added value compared to just using the auto
generated classes directly.



To bootstrap this I've started an initial list of discrete yes/no
questions to help serve as a starting point of what kwayland's
direction specifically should be.

 - Is it kwayland's job to abstract different versions of the same protocol?

 - Is it kwayland's job to abstract different protocols that are
semantically similar?
(including xdgshellv6 and wm_base)

- Is it kwayland's job to turn an event-driven API into a property-driven API?
(i.e turning the request set_minimum_size into an API where you can
query what was last received)

 - Is it kwayland's job to abstract receiving double buffered stuff?
(i.e the getter above only gets the value once committed)

 - Is it kwayland's job to abstract sending double buffered stuff?
(i.e implicitly send "wl_output.done() after Output::setSize())

 - Is it kwayland's job to be a multiplexer?
(i.e updating xdgoutput or plasmawindowmanagement forwards events to
all listening resources)
If so should we express this difference in the API or have them as
methods on the global? Same for the sending to only the focussed
window?

 - Is it kwayland's job to convert basic types into Qt friendly types?

David

D28094: [breeze-icons] fix shadow in some user (applets/128) icons

[Rocket Aaron]

rocka updated this revision to Diff 77814.
rocka added a comment.


  remove svg metadata

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28094?vs=77811=77814

BRANCH
  fix-user-icon-shadow (branched from master)

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

AFFECTED FILES
  icons-dark/applets/128/user-ada-lovelace.svg
  icons-dark/applets/128/user-leonardo-da-vinci.svg
  icons/applets/128/user-ada-lovelace.svg
  icons/applets/128/user-leonardo-da-vinci.svg

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

D28095: Bump required version of KF5 to 5.69.0

[Ahmad Samir]

This revision was automatically updated to reflect the committed changes.
Closed by commit R871:ce461918868d: Bump required version of KF5 to 5.69.0 
(authored by ahmadsamir).

REPOSITORY
  R871 DrKonqi

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28095?vs=77812=77813

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

AFFECTED FILES
  CMakeLists.txt

To: ahmadsamir, #frameworks, dfaure, apol, sitter
Cc: kde-frameworks-devel, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D28095: Bump required version of KF5 to 5.69.0

[Harald Sitter]

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

REPOSITORY
  R871 DrKonqi

BRANCH
  l-kcrash (branched from master)

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

To: ahmadsamir, #frameworks, dfaure, apol, sitter
Cc: kde-frameworks-devel, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D27544: Fix update scenarios with no explicit downloadlink selected

[Arjen Hiemstra]

ahiemstra added inline comments.

INLINE COMMENTS

> engine.cpp:614
> +QString identifiedLink;
> +const QString& payloadToIdentify = d->payloadToIdentify[entry];
> +const QStringList& payloads = d->payloads[entry];

Code style: & attaches to the name, not the type. (Yes I hate it too).

There's a few instances of this around.

> engine.cpp:621
> +// Next simplest option, filename is the same but in a 
> different folder
> +const QStringRef& fileName = 
> payloadToIdentify.splitRef(QChar::fromLatin1('/')).last();
> +for (const QString& payload : payloads) {

This makes a reference to a temporary which I'm not sure is a good idea. Since 
you are already using splitRef, just make a copy.

> engine.cpp:631
> +// Least simple option, no match - ask the user to pick 
> (and if we still haven't got one... that's us done, no installation)
> +Question* question = new 
> Question(Question::SelectFromListQuestion, this);
> +question->setTitle(i18n("Pick Update Item"));

This object will linger until the engine is destroyed, which seems like a 
suboptimal thing. Maybe better to do:

  auto question = std::make_unique(Question::SelectFromListQuestion);

then it will be automatically cleaned up once you exit the scope.

> engine.cpp:644
> +m_installation->install(theEntry);
> +}
> +// As the serverside data may change before next time this is 
> called, even in the same session,

You may want to log something in the else here, at least then it will be 
possible to figure out why an update is not happening.

> EntryDetails.qml:101
>  if (component.downloadCount == 1) {
> -newStuffModel.installItem(component.index);
> +newStuffModel.installItem(component.index, 1);
>  } else {

That "1" here is a bit mysterious, what does it actually mean?

> quickitemsmodel.h:154
>   * @param index The index of the item to install or update
> + * @param linkId The download item to install. If this is -1, it is 
> assumed to be an update with an unknown payload, and a number of heuristics 
> will be applied by the engine
> + * @see Engine::downloadLinkLoaded implementation for details

Rather than using -1, you could add an enum that defines these values a bit 
more, like:

  enum LinkId {
  AutoDetectLink = -1,
  FirstItem = 1
  }

You can then also expose that to QML to avoid the magical 1 up there.

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-update (branched from master)

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

To: leinir, #knewstuff, #frameworks, #plasma, ngraham, apol, 
#discover_software_store
Cc: ahiemstra, alexde, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27986: Allow providing an error message from the application

[Ahmad Samir]

ahmadsamir added a comment.


  In D27986#628977 , @bcooksley 
wrote:
  
  > The following is notice that I intend to revert this change, along with the 
corresponding commits that make use of this functionality in Dr Konqi, as they 
cause a FTBFS on both FreeBSD and Windows which has not been addressed.
  >  This regression is over a week old at this point.
  >
  > Please see 
https://build.kde.org/view/Failing/job/Plasma/job/drkonqi/job/kf5-qt5%20FreeBSDQt5.13/lastFailedBuild/
 and 
https://build.kde.org/view/Failing/job/Plasma/job/drkonqi/job/kf5-qt5%20WindowsMSVCQt5.14/lastFailedBuild/
 for more information.
  
  
  The FreeBSD issue at least should be fixed by D28095 
.

REPOSITORY
  R285 KCrash

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

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

D28095: Bump required version of KF5 to 5.69.0

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, dfaure, apol, sitter.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  KCrash::setErrorMessage() was introduced in 5.69.0.

REPOSITORY
  R871 DrKonqi

BRANCH
  l-kcrash (branched from master)

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

AFFECTED FILES
  CMakeLists.txt

To: ahmadsamir, #frameworks, dfaure, apol, sitter
Cc: kde-frameworks-devel, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D28094: [breeze-icons] fix shadow in some user (applets/128) icons

[Rocket Aaron]

rocka edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D28094: [breeze-icons] fix shadow in some user (applets/128) icons

[Rocket Aaron]

rocka edited the test plan for this revision.
rocka added a reviewer: VDG.

REPOSITORY
  R266 Breeze Icons

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

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

D28094: [breeze-icons] fix shadow in some user (applets/128) icons

[Rocket Aaron]

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

REVISION SUMMARY
  When executing `kdialog --geticon`, some errors would shown:
  
$ kdialog --geticon
qt.svg: /usr/share/icons/breeze-dark/applets/128/user-ada-lovelace.svg:68: 
Could not resolve property: #linearGradient6624
qt.svg: /usr/share/icons/breeze-dark/applets/128/user-ada-lovelace.svg:68: 
Could not resolve property: #linearGradient6626
qt.svg: 
/usr/share/icons/breeze-dark/applets/128/user-leonardo-da-vinci.svg:79: Could 
not resolve property: #linearGradient6634
  
  ... And those 2 icons doesn't have correct shadow. This patch fixes it.

REPOSITORY
  R266 Breeze Icons

BRANCH
  fix-user-icon-shadow (branched from master)

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

AFFECTED FILES
  icons-dark/applets/128/user-ada-lovelace.svg
  icons-dark/applets/128/user-leonardo-da-vinci.svg
  icons/applets/128/user-ada-lovelace.svg
  icons/applets/128/user-leonardo-da-vinci.svg

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

D27617: [breeze-icons] Add telegram-desktop tray icons

[Ilya Bizyaev]

IlyaBizyaev added a comment.


  This breaks for people using Breeze for apps and Breeze Dark for Plasma:
  F8180783: image.png 
  
  > pass environemnt variable TDESKTOP_DISABLE_TRAY_COUNTER=1 to 
telegram-desktop also make it follows KDE color scheme
  
  This is not a solution; we need to make sure things work by default and not 
in terminal-hackish way. So for a novice user with default Kubuntu settings and 
an official Telegram build, "the icon is broken in KDE" now.
  Tbh I'd rather we didn't interfere with 3rd-party apps at all.

REPOSITORY
  R266 Breeze Icons

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

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

D28093: [breeze-icons] add TeamViewer tray icons

[Rocket Aaron]

rocka edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D28093: [breeze-icons] add TeamViewer tray icons

[Rocket Aaron]

rocka added a reviewer: VDG.

REPOSITORY
  R266 Breeze Icons

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

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

D28093: [breeze-icons] add TeamViewer tray icons

[Rocket Aaron]

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

REVISION SUMMARY
  Created TeamViewer tray icons following Breeze style.

REPOSITORY
  R266 Breeze Icons

BRANCH
  teamviewer-tray (branched from master)

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

AFFECTED FILES
  icons-dark/status/22/TeamViewer.svg
  icons/status/22/TeamViewer.svg

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

D27416: Add small icons for KeePassXC

[Wojciech Stachurski]

wsdfhjxc edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D27416: Add small icons for KeePassXC

[Wojciech Stachurski]

wsdfhjxc updated this revision to Diff 77809.
wsdfhjxc added a comment.


  Update small icons for KeePassXC

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27416?vs=75722=77809

BRANCH
  keepassxc-small

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

AFFECTED FILES
  icons-dark/status/22/keepassxc-locked.svg
  icons-dark/status/22/keepassxc-unlocked.svg
  icons/status/22/keepassxc-locked.svg
  icons/status/22/keepassxc-unlocked.svg

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

D27840: Introduce SettingState* elements to ease KCM writing

[Benjamin Port]

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


  From code POV this is ok, but neet to wait feedback from VDG to adapt state 
indicator

REPOSITORY
  R296 KDeclarative

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

To: ervin, crossi, hchain, meven, bport, davidedmundson, mart, ngraham, 
#frameworks, #plasma
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28079: [keditfiletype] Prevent removing the "main" glob pattern for mime types

[Ahmad Samir]

ahmadsamir added a comment.


  For background see: https://bugs.kde.org/show_bug.cgi?id=414742
  
  I may be wrong, but personally I think QMimeDatabase is acting sensibly by 
not allowing to remove the "main" glob pattern of a mimetype.
  
  Also IIUC it seems other tools are sort of doing the same, e.g. gio (see the 
bug report for some testing I posted there).
  
  So, either this diff makes sense, or someone will have to put his upstream Qt 
hat on and change QMimeDatabase :p; I dug into the git history upstream, and 
IIRC it seems QMimeDatabase prepends the main glob pattern since 2012 (from 
before it was merged into qtbase).

REPOSITORY
  R126 KDE CLI Utilities

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

To: ahmadsamir, #plasma, dfaure, davidedmundson, apol
Cc: kde-frameworks-devel, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D27986: Allow providing an error message from the application

[Ben Cooksley]

bcooksley added a comment.


  The following is notice that I intend to revert this change, along with the 
corresponding commits that make use of this functionality in Dr Konqi, as they 
cause a FTBFS on both FreeBSD and Windows which has not been addressed.
  This regression is over a week old at this point.
  
  Please see 
https://build.kde.org/view/Failing/job/Plasma/job/drkonqi/job/kf5-qt5%20FreeBSDQt5.13/lastFailedBuild/
 and 
https://build.kde.org/view/Failing/job/Plasma/job/drkonqi/job/kf5-qt5%20WindowsMSVCQt5.14/lastFailedBuild/
 for more information.

REPOSITORY
  R285 KCrash

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

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

D27855: [Debug] Improve readability of positioninfo debug format

[Ben Cooksley]

bcooksley added a comment.


  One of the changes in this string of 4 revisions has unfortunately broken the 
build of Baloo on the CI system.
  Please see 
https://build.kde.org/view/Failing/job/Frameworks/job/baloo/job/kf5-qt5%20FreeBSDQt5.13/90/console

REPOSITORY
  R293 Baloo

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

To: bruns, #baloo, ngraham
Cc: bcooksley, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17816: Support for xattrs on kio copy/move

[Cochise César]

cochise added inline comments.

INLINE COMMENTS

> dfaure wrote in jobtest.cpp:638
> Was my comment ignored?

Fixed on new commit.

REPOSITORY
  R241 KIO

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

To: cochise, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta
Cc: usta, scheirle, anthonyfieroni, tmarshall, arrowd, cfeck, bruns, phidrho, 
dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, 
spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh

D17816: Support for xattrs on kio copy/move

[Cochise César]

cochise added inline comments.

INLINE COMMENTS

> usta wrote in jobtest.cpp:573
> but you have written it as with single T , i think it must be double ( TT )

You are right. Sorry.
Fixed. =]

REPOSITORY
  R241 KIO

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

To: cochise, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta
Cc: usta, scheirle, anthonyfieroni, tmarshall, arrowd, cfeck, bruns, phidrho, 
dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, 
spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh

D17816: Support for xattrs on kio copy/move

[Cochise César]

cochise updated this revision to Diff 77804.
cochise added a comment.


  Refactored the tests:
  
  Plattform command configured on JobTest::initTestCase
  
  Removal duplicated code of functions [...]WithXattr
  
  Added check is filesystens supports users xattr:
  
  > Try to write xattr on source filesystem. If not able sets to skip.
  >  If destination is in another partition, repeat on dest.
  >  As we don't wanna skip the whole copy test, skipping is done by not 
writing xattr on source file.
  
  Cleanup.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=77559=77804

BRANCH
  arcpatch-D17816_1

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: cochise, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta
Cc: usta, scheirle, anthonyfieroni, tmarshall, arrowd, cfeck, bruns, phidrho, 
dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, 
spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh

Re: KDE CI: Applications » kreversi » kf5-qt5 FreeBSDQt5.13 - Build # 26 - Still Failing!

[laurent Montel]

Le lundi 16 mars 2020, 15:43:29 CET Friedrich W. H. Kossebau a écrit :
> Am Montag, 16. März 2020, 10:39:39 CET schrieb Ben Cooksley:
> > On Mon, Mar 16, 2020 at 10:24 PM David Edmundson
> > 
> >  wrote:
> > > There is not a SIC. KIO is fine
> > > 
> > > It's merely this crap again:
> > > 
> > > if (EXISTS "${CMAKE_SOURCE_DIR}/.git")
> > > 
> > >add_definitions(-DQT_DISABLE_DEPRECATED_BEFORE=0x06)
> > >add_definitions(-DKF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x06)
> > > 
> > > endif()
> > 
> > May I suggest that we tweak this macro so that passing 0x06 makes
> > it no-op and fail open (disabling nothing)?

Hi,

> We could only tweak the macros handling K*_DISABLE_DEPRECATED_BEFORE_AND_AT,
> though those are complicated enough and this would also just be a
> work-around for a usage currently not favoured by many (it will be wanted
> at KF6 preparation/porting times though).
> 
> Instead we should finally create the policy everywhere to use
> QT_DISABLE_DEPRECATED_BEFORE & KF_DISABLE_DEPRECATED_BEFORE_AND_AT only with
> numbers of released versions that have been checked against. And not use
> those with version numbers of the future to trigger failing builds for
> developers once further API in use is deprecated, to force people into
> working on usage of deprecated API instantly.
> 
> Some thought doing 0x06 would be still acceptable with master branches.
> Though this would need active avoiding to have this setup on creating new
> stable branches being copied into those. Which seems not cared for though.
> And myself I am among the people who think also for master being forced into
> deprecated API work is overdoing - it is good to do things as early as
> possible, to "flatten the curve" of Qt6/KF6 porting work. But that is what
> warnings are for.
> 
> Laurent,
> could you do the favour to change all the usages of
> *DISABLE_DEPRECATED_BEFORE_AND_AT you introduced to a version matching to
> the latest released version you are sure works fine? (or organize such a
> change by people working on those repos). And this should be done for
> stable as well (sadly 20.04 was just branched :/ ).
> The "if (EXISTS "${CMAKE_SOURCE_DIR}/.git")" should be removed as well then,
> no longer being needed, and to also ensure released tarballs are built with
> same flags as developer builds (might e.g. affect overloaded methods).

I vote indeed for removing all theses lines in CmakeLists.txt, as it's the 
method used at the moment for fixing compile error.

I liked the Volker's work about deprecated method, he ported all code before 
to deprecate API. It was a good method as he knew how to port code, so he 
ported and after deprecated.

For other devs they deprecated API but no port KDE code and didn't write info 
how to port it, and it's too sad because for sure we will port this code later 
perhaps when we will port to Qt6, all this port will we do quickly perhaps 
with bugs (ok I already made some errors but I make the porting on several 
release so we can see problem on a long time.).

As Friedrich wrote the people/maintainer will port code when they will see 
compile warning, I will trust him but when I look at git commit I don't see a 
lot of commit about it, but I see some commit which removed this 
DISABLE_DEPRECATED_BEFORE_AND_AT macro.

Ok before when we increased kf5 or qt version sometimes it didn't build so the 
solution was to fixing 1 compile error or two, sometime no very hard to fix, 
without theses macros we will accumulate a lot of compile warning and it will 
harder to port at the end when we will migrate at qt6, and not sure that we 
will have more dev at this moment.



To conclude I will remove theses lines in all modules (but in pim* and some 
apps as I maintains them) and "wait and see" how devs will make porting.

Regards 

> 
> Yes, this will break your personal workflow of your deprecated API usage
> work (which IMHO is very valuable, sadly of the kind nobody will really
> notice due to Qt6/KF6 porting life being easier for them without knowing
> how bad it would have been, you will stay an unsung hero).
> I have no simple idea yet how to enable a similar approach yet again.
> Passing "-Werror=deprecated-declarations" to the build configurations via
> env vars is not targeted only at Qt & KF, so might have undesired
> sideeffects.
> 
> Cheers
> Friedrich


-- 
Laurent Montel | laurent.mon...@kdab.com | KDE/Qt Senior Software Engineer 
KDAB (France) S.A.S., a KDAB Group company Tel. France +33 (0)4 90 84 08 53, 
 www.kdab.fr KDAB - The Qt, C++ and OpenGL Experts - Platform-independent 
software solutions