D15081: replace own wildcard matcher with QRegularExpression combining all wildcards

[Christoph Cullmann]

cullmann added a comment.


  I had some more improvement ideas, this can wait a release.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: dfaure, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, 
bruns, demsking, cullmann, sars, dhaumann

KDE CI: Frameworks kio kf5-qt5 WindowsMSVCQt5.11 - Build # 2 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20WindowsMSVCQt5.11/2/
 Project:
Frameworks kio kf5-qt5 WindowsMSVCQt5.11
 Date of build:
Sun, 26 Aug 2018 03:38:43 +
 Build duration:
1 hr 57 min and counting
   JUnit Tests
  Name: (root) Failed: 24 test(s), Passed: 30 test(s), Skipped: 0 test(s), Total: 54 test(s)Failed: TestSuite.kiocore-deletejobtestFailed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kfileitemtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiocore-mkpathjobtestFailed: TestSuite.kiofilewidgets-kfilecopytomenutestFailed: TestSuite.kiofilewidgets-kfilecustomdialogtestFailed: TestSuite.kiofilewidgets-kfileplacesmodeltestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiofilewidgets-kfilewidgettestFailed: TestSuite.kiofilewidgets-knewfilemenutestFailed: TestSuite.kiofilewidgets-kurlnavigatortestFailed: TestSuite.kiowidgets-accessmanagertestFailed: TestSuite.kiowidgets-accessmanagertest-qnamFailed: TestSuite.kiowidgets-dropjobtestFailed: TestSuite.kiowidgets-fileundomanagertestFailed: TestSuite.kiowidgets-jobguitestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltestFailed: TestSuite.kiowidgets-krununittestFailed: TestSuite.kiowidgets-kurifiltertestFailed: TestSuite.kiowidgets-kurlcompletiontestFailed: TestSuite.kiowidgets-kurlcompletiontest-nowaitFailed: TestSuite.kpasswdservertest

D14449: Modify device usage information

[Shubham]

shubham added a comment.


  You plz proceed on it. Right now working on some other patch.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx
Cc: pino, rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.9 - Build # 225 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/225/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 26 Aug 2018 03:38:43 +
 Build duration:
48 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 58 test(s), Skipped: 0 test(s), Total: 59 test(s)Failed: TestSuite.kiowidgets-kdirlistertest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(258/396)65%
(258/396)53%
(31955/59937)38%
(16471/43900)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(55/55)100%
(55/55)95%
(9014/9469)48%
(4251/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.core84%
(98/116)84%
(98/116)58%
(8354/14360)50%
(4665/9293)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3894/7926)34%
(1586/4669)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1022)38%
(315/830)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/245)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1775/4320)35%
(1304/3700)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)51%
   

D14449: Modify device usage information

[Nathaniel Graham]

ngraham added a comment.


  Great! Are you gonna do the QFormLayout patch, or should I?

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx
Cc: pino, rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

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

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/380/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 26 Aug 2018 03:38:43 +
 Build duration:
34 min and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 59 test(s), Skipped: 0 test(s), Total: 59 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(258/396)65%
(258/396)53%
(31911/59935)38%
(16471/43902)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(55/55)100%
(55/55)95%
(9039/9469)48%
(4274/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.core84%
(98/116)84%
(98/116)58%
(8285/14358)50%
(4639/9289)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3893/7926)34%
(1585/4669)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1022)38%
(315/830)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/245)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1775/4320)35%
(1304/3700)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%

D14449: Modify device usage information

[Shubham]

shubham added a comment.


  In D14449#315524 , @ngraham wrote:
  
  > Do you plan to submit a new version of this after the layout has been 
converted to use `QFormLayout`? I do hope some form of this patch makes it in, 
since IMHO it's a nice little quality-of-life improvement.
  
  
  Sure

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx
Cc: pino, rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

KDE CI: Frameworks khtml kf5-qt5 FreeBSDQt5.11 - Build # 2 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20khtml%20kf5-qt5%20FreeBSDQt5.11/2/
 Project:
Frameworks khtml kf5-qt5 FreeBSDQt5.11
 Date of build:
Sun, 26 Aug 2018 03:38:33 +
 Build duration:
17 min and counting

D14449: Modify device usage information

[Nathaniel Graham]

ngraham added a comment.


  Do you plan to submit a new version of this after the layout has been 
converted to use `QFormLayout`? I do hope some form of this patch makes it in, 
since IMHO it's a nice little quality-of-life improvement.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx
Cc: pino, rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards

[Dominik Haumann]

dhaumann added a subscriber: dfaure.
dhaumann added a comment.


  Do you need the public methods in Definition outside of KSyntaxHighlighting?
  
  In general: +1
  
  If you want this to be in 5.50, you need to get a +2 from @vkrause and ping 
@dfaure to retag.

INLINE COMMENTS

> definition.h:196
> + */
> +QRegularExpression extensionsAsRegularExpression() const;
> +

@since 5.50 or 5.51

> definition.h:202
> + * @return regular expression matching them
> + */
> +static QRegularExpression convertExtensionsToRegularExpression(const 
> QVector );

@since 5.50 or 5.51

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: dfaure, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, 
bruns, demsking, cullmann, sars, dhaumann

D9987: Lessen log spam by not checking for existence of file with empty name

[Dominik Haumann]

dhaumann added a comment.


  @rkflx can you please reopen so we can accept this?

REPOSITORY
  R303 KInit

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

To: rkflx, #frameworks
Cc: dfaure, kde-frameworks-devel, dhaumann, michaelh, ngraham, bruns

D15076: Build failures with KSyntaxHighlighting 5.49

[Michael Pyne]

mpyne added a comment.


  The ECM documentation points to a specific change that may be related: 
https://api.kde.org/ecm/kde-module/KDECMakeSettings.html#build-settings
  
  Using the `KDE_SKIP_BUILD_SETTINGS` option that is described just leads to 
different build failures for me, however. I think it's just a matter of 
adjusting some buildsystem-specific paths to reflect the new locations with 
newer ECM.

REPOSITORY
  R55 Cantor

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

To: asemke, #kde_edu, #cantor, #frameworks
Cc: mpyne, cullmann, kde-edu, narvaez, apol

D15076: Build failures with KSyntaxHighlighting 5.49

[Michael Pyne]

mpyne added a comment.


  I don't know the cause myself but the ECM version works up until 5.38.0 in my 
own testing. So presumably the change in behavior is something introduced in 
that release of ECM?

REPOSITORY
  R55 Cantor

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

To: asemke, #kde_edu, #cantor, #frameworks
Cc: mpyne, cullmann, kde-edu, narvaez, apol

D15076: Build failures with KSyntaxHighlighting 5.49

[Christoph Cullmann]

cullmann edited reviewers, added: Frameworks; removed: Framework: Syntax 
Highlighting.
cullmann added a comment.


  Perhaps other frameworks people know the exact cause.

REPOSITORY
  R55 Cantor

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

To: asemke, #kde_edu, #cantor, #frameworks, #framework_syntax_highlighting
Cc: cullmann, kde-edu, narvaez, apol

D9987: Lessen log spam by not checking for existence of file with empty name

[David Faure]

dfaure added a comment.


  +2

REPOSITORY
  R303 KInit

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

To: rkflx, #frameworks
Cc: dfaure, kde-frameworks-devel, dhaumann, michaelh, ngraham, bruns

D15084: Change documentation to reflect the real toolchain CMake name

[Alexey Min]

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


  Looks harmless and correct

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  fix_android_toolchain_documentation

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

To: bport, apol, alexeymin
Cc: alexeymin, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D15084: Change documentation to reflect the real toolchain CMake name

[Benjamin Port]

bport created this revision.
bport added a reviewer: apol.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
bport requested review of this revision.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  fix_android_toolchain_documentation

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

AFFECTED FILES
  toolchain/Android.cmake

To: bport, apol
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D15000: Display mounted file system type and mounted from fields in properties dialog

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added reviewers: Frameworks, cfeck.
ngraham added a comment.
This revision is now accepted and ready to land.


  Works great and looks fine to me, and the code seems sensible too given that 
this dialog is currently implemented with a GridLayout. I'd like to see this 
ported to use a FormLayout at some point, which is both more semantically 
appropriate, and would also simplify this type of code. But that's material for 
another patch of course. :)
  
  Please wait for at least one more review before landing.

REPOSITORY
  R241 KIO

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

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

D13643: Add LabPlot project file icon

[Nathaniel Graham]

ngraham added a comment.


  Thanks again for your contribution! Next time, we'll try to be more on the 
ball about the review. :)

REPOSITORY
  R266 Breeze Icons

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

To: mtrescott, #labplot, #vdg, #breeze, ngraham
Cc: abetts, asemke, ngraham, kde-frameworks-devel, michaelh, bruns

D13643: Add LabPlot project file icon

[Nathaniel Graham]

ngraham closed this revision.

REPOSITORY
  R266 Breeze Icons

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

To: mtrescott, #labplot, #vdg, #breeze, ngraham
Cc: abetts, asemke, ngraham, kde-frameworks-devel, michaelh, bruns

D13643: Add LabPlot project file icon

[Matthew Trescott]

mtrescott added a comment.


  Sure.
  Real name: Matthew Trescott
  Email: matthewtresc...@gmail.com

REPOSITORY
  R266 Breeze Icons

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

To: mtrescott, #labplot, #vdg, #breeze, ngraham
Cc: abetts, asemke, ngraham, kde-frameworks-devel, michaelh, bruns

D15071: Don't draw frames and shadows around images with transparency

[Nathaniel Graham]

ngraham added a comment.


  @markg I'm in favor of keeping the conditional logic for the frames. If we 
think of this from an aesthetic point of view, we should draw frames and 
shadows around images that actually look better as a result. Those would be 
square and rectangular images with no transparency, which are pretty common for 
typical users. Images with transparency don't look good with the frames and 
shadows, so this patch turns them off. I don't think the inconsistency will 
bother people. On the contrary, the unnecessary *consistency* is what's 
bothering some people! :)
  
  I suspect you're right that originally, this feature was an attempt to mimic 
macOS Finder. Finder IMHO does a much worse job than we currently do or that we 
would do with this patch: it currently puts a frame around every image file 
unconditionally. It makes no attempt to detect icon files that look better with 
no frame, and it suffers from the "double frame" issue for window screenshots 
that include a shadow. I think if we land this patch and D15069 
, our file dialog and Dolphin will have a 
better behavior. :)

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, #vdg, broulik, cfeck
Cc: markg, abetts, bruns, kde-frameworks-devel, michaelh, ngraham

D15071: Don't draw frames and shadows around images with transparency

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, #vdg, broulik, cfeck
Cc: markg, abetts, bruns, kde-frameworks-devel, michaelh, ngraham

D15071: Don't draw frames and shadows around images with transparency

[Mark Gaiser]

markg added a comment.


  (repost from D15069 )
  
  Hi Nate,
  
  I'm afraid this will give inconsistent frames, at least that will be the 
perception of the user.
  The hasAlpha function on a QPixmap (probably) boils down to executing this 
function:
  
bool QX11PlatformPixmap::hasAlphaChannel() const


   
{   


   
if (picture && d == 32) 


   
return true;


   



   
if (x11_mask && d == 1) 


   
return true;


   



   
return false;   


   
} 
  
  But there are quite some places in Qt where the alpha channel checks are done 
differently.The above would be for X11, it's likely different on wayland, etc...
  
  So where is this going to be inconsistent? Well, with images that "have" an 
alpha channel but don't use it.
  This happens when saving an image. It's often a setting to keep transparency 
or not (it is in photoshop and gimp).
  Therefore you can - and will - have people that have images with an alpha 
channel but don't use it so they don't get a frame. While the very same image 
when saved differently (and in png as well) will have a frame. That's going to 
be a bug report from that user i guess ;)
  
  Having said that, i'm much more in favor of an all-or-nothing approach. Not 
some logic somewhere that dictates when an image has a frame and when it 
doesn't.
  We've had frames for years so perhaps it's time to just flip the switch and 
see how users like it. So just flip that switch by default to no frames.
  
  - end of previous comment.
  
  As @ngraham figured out, Qt apparently does some more in depth transparency 
checking. I wonder if it does a pixel-by-pixel test, that would be expensive.
  Anyhow, i'd just say - to be consistent - to flip the switch. No frames at 
all anywhere. It looks just weird to me to have folders where some images have 
frames and some don't.
  For users it will be weird as well, they might even consider it a bug and 
report it.
  
  It would be a whole different story if the frames were part of a larger area 
(so including the filename), kinda like this 
. But it isn't. It's 
merely directly around the image.
  It probably (assumption) was a way to imitate Finder (of macOS) and the old 
"Windows Live Photo Gallery" specially the later one has a nearly 1-on-1 
matching with the frame Dolphin draws.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, #vdg, broulik, cfeck
Cc: markg, abetts, bruns, kde-frameworks-devel, michaelh, ngraham

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards

[Christoph Cullmann]

cullmann added a comment.


  As Sebastian agreed to MIT,  we can think about this after the 5.50 release.

REPOSITORY
  R216 Syntax Highlighting

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

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

D15000: Display mounted file system type and mounted from fields in properties dialog

[Shubham]

shubham added a comment.


  ping?

REPOSITORY
  R241 KIO

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

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

D15071: Don't draw frames and shadows around images with transparency

[Nathaniel Graham]

ngraham retitled this revision from "Don't draw frames and shadows around 
images with an alpha channel" to "Don't draw frames and shadows around images 
with transparency".

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, #vdg, broulik, cfeck
Cc: markg, abetts, bruns, kde-frameworks-devel, michaelh, ngraham

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards

[Christoph Cullmann]

cullmann updated this revision to Diff 40435.
cullmann added a comment.


  avoid to use capture groups

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15081?vs=40429=40435

BRANCH
  master

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

AFFECTED FILES
  COPYING
  COPYING.LIB
  autotests/CMakeLists.txt
  autotests/definition_test.cpp
  autotests/syntaxrepository_test.cpp
  autotests/wildcardmatcher_test.cpp
  data/syntax/asterisk.xml
  data/syntax/gdbinit.xml
  data/syntax/gnuplot.xml
  data/syntax/pango.xml
  data/syntax/stata.xml
  src/indexer/katehighlightingindexer.cpp
  src/lib/CMakeLists.txt
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/definition_p.h
  src/lib/repository.cpp
  src/lib/wildcardmatcher.cpp
  src/lib/wildcardmatcher_p.h

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

D13643: Add LabPlot project file icon

[Nathaniel Graham]

ngraham added a comment.


  Whoops, sorry this was missed. I can merge it for you, since it looks great! 
In order to do this, I'll need to know your real name and preferred email 
address. Can you tell me those?

REPOSITORY
  R266 Breeze Icons

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

To: mtrescott, #labplot, #vdg, #breeze, ngraham
Cc: abetts, asemke, ngraham, kde-frameworks-devel, michaelh, bruns

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards

[Christoph Cullmann]

cullmann added a comment.


  For the accessor: I actually would prefer a const QRegularExpression & as 
return value, but most other methods use values to hide the internals.

REPOSITORY
  R216 Syntax Highlighting

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

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

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards

[Christoph Cullmann]

cullmann added a comment.


  Some trivial QBenchmark stuff tells
  
QBENCHMARK {
  m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.c"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.sdfdsf"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.ada"));
  m_repo->definitionForFileName(QLatin1String("12323.cklsjdf"));
  m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.c"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.sdfdsf"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.ada"));
  m_repo->definitionForFileName(QLatin1String("12323.cklsjdf"));
  m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.c"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.sdfdsf"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.ada"));
  m_repo->definitionForFileName(QLatin1String("12323.cklsjdf"));
  m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.c"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.sdfdsf"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.ada"));
  m_repo->definitionForFileName(QLatin1String("12323.cklsjdf"));
  m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.c"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.sdfdsf"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.ada"));
  m_repo->definitionForFileName(QLatin1String("12323.cklsjdf"));
  m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.c"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.sdfdsf"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.ada"));
  m_repo->definitionForFileName(QLatin1String("12323.cklsjdf"));
  m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.c"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.sdfdsf"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.ada"));
  m_repo->definitionForFileName(QLatin1String("12323.cklsjdf"));
  m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.c"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.sdfdsf"));
  
m_repo->definitionForFileName(QLatin1String("ksldjfklsdjfkl.ada"));
  m_repo->definitionForFileName(QLatin1String("12323.cklsjdf"));
  }
  
  tells:
  
  new: between 4 and 5 msecs per iteration
  old: between 5 and 6 msecs per iteration
  
  I think the main issue in the "very old code" was, that we did a QRegExp per 
extension, not a combined one.

REPOSITORY
  R216 Syntax Highlighting

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

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

D15071: Don't draw frames and shadows around images with an alpha channel

[Nathaniel Graham]

ngraham added a subscriber: markg.
ngraham added a comment.


  @bruns, done.
  
  @broulik, @markg I too was concerned about the case of images that have an 
alpha channel but not actual transparency. I did not actually have any images 
on my computer that had an alpha channel but no transparency, so I made one 
with GIMP: I took a PNG with an alpha channel and transparency and I cropped it 
to remove all the transparency, but I left the alpha channel. I then made 
another version of the same image that has no alpha channel. Both are correctly 
seen as not having any transparency and given a frame. See the two top-left 
images:
  
  F6215000: Alpha but no transparency.png 

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, #vdg, broulik, cfeck
Cc: markg, abetts, bruns, kde-frameworks-devel, michaelh, ngraham

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards

[Christoph Cullmann]

cullmann retitled this revision from "replace own wildcard matcher with 
QRegularExpression combining all wildcards add validator to index tool, only ? 
and * wildcard stuff is valid fix some syntax files that had e.g. , instead of 
; and spaces mixed in remove asterisk definition, that..." to "replace own 
wildcard matcher with QRegularExpression combining all wildcards".
cullmann edited the summary of this revision.

REPOSITORY
  R216 Syntax Highlighting

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

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

D14938: Force ki18n to build with the libintl.so path

[Hannah von Reth]

vonreth added a comment.


  Does anyone know why it prevents the usage of libintl? 
  And if there is a specific reason for that behaviour should we stop providing 
gettext on mac?

REPOSITORY
  R877 Craft Blueprints for KDE

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

To: sdepiets, #craft, vonreth, #frameworks
Cc: vonreth

KDE CI: Frameworks syndication kf5-qt5 WindowsMSVCQt5.11 - Build # 2 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20syndication%20kf5-qt5%20WindowsMSVCQt5.11/2/
 Project:
Frameworks syndication kf5-qt5 WindowsMSVCQt5.11
 Date of build:
Sat, 25 Aug 2018 16:07:20 +
 Build duration:
2 min 30 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 70 test(s), Skipped: 0 test(s), Total: 70 test(s)

D15071: Don't draw frames and shadows around images with an alpha channel

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, #vdg, broulik, cfeck
Cc: abetts, bruns, kde-frameworks-devel, michaelh, ngraham

D15071: Don't draw frames and shadows around images with an alpha channel

[Andres Betts]

abetts added a comment.


  I support this 100%

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, #vdg, broulik, cfeck
Cc: abetts, bruns, kde-frameworks-devel, michaelh, ngraham

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards add validator to index tool, only ? and * wildcard stuff is valid fix some syntax files that had e.g. , instead of

[Dominik Haumann]

dhaumann added a comment.


  The old wildcard matcher existed for performance reasons. Did you check?
  
  PS: When doing review requests, could you please have a nice subject, and not 
put very long lines into its stead? :-p Especially since this line will also 
appear in Davids release notes?

REPOSITORY
  R216 Syntax Highlighting

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

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

KDE CI: Frameworks syndication kf5-qt5 SUSEQt5.10 - Build # 1 - Successful!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20syndication%20kf5-qt5%20SUSEQt5.10/1/
 Project:
Frameworks syndication kf5-qt5 SUSEQt5.10
 Date of build:
Sat, 25 Aug 2018 15:28:12 +
 Build duration:
5 min 0 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 71 test(s), Skipped: 0 test(s), Total: 71 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report86%
(6/7)83%
(71/86)83%
(71/86)61%
(2347/3820)31%
(2697/8625)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(2/2)100%
(2/2)68%
(65/95)39%
(126/320)src70%
(16/23)70%
(16/23)69%
(454/660)42%
(574/1360)src.atom91%
(10/11)91%
(10/11)48%
(337/706)22%
(407/1885)src.mapper100%
(16/16)100%
(16/16)91%
(446/492)49%
(352/724)src.rdf87%
(20/23)87%
(20/23)65%
(805/1241)37%
(1037/2828)src.rss270%
(7/10)70%
(7/10)41%
(240/584)14%
(201/1418)tests0%
(0/1)0%
(0/1)0%
(0/42)0%
(0/90)

KDE CI: Frameworks syndication kf5-qt5 SUSEQt5.9 - Build # 1 - Successful!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20syndication%20kf5-qt5%20SUSEQt5.9/1/
 Project:
Frameworks syndication kf5-qt5 SUSEQt5.9
 Date of build:
Sat, 25 Aug 2018 15:28:09 +
 Build duration:
4 min 9 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 71 test(s), Skipped: 0 test(s), Total: 71 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report86%
(6/7)83%
(71/86)83%
(71/86)61%
(2347/3820)31%
(2678/8577)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(2/2)100%
(2/2)68%
(65/95)40%
(124/312)src70%
(16/23)70%
(16/23)69%
(454/660)42%
(567/1342)src.atom91%
(10/11)91%
(10/11)48%
(337/706)21%
(401/1871)src.mapper100%
(16/16)100%
(16/16)91%
(446/492)49%
(352/724)src.rdf87%
(20/23)87%
(20/23)65%
(805/1241)37%
(1037/2828)src.rss270%
(7/10)70%
(7/10)41%
(240/584)14%
(197/1410)tests0%
(0/1)0%
(0/1)0%
(0/42)0%
(0/90)

KDE CI: Frameworks syndication kf5-qt5 WindowsMSVCQt5.11 - Build # 1 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20syndication%20kf5-qt5%20WindowsMSVCQt5.11/1/
 Project:
Frameworks syndication kf5-qt5 WindowsMSVCQt5.11
 Date of build:
Sat, 25 Aug 2018 15:28:15 +
 Build duration:
2 min 22 sec and counting
   JUnit Tests
  Name: (root) Failed: 63 test(s), Passed: 7 test(s), Skipped: 0 test(s), Total: 70 test(s)Failed: TestSuite.Atom-atom10_authors_and_contributors.xmlFailed: TestSuite.Atom-atom10_authors_priority.xmlFailed: TestSuite.Atom-atom10_entry_category.xmlFailed: TestSuite.Atom-atom10_entry_id.xmlFailed: TestSuite.Atom-atom10_entry_link.xmlFailed: TestSuite.Atom-atom10_entry_summary.xmlFailed: TestSuite.Atom-atom10_entry_title.xmlFailed: TestSuite.Atom-atom10_feed_generator.xmlFailed: TestSuite.Atom-atom10_feed_icon.xmlFailed: TestSuite.Atom-atom10_feed_logo.xmlFailed: TestSuite.Atom-atom10_feed_rights.xmlFailed: TestSuite.Atom-atom10_feed_rights_escaped_markup.xmlFailed: TestSuite.Atom-atom10_feed_rights_inline_markup.xmlFailed: TestSuite.Atom-atom10_feed_rights_inline_markup_2.xmlFailed: TestSuite.Atom-atom10_feed_rights_text.xmlFailed: TestSuite.Atom-atom10_relative_uris.xmlFailed: TestSuite.Atom-bildblog.xmlFailed: TestSuite.Atom-blogspot.com-plaintext.xmlFailed: TestSuite.Atom-blogspot.com_heimwege.xmlFailed: TestSuite.Atom-bug-190068.xmlFailed: TestSuite.Atom-heise-atom.xmlFailed: TestSuite.Atom-qtblog.xmlFailed: TestSuite.Atom-rssowl.org.xmlFailed: TestSuite.Atom-tbray.org_nested_xmlbase.xmlFailed: TestSuite.Atom-wordpress2_atom03_example.xmlFailed: TestSuite.Rdf-heise.de-newsticker.xmlFailed: TestSuite.Rdf-javaworld.xmlFailed: TestSuite.Rdf-laut.de.xmlFailed: TestSuite.Rdf-rss090_channel_title.xmlFailed: TestSuite.Rdf-rss090_item_title.xmlFailed: TestSuite.Rdf-security_debian.rdf.xmlFailed: TestSuite.Rdf-slashdot.xmlFailed: TestSuite.Rdf-tagesschau.de.xmlFailed: TestSuite.Rss2-akregator_blog_rss2.xmlFailed: TestSuite.Rss2-assembla.xmlFailed: TestSuite.Rss2-chaosradio-podcast.xmlFailed: TestSuite.Rss2-cia_osterfeld.xmlFailed: TestSuite.Rss2-content-f.xmlFailed: TestSuite.Rss2-content-g.xmlFailed: TestSuite.Rss2-desc-a.xmlFailed: TestSuite.Rss2-desc-b.xmlFailed: TestSuite.Rss2-desc-c.xmlFailed: TestSuite.Rss2-desc-d.xmlFailed: TestSuite.Rss2-desc-e.xmlFailed: TestSuite.Rss2-enclosure.xmlFailed: TestSuite.Rss2-feedster_search_akregator.xmlFailed: TestSuite.Rss2-kde-apps.rss2.xmlFailed: TestSuite.Rss2-linux_org_ru.xmlFailed: TestSuite.Rss2-linuxfr_forums.xmlFailed: TestSuite.Rss2-livejournal.com_madfire.xmlFailed: TestSuite.Rss2-lunatic_fringe.xmlFailed: TestSuite.Rss2-pcinpact_news.xmlFailed: TestSuite.Rss2-pennyarcade.xmlFailed: TestSuite.Rss2-pro-linux.rss091.xmlFailed: TestSuite.Rss2-reddit.com.xmlFailed: TestSuite.Rss2-spreeblick.xmlFailed: TestSuite.Rss2-stefandecker.org.xmlFailed: TestSuite.Rss2-sueddeutsche.rss2.xmlFailed: TestSuite.Rss2-xhtml-h.xmlFailed: TestSuite.Rss2-xhtml-i.xmlFailed: TestSuite.Rss2-xhtml-j.xmlFailed: TestSuite.Rss2-xhtml-k.xmlFailed: TestSuite.Rss2-zeit.de.xml

KDE CI: Frameworks syndication kf5-qt5 FreeBSDQt5.11 - Build # 1 - Successful!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20syndication%20kf5-qt5%20FreeBSDQt5.11/1/
 Project:
Frameworks syndication kf5-qt5 FreeBSDQt5.11
 Date of build:
Sat, 25 Aug 2018 15:28:14 +
 Build duration:
1 min 1 sec and counting

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards add validator to index tool, only ? and * wildcard stuff is valid fix some syntax files that had e.g. , instead of

[Christoph Cullmann]

cullmann updated this revision to Diff 40429.
cullmann added a comment.


  improve definitionForName code, already for matching take into account the 
priority
  that saves some matching time and avoid later sorting

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15081?vs=40428=40429

BRANCH
  master

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

AFFECTED FILES
  COPYING
  COPYING.LIB
  autotests/CMakeLists.txt
  autotests/definition_test.cpp
  autotests/syntaxrepository_test.cpp
  autotests/wildcardmatcher_test.cpp
  data/syntax/asterisk.xml
  data/syntax/gdbinit.xml
  data/syntax/gnuplot.xml
  data/syntax/pango.xml
  data/syntax/stata.xml
  src/indexer/katehighlightingindexer.cpp
  src/lib/CMakeLists.txt
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/definition_p.h
  src/lib/repository.cpp
  src/lib/wildcardmatcher.cpp
  src/lib/wildcardmatcher_p.h

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

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards add validator to index tool, only ? and * wildcard stuff is valid fix some syntax files that had e.g. , instead of

[Christoph Cullmann]

cullmann updated this revision to Diff 40428.
cullmann added a comment.


  all code is now MIT
  only non-MIT parts are e.g. some generator scripts for highlighting and many 
highlighting data files

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15081?vs=40427=40428

BRANCH
  master

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

AFFECTED FILES
  COPYING
  COPYING.LIB
  autotests/CMakeLists.txt
  autotests/definition_test.cpp
  autotests/syntaxrepository_test.cpp
  autotests/wildcardmatcher_test.cpp
  data/syntax/asterisk.xml
  data/syntax/gdbinit.xml
  data/syntax/gnuplot.xml
  data/syntax/pango.xml
  data/syntax/stata.xml
  src/indexer/katehighlightingindexer.cpp
  src/lib/CMakeLists.txt
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/definition_p.h
  src/lib/repository.cpp
  src/lib/wildcardmatcher.cpp
  src/lib/wildcardmatcher_p.h

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

D13643: Add LabPlot project file icon

[Matthew Trescott]

mtrescott added a comment.


  Bump. Please, can someone merge this? For all that I read on Planet KDE about 
"streamlined onboarding of contributors" nothing has happened here.

REPOSITORY
  R266 Breeze Icons

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

To: mtrescott, #labplot, #vdg, #breeze, ngraham
Cc: abetts, asemke, ngraham, kde-frameworks-devel, michaelh, bruns

D15071: Don't draw frames and shadows around images with an alpha channel

[Stefan Brüns]

bruns added a comment.


  I think this is welcome.
  Nate, can you prepare a pair of screenshots where you have one icon per type 
(SVG, PNG, PNG+alpha, JPEG, ...), giving each file a speaking name?

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, #vdg, broulik, cfeck
Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards add validator to index tool, only ? and * wildcard stuff is valid fix some syntax files that had e.g. , instead of

[Christoph Cullmann]

cullmann added a comment.


  Rational for the static convertExtensionsToRegularExpression method: Allows 
to nice unit tests + allows e.g. KTextEditor to use that later, too, as 
replacement for its own copy of the wildcard matcher code for the modelines.

REPOSITORY
  R216 Syntax Highlighting

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

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

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards add validator to index tool, only ? and * wildcard stuff is valid fix some syntax files that had e.g. , instead of

[Christoph Cullmann]

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

REVISION SUMMARY
  ...will never match as it tried to match the file path and is anyways 'very' 
basic add auto-test for new regex stuff

TEST PLAN
  make && make test still works, autotest added

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/definition_test.cpp
  autotests/syntaxrepository_test.cpp
  autotests/wildcardmatcher_test.cpp
  data/syntax/asterisk.xml
  data/syntax/gdbinit.xml
  data/syntax/gnuplot.xml
  data/syntax/pango.xml
  data/syntax/stata.xml
  src/indexer/katehighlightingindexer.cpp
  src/lib/CMakeLists.txt
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/definition_p.h
  src/lib/repository.cpp
  src/lib/wildcardmatcher.cpp
  src/lib/wildcardmatcher_p.h

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

D14912: Bindings: Make generator forward compatible with Python 3

[Stefan Brüns]

bruns added a comment.


  In D14912#315093 , @arojas wrote:
  
  > Python3 still can't be used after this change: python2 is explicitely 
required at FindPythonModuleGeneration.cmake:177 (and all subsequent calls to 
sip_generator.py are done with ${GPB_PYTHON2_COMMAND}). This can be adjusted in 
another review though.
  
  
  Already there - D14914 . Also check 
D14915 , D15068 
, D15070 


REPOSITORY
  R240 Extra CMake Modules

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

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

D14914: Bindings: Use python version matching the found clang python module

[Stefan Brüns]

bruns added a dependency: D14912: Bindings: Make generator forward compatible 
with Python 3.

REPOSITORY
  R240 Extra CMake Modules

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

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

D14912: Bindings: Make generator forward compatible with Python 3

[Stefan Brüns]

bruns added a dependent revision: D14914: Bindings: Use python version matching 
the found clang python module.

REPOSITORY
  R240 Extra CMake Modules

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

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

Re: KDE CI: Frameworks kservice kf5-qt5 WindowsMSVCQt5.10 - Build # 41 - Still unstable!

[Ben Cooksley]

On Sun, Aug 26, 2018 at 12:00 AM David Faure  wrote:
>
> On vendredi 24 août 2018 14:00:12 CEST Ben Cooksley wrote:
> > > > I guess this is something the CI tooling will need to be handling?
> > >
> > > I'm confused, because surely something has been done already, no?
> > > Any lookup done by lib code has to find the files installed by that lib.
> > > I know we moved some stuff to qrc files, but afaik not everything.
> > > [though maybe the missing stuff isn't covered by unittests]
> >
> > We unpack everything needed into the install prefix yes.
> > CMake and the rest of the tools are more than happy to use the
> > resources in there as we can point them in that direction using PATH,
> > CMAKE_PREFIX_PATH, etc.
>
> OK. I guess using the install prefix for this made sense when we installed
> before running tests ;-)
> Now with the new logic it seems like a bit of a strange location (since the
> stuff isn't installed there), but why not, it makes no difference on Unix
> (it's the "future install prefix" now).

Yeah. To a certain extent it does still make sense as we need to
capture the stuff which is installed for later builds.

>
> > Qt of course when it comes to QStandardPaths can't be given that
> > direction, hence the test failures.
>
> Right. On Windows the only solution I can think of, is to copy that stuff into
> bin/data in the builddir.

I've now arranged for that to start happening.
Unfortunately it looks like KService is still unhappy, just with
different failure messages :)

>
> Indeed for a developer trying to work in a custom prefix on Windows, the setup
> isn't ideal. We still need a solution for that in QStandardPaths, but I don't
> know what it would be (env var? config file?). It's been hard to get this
> changed in Qt because any of this would not be "standard", breaking the name
> of the class :-)

I'd suggest an environment variable named something along the lines of
QT_DATA_PATH.
I know historically we've faced quite a bit of resistance from both
module maintainers as well as Qt Company employees on this whole thing
though.

>
> AFAICS a developer would have to either install/copy dependencies into
> build/bin/data or to install them into the actual standard paths.

Yes. Installing dependencies into build/bin/data/ isn't really
workable unfortunately as if you're building multiple things they'll
get mixed up with each other so it'll end up being copying in practice
I think.

>
> At runtime for deployed applications I think the KDE-windows people are fine
> with everything in bin/data?

>From my understanding that works perfectly yes.

>
> > > > My only concern about that though is it will make running tests
> > > > locally for devs harder as they'll need to know to do this.
> > >
> > > Devs don't install each framework into its own install prefix, so their
> > > situation is actually much easier than the CI situation :-)
> >
> > Ah, sorry it seems i've not explained what the CI system does here
> > completely. On Linux and FreeBSD we use the same install prefix for
> > everything - only populating it with what's needed for each build.
>
> Oh indeed. I remembered wrong.
>
> So /home/jenkins/install-prefix/share has the stuff needed by the current job
> (and only one job at a time can run). OK.

Yep.

>
> > On Windows we follow the same concept, with the difference that the
> > install prefix in question is always within the Jenkins workspace for
> > that build.
>
> I'm curious: why? Why not a "fixed" path like on Unix?

I set it up like this because on Windows it is extremely foreseeable
that software gets installed to a path which is wildly different
compared to what we build it with.
If we were to use a single fixed path then any code which relies on
this single fixed path would pass fine on the CI system, and then end
up failing on end-users systems which is not ideal.

>
> > In the case of a KService build for instance, it will find kdoctools,
> > ki18n, etc all within C:\CI\workspace\Frameworks kservice kf5-qt5
> > WindowsMSVCQt5.11/install-prefix/
> >
> > This should (theoretically at least) be reasonably close to a developers
> > system.
>
> Not on Windows, since QStanrdPaths won't find stuff in a random custom prefix
> :-)
>
> KDE-Windows people, any input?
>
> Otherwise, short term, if the CI could use build/bin instead as install prefix
> for the dependencies, it seems to me that it would help.

I've put code in place now to copy everything over, which should make
this stuff available to tests for now at least.
Ideally, we would have a solution like XDG_DATA_DIRS for Windows and
macOS (which is the proper solution to this)

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

Cheers,
Ben

D9987: Lessen log spam by not checking for existence of file with empty name

[Dominik Haumann]

dhaumann added a subscriber: dfaure.
dhaumann added a comment.


  @rkflx I think this patch is definitely good enough to not let it go.
  
  @dfaure Can you give another +1?

REPOSITORY
  R303 KInit

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

To: rkflx, #frameworks
Cc: dfaure, kde-frameworks-devel, dhaumann, michaelh, ngraham, bruns

Re: KDE CI: Frameworks kservice kf5-qt5 WindowsMSVCQt5.10 - Build # 41 - Still unstable!

[David Faure]

On vendredi 24 août 2018 14:00:12 CEST Ben Cooksley wrote:
> > > I guess this is something the CI tooling will need to be handling?
> > 
> > I'm confused, because surely something has been done already, no?
> > Any lookup done by lib code has to find the files installed by that lib.
> > I know we moved some stuff to qrc files, but afaik not everything.
> > [though maybe the missing stuff isn't covered by unittests]
> 
> We unpack everything needed into the install prefix yes.
> CMake and the rest of the tools are more than happy to use the
> resources in there as we can point them in that direction using PATH,
> CMAKE_PREFIX_PATH, etc.

OK. I guess using the install prefix for this made sense when we installed 
before running tests ;-)
Now with the new logic it seems like a bit of a strange location (since the 
stuff isn't installed there), but why not, it makes no difference on Unix
(it's the "future install prefix" now).

> Qt of course when it comes to QStandardPaths can't be given that
> direction, hence the test failures.

Right. On Windows the only solution I can think of, is to copy that stuff into 
bin/data in the builddir.

Indeed for a developer trying to work in a custom prefix on Windows, the setup 
isn't ideal. We still need a solution for that in QStandardPaths, but I don't 
know what it would be (env var? config file?). It's been hard to get this 
changed in Qt because any of this would not be "standard", breaking the name 
of the class :-)

AFAICS a developer would have to either install/copy dependencies into 
build/bin/data or to install them into the actual standard paths.

At runtime for deployed applications I think the KDE-windows people are fine 
with everything in bin/data?

> > > My only concern about that though is it will make running tests
> > > locally for devs harder as they'll need to know to do this.
> > 
> > Devs don't install each framework into its own install prefix, so their
> > situation is actually much easier than the CI situation :-)
> 
> Ah, sorry it seems i've not explained what the CI system does here
> completely. On Linux and FreeBSD we use the same install prefix for
> everything - only populating it with what's needed for each build.

Oh indeed. I remembered wrong.

So /home/jenkins/install-prefix/share has the stuff needed by the current job 
(and only one job at a time can run). OK.

> On Windows we follow the same concept, with the difference that the
> install prefix in question is always within the Jenkins workspace for
> that build.

I'm curious: why? Why not a "fixed" path like on Unix?

> In the case of a KService build for instance, it will find kdoctools,
> ki18n, etc all within C:\CI\workspace\Frameworks kservice kf5-qt5
> WindowsMSVCQt5.11/install-prefix/
> 
> This should (theoretically at least) be reasonably close to a developers
> system.

Not on Windows, since QStanrdPaths won't find stuff in a random custom prefix 
:-)

KDE-Windows people, any input?

Otherwise, short term, if the CI could use build/bin instead as install prefix 
for the dependencies, it seems to me that it would help.

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

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

[David Faure]

dfaure accepted this revision.
dfaure added a comment.


  Makes sense to me, actually. A unittest is difficult to write but an 
interactive test program would be an easy way to see the difference that this 
makes.
  
  Henrik: everything OK? I see you unsubscribed from a number of phabricator 
requests

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

To: brauch, cfeck, dfaure
Cc: dfaure, dhaumann, aacid, #frameworks, michaelh, ngraham, bruns

D14940: kformattest: Use plural suffix (s) consequently

[Ralf Habacker]

habacker added a comment.


  
https://forum.wordreference.com/threads/decimals-plural-singular.2054431/#post-10279600
 listed many public organisations that are using signular for values between 1 
and -1.

REPOSITORY
  R244 KCoreAddons

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

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

D15072: [server] Abort drag start on correct conditions and without posting error

[Roman Gilg]

romangg edited the test plan for this revision.

REPOSITORY
  R127 KWayland

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

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

D15074: [server] Do not try to create data offers without source

[Roman Gilg]

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

REVISION SUMMARY
  An internal drag is without data source. Still we tried to create offers.

TEST PLAN
  This change makes the updated autotest in D15072 
 pass without errors.

REPOSITORY
  R127 KWayland

BRANCH
  offerWithSourceOnly

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

AFFECTED FILES
  src/server/datadevice_interface.cpp

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

D15072: [server] Abort drag start on correct conditions and without posting error

[Roman Gilg]

romangg added a comment.


  I updated the autotest. Note that the old autotest was broken anyway because 
the wrong pointer button serial was used when the drag was started and by that 
aborted in SeatInterface. The new test fails on `testDragInternally` with:
  
QFATAL : TestDataDevice::testDragInternally(grab and focus) ASSERT: 
"source" in file 
/home/roman/dev/kde/src/frameworks/kwayland/src/server/dataoffer_interface.cpp, 
line 153
FAIL!  : TestDataDevice::testDragInternally(grab and focus) Received a 
fatal error.
  
  what is a different problem. A data offer should not be created when there is 
no data source. The old autotest did not capture this problem, since the drag 
was aborted on the SeatInterface.

REPOSITORY
  R127 KWayland

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

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

D15072: [server] Abort drag start on correct conditions and without posting error

[Roman Gilg]

romangg updated this revision to Diff 40406.
romangg added a comment.


  - Update autotest

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15072?vs=40403=40406

BRANCH
  startDragAbortFix

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

AFFECTED FILES
  autotests/client/test_datadevice.cpp
  src/server/datadevice_interface.cpp

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

D13906: add missing include

[Alexander Stippich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:9ba0d27ef263: add missing include (authored by astippich).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13906?vs=37210=40405

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

AFFECTED FILES
  src/embeddedimagedata.h

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

D15072: [server] Abort drag start on correct conditions and without posting error

[Roman Gilg]

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

REVISION SUMMARY
  A drag start request should be dismissed when the client does not have an
  implicit pointer grab or the currently focused pointer surface is not the
  origin. The conditions for that were wrong in the past.
  
  Also just ignore the request and not post directly an error, that potentially
  kills the client since by concurrency the client might have send a valid
  request, that got invalidated through grab or focus change at the same time
  on the server side.

TEST PLAN
  Manually.

REPOSITORY
  R127 KWayland

BRANCH
  startDragAbortFix

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

AFFECTED FILES
  src/server/datadevice_interface.cpp

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

D14912: Bindings: Make generator forward compatible with Python 3

[Antonio Rojas]

arojas added a comment.


  Python3 still can't be used after this change: python2 is explicitely 
required at FindPythonModuleGeneration.cmake:177 (and all subsequent calls to 
sip_generator.py are done with ${GPB_PYTHON2_COMMAND}). This can be adjusted in 
another review though.

REPOSITORY
  R240 Extra CMake Modules

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

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

D9987: Lessen log spam by not checking for existence of file with empty name

[Henrik Fehlauer]

rkflx abandoned this revision.
Herald added a subscriber: kde-frameworks-devel.

REPOSITORY
  R303 KInit

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

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