D16299: RFC fallback to dnssd service discovery if smb listDir failed on root

[Nathaniel Graham]

ngraham added a comment.


  FWIW, in practice the workgroup is much less prominent to even unnecessary in 
a macOS or Linux shared file environment. I've only ever seen shares on 
multiple workgroups used in a Windows environment.

REPOSITORY
  R320 KIO Extras

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

To: sitter, #frameworks, #dolphin
Cc: ngraham, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, bruns, emmanuelp

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Nathaniel Graham]

ngraham added a comment.


  Tested this out and couldn't actually make it work the way I asked--with the 
FAT32 file size limit checked first. I think I see why: there are two `total 
size > available space?` checks that output `ERR_DISK_FULL` on error, and both 
of them execute first. The FAT32 file size limit check should be before the 
first one, probably near `// Stat the next src url`.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: cfeck, bruns, kde-frameworks-devel, michaelh, ngraham

D16301: Remove ComponentInstaller

[Aleix Pol Gonzalez]

apol added a comment.


  +1

REPOSITORY
  R242 Plasma Framework (Library)

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

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

D16306: [Codecs] Remove unneeded const_cast

[Stefan Brüns]

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

REVISION SUMMARY
  QByteArray accepts a const char* in the constructor, no need to cast
  the const away.
  Also make it obvious we are using the const overload of data() by
  replacing it with constData().

REPOSITORY
  R293 Baloo

BRANCH
  oob

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

AFFECTED FILES
  src/codecs/postingcodec.cpp

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

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> cfeck wrote in copyjob.cpp:1622
> Does this even work on a -m32 system?

`(1ul << 32) -1` then ...

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: cfeck, bruns, kde-frameworks-devel, michaelh, ngraham

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> job_error.cpp:248
>  break;
> +case KIO::ERR_FILE_TOO_LARGE_FOR_FAT32:
> +result = i18n("The file named %1 cannot be transferred because its 
> size is greater than 4 GB,"

Should also be handled in `KIO::rawErrorDetail(...)`, same file below.

> ngraham wrote in job_error.cpp:249
> None of the other error messages use quotes around the filenames. What's most 
> semantically correct would be to change `i18n` to `xi18n` and wrap the 
> filename/path token in `` tags (this is exactly what 
> they're for). We can do that in this patch, and then change it for all the 
> other error messages (as appropriate) in another patch.
> 
> Also, in terms of wording, the HIG recommends putting the most important part 
> of the sentence first, which in this case would be the error message; the 
> explanation would go second. See 
> https://hig.kde.org/style/writing/wording.html. So the current sentence may 
> be a bit long, but it's structurally correct as-is.

`"Could not transfer %1 because it is to large. The destination filesystem only 
supports files up to 4GB.` is shorter ...

The second part may even be removed alltogether, see `KIO::rawErrorDetail`, 
https://github.com/KDE/kio/blob/master/src/core/job_error.cpp#L311

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: cfeck, bruns, kde-frameworks-devel, michaelh, ngraham

D16300: Remove double underscore (__) from header include guards

[Dominik Haumann]

dhaumann added a comment.


  Thanks!
  
  Qt decided against #pragma once. I wouldn't want to use it in KDE unless it 
is decided on kde-frameworks-devel.

REPOSITORY
  R39 KTextEditor

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

To: kossebau, #kate, cullmann
Cc: dhaumann, winterz, bcooksley, broulik, cullmann, kwrite-devel, 
kde-frameworks-devel, michaelh, ngraham, bruns, demsking, sars

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> bruns wrote in copyjob.cpp:1622
> why not `1 << 32 - 1` ?
> Also note the `- 1`, 2  ^ 32  is 0, as FAT32 uses a uint32_t for the file 
> size.

Does this even work on a -m32 system?

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: cfeck, bruns, kde-frameworks-devel, michaelh, ngraham

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> bruns wrote in job_error.cpp:249
> %1 with quotes please, as it can contain whitespace, so it would be hard to 
> spot where the filename ends.
> I would also prefer a sentence where the reason is given first, e.g.
> `The destination filesystem only supports files up to 4GB. "%1" is to large 
> an can not be transferred.`

None of the other error messages use quotes around the filenames. What's most 
semantically correct would be to change `i18n` to `xi18n` and wrap the 
filename/path token in `` tags (this is exactly what 
they're for). We can do that in this patch, and then change it for all the 
other error messages (as appropriate) in another patch.

Also, in terms of wording, the HIG recommends putting the most important part 
of the sentence first, which in this case would be the error message; the 
explanation would go second. See 
https://hig.kde.org/style/writing/wording.html. So the current sentence may be 
a bit long, but it's structurally correct as-is.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> copyjob.cpp:1622
> +
> +if (fileSystem == KFileSystemType::Fat && (*it).size > 
> 4294967296) { // 4 GB = 4294967296 Bytes
> +q->setError(ERR_FILE_TOO_LARGE_FOR_FAT32);

why not `1 << 32 - 1` ?
Also note the `- 1`, 2  ^ 32  is 0, as FAT32 uses a uint32_t for the file size.

> job_error.cpp:249
> +case KIO::ERR_FILE_TOO_LARGE_FOR_FAT32:
> +result = i18n("The file named %1 cannot be transferred because its 
> size is greater than 4 GB,"
> +  " and the destination's filesystem does not support 
> files that large.", errorText);

%1 with quotes please, as it can contain whitespace, so it would be hard to 
spot where the filename ends.
I would also prefer a sentence where the reason is given first, e.g.
`The destination filesystem only supports files up to 4GB. "%1" is to large an 
can not be transferred.`

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> global.h:249
> +ERR_CANNOT_CREATE_SLAVE = KJob::UserDefinedError + 73, ///< used by 
> Slave::createSlave, @since 5.30
> +ERR_FILE_TOO_LARGE_FOR_FAT32 = KJob::UserDefinedError + 74
>  };

Please add a comment with `@since` to this

> job_error.cpp:253
>  default:
> -result = i18n("Unknown error code %1\n%2\nPlease send a full bug 
> report at https://bugs.kde.org.;,  errorCode,  errorText);
> +result = i18n("Unknown error code %1\n%2\nPlease send a full bug 
> report at http://bugs.kde.org.;,  errorCode,  errorText);
>  break;

Unrelated and incorrect change: `https` is correct here, so please revert.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Nathaniel Graham]

ngraham added a comment.


  Thank you for indulging me 

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in FAT32 file system(4 GB)

[Shubham]

shubham updated this revision to Diff 43895.
shubham retitled this revision from "Warn user before copy/move job if the file 
size exceeds the maximum possible file size(4 GB) in FAT32 file system" to 
"Warn user before copy/move job if the file size exceeds the maximum possible 
file size in FAT32 file system(4 GB) ".

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16249?vs=43894=43895

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

AFFECTED FILES
  src/core/copyjob.cpp
  src/core/global.h
  src/core/job_error.cpp

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size(4 GB) in FAT32 file system

[Nathaniel Graham]

ngraham added a comment.


  Thanks, this is looking better. I will test it out later.
  
  It's not a huge deal, but `40` might be clearer and more readable 
than `qPow(4*M_E, 9)`. Then you could avoid adding the `` import too.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size(4 GB) in FAT32 file system

[Shubham]

shubham marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size(4 GB) in FAT32 file system

[Shubham]

shubham updated this revision to Diff 43894.
shubham added a comment.


  Cache file system time instead of creating it every time

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16249?vs=43890=43894

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

AFFECTED FILES
  src/core/copyjob.cpp
  src/core/global.h
  src/core/job_error.cpp

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16305: Add a QIconEnginePlugin to allow QIcon deserialization

[Fabian Vogt]

fvogt created this revision.
fvogt added a reviewer: Frameworks.
Herald added a project: Frameworks.
fvogt requested review of this revision.

REVISION SUMMARY
  This is necessary to allow QIcons with a KIconEngine as engine to deserialize
  properly.
  
  BUG: 399989

TEST PLAN
  Ran the PoC in the bug report, works fine.

REPOSITORY
  R302 KIconThemes

BRANCH
  master

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kiconengineplugin.cpp
  src/kiconengineplugin.json

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

D16301: Remove ComponentInstaller

[Stefan Brüns]

bruns added a comment.


  
https://github.com/hughsie/PackageKit/blob/master/src/org.freedesktop.PackageKit.Transaction.xml
  
  Method InstallPackages
  
  This requires of course we already have a valid package name. For this we 
probably have to rely on appstream, but need to define an appropriate 
 id first.
  e.g.
  

   org.kde.plasma.dataengine.time
   org.kde.plasma
   Time Dataengine


REPOSITORY
  R242 Plasma Framework (Library)

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

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

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size(4 GB) in FAT32 file system

[Nathaniel Graham]

ngraham added a comment.


  Maybe we could cache the value of `KFileSystemType::fileSystemType()` 
somewhere so we don't need to calculate it again for every file, since it's the 
same for every one.

INLINE COMMENTS

> job_error.cpp:249
> +case KIO::ERR_FILE_TOO_LARGE_FOR_FAT32:
> +result = i18n("The file named %1 cannot be transferred because it's 
> size is greater than 4 GB,"
> +  " and the destination's filesystem does not support 
> files that large.", errorText);

it's -> its

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size(4 GB) in FAT32 file system

[Shubham]

shubham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size(4 GB) in FAT32 file system

[Shubham]

shubham updated this revision to Diff 43890.
shubham retitled this revision from "Warn user before copy/move job if the file 
size exceeds the maximum possible file size in a File System" to "Warn user 
before copy/move job if the file size exceeds the maximum possible file size(4 
GB) in FAT32 file system".

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16249?vs=43735=43890

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

AFFECTED FILES
  src/core/copyjob.cpp
  src/core/global.h
  src/core/job_error.cpp

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16197: provide a list of supported mimetypes for embeddedimagedata

[Alexander Stippich]

astippich added a comment.


  I've decided to name it similar to the extractor plugins

REPOSITORY
  R286 KFileMetaData

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

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

D16197: provide a list of supported mimetypes for embeddedimagedata

[Alexander Stippich]

astippich updated this revision to Diff 43888.
astippich marked an inline comment as done.
astippich added a comment.


  - remove read prefix

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16197?vs=43819=43888

BRANCH
  mimetypes_embedded_image

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

AFFECTED FILES
  autotests/embeddedimagedatatest.cpp
  src/embeddedimagedata.cpp
  src/embeddedimagedata.h

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

D16281: update epub test data and test for comment property

[Alexander Stippich]

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


  - fix test file

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16281?vs=43820=43887

BRANCH
  description_test_preparation

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

AFFECTED FILES
  autotests/epubextractortest.cpp
  autotests/samplefiles/test.epub

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

D16283: implement more tags for asf metadata

[Alexander Stippich]

astippich added inline comments.

INLINE COMMENTS

> bruns wrote in taglibextractor.cpp:739
> this makes me always wonder why we don't do:
> 
>   // decltype(data.albumArtists) == QStringList
>   for (attribute : lstAsf) {
> QString t = TStringToQString(attribute.toString());
> data.albumArtists << contactsFromString(t);
>   }

Yeah, there is a lot of unneccesary stuff in there that must be cleaned up. But 
before I get to that, I would like to add tests for multiple entries, and 
before that we have to agree how the output of mutliple entries shall look 
like, which brings me to D12950  :)

> bruns wrote in taglibextractor.h:28
> Side note - I think `tfilestream.h` is no longer needed.

Is it okay if I push this directly?

REPOSITORY
  R286 KFileMetaData

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

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

D16283: implement more tags for asf metadata

[Alexander Stippich]

astippich updated this revision to Diff 43885.
astippich marked 3 inline comments as done.
astippich added a comment.


  - fix comment and simplify iterating
  - cleanup

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16283?vs=43822=43885

BRANCH
  extract_asf

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

AFFECTED FILES
  autotests/samplefiles/test.wma
  autotests/taglibextractortest.cpp
  autotests/taglibextractortest.h
  src/extractors/taglibextractor.cpp
  src/extractors/taglibextractor.h

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

D16300: Remove double underscore (__) from header include guards

[Allen Winter]

winterz added a comment.


  fyi, krazy in strict mode will check for leading and trailing underscores on 
the include guards.
  I don't run krazy in strict mode on the EBN, however.
  
  I could make this a standard check if we want so the EBN would pick it up too.

REPOSITORY
  R39 KTextEditor

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

To: kossebau, #kate, cullmann
Cc: winterz, bcooksley, broulik, cullmann, kwrite-devel, kde-frameworks-devel, 
michaelh, ngraham, bruns, demsking, sars, dhaumann

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.9 - Build # 284 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.9/284/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Thu, 18 Oct 2018 17:11:58 +
 Build duration:
20 min and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 60 test(s), Skipped: 0 test(s), Total: 60 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(261/397)66%
(261/397)53%
(32021/59991)38%
(16504/43892)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(56/56)100%
(56/56)95%
(9077/9516)48%
(4271/8920)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%
(8339/14316)50%
(4660/9263)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3893/7935)34%
(1589/4691)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)51%
(526/1023)37%
(316/850)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/1364)0%
(0/1414)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1769/4316)35%
(1306/3684)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(629/1331)55%
(619/1123)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/257)7%
(14/212)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/30)src.ioslaves.trash56%
(5/9)56%
(5/9)52%
(718/1372)43%
  

D16300: Remove double underscore (__) from header include guards

[Ben Cooksley]

bcooksley added a comment.


  In certain instances `#pragma once` fails when using MSVC, while traditional 
`#define` include guards are fully reliable.

REPOSITORY
  R39 KTextEditor

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

To: kossebau, #kate, cullmann
Cc: bcooksley, broulik, cullmann, kwrite-devel, kde-frameworks-devel, michaelh, 
ngraham, bruns, demsking, sars, dhaumann

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.10 - Build # 439 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/439/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Thu, 18 Oct 2018 17:11:58 +
 Build duration:
5 min 25 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 60 test(s), Skipped: 0 test(s), Total: 60 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(261/397)66%
(261/397)53%
(31951/59991)38%
(16469/43892)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(56/56)100%
(56/56)95%
(9077/9516)48%
(4268/8920)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%
(8280/14316)50%
(4636/9259)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3893/7935)34%
(1589/4691)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)51%
(525/1023)37%
(315/850)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/1364)0%
(0/1414)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1774/4316)35%
(1304/3684)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1331)55%
(620/1123)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/257)7%
(14/212)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/30)src.ioslaves.trash56%
(5/9)56%
(5/9)51%
(697/1372)42%
  

D16301: Remove ComponentInstaller

[Kai Uwe Broulik]

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

REVISION SUMMARY
  When a dataengine failed to load it would ask PackageKit to install it but 
it's generally not how we want to distribute plasmoid infrastructure these days 
and didn't work.

TEST PLAN
  The interface didn't exist here, and I couldn't find it in the docs, and it's 
ancient anyway. Even if it worked, it would still fail to load the engine, have 
you install it (assuming it even finds it), and then you have to restart plasma 
to load it.

REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  autotests/CMakeLists.txt
  src/plasma/CMakeLists.txt
  src/plasma/private/componentinstaller.cpp
  src/plasma/private/componentinstaller_p.h
  src/plasma/private/dataenginemanager.cpp
  src/plasma/scripting/scriptengine.cpp

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

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/plasma-framework/job/kf5-qt5%20SUSEQt5.9/169/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Thu, 18 Oct 2018 14:45:28 +
 Build duration:
9 min 38 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 14 test(s), Skipped: 0 test(s), Total: 15 test(s)Failed: TestSuite.plasma-iconitemtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report50%
(9/18)45%
(57/126)45%
(57/126)39%
(5180/13181)29%
(2734/9444)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests93%
(13/14)93%
(13/14)96%
(1068/1117)51%
(552/1084)src.declarativeimports.calendar0%
(0/6)0%
(0/6)0%
(0/463)0%
(0/231)src.declarativeimports.core44%
(7/16)44%
(7/16)33%
(749/2240)27%
(386/1442)src.declarativeimports.plasmacomponents0%
(0/6)0%
(0/6)0%
(0/497)0%
(0/187)src.declarativeimports.plasmaextracomponents0%
(0/3)0%
(0/3)0%
(0/42)0%
(0/22)src.declarativeimports.platformcomponents0%
(0/3)0%
(0/3)0%
(0/58)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/14)0%
(0/2)src.plasma64%
(14/22)64%
(14/22)48%
(1700/3506)39%
(1030/2633)src.plasma.packagestructure57%
(4/7)57%
(4/7)37%
(51/138)42%
(5/12)src.plasma.private63%
(12/19)63%
(12/19)61%
(945/1558)42%
(424/1003)src.plasma.scripting67%
(2/3)67%
(2/3)20%
(34/166)10%
(13/128)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick27%
(3/11)27%
(3/11)30%
(589/1977)19%
(319/1702)src.plasmaquick.private50%
(1/2)50%
(1/2)29%
(31/107)36%
(5/14)src.scriptengines.qml.plasmoid17%
(1/6)17%
(1/6)1%
(13/1098)0%
(0/906)tests.dpi0%
(0/2)0%
(0/2)0%
(0/21)0%
(0/2)tests.kplugins0%
(0/2)0%
(0/2)0%
(0/61)0%
(0/16)tests.testengine0%

D16300: Remove double underscore (__) from header include guards

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D16300#345405 , @broulik wrote:
  
  > It could have just gone `#pramga once`
  
  
  conjunctive "could" -> not specified in KF policies if possible.
  
  Seems even Qt devs are undecided about it and rejected it again, so myself 
tend to avoid for now. As much as something like it is desired.

REPOSITORY
  R39 KTextEditor

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

To: kossebau, #kate, cullmann
Cc: broulik, cullmann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D16295: Remove PLASMA_NO_KIO

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:e92ed94d9d1a: Remove PLASMA_NO_KIO option (authored by 
broulik).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16295?vs=43857=43872

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

AFFECTED FILES
  autotests/CMakeLists.txt
  src/plasma/CMakeLists.txt
  src/plasma/config-plasma.h.cmake
  src/plasma/containment.cpp
  src/plasma/pluginloader.cpp
  src/plasma/private/associatedapplicationmanager.cpp
  src/scriptengines/qml/CMakeLists.txt
  src/scriptengines/qml/plasmoid/containmentinterface.cpp

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

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/plasma-framework/job/kf5-qt5%20SUSEQt5.10/244/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Thu, 18 Oct 2018 14:45:28 +
 Build duration:
2 min 33 sec and counting
   JUnit Tests
  Name: (root) Failed: 6 test(s), Passed: 9 test(s), Skipped: 0 test(s), Total: 15 test(s)Failed: TestSuite.dialognativetestFailed: TestSuite.plasma-configmodeltestFailed: TestSuite.plasma-fallbackpackagetestFailed: TestSuite.plasma-iconitemtestFailed: TestSuite.plasma-packagestructuretestFailed: TestSuite.plasma-storagetest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report33%
(6/18)35%
(44/126)35%
(44/126)27%
(3601/13121)19%
(1821/9446)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests86%
(12/14)86%
(12/14)55%
(607/1113)29%
(313/1084)src.declarativeimports.calendar0%
(0/6)0%
(0/6)0%
(0/463)0%
(0/231)src.declarativeimports.core31%
(5/16)31%
(5/16)13%
(299/2221)7%
(96/1444)src.declarativeimports.plasmacomponents0%
(0/6)0%
(0/6)0%
(0/497)0%
(0/187)src.declarativeimports.plasmaextracomponents0%
(0/3)0%
(0/3)0%
(0/42)0%
(0/22)src.declarativeimports.platformcomponents0%
(0/3)0%
(0/3)0%
(0/58)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/14)0%
(0/2)src.plasma64%
(14/22)64%
(14/22)41%
(1420/3492)30%
(794/2633)src.plasma.packagestructure0%
(0/7)0%
(0/7)0%
(0/135)0%
(0/12)src.plasma.private47%
(9/19)47%
(9/19)43%
(665/1544)30%
(301/1003)src.plasma.scripting0%
(0/3)0%
(0/3)0%
(0/161)0%
(0/128)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick27%
(3/11)27%
(3/11)29%
(579/1978)18%
(312/1702)src.plasmaquick.private50%
(1/2)50%
(1/2)29%
(31/107)36%
(5/14)src.scriptengines.qml.plasmoid0%
(0/6)0%
(0/6)0%
(0/1096)0%
(0/906)tests.dpi0%
(0/2)0%
(0/2)0%
(0/21)0%
(0/2)tests.kplugins0%
(0/2)0%
  

D16300: Remove double underscore (__) from header include guards

[Kai Uwe Broulik]

broulik added a comment.


  It could have just gone `#pramga once`

REPOSITORY
  R39 KTextEditor

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

To: kossebau, #kate, cullmann
Cc: broulik, cullmann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D16300: Remove double underscore (__) from header include guards

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:38a9214aaa0e: Remove double underscore (__) from header 
include guards (authored by kossebau).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16300?vs=43870=43871

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

AFFECTED FILES
  config.h.cmake
  src/dialogs/kateconfigpage.h
  src/dialogs/katedialogs.h
  src/document/katebuffer.h
  src/inputmode/kateabstractinputmode.h
  src/inputmode/kateabstractinputmodefactory.h
  src/inputmode/katenormalinputmode.h
  src/inputmode/katenormalinputmodefactory.h
  src/inputmode/kateviinputmode.h
  src/inputmode/kateviinputmodefactory.h
  src/mode/katemodeconfigpage.h
  src/mode/katemodemanager.h
  src/mode/katemodemenu.h
  src/printing/kateprinter.h
  src/printing/printconfigwidgets.h
  src/printing/printpainter.h
  src/render/katerenderer.h
  src/schema/kateschema.h
  src/schema/kateschemaconfig.h
  src/spellcheck/spellcheckdialog.h
  src/syntax/katehighlight.h
  src/syntax/katehighlightmenu.h
  src/utils/kateautoindent.h
  src/utils/katebookmarks.h
  src/utils/katecmds.h
  src/utils/kateconfig.h
  src/utils/katedefaultcolors.h
  src/utils/kateglobal.h
  src/utils/katesedcmd.h
  src/view/kateviewhelpers.h

To: kossebau, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, 
demsking, sars, dhaumann

D16299: RFC fallback to dnssd service discovery if smb listDir failed on root

[Harald Sitter]

sitter added a comment.


  This diff does somewhat depend on D16298  
as currently KDNSSD can easily lock indefinitely on resolving services. We 
could kinda bypass this with a timeout on service resolution, that would 
however have the disadvantage of either having to repeat the resolution or not 
showing the device. Both of which suck a bit. My plan is to simply have the 
fallback only run with KDNSSD 5.52 (assuming that gets the fix landed).
  Besides that this is working nicely for me.
  I would love to have a way to actually get the workgroup from a share, but 
from some research that seems simply unobtainable (via smbclient anyway) unless 
SMB1 is used (as in: if you use the smbclient CLI it would transparently drop 
down to SMB1 for the workgroup query but even that doesn't work if the minimum 
version is set >=SMB2). So we could drop to SMB1, but honestly I fail to 
appreciate the use in that in particular since the maintainer of SMB within 
Microsoft is very insistent on people not using a 30 year old insecure protocol 
anymore (rightfully, I should add ;))
  
  In D16299#345363 , @ngraham wrote:
  
  > Wow, awesome work. What more is needed to support Windows? Support for the 
`WS-Discovery` protocol?
  
  
  Yes. I am not sure we have a production quality lib for that anywhere though. 
I haven't had much of a look since it's tricky to try in my network. There is 
however a good chance windows 10 already does, or possibly will at some point 
in the future, embrace dnssd like the rest of the world 
https://channel9.msdn.com/Events/Build/2015/3-79

REPOSITORY
  R320 KIO Extras

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

To: sitter, #frameworks, #dolphin
Cc: ngraham, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, bruns, emmanuelp

D16300: Remove double underscore (__) from header include guards

[Christoph Cullmann]

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


  Yep, makes sense!

REPOSITORY
  R39 KTextEditor

BRANCH
  remove__fromincludeguards

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

To: kossebau, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, 
demsking, sars, dhaumann

D16300: Remove double underscore (__) from header include guards

[Friedrich W. H. Kossebau]

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

REVISION SUMMARY
  Names containing a double underscore (__) are reserved to the
  C++ implementation, so better just avoid it, even with KATE namespace.
  
  Needs rename of global config include guards to avoid guard name clash.

TEST PLAN
  Still builds

REPOSITORY
  R39 KTextEditor

BRANCH
  remove__fromincludeguards

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

AFFECTED FILES
  config.h.cmake
  src/dialogs/kateconfigpage.h
  src/dialogs/katedialogs.h
  src/document/katebuffer.h
  src/inputmode/kateabstractinputmode.h
  src/inputmode/kateabstractinputmodefactory.h
  src/inputmode/katenormalinputmode.h
  src/inputmode/katenormalinputmodefactory.h
  src/inputmode/kateviinputmode.h
  src/inputmode/kateviinputmodefactory.h
  src/mode/katemodeconfigpage.h
  src/mode/katemodemanager.h
  src/mode/katemodemenu.h
  src/printing/kateprinter.h
  src/printing/printconfigwidgets.h
  src/printing/printpainter.h
  src/render/katerenderer.h
  src/schema/kateschema.h
  src/schema/kateschemaconfig.h
  src/spellcheck/spellcheckdialog.h
  src/syntax/katehighlight.h
  src/syntax/katehighlightmenu.h
  src/utils/kateautoindent.h
  src/utils/katebookmarks.h
  src/utils/katecmds.h
  src/utils/kateconfig.h
  src/utils/katedefaultcolors.h
  src/utils/kateglobal.h
  src/utils/katesedcmd.h
  src/view/kateviewhelpers.h

To: kossebau, #kate
Cc: kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, 
cullmann, sars, dhaumann

D16299: RFC fallback to dnssd service discovery if smb listDir failed on root

[Nathaniel Graham]

ngraham added reviewers: Frameworks, Dolphin.
ngraham added a comment.


  Wow, awesome work. What more is needed to support Windows? Support for the 
`WS-Discovery` protocol?

REPOSITORY
  R320 KIO Extras

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

To: sitter, #frameworks, #dolphin
Cc: ngraham, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, bruns, emmanuelp

D16299: RFC fallback to dnssd service discovery if smb listDir failed on root

[Harald Sitter]

sitter created this revision.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  This elevates a problem with newer SMB protocol versions and smbclient
  not supporting discovery/browsing. when not using older (as in:
  ancient) protocol versions discovery doesn't work and smb:/ gives no
  results.
  
  By falling back to DNSSD based discovery we can ensure discovery of DNSSD
  remotes (namely linux and osx) is always working. Windows unfortunately
  does not support DNSSD and as such will not benefit from this mode of
  discovery and continue to be unlisted when using a protocol version
  without browsing support.
  
  CCBUG: 392447
  CCBUG: 390551

TEST PLAN
  smb.conf:
  
[global]
client min protocol = SMB2
  
  Lists devices

REPOSITORY
  R320 KIO Extras

BRANCH
  smb-discovery

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

AFFECTED FILES
  smb/CMakeLists.txt
  smb/kio_smb.h
  smb/kio_smb_browse.cpp

To: sitter
Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D16295: Remove PLASMA_NO_KIO

[David Edmundson]

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


  ++

REPOSITORY
  R242 Plasma Framework (Library)

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

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

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 43868.
kossebau marked an inline comment as done.
kossebau added a comment.


  update to Dominik's last review:
  
  - remove __ from include guard
  - add comment that Option::view is always set

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8708?vs=43516=43868

BRANCH
  addAnnotationItemDelegate

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

AFFECTED FILES
  src/CMakeLists.txt
  src/include/CMakeLists.txt
  src/include/ktexteditor/abstractannotationitemdelegate.h
  src/include/ktexteditor/annotationinterface.h
  src/utils/ktexteditor.cpp
  src/view/kateannotationitemdelegate.cpp
  src/view/kateannotationitemdelegate.h
  src/view/kateview.cpp
  src/view/kateview.h
  src/view/kateviewhelpers.cpp
  src/view/kateviewhelpers.h
  src/view/kateviewinternal.h

To: kossebau, #kate, #kdevelop, dhaumann
Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, 
bruns, demsking, sars

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

[Friedrich W. H. Kossebau]

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


  In D8708#344872 , @dhaumann wrote:
  
  > Feel free to push this for 5.52 (you'd have to adapt the @since 5.53 again).
  
  
  Thanks for review again :)
  
  Would prefer waiting for 5.53, for some full possible period of testing by 
people running from master, which should be at least > number of people geting 
and trying cross-project/repo patches :)
  
  > I only have minor comments that should not hold back this patch. One thing 
that pops into my eyes is that there are still some "todo" comments, which even 
talk about cornercases / bugs. Since I think you know what you are doing, I 
leave it up to you to decide wither it's ok as is or whether there need to be 
more fixes.
  
  Yes, the TODOs left are about non-critical things, like issues already in the 
old code (those about rendering group delimiters at the lines on the view 
top/bottom borders), old magic numbers which could see some reasoning, or some 
check which is practically not needed, but might be expected by some looking at 
patterns. Left as TODO remarks for my older self or someone else, in case they 
work on this, so the rough edges are documented and not silently in the code.
  
  > In any case, it was never my (and I think I can also safely say) nor any 
other's intention to hold this patch back. So yes, pinging more often is ok 
imo. Luckily, you are not a first-time contributor (who would be very much 
discouraged by our review delays), so I am sure / hope we'll see more patches 
from you in the future as well! :-)
  
  Okay, white card for more poking about reviews happily received :)
  Some other reason might also have been that I left the ""WIP" too long in the 
title, which might also have sent a wrong signal, when this was rather RFC on 
the working prototype/pre-production sample.

INLINE COMMENTS

> dhaumann wrote in kateannotationitemdelegate.h:20
> All keywords starting with __ are reserved, I suggest to remove __ at the 
> beginning and at the end.

Had those to be in line with other headers in that dir. Will do a separate 
patch to change that for the existing headers then.

REPOSITORY
  R39 KTextEditor

BRANCH
  addAnnotationItemDelegate

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

To: kossebau, #kate, #kdevelop, dhaumann
Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, 
bruns, demsking, sars

D16294: [test/xdgtest] Create/destroy popup on click

[Vlad Zagorodniy]

zzag added inline comments.

INLINE COMMENTS

> xdgtest.cpp:202
>  QImage image(buffer->address(), size.width(), size.height(), 
> QImage::Format_ARGB32_Premultiplied);
> -image.fill(QColor(255, 0, 0, 255));
> +image.fill(QColor(0, 0, 255, 255));
>  

Also, you could use Qt::blue. :-)

REPOSITORY
  R127 KWayland

BRANCH
  master

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

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

D16298: prevent avahi signal racing

[Harald Sitter]

sitter marked 2 inline comments as done.

REPOSITORY
  R272 KDNSSD

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

To: sitter, mdawson, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16298: prevent avahi signal racing

[Harald Sitter]

sitter updated this revision to Diff 43863.
sitter added a comment.


  typo-- && make isOurMsg const

REPOSITORY
  R272 KDNSSD

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16298?vs=43861=43863

BRANCH
  master

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

AFFECTED FILES
  src/CMakeLists.txt
  src/avahi-domainbrowser.cpp
  src/avahi-domainbrowser_p.h
  src/avahi-publicservice.cpp
  src/avahi-publicservice_p.h
  src/avahi-remoteservice.cpp
  src/avahi-remoteservice_p.h
  src/avahi-servicebrowser.cpp
  src/avahi-servicebrowser_p.h
  src/avahi-servicetypebrowser.cpp
  src/avahi-servicetypebrowser_p.h
  src/avahi_listener.cpp
  src/avahi_listener_p.h

To: sitter, mdawson, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16298: prevent avahi signal racing

[Kai Uwe Broulik]

broulik added a comment.


  Not sure if evil or genius ;)
  Looks sane, as sane as it gets, obviously.

INLINE COMMENTS

> avahi_listener_p.h:29
> +
> +// Assits with listening to Avahi for all signals and then checking if the
> +// a given dbus message is meant for us or not.

*Assists

> avahi_listener_p.h:41
> +// This method gets called a lot but doesn't do much. Suggest inlining.
> +inline bool isOurMsg(const QDBusMessage )
> +{

Make `const`

REPOSITORY
  R272 KDNSSD

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

To: sitter, mdawson, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16294: [test/xdgtest] Create/destroy popup on click

[Vlad Zagorodniy]

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

INLINE COMMENTS

> xdgtest.cpp:172
> +positioner.setGravity(Qt::BottomEdge);
> +positioner.setConstraints(XdgPositioner::Constraint::FlipX | 
> XdgPositioner::Constraint::FlipX);
> +m_xdgShellPopup = m_xdgShell->createPopup(m_popupSurface, 
> m_xdgShellSurface, positioner, m_popupSurface);

FlipY?

REPOSITORY
  R127 KWayland

BRANCH
  master

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

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

D16298: prevent avahi signal racing

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> avahi-remoteservice.cpp:104
> +
> +new org::freedesktop::Avahi::ServiceResolver(
> +s.service(),

FTR: this is obviously leaking and was doing so before as well. I'll fix it 
after this diff.

REPOSITORY
  R272 KDNSSD

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

To: sitter, mdawson, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16298: prevent avahi signal racing

[Harald Sitter]

sitter added reviewers: mdawson, broulik.

REPOSITORY
  R272 KDNSSD

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

To: sitter, mdawson, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16298: prevent avahi signal racing

[Harald Sitter]

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

REVISION SUMMARY
  https://github.com/lathiat/avahi/issues/9
  
  Avahi has upstream signal races which can have any number of side effect
  bugs on our end ranging from unreliable discovery, over missing servers
  in kio slave listings, to outright deadlocking when waiting for a result
  which has already raced passed us (specifically RemoteService is vulnerable
  to that).
  
  When we make a request to avahi it will immediately start sending
  discovery signals even when we haven't even connected to the signals yet.
  This means any number of signals may be lost in the time between our
  request and us connecting to the signals.
  As this applies to (all?) in-the-wild versions of avahi even if we got an
  upstream fix today it'd not help the user until they get the new avahi
  integrated.
  
  To prevent us from losing the race all dbus signal listening is now done
  via blanket connects.
  Ordinarily we'd make a request, dbus would give us an object path, we'd
  connect to the object path's signals and then get signals for our request.
  Since the signals are racey we'll instead connect to all signals of a given
  interface (e.g. org.freedesktop.Avahi.DomainBrowser) completely ignoring
  the object path. We'll then "locally" filter all signals which belong to
  the specific DomainBrowser object as identified by the signals' dbus object
  path. This eliminates all risk of racing. Before we make the request
  we setup our blanket connections and the slots won't get called until after
  the event loop returns, which won't be until we've set "our" dbus object
  path for filtering later in the relevant functions.
  
  The obvious downsides of this are:
  
  - boilerplatey code for handling this stuff
  - runtime checked signal-slot compatibility
  - much more traffic going into our slots (doesn't actually end up doing any 
heavy lifting)

TEST PLAN
  - wrote a crappy test app to use all browser -> still browse (also service 
resolves)

REPOSITORY
  R272 KDNSSD

BRANCH
  master

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

AFFECTED FILES
  src/CMakeLists.txt
  src/avahi-domainbrowser.cpp
  src/avahi-domainbrowser_p.h
  src/avahi-publicservice.cpp
  src/avahi-publicservice_p.h
  src/avahi-remoteservice.cpp
  src/avahi-remoteservice_p.h
  src/avahi-servicebrowser.cpp
  src/avahi-servicebrowser_p.h
  src/avahi-servicetypebrowser.cpp
  src/avahi-servicetypebrowser_p.h
  src/avahi_listener.cpp
  src/avahi_listener_p.h

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

D16295: Remove PLASMA_NO_KIO

[Kai Uwe Broulik]

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

REVISION SUMMARY
  It is a remnant of the Active days and didn't even compile.
  This patch only removes those ifdefs, it doesn't fix any issues I found 
during (e.g. double lookups or the fact that `KRun::autoDelete` is the default 
now)

TEST PLAN
  I think the entire `KDE_PLATFORM_FEATURE_BINARY_COMPATIBLE_FEATURE_REDUCTION` 
could be removed, but one step at a time
  
  Compiles, dropping files on desktop still asks what do to and adding a widget 
or setting as wallpaper works

REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  autotests/CMakeLists.txt
  src/plasma/CMakeLists.txt
  src/plasma/config-plasma.h.cmake
  src/plasma/containment.cpp
  src/plasma/pluginloader.cpp
  src/plasma/private/associatedapplicationmanager.cpp
  src/scriptengines/qml/CMakeLists.txt
  src/scriptengines/qml/plasmoid/containmentinterface.cpp

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

D16294: [test/xdgtest] Create/destroy popup on click

[David Edmundson]

davidedmundson edited the summary of this revision.

REPOSITORY
  R127 KWayland

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

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

D16294: [test/xdgtest] Create/destroy popup on click

[David Edmundson]

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

REVISION SUMMARY
  This allows a user to move the windoow before creating the popup which
  is extremely useful in testing constraints. Also makes it very easy to
  mod thsi code into a grabbing popup for other tests.
  
  This patch also improve the painted surfaces to show the anchor rect
  around where we place the popup which is easier for visual debugging.
  
  No library code changes

TEST PLAN
  Ran the test
  KWin doesn't position the popup according to all constraints
  Soon will

REPOSITORY
  R127 KWayland

BRANCH
  master

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

AFFECTED FILES
  tests/xdgtest.cpp

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

D15070: Bindings: Support using sys paths for python install directory

[Christophe Giboudeaux]

cgiboudeaux added a comment.


  In D15070#344900 , @bruns wrote:
  
  > In D15070#344884 , @cgiboudeaux 
wrote:
  >
  > > In D15070#344871 , @bruns 
wrote:
  > >
  > > > As all the raised concerns have been dealed with, can we give this a 
try while the next KF release is still somewhat in the future?
  > >
  > >
  > > No, the empty if must be removed. Code that does nothing is useless.
  >
  >
  > Its not empty, there is a comment inside. Of course I can add a 
`set(KDE_INSTALL_PYTHON${pyversion}DIR ${KDE_INSTALL_PYTHON${pyversion}DIR})` 
if you insist ...
  >
  > And if you restructure it you either end up with a lengthy if condition - 
`if (NOT KDE_INSTALL_PYTHON${pyversion}DIR AND 
KDE_INSTALL_USE_PYTHON${pyversion}_SYS_PATHS)` - or another nesting level. Both 
are significantly harder to read.
  
  
  If KDE_INSTALL_USE_PYTHON${pyversion}_SYS_PATHS is true, 
KDE_INSTALL_PYTHON${pyversion}DIR can be ignored. This was in the review

REPOSITORY
  R240 Extra CMake Modules

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

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

D16291: Center icons properly if size doesn't fit

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R302:2699d36365fa: Center icons properly if size doesnt 
fit (authored by broulik).

REPOSITORY
  R302 KIconThemes

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16291?vs=43847=43848

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

AFFECTED FILES
  src/kiconengine.cpp

To: broulik, dfaure, cfeck, ngraham, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16291: Center icons properly if size doesn't fit

[David Edmundson]

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

REPOSITORY
  R302 KIconThemes

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

To: broulik, dfaure, cfeck, ngraham, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16291: Center icons properly if size doesn't fit

[Kai Uwe Broulik]

broulik updated this revision to Diff 43847.
broulik edited the test plan for this revision.
broulik added a comment.


  - Also properly create new pixmap

REPOSITORY
  R302 KIconThemes

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16291?vs=43845=43847

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

AFFECTED FILES
  src/kiconengine.cpp

To: broulik, dfaure, cfeck, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16291: Center icons properly if size doesn't fit

[Kai Uwe Broulik]

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

REVISION SUMMARY
  When requesting an icon size that cannot be served, the pixmap is centered in 
the requested size.
  By using the original pixmap's device pixel ratio we ensure the padded pixmap 
is painted correctly.
  
  BUG: 396990

TEST PLAN
  There's still various issues in Dolphin's view engine when using fractional 
scaling, such as icons being too small, or preview icons larger then file 
icons, etc but at least icons aren't cut-off and misaligned anymore.
  I don't fully understand why for SVGs it fails to get a properly sized pixmap 
and instead pads them but that's fractional scaling for you right there...
  
  - deleted icon cache in `~/.cache`, ran `QT_SCREEN_SCALE_FACTORS=1.4 
dolphin`, no longer have icons rendered out of bounds

REPOSITORY
  R302 KIconThemes

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

AFFECTED FILES
  src/kiconengine.cpp

To: broulik, dfaure, cfeck, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16249: Warn user before copy/move job if the file size exceeds the maximum possible file size in a File System

[Shubham]

shubham added a comment.


  Is there any predefined function which returns the file size limit for a 
given file system?

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, elvisangelaccio, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns