D15367: [Comic Thumbnailer] fix CBR thumbnail generation

[Florian Léger]

fleger added a comment.


  Please use florian.leg...@gmail.com

REPOSITORY
  R320 KIO Extras

BRANCH
  comicbook-fix-cbr

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

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

D17342: team-port setting

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> teamportsettingtest.cpp:29
> +
> +#if !NM_CHECK_VERSION(1, 12, 0)
> +#define NM_SETTING_TEAM_PORT_CONFIG"config"

Those defines are already in NM 1.10

> teamportsettingtest.cpp:62
> +<< (qint32)1 // queueId
> +<< false // sticky
> +<< linkWatchers; // linkWatchers

set it to TRUE, otherwise it will fail once you fix the test

> teamportsettingtest.cpp:92
> +
> +QVariantMap::const_iterator it = map.constBegin();
> +NMVariantMapList list = it.value().value();

This is weird, this way only link-watchers should be compared, the rest should 
be identical to other unit tests.

> setting.cpp:34
>  
> +#if !NM_CHECK_VERSION(1, 12, 0)
> +#define NM_SETTING_TEAM_PORT_SETTING_NAME  "team-port"

NM 1.10.0

> teamportsetting.cpp:26
> +
> +#if !NM_CHECK_VERSION(1, 12, 0)
> +#define NM_SETTING_TEAM_PORT_SETTING_NAME "team-port"

NM 1.10.0

> teamportsetting.cpp:210
> +
> +if (lacpKey() >= 0) {
> +setting.insert(QLatin1String(NM_SETTING_TEAM_PORT_LACP_KEY), 
> lacpKey());

> 0

> teamportsetting.cpp:214
> +
> +if (lacpPrio() >= 0) {
> +setting.insert(QLatin1String(NM_SETTING_TEAM_PORT_LACP_PRIO), 
> lacpPrio());

!= 255

> teamportsetting.cpp:218
> +
> +if (prio() >= 0) {
> +setting.insert(QLatin1String(NM_SETTING_TEAM_PORT_PRIO), prio());

> 0

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16671: Refactor embedded image extractor for greater extensibility

[Alexander Stippich]

astippich marked 2 inline comments as done.
astippich added inline comments.

INLINE COMMENTS

> bruns wrote in embeddedimagedata.cpp:134
> Does Taglib signal an error when it fails to parse the file? Or is calling 
> pictureList() always safe?

Docs don't say, so I fed a musepack file into this codepath and it still ran 
without issues (no cover extracted of course). Good enough?

> bruns wrote in embeddedimagedata.cpp:148
> Not sure if this is an issue, but the old code only called 
> `tag()->pictureList()` after `!tag()->isEmpty()` - is this safe without?
> Dito for oggFile above.

Yes, the list will be empty.

REPOSITORY
  R286 KFileMetaData

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

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

D16671: Refactor embedded image extractor for greater extensibility

[Alexander Stippich]

astippich updated this revision to Diff 46880.
astippich added a comment.


  - new lines, handle dataPosition == -1

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16671?vs=46860=46880

BRANCH
  refactor_embedded_image

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

AFFECTED FILES
  src/embeddedimagedata.cpp

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

KDE CI: Frameworks » kcrash » kf5-qt5 WindowsMSVCQt5.11 - Build # 16 - Aborted!

[CI System]

BUILD ABORTED
 Build URL
https://build.kde.org/job/Frameworks/job/kcrash/job/kf5-qt5%20WindowsMSVCQt5.11/16/
 Project:
kf5-qt5 WindowsMSVCQt5.11
 Date of build:
Sun, 02 Dec 2018 06:21:04 +
 Build duration:
3 days 0 hr and counting

D17350: Add the missing api for multilevel KCMs to control the columns

[Jan Grulich]

jgrulich added a comment.


  @mart , can you push your changes for plasma-nm so I can try this?

REPOSITORY
  R296 KDeclarative

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

To: mart, #plasma, jgrulich
Cc: abetts, ngraham, kde-frameworks-devel, michaelh, bruns

D17350: Add the missing api for multilevel KCMs to control the columns

[Jan Grulich]

jgrulich added a comment.


  In D17350#371191 , @ngraham wrote:
  
  > That plasma-nm QML rewrite is looking totally awesome! How soon can we get 
it committed?
  
  
  I unfortunately didn't have time to work on that since Akademy so there is no 
progress and lots of work ahead. I was thinking of working on that during 
Christmas break, to move it forward a bit, but my best guess would be to have 
it ready for Plasma 5.16, definitely not 5.15.

REPOSITORY
  R296 KDeclarative

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

To: mart, #plasma, jgrulich
Cc: abetts, ngraham, kde-frameworks-devel, michaelh, bruns

D17351: make use of the new currentIndex api of QML kcms

[Jan Grulich]

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


  Looks good to me.

REPOSITORY
  R295 KCMUtils

BRANCH
  phab/missingapi

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

To: mart, #plasma, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

Re: Time formats / LC_TIME challenge for 4-digit year support

[Thiago Macieira]

On Tuesday, 4 December 2018 11:42:41 PST Jaroslaw Staniek wrote:
> I did not know there are such differences for the locale system. But by no
> means my email is a call to "patch" the Qt-level problem by having a KF5
> solution. And I am skeptical if Qt can be patched in version 5 with such
> incompatible change. Hopefully these poor man analysis help some other app
> developers 

If you want that, please submit your patch to unicode.org. They need to update 
the CLDR to have 4-digit years for the locale of your choice.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.10 - Build # 512 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/512/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Wed, 05 Dec 2018 04:42:40 +
 Build duration:
23 min and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 59 test(s), Skipped: 0 test(s), Total: 61 test(s)Failed: TestSuite.kiofilewidgets-knewfilemenutestFailed: TestSuite.kiogui-favicontest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(262/398)66%
(262/398)53%
(31957/60116)37%
(16497/44086)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(57/57)100%
(57/57)95%
(9091/9538)48%
(4281/8965)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)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.core86%
(100/116)86%
(100/116)58%
(8345/14337)50%
(4666/9259)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3908/7971)34%
(1603/4743)src.gui100%
(2/2)100%
(2/2)88%
(95/108)68%
(45/66)src.ioslaves.file100%
(5/5)100%
(5/5)51%
(532/1036)37%
(319/868)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1344)0%
(0/1416)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/248)0%
(0/148)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1765/4288)35%
(1306/3692)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1330)55%
(626/1135)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(73/267)8%
(14/184)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/32)src.ioslaves.trash56%
(5/9)56%
  

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.9 - Build # 361 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.9/361/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Wed, 05 Dec 2018 04:42:40 +
 Build duration:
8 min 12 sec and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 59 test(s), Skipped: 0 test(s), Total: 61 test(s)Failed: TestSuite.kiofilewidgets-knewfilemenutestFailed: TestSuite.kiogui-favicontest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(262/398)66%
(262/398)53%
(31984/60117)37%
(16515/44086)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(57/57)100%
(57/57)95%
(9091/9538)48%
(4280/8965)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)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.core86%
(100/116)86%
(100/116)58%
(8379/14338)51%
(4689/9263)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3909/7971)34%
(1604/4743)src.gui100%
(2/2)100%
(2/2)88%
(95/108)68%
(45/66)src.ioslaves.file100%
(5/5)100%
(5/5)51%
(531/1036)37%
(318/868)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1344)0%
(0/1416)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/248)0%
(0/148)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1770/4288)35%
(1304/3692)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(629/1330)55%
(625/1135)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(73/267)8%
(14/184)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/32)src.ioslaves.trash56%
(5/9)56%
  

D17301: add documentation to result class

[Stefan Brüns]

bruns added a comment.


  In D17301#371323 , @astippich 
wrote:
  
  > Thanks for your patience :)
  
  
  Thanks for bearing with my nagging ;-)

REPOSITORY
  R293 Baloo

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

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

D17358: implement cover extraction for asf files

[Stefan Brüns]

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

INLINE COMMENTS

> embeddedimagedata.cpp:273
>  }
> +QByteArray
> +EmbeddedImageData::Private::getFrontCoverFromAsf(TagLib::ASF::Tag* asfTags) 
> const

Nitpick - empty line between functions

REPOSITORY
  R286 KFileMetaData

BRANCH
  wma_cover

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

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

D16671: Refactor embedded image extractor for greater extensibility

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> embeddedimagedata.cpp:181
>  }
> +QByteArray
> +EmbeddedImageData::Private::getFrontCoverFromMp4(TagLib::MP4::Tag* mp4Tags) 
> const

Nitpick - add an empty line in-between functions

REPOSITORY
  R286 KFileMetaData

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

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

D17357: extend list of supported mimetypes for embedded image extractor

[Stefan Brüns]

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


  Can you mention the file types and their corresponding mimetypes in the 
Summary? Thx.

INLINE COMMENTS

> embeddedimagedata.cpp:199
> +if (speexFile.tag()) {
> +return 
> getFrontCoverFromFlacPicture(speexFile.tag()->pictureList());
> +}

Comment about tag->isEmpty() from D16671  
applies here as well

REPOSITORY
  R286 KFileMetaData

BRANCH
  image_extend

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

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

D16671: Refactor embedded image extractor for greater extensibility

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> embeddedimagedata.cpp:134
>  TagLib::FLAC::File flacFile(, 
> TagLib::ID3v2::FrameFactory::instance(), true);
> -TagLib::List lstPic = 
> flacFile.pictureList();
> -
> -if (!lstPic.isEmpty()) {
> -for (TagLib::List::Iterator it = 
> lstPic.begin(); it != lstPic.end(); ++it) {
> -TagLib::FLAC::Picture *picture = *it;
> -if (picture->type() == picture->FrontCover) {
> -return QByteArray(picture->data().data(), 
> picture->data().size());
> -}
> -}
> -}
> +return getFrontCoverFromFlacPicture(flacFile.pictureList());
> +

Does Taglib signal an error when it fails to parse the file? Or is calling 
pictureList() always safe?

> embeddedimagedata.cpp:148
> +TagLib::Ogg::Opus::File opusFile(, true);
> +if (opusFile.tag()) {
> +return 
> getFrontCoverFromFlacPicture(opusFile.tag()->pictureList());

Not sure if this is an issue, but the old code only called 
`tag()->pictureList()` after `!tag()->isEmpty()` - is this safe without?
Dito for oggFile above.

> astippich wrote in embeddedimagedata.cpp:204
> The code also works if only the picture data is there without the name.
> I could return an empty QByteArray if dataPosition == -1 and pictureData.size 
> == 0.
> Do you prefer if I do that separately?

Is it safe to call find on an empty ByteVector - most likely yes ...
Then return QByteArray on dataPosition == -1, so it is obvious the error case 
is handled, and keep the rest as is.

REPOSITORY
  R286 KFileMetaData

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

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

D17355: Align Plasma QML button content in center if it has an icon

[David Edmundson]

davidedmundson added a comment.


  Code wise:
  
  - Any changes to PlasmaStyle need to happen in the QQC2 (PlasmaComponents3) 
version too before shipping
  - Please be sure to run qmlscene tests/components/button.qml
  - Why do we have nested RowLayouts?
  
  
  
  > If we changed that so that buttons could be narrower when the icon + label 
is small, than we probably wouldn't need to horizontally center anything.
  
  That's do-able from application space.
  
  Button {
  
implicitWidth: minimumWidth
  
  }
  
  If you want that, there's no need to change this

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: davidedmundson, apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, 
IohannesPetros, trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, 
ZrenBot, firef, bruns, skadinna, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, mbohlender, mart

D17355: Align Plasma QML button content in center if it has an icon

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> ButtonStyle.qml:41
>  label: RowLayout {
> -spacing: units.smallSpacing
> +RowLayout {
> +Layout.alignment: Qt.AlignHCenter | Qt.AlignVCenter

This looks wrong, a RowLayout inside another one?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: apol, ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, IohannesPetros, 
trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, ZrenBot, firef, bruns, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, 
sebas, mbohlender, mart

D17324: Move internal helper from BasicIndexingJob to anonymous namespace

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:2c8d35247cae: Move internal helper from BasicIndexingJob 
to anonymous namespace (authored by bruns).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17324?vs=46758=46876

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

AFFECTED FILES
  src/file/basicindexingjob.cpp
  src/file/basicindexingjob.h

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

Re: [QUESTION] KIO slave-socket shortcut - does it exist?

[Albert Astals Cid]

El dimarts, 4 de desembre de 2018, a les 16:37:34 CET, Smits Katze va escriure:
> Background: I want to sandbox KDE apps and need to understand better how
> KIO works.
> 
> My current level of understanding is that apps ask klauncher/kdeinit for a
> KIO slave if they need one. Then either kdeinit spawns a new slave process,
> or there is already an idle slave and it is reused. Idle slaves kill
> themselves after a couple of minutes if no request is coming in.
> Communication between the slave and the app happens via a socket, usually
> to find in /run/user/$UID.
> 
> The question is if, or rather when, it is possible to shortcut this
> process. Do slaves, especially idle ones, accept commands issued by third
> programs via these sockets? Try to take the perspective of an evil-minded
> adversary.

Do you mean this as a security issue?

Albert

> 
> Thank you very much!
> 

Re: [QUESTION] KIO slave-socket shortcut - does it exist?

[Elvis Angelaccio]

On 04/12/18 16:37, Smits Katze wrote:
> Background: I want to sandbox KDE apps and need to understand better how
> KIO works.
> 
> My current level of understanding is that apps ask klauncher/kdeinit for
> a KIO slave if they need one. Then either kdeinit spawns a new slave
> process, or there is already an idle slave and it is reused. Idle slaves
> kill themselves after a couple of minutes if no request is coming in.
> Communication between the slave and the app happens via a socket,
> usually to find in /run/user/$UID.
> 
> The question is if, or rather when, it is possible to shortcut this
> process. 

You can bypass klauncher/kdeinit by exporting the KDE_FORK_SLAVES
environment variable set to 1. Then the applications will spawn the
ioslave process on their own.

Not sure if this actually helps you, though.

> Do slaves, especially idle ones, accept commands issued by
> third programs via these sockets? Try to take the perspective of an
> evil-minded adversary.
> 
> Thank you very much!

Cheers,
Elvis

D17324: Move internal helper from BasicIndexingJob to anonymous namespace

[Alexander Stippich]

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

REPOSITORY
  R293 Baloo

BRANCH
  submit

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

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

D17358: implement cover extraction for asf files

[Alexander Stippich]

astippich updated this revision to Diff 46868.
astippich added a comment.


  - fix

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17358?vs=46863=46868

BRANCH
  wma_cover

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

AFFECTED FILES
  autotests/embeddedimagedatatest.cpp
  autotests/samplefiles/test.wma
  src/embeddedimagedata.cpp

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

D17357: extend list of supported mimetypes for embedded image extractor

[Alexander Stippich]

astippich added a dependent revision: D17358: implement cover extraction for 
asf files.

REPOSITORY
  R286 KFileMetaData

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

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

D17358: implement cover extraction for asf files

[Alexander Stippich]

astippich added a dependency: D17357: extend list of supported mimetypes for 
embedded image extractor.

REPOSITORY
  R286 KFileMetaData

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

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

D16671: Refactor embedded image extractor for greater extensibility

[Alexander Stippich]

astippich added a dependent revision: D17357: extend list of supported 
mimetypes for embedded image extractor.

REPOSITORY
  R286 KFileMetaData

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

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

D17357: extend list of supported mimetypes for embedded image extractor

[Alexander Stippich]

astippich added a dependency: D16671: Refactor embedded image extractor for 
greater extensibility.

REPOSITORY
  R286 KFileMetaData

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

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

Re: Time formats / LC_TIME challenge for 4-digit year support

[Jaroslaw Staniek]

On Tue, 4 Dec 2018 at 20:10, Albert Astals Cid  wrote:

> El dilluns, 3 de desembre de 2018, a les 14:47:56 CET, Jaroslaw Staniek va
> escriure:
> > Hello,
> > The need: 4-digit year support for short date formats to avoid issues
> like
> > "10/11/12" dates.
>
> What's wrong with 10/11/12 ?
>

It's a problem. Year 2000 problem.

>
> > This is just my initial conclusion from for the Qt date format issue:
> >
> >
> https://bugreports.qt.io/browse/QTBUG-59382?focusedCommentId=436861=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-436861
> > lt;dr: KF5 context: KCallendarSystem is part of the old APIs and the only
> > code I found related to LC_TIME.
> >
> > So I've not found any common code KDE app can use instead of hard coding
> > some internal logic in order to support non-Plasma environments and/or
> > non-global settings. Basically even if an app implements locale name
> > selection in its private settings, "pure" Qt code is not enough to
> support
> > it in the discussed cases. Users may need alter date format globally (for
> > all apps) by hand using environment's settings, setting e.g. -mm-dd.
> Or
> > app developers may add private setting for that, one per app.
> >
> > Opinions welcome.
>
> I am not sure i understand your email/problem.
>
> As far as i read this email your concern is:
>  * localized dates use the locale date format
>  * locale date format is "wrong".


Yes. "Opinions welcome" mostly meant sending "do you have this kind of
issues in your apps" question to app developers who happen to use KF5+Qt.


>
> So the two options are:
>  a) User changes their locale
>
 b) Apps don't use localized dates and use a custom format.
>
> Am I understanding your email correctly?
>

Yes, I discussed pros and cons of a) and b).

Doing a) means the user changing global locale is responsible for finding
locale that matches format which Qt implements as expected (if not, that's
rare problem but unfortunately happened for the ISO format explained in the
Qt ticket). And possibly, finding format that won't break non-Qt apps that
may use different locale infra.
Or if the user changes the format by hand, but still does so in the system
locale settings, she's more in control as this is a global setting, not
per-app.

b) Has pros and cons but cons can be reduced by this option being opt-in
and default coming from the locale.

I did not know there are such differences for the locale system. But by no
means my email is a call to "patch" the Qt-level problem by having a KF5
solution. And I am skeptical if Qt can be patched in version 5 with such
incompatible change. Hopefully these poor man analysis help some other app
developers :)

-- 
regards, Jaroslaw Staniek

KDE:
: A world-wide network of software engineers, artists, writers, translators
: and facilitators committed to Free Software development - kde.org
KEXI:
: A visual database apps builder - kexi-project.org calligra.org/kexi
  twitter.com/kexi_project facebook.com/kexi.project t.me/kexi_project
Qt Certified Specialist:
: linkedin.com/in/jstaniek 

D17301: add documentation to result class

[Alexander Stippich]

astippich added a comment.


  Thanks for your patience :)

REPOSITORY
  R293 Baloo

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

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

D17301: add documentation to result class

[Alexander Stippich]

astippich updated this revision to Diff 46864.
astippich added a comment.


  - remove docstring for reimplemented functions

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17301?vs=46750=46864

BRANCH
  result_documentation

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

AFFECTED FILES
  src/file/extractor/result.h

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

D17355: Align Plasma QML button content in center if it has an icon

[Björn Feber]

GB_2 added a comment.


  It works properly, no matter how big the button is or if have an icon or not.
  In my opinion we should leave it how it is now.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, IohannesPetros, 
trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, ZrenBot, firef, bruns, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, 
sebas, apol, mbohlender, mart

D17358: implement cover extraction for asf files

[Alexander Stippich]

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

REVISION SUMMARY
  implement support for last missing mimetype
  compared to tagllibextractor

TEST PLAN
  test passes

REPOSITORY
  R286 KFileMetaData

BRANCH
  wma_cover

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

AFFECTED FILES
  autotests/embeddedimagedatatest.cpp
  autotests/samplefiles/test.wma
  src/embeddedimagedata.cpp

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

D17355: Align Plasma QML button content in center if it has an icon

[Björn Feber]

GB_2 updated this revision to Diff 46862.

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17355?vs=46856=46862

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

AFFECTED FILES
  src/declarativeimports/plasmastyle/ButtonStyle.qml

To: GB_2, #plasma, #vdg
Cc: ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, IohannesPetros, 
trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, ZrenBot, firef, bruns, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, 
sebas, apol, mbohlender, mart

D17357: extend list of supported mimetypes for embedded image extractor

[Alexander Stippich]

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

REVISION SUMMARY
  use existing functions to add support for new
  mimetypes

TEST PLAN
  tests pass

REPOSITORY
  R286 KFileMetaData

BRANCH
  image_extend

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

AFFECTED FILES
  autotests/embeddedimagedatatest.cpp
  autotests/samplefiles/test.spx
  src/embeddedimagedata.cpp

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

D16671: Refactor embedded image extractor for greater extensibility

[Alexander Stippich]

astippich updated this revision to Diff 46860.
astippich added a comment.


  - rebase

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16671?vs=44982=46860

BRANCH
  refactor_embedded_image

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

AFFECTED FILES
  src/embeddedimagedata.cpp

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

D17245: Add string formatting function to property info

[Alexander Stippich]

astippich marked 3 inline comments as done.
astippich added a comment.


  I'm wondering, anything else to do if I add a new dependency to KFileMetaData?

INLINE COMMENTS

> bruns wrote in propertyinfotest.cpp:63
> I think this should be "44.1 kHz". Insert the correct expected value and make 
> it QEXPECT_FAIL?

That was a localization issue.

> bruns wrote in propertyinfotest.cpp:65
> dito, "128 kb/s"

I'm going to fix this shortly after, so I don't bother

> bruns wrote in formatstrings.cpp:80
> for 180°,  CCW is irrelevant.

That was pretty stupid :)

REPOSITORY
  R286 KFileMetaData

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

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

D17245: Add string formatting function to property info

[Alexander Stippich]

astippich updated this revision to Diff 46859.
astippich added a comment.


  - small fixes

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17245?vs=46688=46859

BRANCH
  display_value

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

AFFECTED FILES
  CMakeLists.txt
  autotests/propertyinfotest.cpp
  autotests/propertyinfotest.h
  src/CMakeLists.txt
  src/formatstrings.cpp
  src/formatstrings.h
  src/propertyinfo.cpp
  src/propertyinfo.h

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

Re: Transitioning CI builds of all non-Frameworks from Qt 5.9

[Albert Astals Cid]

El dimarts, 4 de desembre de 2018, a les 20:13:43 CET, Ben Cooksley va escriure:
> On Wed, Dec 5, 2018 at 7:22 AM Albert Astals Cid  wrote:
> >
> > El dimarts, 4 de desembre de 2018, a les 18:10:44 CET, Thiago Macieira va 
> > escriure:
> > > On Monday, 3 December 2018 01:30:25 PST René J.V. Bertin wrote:
> > > > Can't you just configure the CI to use Qt 5.10? I think it's not good to
> > > > hardcode an "overzealous" (for lack of a better word) Qt version in
> > > > projects that don't require them AND I think that one should support the
> > > > current LTS release in as many projects as possible as a general rule of
> > > > principle.
> > > >
> > > > There's a reason why those LTS releases exist and that should probably 
> > > > be
> > > > taken into consideration ESPECIALLY for the KF5 Frameworks (remember why
> > > > kdelibs4 was split up)!
> > >
> > > Which is exactly why 5.11.3 (released today) should be picked. It 
> > > contains all
> > > fixes that 5.9.7 contains, whereas 5.10.1 does not. Moving from 5.9.7 to
> > > 5.10.1 means regressing all those fixes.
> >
> > It doesn't matter, apps need 5.10 to compile, so CI needs to use 5.10.
> >
> > Of course users should be using 5.11.3, but that's a different story.
> 
> So you'd prefer we move to Qt 5.10 then, and then do another move to
> 5.11 when Frameworks moves it's bar up?

Personally, yes, it's much easier to detect apps requiring 5.11 API when they 
don't pretend to (not sure there's much new api in 5.11 but anyhow). I mean in 
an ideal world we'd compile each app with their required min Qt version to 
detect those things, but since that's not possible with the current setup let's 
settle for the smallest possible version.

And Frameworks requiring 5.11 is still kind of far no?

Cheers,
  Albert

> 
> >
> > Cheers,
> >   Albert
> >
> >
> 
> Regards,
> Ben

Re: Transitioning CI builds of all non-Frameworks from Qt 5.9

[Ben Cooksley]

On Wed, Dec 5, 2018 at 7:22 AM Albert Astals Cid  wrote:
>
> El dimarts, 4 de desembre de 2018, a les 18:10:44 CET, Thiago Macieira va 
> escriure:
> > On Monday, 3 December 2018 01:30:25 PST René J.V. Bertin wrote:
> > > Can't you just configure the CI to use Qt 5.10? I think it's not good to
> > > hardcode an "overzealous" (for lack of a better word) Qt version in
> > > projects that don't require them AND I think that one should support the
> > > current LTS release in as many projects as possible as a general rule of
> > > principle.
> > >
> > > There's a reason why those LTS releases exist and that should probably be
> > > taken into consideration ESPECIALLY for the KF5 Frameworks (remember why
> > > kdelibs4 was split up)!
> >
> > Which is exactly why 5.11.3 (released today) should be picked. It contains 
> > all
> > fixes that 5.9.7 contains, whereas 5.10.1 does not. Moving from 5.9.7 to
> > 5.10.1 means regressing all those fixes.
>
> It doesn't matter, apps need 5.10 to compile, so CI needs to use 5.10.
>
> Of course users should be using 5.11.3, but that's a different story.

So you'd prefer we move to Qt 5.10 then, and then do another move to
5.11 when Frameworks moves it's bar up?

>
> Cheers,
>   Albert
>
>

Regards,
Ben

D17355: Align Plasma QML button content in center if it has an icon

[Nathaniel Graham]

ngraham added a comment.


  Cool!
  
  Does this still work properly when the text is so long that the button 
expands to accommodate it?
  
  Now that I think about this a bit, it seems like maybe the problem here is 
that there's a minimum button width in the first place. If we changed that so 
that buttons could be narrower when the icon + label is small, than we probably 
wouldn't need to horizontally center anything.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: GB_2, #plasma, #vdg
Cc: ngraham, #vdg, kde-frameworks-devel, #plasma, alexde, IohannesPetros, 
trickyricky26, ragreen, Pitel, michaelh, crozbo, ndavis, ZrenBot, firef, bruns, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, 
sebas, apol, mbohlender, mart

Re: Time formats / LC_TIME challenge for 4-digit year support

[Albert Astals Cid]

El dilluns, 3 de desembre de 2018, a les 14:47:56 CET, Jaroslaw Staniek va 
escriure:
> Hello,
> The need: 4-digit year support for short date formats to avoid issues like
> "10/11/12" dates.

What's wrong with 10/11/12 ?

> 
> This is just my initial conclusion from for the Qt date format issue:
> 
> https://bugreports.qt.io/browse/QTBUG-59382?focusedCommentId=436861=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-436861
> lt;dr: KF5 context: KCallendarSystem is part of the old APIs and the only
> code I found related to LC_TIME.
> 
> So I've not found any common code KDE app can use instead of hard coding
> some internal logic in order to support non-Plasma environments and/or
> non-global settings. Basically even if an app implements locale name
> selection in its private settings, "pure" Qt code is not enough to support
> it in the discussed cases. Users may need alter date format globally (for
> all apps) by hand using environment's settings, setting e.g. -mm-dd. Or
> app developers may add private setting for that, one per app.
> 
> Opinions welcome.

I am not sure i understand your email/problem.

As far as i read this email your concern is:
 * localized dates use the locale date format
 * locale date format is "wrong".

So the two options are:
 a) User changes their locale
 b) Apps don't use localized dates and use a custom format.

Am I understanding your email correctly?

Cheers,
  Albert

D17355: Align Plasma QML button content in center if it has an icon

[Björn Feber]

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

REVISION SUMMARY
  Aligns the Plasma QML Button content in the center if it has an icon.
  It makes this look better: https://phabricator.kde.org/D17323
  
  Before: F6455121: Plasma QML Button Content Align (before).png 

  
  After: F6455122: Plasma QML Button Content Align (after).png 


TEST PLAN
  Run this test QML file with qmlscene: F6455115: main.qml 


REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  src/declarativeimports/plasmastyle/ButtonStyle.qml

To: GB_2, #plasma, #vdg
Cc: #vdg, kde-frameworks-devel, #plasma, alexde, IohannesPetros, trickyricky26, 
ragreen, Pitel, michaelh, crozbo, ndavis, ZrenBot, firef, ngraham, bruns, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, 
sebas, apol, mbohlender, mart

Re: Transitioning CI builds of all non-Frameworks from Qt 5.9

[Ben Cooksley]

On Tue, Dec 4, 2018 at 9:45 AM Elvis Angelaccio
 wrote:
>
>
>
> On 03/12/18 09:46, Ben Cooksley wrote:
> > Hi all,
>
> Hi Ben

Hi Elvis,

>
> >
> > I've been informed by the PIM developers that they would like to bump
> > the dependency of the Qt version they use, from Qt 5.9 which it's on
> > currently, to Qt 5.10.
> >
> > As a consequence, due to many KDE projects using PIM libraries, their
> > dependency on Qt will also be effectively bumped. To minimize the
> > maintenance cost of this bump for the CI system, I would like to bump
> > everyone up from Qt 5.9 to a newer version of Qt (otherwise we'll end
> > up chasing down build failures for a long time)
>
> It's not clear if you are suggesting to bump the minimum required Qt
> version in each CMakeLists.txt file, or if you are just going to bump
> the Qt version used by the CI server.
>
> Could you please clarify? Thanks!

I'll only be bumping the version used by the CI system, the
applications themselves will be left unchanged.

>
> >
> > As most distributions have moved on from 5.10, and use Qt 5.11 now
> > (and will in many cases soon move to Qt 5.12), i'd like to suggest
> > that instead of going to Qt 5.10, we move straight to 5.11.
> >
> > Because Frameworks has a two versions prior support policy, it'll
> > remain on 5.9 for now until it's ready to move up to 5.10 (assuming
> > 5.12 is a usable Qt version)
> >
> > If nobody has any objections, i'll proceed with this change in around
> > 3 days time.
> >
> > Cheers,
> > Ben
> >
>
> Cheers,
> Elvis

Cheers,
Ben

Re: Transitioning CI builds of all non-Frameworks from Qt 5.9

[Albert Astals Cid]

El dimarts, 4 de desembre de 2018, a les 18:10:44 CET, Thiago Macieira va 
escriure:
> On Monday, 3 December 2018 01:30:25 PST René J.V. Bertin wrote:
> > Can't you just configure the CI to use Qt 5.10? I think it's not good to
> > hardcode an "overzealous" (for lack of a better word) Qt version in
> > projects that don't require them AND I think that one should support the
> > current LTS release in as many projects as possible as a general rule of
> > principle.
> > 
> > There's a reason why those LTS releases exist and that should probably be
> > taken into consideration ESPECIALLY for the KF5 Frameworks (remember why
> > kdelibs4 was split up)!
> 
> Which is exactly why 5.11.3 (released today) should be picked. It contains 
> all 
> fixes that 5.9.7 contains, whereas 5.10.1 does not. Moving from 5.9.7 to 
> 5.10.1 means regressing all those fixes.

It doesn't matter, apps need 5.10 to compile, so CI needs to use 5.10.

Of course users should be using 5.11.3, but that's a different story.

Cheers,
  Albert

D17350: Add the missing api for multilevel KCMs to control the columns

[Andres Betts]

abetts added a comment.


  Instead of connection name, would it make sense to call it, network name 
instead?

REPOSITORY
  R296 KDeclarative

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

To: mart, #plasma, jgrulich
Cc: abetts, ngraham, kde-frameworks-devel, michaelh, bruns

[QUESTION] KIO slave-socket shortcut - does it exist?

[Smits Katze]

Background: I want to sandbox KDE apps and need to understand better how
KIO works.

My current level of understanding is that apps ask klauncher/kdeinit for a
KIO slave if they need one. Then either kdeinit spawns a new slave process,
or there is already an idle slave and it is reused. Idle slaves kill
themselves after a couple of minutes if no request is coming in.
Communication between the slave and the app happens via a socket, usually
to find in /run/user/$UID.

The question is if, or rather when, it is possible to shortcut this
process. Do slaves, especially idle ones, accept commands issued by third
programs via these sockets? Try to take the perspective of an evil-minded
adversary.

Thank you very much!

Re: Transitioning CI builds of all non-Frameworks from Qt 5.9

[Thiago Macieira]

On Monday, 3 December 2018 01:30:25 PST René J.V. Bertin wrote:
> Can't you just configure the CI to use Qt 5.10? I think it's not good to
> hardcode an "overzealous" (for lack of a better word) Qt version in
> projects that don't require them AND I think that one should support the
> current LTS release in as many projects as possible as a general rule of
> principle.
> 
> There's a reason why those LTS releases exist and that should probably be
> taken into consideration ESPECIALLY for the KF5 Frameworks (remember why
> kdelibs4 was split up)!

Which is exactly why 5.11.3 (released today) should be picked. It contains all 
fixes that 5.9.7 contains, whereas 5.10.1 does not. Moving from 5.9.7 to 
5.10.1 means regressing all those fixes.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

D16954: Add find module for Google's libphonenumber

[Volker Krause]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:ebb05ecafb02: Add find module for Googles 
libphonenumber (authored by vkrause).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16954?vs=46805=46848

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

AFFECTED FILES
  find-modules/FindPhoneNumber.cmake

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

D17351: make use of the new currentIndex api of QML kcms

[Marco Martin]

mart edited the summary of this revision.

REPOSITORY
  R295 KCMUtils

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

To: mart, #plasma, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17351: make use of the new currentIndex api of QML kcms

[Marco Martin]

mart added a reviewer: jgrulich.

REPOSITORY
  R295 KCMUtils

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

To: mart, #plasma, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17350: Add the missing api for multilevel KCMs to control the columns

[Marco Martin]

mart added a reviewer: jgrulich.

REPOSITORY
  R296 KDeclarative

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

To: mart, #plasma, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17350: Add the missing api for multilevel KCMs to control the columns

[Marco Martin]

mart added a comment.


  F6454962: image.png 

REPOSITORY
  R296 KDeclarative

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

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

D17351: make use of the new currentIndex api of QML kcms

[Marco Martin]

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

REVISION SUMMARY
  sync kcm currentindex with the pagerow's

TEST PLAN
  tested with newtowrkmanager qml rewrite of the KCM

REPOSITORY
  R295 KCMUtils

BRANCH
  phab/missingapi

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

AFFECTED FILES
  src/kcmoduleqml.cpp
  src/kcmoduleqml_p.h

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

D17350: Add the missing api for multilevel KCMs to control the columns

[Marco Martin]

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

REVISION SUMMARY
  - add currentIndex and depth, which would map to the view in SystemSettings 
using a Kirigami PageRow (but not necessarly)
  
  - On multilevel kcms, Fix layout alignment issues that would prevent
  
  ScrollViewKCM and SimpleKCM when displayed one besides the other to look
  aligned with each other

TEST PLAN
  tested most current QML kcms, tested the new features extensively
  with the QML networkmanager kcm rewrite

REPOSITORY
  R296 KDeclarative

BRANCH
  phab/missingapi

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

AFFECTED FILES
  src/qmlcontrols/kcmcontrols/qml/ScrollViewKCM.qml
  src/qmlcontrols/kcmcontrols/qml/SimpleKCM.qml
  src/quickaddons/configmodule.cpp
  src/quickaddons/configmodule.h

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

D17345: Support Bluetooth batteries

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R245:b983836f05cd: Support Bluetooth batteries (authored by 
broulik).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D17345?vs=46834=46837#toc

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17345?vs=46834=46837

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

AFFECTED FILES
  src/solid/devices/backends/fakehw/fakebattery.cpp
  src/solid/devices/backends/upower/upowerbattery.cpp
  src/solid/devices/backends/upower/upowerdevice.cpp
  src/solid/devices/frontend/battery.h

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

D17345: Support Bluetooth batteries

[Bhushan Shah]

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

REPOSITORY
  R245 Solid

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

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

D17345: Support Bluetooth batteries

[Kai Uwe Broulik]

broulik added a dependent revision: D17346: Support Bluetooth batteries.

REPOSITORY
  R245 Solid

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

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

D17345: Support Bluetooth batteries

[Kai Uwe Broulik]

broulik added a reviewer: bshah.

REPOSITORY
  R245 Solid

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

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

D17345: Support Bluetooth batteries

[Kai Uwe Broulik]

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

REVISION SUMMARY
  There's currently no generic Bluetooth device type in UPower, so we get them 
of "unknown" type.
  Check for whether it comes from Bluez to determine it's a Bluetooth battery.

TEST PLAN
  Using upower and bluez git master and a patch to plasma-workspace I now have 
my headset show report its battery level.
  Without plasma-workspace patch it shows up as generic battery but at least it 
shows up
  F6454744: Screenshot_20181204_151718.png 

  "Kopfhörer" is the actual device name, not type, very creative :)

REPOSITORY
  R245 Solid

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

AFFECTED FILES
  src/solid/devices/backends/fakehw/fakebattery.cpp
  src/solid/devices/backends/upower/upowerbattery.cpp
  src/solid/devices/backends/upower/upowerdevice.cpp
  src/solid/devices/frontend/battery.h

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

D17241: WIP:Disable highlighting for lines longer than 1024 characters.

[Kåre Särs]

sars retitled this revision from "Disable highlighting for lines longer than 
1024 characters." to "WIP:Disable highlighting for lines longer than 1024 
characters.".

REPOSITORY
  R39 KTextEditor

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

To: sars, cullmann, vkrause, dhaumann, mwolff
Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D17241: Disable highlighting for lines longer than 1024 characters.

[Kåre Särs]

sars added a comment.


  I agree, we need to do something about the line length limit (that only wraps 
the lines at the limit when opening the file).
  
  The question is: do we totally remove the option and limit or only the option 
and raise the limit.
  
  You can still edit the document at 200 000 characters, but it starts to get 
sluggish. At 500 000 it takes almost a second to insert a space (on my 
computer). If we keep the limit, we probably could be one of the few editor 
that could open and read files with millions of characters on a line.
  
  If we keep the limit what would it be? 10, 65536, 5 or what?
  
  Or do we keep the option and just raise the default limit?
  
  I'm currently working on adding a notice message about the disabled 
highlighting.
  
  There also seems to be a test for the long lines that needs to be updated.

REPOSITORY
  R39 KTextEditor

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

To: sars, cullmann, vkrause, dhaumann, mwolff
Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D17342: team-port setting

[Pranav Gade]

pranavgade updated this revision to Diff 46830.
pranavgade added a comment.


  add missing indents

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17342?vs=46829=46830

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/teamportsettingtest.cpp
  autotests/settings/teamportsettingtest.h
  src/CMakeLists.txt
  src/settings/setting.cpp
  src/settings/setting.h
  src/settings/teamportsetting.cpp
  src/settings/teamportsetting.h
  src/settings/teamportsetting_p.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17342: team-port setting

[Pranav Gade]

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

REVISION SUMMARY
  Added team-port setting according to:
  https://developer.gnome.org/NetworkManager/stable/settings-team-port.html

REPOSITORY
  R282 NetworkManagerQt

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/teamportsettingtest.cpp
  autotests/settings/teamportsettingtest.h
  src/CMakeLists.txt
  src/settings/setting.cpp
  src/settings/setting.h
  src/settings/teamportsetting.cpp
  src/settings/teamportsetting.h
  src/settings/teamportsetting_p.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17241: Disable highlighting for lines longer than 1024 characters.

[Milian Wolff]

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


  what sven said, we should also remove the code to disable highlighting 
altogether when the line limit is reached, no?

INLINE COMMENTS

> katerenderer.cpp:387
>  
> +if (textLine->length() > 1024 && !selectionsOnly) {
> +return newHighlight;

put the 1024 into a constant and use it here and below such that we ensure the 
number stays in sync

also, don't we have a setting for the line length limit? shouldn't that be used 
instead here?

> katerenderer.cpp:400
> +const QVector  = 
> textLine->attributesList();
> +for (int i = 0; i < al.count(); ++i) {
> +if (al[i].length > 0 && al[i].attributeValue > 0) {

this style-change should be submitted independently of this code review

REPOSITORY
  R39 KTextEditor

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

To: sars, cullmann, vkrause, dhaumann, mwolff
Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D17317: match and tc setting

[Jan Grulich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R282:e95bb9bdf98c: match and tc setting (authored by jgrulich).

REPOSITORY
  R282 NetworkManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17317?vs=46826=46827

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/matchsettingtest.cpp
  autotests/settings/matchsettingtest.h
  autotests/settings/tcsettingtest.cpp
  autotests/settings/tcsettingtest.h
  src/CMakeLists.txt
  src/settings/matchsetting.cpp
  src/settings/matchsetting.h
  src/settings/matchsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h
  src/settings/tcsetting.cpp
  src/settings/tcsetting.h
  src/settings/tcsetting_p.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

[Jan Grulich]

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

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

[Pranav Gade]

pranavgade updated this revision to Diff 46826.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17317?vs=46825=46826

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/matchsettingtest.cpp
  autotests/settings/matchsettingtest.h
  autotests/settings/tcsettingtest.cpp
  autotests/settings/tcsettingtest.h
  src/CMakeLists.txt
  src/settings/matchsetting.cpp
  src/settings/matchsetting.h
  src/settings/matchsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h
  src/settings/tcsetting.cpp
  src/settings/tcsetting.h
  src/settings/tcsetting_p.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

[Jan Grulich]

jgrulich added a comment.


  It again doesn't apply, rebase it to current master.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

[Pranav Gade]

pranavgade updated this revision to Diff 46825.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17317?vs=46824=46825

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/matchsettingtest.cpp
  autotests/settings/matchsettingtest.h
  autotests/settings/tcsettingtest.cpp
  autotests/settings/tcsettingtest.h
  src/CMakeLists.txt
  src/settings/matchsetting.cpp
  src/settings/matchsetting.h
  src/settings/matchsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h
  src/settings/tcsetting.cpp
  src/settings/tcsetting.h
  src/settings/tcsetting_p.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

[Jan Grulich]

jgrulich added a comment.


  In D17317#371010 , @pranavgade 
wrote:
  
  > In D17317#371009 , @jgrulich 
wrote:
  >
  > > The tcsettingtest is failing.
  >
  >
  > Why, exactly?
  
  
  Because it's broken in numerous ways :).
  
void TcSettingTest::testSetting()
{
QFETCH(NMVariantMapList, tfilters);
QFETCH(NMVariantMapList, qdiscs);

QVariantMap map;

map.insert(QLatin1String(NM_SETTING_TC_CONFIG_TFILTERS), 
QVariant::fromValue(tfilters));
map.insert(QLatin1String(NM_SETTING_TC_CONFIG_QDISCS), 
QVariant::fromValue(qdiscs));

NetworkManager::TcSetting setting;
setting.fromMap(map);

QVariantMap map1 = setting.toMap();

QVariantMap::const_iterator it = map.constBegin();
while (it != map.constEnd()) {
NMVariantMapList list = it.value().value();
NMVariantMapList list1 = 
map1.value(it.key()).value();

QCOMPARE(list.count(), list1.count());

int comparedMaps = 0;
for (int i = 0; i < list.size(); ++i) {
QVariantMap varMap = list.at(i);
for (int j = 0; j < list1.size(); ++j) {
QVariantMap varMap1 = list1.at(i);
QVariantMap::const_iterator ite = varMap.constBegin();
int comparedvals = 0;
while (ite != varMap.constEnd()) {
QVariantMap::const_iterator val1 = 
varMap1.constFind(ite.key());
if (val1 != varMap1.constEnd()) {
if (varMap.value(ite.key()) == val1.value()) {
++comparedvals;
}
}
++ite;
}
if (comparedvals == varMap.size()) {
comparedMaps++;
}
}
}
++it;
QCOMPARE(comparedMaps, list.count());
}
}
  
  It should be like this.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

[Pranav Gade]

pranavgade added a comment.


  In D17317#371009 , @jgrulich wrote:
  
  > The tcsettingtest is failing.
  
  
  Why, exactly?

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

[Jan Grulich]

jgrulich added a comment.


  The tcsettingtest is failing.

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17317: match and tc setting

[Pranav Gade]

pranavgade updated this revision to Diff 46824.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17317?vs=46792=46824

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

AFFECTED FILES
  autotests/settings/CMakeLists.txt
  autotests/settings/matchsettingtest.cpp
  autotests/settings/matchsettingtest.h
  autotests/settings/tcsettingtest.cpp
  autotests/settings/tcsettingtest.h
  src/CMakeLists.txt
  src/settings/matchsetting.cpp
  src/settings/matchsetting.h
  src/settings/matchsetting_p.h
  src/settings/setting.cpp
  src/settings/setting.h
  src/settings/tcsetting.cpp
  src/settings/tcsetting.h
  src/settings/tcsetting_p.h

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

Re: Transitioning CI builds of all non-Frameworks from Qt 5.9

[Elvis Angelaccio]

On 03/12/18 09:46, Ben Cooksley wrote:
> Hi all,

Hi Ben

> 
> I've been informed by the PIM developers that they would like to bump
> the dependency of the Qt version they use, from Qt 5.9 which it's on
> currently, to Qt 5.10.
> 
> As a consequence, due to many KDE projects using PIM libraries, their
> dependency on Qt will also be effectively bumped. To minimize the
> maintenance cost of this bump for the CI system, I would like to bump
> everyone up from Qt 5.9 to a newer version of Qt (otherwise we'll end
> up chasing down build failures for a long time)

It's not clear if you are suggesting to bump the minimum required Qt
version in each CMakeLists.txt file, or if you are just going to bump
the Qt version used by the CI server.

Could you please clarify? Thanks!

> 
> As most distributions have moved on from 5.10, and use Qt 5.11 now
> (and will in many cases soon move to Qt 5.12), i'd like to suggest
> that instead of going to Qt 5.10, we move straight to 5.11.
> 
> Because Frameworks has a two versions prior support policy, it'll
> remain on 5.9 for now until it's ready to move up to 5.10 (assuming
> 5.12 is a usable Qt version)
> 
> If nobody has any objections, i'll proceed with this change in around
> 3 days time.
> 
> Cheers,
> Ben
>

Cheers,
Elvis

Re: Transitioning CI builds of all non-Frameworks from Qt 5.9

[Ben Cooksley]

On Tue, Dec 4, 2018 at 9:45 AM Elvis Angelaccio
 wrote:
>
>
>
> On 03/12/18 09:46, Ben Cooksley wrote:
> > Hi all,
>
> Hi Ben

Hi Elvis,

>
> >
> > I've been informed by the PIM developers that they would like to bump
> > the dependency of the Qt version they use, from Qt 5.9 which it's on
> > currently, to Qt 5.10.
> >
> > As a consequence, due to many KDE projects using PIM libraries, their
> > dependency on Qt will also be effectively bumped. To minimize the
> > maintenance cost of this bump for the CI system, I would like to bump
> > everyone up from Qt 5.9 to a newer version of Qt (otherwise we'll end
> > up chasing down build failures for a long time)
>
> It's not clear if you are suggesting to bump the minimum required Qt
> version in each CMakeLists.txt file, or if you are just going to bump
> the Qt version used by the CI server.
>
> Could you please clarify? Thanks!

I'll only be bumping the version used by the CI system, the
applications themselves will be left unchanged.

>
> >
> > As most distributions have moved on from 5.10, and use Qt 5.11 now
> > (and will in many cases soon move to Qt 5.12), i'd like to suggest
> > that instead of going to Qt 5.10, we move straight to 5.11.
> >
> > Because Frameworks has a two versions prior support policy, it'll
> > remain on 5.9 for now until it's ready to move up to 5.10 (assuming
> > 5.12 is a usable Qt version)
> >
> > If nobody has any objections, i'll proceed with this change in around
> > 3 days time.
> >
> > Cheers,
> > Ben
> >
>
> Cheers,
> Elvis

Cheers,
Ben