D25939: TypeScript: add "bigint" primitive type

[Christoph Cullmann]

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


  Seems fine, thanks!

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  typescript-bigint-type

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

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

D25939: TypeScript: add "bigint" primitive type

[Nibaldo González]

nibags created this revision.
nibags added a reviewer: Framework: Syntax Highlighting.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
nibags requested review of this revision.

REVISION SUMMARY
  Add the "bigint" type: 
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-2.html#bigint

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  typescript-bigint-type

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

AFFECTED FILES
  autotests/folding/test.ts.fold
  autotests/html/test.ts.html
  autotests/input/test.ts
  autotests/reference/test.ts.ref
  data/syntax/typescript.xml

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

D25869: Python: improve numbers, add octals, binaries and "breakpoint" keyword

[Nibaldo González]

nibags closed this revision.

REPOSITORY
  R216 Syntax Highlighting

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

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

D24433: Move URL parsing methods from kioslave to query object

[Stefan Brüns]

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


  just nitpicks ...

REPOSITORY
  R293 Baloo

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

To: iasensio, #baloo, meven, ngraham, astippich, bruns
Cc: broulik, bruns, kde-frameworks-devel, #dolphin, hurikhan77, lots0logs, 
LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D24433: Move URL parsing methods from kioslave to query object

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> query.cpp:342
> +QString jsonString = urlQuery.queryItemValue(QStringLiteral("json"), 
> QUrl::FullyDecoded);
> +q = Query::fromJSON(jsonString.toUtf8());
> +} else if (urlQuery.hasQueryItem(QStringLiteral("query"))) {

just `return Query::fromJSON(jsonString.toUtf8());`

> query.cpp:343
> +q = Query::fromJSON(jsonString.toUtf8());
> +} else if (urlQuery.hasQueryItem(QStringLiteral("query"))) {
> +QString queryString = 
> urlQuery.queryItemValue(QStringLiteral("query"), QUrl::FullyDecoded);

remove the `else`

> query.cpp:345
> +QString queryString = 
> urlQuery.queryItemValue(QStringLiteral("query"), QUrl::FullyDecoded);
> +q.setSearchString(queryString);
> +} else {

just `return q;`

> query.cpp:346
> +q.setSearchString(queryString);
> +} else {
> +const QString jsonString = jsonQueryFromUrl(url);

remove `else`

> query.cpp:349
> +if (!jsonString.isEmpty()) {
> +q = Query::fromJSON(jsonString.toUtf8());
> +}

just `return Query::fromJSON(jsonString.toUtf8());`

REPOSITORY
  R293 Baloo

BRANCH
  extend_query_url

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

To: iasensio, #baloo, meven, ngraham, astippich
Cc: broulik, bruns, kde-frameworks-devel, #dolphin, hurikhan77, lots0logs, 
LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D25622: partial solution to accept accentuated characters on windows

[Matthieu Gallien]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:9220e5045687: partial solution to accept accentuated 
characters on windows (authored by mgallien).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25622?vs=70592&id=71404

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

AFFECTED FILES
  src/extractors/taglibextractor.cpp

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

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/219/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Thu, 12 Dec 2019 22:34:38 +
 Build duration:
10 min and counting
   JUnit Tests
  Name: projectroot Failed: 3 test(s), Passed: 49 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.13 - Build # 234 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.13/234/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Thu, 12 Dec 2019 22:34:38 +
 Build duration:
7 min 18 sec and counting
   BUILD ARTIFACTS
  acc/KF5KIO-5.65.0.xmllogs/KF5KIO/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: 0 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 53 test(s)Name: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report67%
(24/36)67%
(270/405)67%
(270/405)56%
(34649/61665)40%
(17600/43559)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(58/58)100%
(58/58)95%
(9645/10105)47%
(4504/9556)autotests.http100%
(5/5)100%
(5/5)99%
(580/581)68%
(108/160)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core88%
(104/118)88%
(104/118)59%
(8659/14558)51%
(4521/8823)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets68%
(26/38)68%
(26/38)56%
(4658/8286)43%
(2028/4744)src.gui100%
(2/2)100%
(2/2)94%
(102/108)74%
(49/66)src.ioslaves.file100%
(7/7)100%
(7/7)56%
(678/1206)40%
(386/954)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/168)0%
(0/89)src.ioslaves.ftp100%
(2/2)100%
(2/2)47%
(644/1370)37%
(524/1418)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/148)src.ioslaves.http88%
(7/8)88%
(7/8)42%
(1796/4288)36%
(1309/3636)src.ioslaves.http.kcookiejar40%
(2/5)40%
(2/5)47%
(631/1330)56%
(576/1027)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(73/267)8%
(14/184)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
  

D25937: pic: Fix Invalid-enum-value undefined behaviour

[Albert Astals Cid]

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

REVISION SUMMARY
  Instead of directly casting the quint8 to PicChannelEncoding we just store 
the quint8
  and compare it to the possible PicChannelEncoding values when needed
  
  oss-fuzz/19344

REPOSITORY
  R287 KImageFormats

BRANCH
  master

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

AFFECTED FILES
  src/imageformats/pic.cpp
  src/imageformats/pic_p.h

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

D25622: partial solution to accept accentuated characters on windows

[Hannah von Reth]

vonreth added a comment.


  In D25622#576498 , @bruns wrote:
  
  > Again, #windows  does not care ...
  
  
  The team is very small...

REPOSITORY
  R286 KFileMetaData

BRANCH
  master

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

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

D21204: Ensure mounted nfs filesystems matches their fstab declared counterpart

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R245 Solid

BRANCH
  arcpatch-D21204

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

To: meven, bruns, #frameworks, ngraham
Cc: broulik, dhaumann, anthonyfieroni, ngraham, bruns, apol, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh

D24433: Move URL parsing methods from kioslave to query object

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  Let's do this unless @bruns objects soon. :)

REPOSITORY
  R293 Baloo

BRANCH
  extend_query_url

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

To: iasensio, #baloo, meven, ngraham, astippich
Cc: broulik, bruns, kde-frameworks-devel, #dolphin, hurikhan77, lots0logs, 
LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D25414: xattr: fix crash on dangling symlinks

[Ismael Asensio]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:4bb4195a6fc6: xattr: fix crash on dangling symlinks 
(authored by iasensio).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25414?vs=70035&id=71397

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

AFFECTED FILES
  autotests/usermetadatawritertest.cpp
  autotests/usermetadatawritertest.h
  src/xattr_p.h

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

D25622: partial solution to accept accentuated characters on windows

[Stefan Brüns]

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


  Again, #windows  does not care ...

REPOSITORY
  R286 KFileMetaData

BRANCH
  master

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

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

D25934: [xattr] Fix passing negative size to QByteArray

[Anthony Fieroni]

anthonyfieroni abandoned this revision.

REPOSITORY
  R286 KFileMetaData

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

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

D21204: Ensure mounted nfs filesystems matches their fstab declared counterpart

[Stefan Brüns]

bruns added a comment.


  please remove the 2 comments.

REPOSITORY
  R245 Solid

BRANCH
  arcpatch-D21204

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

To: meven, bruns, #frameworks, ngraham
Cc: broulik, dhaumann, anthonyfieroni, ngraham, bruns, apol, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh

D21204: Ensure mounted nfs filesystems matches their fstab declared counterpart

[Stefan Brüns]

bruns accepted this revision.
bruns added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> fstabhandling.cpp:246
> +if (deviceName.endsWith(QLatin1Char('/'))) {
> +// remove trailing slash
> +deviceName.chop(1);

This comment is stating the obvious ...

> fstabhandling.cpp:249
> +} else {
> +// add a trailing slash
> +deviceName.append(QLatin1Char('/'));

dito

REPOSITORY
  R245 Solid

BRANCH
  arcpatch-D21204

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

To: meven, bruns, #frameworks, ngraham
Cc: broulik, dhaumann, anthonyfieroni, ngraham, bruns, apol, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh

D25934: [xattr] Fix passing negative size to QByteArray

[Stefan Brüns]

bruns added a comment.


  Duplicate of D25414 

REPOSITORY
  R286 KFileMetaData

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

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

D25414: xattr: fix crash on dangling symlinks

[Stefan Brüns]

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

REPOSITORY
  R286 KFileMetaData

BRANCH
  fix_symlink

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

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

D25516: Remove unrequired private declarations for taglibextractor

[Stefan Brüns]

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

REPOSITORY
  R286 KFileMetaData

BRANCH
  cleanupTagLib

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

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

D25934: [xattr] Fix passing negative size to QByteArray

[Anthony Fieroni]

anthonyfieroni created this revision.
anthonyfieroni added reviewers: bruns, astippich.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
anthonyfieroni requested review of this revision.

REVISION SUMMARY
  CCBUG: 414227

TEST PLAN
  Not tested

REPOSITORY
  R286 KFileMetaData

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

AFFECTED FILES
  src/xattr_p.h

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

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

[Alexander Saoutkin]

feverfew added inline comments.

INLINE COMMENTS

> dfaure wrote in desktopexecparser.cpp:323
> Could we have a kill switch for KIOFuse if it fails to work properly in some 
> release?
> 
> E.g. some `export KIO_FUSE=0` or `export KIO_FUSE_DISABLE=1`.
> Or a KConfig entry (use `KSharedConfig::openConfig()->group("KIO")` for best 
> performance and flexibility -- then it can be disabled for one calling 
> application or for all)

One can always remove/uninstall kiofuse if it doesn't work or if they don't 
really want the feature?

> dfaure wrote in desktopexecparser.cpp:331
> The comment says "in case there is a password", the code doesn't.
> Probably historical, please fix :)
> 
> On this topic I saw earlier comments in the merge request which I found 
> confusing.
> It's very rare for an application to have the password stored inside the URL 
> itself. That would mean the user typing it in clear in a location bar or in a 
> terminal, bad idea. Instead, the user typically types a URL like 
> ftp://user@host/ and the kioslave asks for the password, and stores it in 
> kpasswdserver.
> 
> Also you wrote "we strip out the stuff that's in userInfo()". (where 
> QUrl::userInfo is username+password). But surely, while we might strip out 
> passwords for security reasons, we never strip out the username, do we?

From what others have tested `userInfo()` is always empty even when the URL is 
actually a protected Samba share... This means I can't know if a certain URL is 
likely to have a password or not, I just have to assume it does. I'll check 
again just in case, as indeed it does seem odd, but that was my conclusion.

> broulik wrote in desktopexecparser.cpp:356
> Blocking IO is worse than blocking DBus, still not nice, especially given the 
> "job"-like nature of `KRun` I would expect it to be fully async.

In these methods I have to return the URLs within the function, so blocking is 
inevitable. If I could return the URLs via a signal then blocking could be 
avoided.

REPOSITORY
  R241 KIO

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

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

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

[Alexander Saoutkin]

feverfew added a comment.


  In D23384#572363 , @dfaure wrote:
  
  > Why does KRun duplicate all of the (new) code from DesktopExecParser, when 
DesktopExecParser is actually a helper class for KRun?
  >  I would expect it to have solved all this already, unless I'm missing 
something about the various code paths.
  >  The code you changed in krun.cpp was supposed to simply resolve 
desktop:/foo to file:///bleh but everything else about %u/%f is done by 
DesktopExecParser.
  
  
  Using git grep it seems to me that `resolveURLs` is called in 
`KRun::runApplicationImpl` and `KRun::runService` but 
`KIO::DesktopExecParser::resultingArguments` isn't called in those two 
functions. If URL conversion isn't necessary in those functions then I'll 
happily get rid of the code.

REPOSITORY
  R241 KIO

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

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

D25902: Fix decrement index and not iterator as discussed with David

[Laurent Montel]

This revision was automatically updated to reflect the committed changes.
Closed by commit R320:6f1b8a6c61d8: Fix decrement index and not iterator as 
discussed with David (authored by mlaurent).

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25902?vs=71322&id=71392

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

AFFECTED FILES
  network/kded/kioslavenotifier.cpp

To: mlaurent, dfaure, anthonyfieroni
Cc: anthonyfieroni, 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

D25699: [PC3 ToolButton] Have the label take into account complementary color schemes

[Fabian Vogt]

fvogt added a comment.


  This fixed the button label, but the menu itself is unsuable due to a black 
text on dark background: 
https://openqa.opensuse.org/tests/1110939#step/start_wayland_plasma5/21

REPOSITORY
  R242 Plasma Framework (Library)

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

To: filipf, #plasma, ngraham, davidedmundson
Cc: mart, davidedmundson, fvogt, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham, bruns

D25902: Fix decrement index and not iterator as discussed with David

[David Faure]

dfaure accepted this revision.
dfaure added a comment.


  We can thank Qt for deprecating QHash::iterator::operator-- which allowed 
Laurent to catch this :-)

REPOSITORY
  R320 KIO Extras

BRANCH
  fix_kioslave_notifier (branched from master)

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

To: mlaurent, dfaure, anthonyfieroni
Cc: anthonyfieroni, 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

D25902: Fix decrement index and not iterator as discussed with David

[Anthony Fieroni]

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

REPOSITORY
  R320 KIO Extras

BRANCH
  fix_kioslave_notifier (branched from master)

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

To: mlaurent, dfaure, anthonyfieroni
Cc: anthonyfieroni, 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

D25902: Fix decrement index and not iterator as discussed with David

[Laurent Montel]

mlaurent added a comment.


  Ping ?:)

REPOSITORY
  R320 KIO Extras

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

To: mlaurent, dfaure
Cc: anthonyfieroni, 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

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

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> broulik wrote in ksortfilterproxymodel.h:44
> Did you try that `QRegularExpression` vs `QRegExp` thing?

We did some research

- Old Qt which we support doesn't convert JS to QRegularExpresssion
- We don't want to write new code with QRegExp
- I don't want to expose two properties and have QML doing ifdefs

That doesn't help draw a conclusion. Though is there any benefit to creating a 
reg exp in JS space rather than here?

> broulik wrote in ksortfilterproxymodel.h:58
> "but you can set it to null" - I think this thing needs a `RESET` method

what do we gain doing that?

You would use that with
filterRowCallback = undefined;

which is the same as now.

I think RESET is only usable if you can't represent null in the passed data 
type.

> broulik wrote in ksortfilterproxymodel.h:118
> Shouldn't this be an `int` and then you can drop all of that role names stuff?

roleNames are always exported. Some people expose the roles as an enum, but not 
that many

I certainly don't want to encourage magic numbers being used in QML.

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

D25296: [RFC] Fix Display Configuration icon margins

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:5172b920f9e1: [RFC] Fix Display Configuration icon 
margins (authored by ngraham).

REPOSITORY
  R242 Plasma Framework (Library)

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

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

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

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

D25924: fix static linking on e.g. macOS

[Christoph Cullmann]

cullmann abandoned this revision.
cullmann added a comment.


  Let's just close that.
  If I have enough time somewhen, will report a Qt bug (or try .1)

REPOSITORY
  R216 Syntax Highlighting

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

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

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

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> ksortfilterproxymodel.h:2
> +/*
> + *   Copyright 2010 by Marco MArtin 
> +

*Martin

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

Did you try that `QRegularExpression` vs `QRegExp` thing?

> ksortfilterproxymodel.h:58
> + * that uses filterRegExp or filterString; while filterCallback is set 
> those two properties are
> + * ignored. Attempts to write a non-callable to this property are 
> silently ignored, but you can set
> + * it to null.

"but you can set it to null" - I think this thing needs a `RESET` method

> ksortfilterproxymodel.h:80
> + */
> +Q_PROPERTY(QString filterRole READ filterRole WRITE setFilterRole)
> +

needs `NOTIFY` signals, as do the ones below

> ksortfilterproxymodel.h:99
> +explicit KSortFilterProxyModel(QObject *parent = nullptr);
> +~KSortFilterProxyModel();
> +

`override`?

> ksortfilterproxymodel.h:118
> +
> +void setSortRole(const QString &role);
> +QString sortRole() const;

Shouldn't this be an `int` and then you can drop all of that role names stuff?

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

D25296: [RFC] Fix Display Configuration icon margins

[Noah Davis]

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

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  fix-xrandr-icon (branched from master)

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

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[Nathaniel Graham]

ngraham added a comment.


  Yeah, I guess my idea precludes the user forcing a specific color scheme that 
happens to be equal to the current system default color scheme.
  
  If that's something we think we should support, then feel free to ignore me.

REPOSITORY
  R265 KConfigWidgets

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

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

D25924: fix static linking on e.g. macOS

[Christoph Cullmann]

cullmann added a comment.


  If you think that is just a Qt issue, we can close this again =)
  In-house we will just patch this :-0 Debugging the static linking in Qt is 
too much work.

REPOSITORY
  R216 Syntax Highlighting

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

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

D25927: [PC3] Complete plasma progress bar animation

[David Edmundson]

davidedmundson added a reviewer: filipf.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

D25926: [PC3] Only show progress bar indicator when the ends won't overlap

[David Edmundson]

davidedmundson added a reviewer: filipf.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

D25927: [PC3] Complete plasma progress bar animation

[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
  Otherwside if we toggle indeterminate the bar resizes to 50% of the view
  and X stops on the current value of the animation. Resulting in
  something that looks super odd.

TEST PLAN
  Plasma Components manual uit test looks sane

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

AFFECTED FILES
  src/declarativeimports/plasmacomponents3/ProgressBar.qml

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

D25926: [PC3] Only show progress bar indicator when the ends won't overlap

[David Edmundson]

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

REVISION SUMMARY
  If the width is smaller than LeftMargin + Right Margin the two ends
  overlap and it looks super weird

TEST PLAN
  Progressbar manual unit test looks sane

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

AFFECTED FILES
  src/declarativeimports/plasmacomponents3/ProgressBar.qml

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

D25924: fix static linking on e.g. macOS

[Christoph Cullmann]

cullmann added a comment.


  Therefore I made this review request ;)
  For me, for Qt 5.14 static linked, it doesn't seem to propagate.
  I am not sure why that worked before thought.

REPOSITORY
  R216 Syntax Highlighting

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

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

D25924: fix static linking on e.g. macOS

[Volker Krause]

vkrause added a comment.


  This looks wrong though, QtNetwork is not used in the public interface, is 
it? And the cmake config files are supposed to propagate private link libraries 
in case of static linking too I think, otherwise LINK_PRIVATE could basically 
never work, no?

REPOSITORY
  R216 Syntax Highlighting

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

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

D25924: fix static linking on e.g. macOS

[Christoph Cullmann]

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

REVISION SUMMARY
  Before, linking fails for me here (static Qt)

TEST PLAN
  make && make test

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

AFFECTED FILES
  src/lib/CMakeLists.txt

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[David Redondo]

davidre added a comment.


  Also if then the color scheme is changed while the application is running it 
would display "Breeze (system default)" checked, breeze dark is unchecked but 
the app is dark because breeze dark is the global color scheme.
  
  I think it should be possible to use KConfigWatcher to monitor the global 
theme.

REPOSITORY
  R265 KConfigWidgets

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

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[David Redondo]

davidre added a comment.


  But isn't that going away from the problem that this is trying to solve? To 
have an option to go back from a fixed colorscheme to the system one.
  Also I'm sure it can be done I just need to figure out how stuff works :D

REPOSITORY
  R265 KConfigWidgets

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

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[Nathaniel Graham]

ngraham added a comment.


  Then maybe we could even do it like so:
  
  Instead of having a separate "use system default" option, we could make the 
color scheme that has the same name as the default into the "use system default 
option, and present it thusly in the menu:
  
( ) Arc
( ) Arc Dark
(o) Breeze (System default)
( ) Breeze Dark

REPOSITORY
  R265 KConfigWidgets

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

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

D25922: Don't use a verified nullptr as a data source

[Dan Leinir Turthra Jensen]

leinir added reviewers: Frameworks, KNewStuff, Plasma, ronaldv.

REPOSITORY
  R235 Attica

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

To: leinir, #frameworks, #knewstuff, #plasma, ronaldv
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25922: Don't use a verified nullptr as a data source

[Dan Leinir Turthra Jensen]

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

REVISION SUMMARY
  Actually use some variables with usable information in version.h
  
  Don't de-reference what will at this point definitely be a null ptr
  
  Also use the information from version.h to perform the same task,
  now that that information is, in fact, useful

REPOSITORY
  R235 Attica

BRANCH
  dont-use-null-qcoreapp-instance (branched from master)

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

AFFECTED FILES
  src/provider.cpp
  src/version.h.cmake

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[David Redondo]

davidre added a comment.


  In D25877#576205 , @ngraham wrote:
  
  > Is there actually a way for this to see the name of the system color scheme?
  
  
  I think so. We can certainly see the name of the system color scheme at 
startup. The tricky thing is when it changes but I will try to do it.

REPOSITORY
  R265 KConfigWidgets

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

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

D25918: Consistently define the tooltip delay

[Nathaniel Graham]

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

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  coco

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

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[Nathaniel Graham]

ngraham added a comment.


  Is there actually a way for this to see the name of the system color scheme?

REPOSITORY
  R265 KConfigWidgets

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

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[David Redondo]

davidre added a task: T12147: KConfigWidgets.

REPOSITORY
  R265 KConfigWidgets

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

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[David Redondo]

davidre updated this revision to Diff 71355.
davidre added a comment.


  Really fix typo

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25877?vs=71354&id=71355

BRANCH
  systemthem (branched from master)

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

AFFECTED FILES
  src/kcolorschememanager.cpp
  src/kcolorschememanager.h

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[David Redondo]

davidre updated this revision to Diff 71354.
davidre added a comment.


  Typo

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25877?vs=71353&id=71354

BRANCH
  systemthem (branched from master)

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

AFFECTED FILES
  src/kcolorschememanager.cpp
  src/kcolorschememanager.h

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[David Redondo]

davidre updated this revision to Diff 71353.
davidre added a comment.


  Fix things, new api + documentation

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25877?vs=71347&id=71353

BRANCH
  systemthem (branched from master)

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

AFFECTED FILES
  src/kcolorschememanager.cpp
  src/kcolorschememanager.h

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

D25918: Consistently define the tooltip delay

[Aleix Pol Gonzalez]

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

REVISION SUMMARY
  Set the right value on the Kirigami.Units and use it from tooltip.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  coco

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

AFFECTED FILES
  src/declarativeimports/kirigamiplasmastyle/Units.qml
  src/declarativeimports/plasmacomponents3/ToolTip.qml

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[David Redondo]

davidre updated this revision to Diff 71347.
davidre added a comment.


  Update

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25877?vs=71261&id=71347

BRANCH
  systemthem (branched from master)

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

AFFECTED FILES
  src/kcolorschememanager.cpp

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

D25296: [RFC] Fix Display Configuration icon margins

[Nathaniel Graham]

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

REPOSITORY
  R242 Plasma Framework (Library)

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

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

D25296: [RFC] Fix Display Configuration icon margins

[Nathaniel Graham]

ngraham updated this revision to Diff 71346.
ngraham added a comment.


  Finally fix this thing

REPOSITORY
  R242 Plasma Framework (Library)

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

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: trickyricky26, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D25914: Remove ColorSchemeFilter from KStyle

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R252:9fc5bade9c26: Remove ColorSchemeFilter from KStyle 
(authored by davidre).

REPOSITORY
  R252 Framework Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25914?vs=71342&id=71344

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

AFFECTED FILES
  src/kstyle/kstyle.cpp

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

D25875: [KColorschemeManager] Prevent changes to the palette if a custom one is set

[David Redondo]

davidre abandoned this revision.
davidre added a comment.


  Abandoned in favor of D25915 

REPOSITORY
  R265 KConfigWidgets

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

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

D25914: Remove ColorSchemeFilter from KStyle

[David Edmundson]

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

REPOSITORY
  R252 Framework Integration

BRANCH
  remove (branched from master)

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

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

D25914: Remove ColorSchemeFilter from KStyle

[David Redondo]

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

REVISION SUMMARY
  The code is duplciated in plasma-integration. According to the summary of
  e47245f96ed1f7decc7209eafe969889579b1c38 
 
when it was added to plasma-integration
  it could/should have been removed.

TEST PLAN
  When changing color scheme the titlebar color still changes

REPOSITORY
  R252 Framework Integration

BRANCH
  remove (branched from master)

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

AFFECTED FILES
  src/kstyle/kstyle.cpp

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[David Redondo]

davidre edited the test plan for this revision.

REPOSITORY
  R265 KConfigWidgets

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

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

D25877: [KColorschemeManager] Add option to reenable following global theme

[David Redondo]

davidre planned changes to this revision.
davidre added a comment.


  Make it a standalone diff

REPOSITORY
  R265 KConfigWidgets

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

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

D25875: [KColorschemeManager] Prevent changes to the palette if a custom one is set

[David Redondo]

davidre edited the test plan for this revision.

REPOSITORY
  R265 KConfigWidgets

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

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

D25875: [KColorschemeManager] Prevent changes to the palette if a custom one is set

[David Redondo]

davidre added a comment.


  Maybe a better place to do this would be plasma-integration?

REPOSITORY
  R265 KConfigWidgets

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

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

D25897: Add 32, 48 and 64 px user-desktop icons

[Nathaniel Graham]

ngraham added a comment.


  The straight lines in the system tray items from my screenshot are definitely 
1px strokes. But all the curved lines look thicker by virtue of not being able 
to align to the pixel grid. And most monochrome widgets have thicker lines when 
added to a tall panel:
  
  F7813795: Screenshot_20191212_101746.png 

  
  This may be a side effect of scaling up smaller icons, but I think the 
thicker strokes work well for a large panel.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

D25149: Add a new template for KCMs

[Kevin Ottens]

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

INLINE COMMENTS

> %{APPNAMELC}settings.kcfg:8
> +  
> +
> +  The Exam

Now that I think about it, what about using both key and name here? To have a 
nice property name and a "stranger" storage key?
Most people seem to miss that feature somehow, that'd make it more obvious.

> Messages.sh:3
> +$EXTRACTRC `find . -name "*.ui"` >> rc.cpp || exit 11
> +$XGETTEXT `find . -name "*.cpp"` -o $podir/kcm5_%{APPNAMELC}.pot
> +rm -f rc.cpp

I'm talking mostly out of ignorance here, but shouldn't that harvest also the 
qml files somehow?

> kcm.cpp:30
> +
> +%{APPNAME}::%{APPNAME}(QObject* parent, const QVariantList& args)
> +: KQuickAddons::ManagedConfigModule(parent, args)

Ditto

> kcm.cpp:36
> +
> +KAboutData* about = new KAboutData(
> +QStringLiteral("kcm_%{APPNAMELC}"), i18n("%{APPNAME} Configuration 
> Module"),

Ditto

Beside that could use auto

> kcm.h:32
> +public:
> +%{APPNAME}(QObject* parent, const QVariantList& args);
> +virtual ~%{APPNAME}() override;

Nitpick: space should be before * and & not after.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: tcanabrava, #plasma, #frameworks, mart, ervin
Cc: #plasma, GB_2, yurchor, davidedmundson, ognarb, ervin, 
kde-frameworks-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
ragreen, michaelh, ZrenBot, ngraham, bruns, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart