D19324: Add code-oss icon

2019-02-26 Thread Noah Davis
ndavis added a comment.


  You also need to optimize the SVG. You can do that by using one of the three 
SVG optimizers on this page: 
https://invent.kde.org/ndavis/hig-kde-org/wikis/Icon-Workflow#svg-optimization
  
  I should add that to the HIG at some point in the future.

REPOSITORY
  R266 Breeze Icons

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

To: axionl, #vdg
Cc: ndavis, rooty, ngraham, kde-frameworks-devel, michaelh, bruns


D19324: Add code-oss icon

2019-02-26 Thread Noah Davis
ndavis added a comment.


  I see you've kept the green on the light theme version instead of using the 
same icon for both versions. Is there a reason for this or do you just prefer 
it?
  
  This looks just about ready to land on the master branch, there are just a 
few more things you need to do.

INLINE COMMENTS

> code.svg:903
> +   height="10.58" />
> + +   
> style="opacity:0.35;fill:none;fill-opacity:1;stroke:none;stroke-width:0.26458332"

Delete this invisible rectangle.

> code.svg:884
> +   height="10.58" />
> + +   
> style="opacity:0.35;fill:none;fill-opacity:1;stroke:none;stroke-width:0.26458332"

Delete this invisible rectangle.

REPOSITORY
  R266 Breeze Icons

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

To: axionl, #vdg
Cc: ndavis, rooty, ngraham, kde-frameworks-devel, michaelh, bruns


D18951: HTML: highlight JSX, TypeScript & MustacheJS code in the

2019-02-26 Thread Nibaldo González
nibags added a comment.


  The MustacheJS highlight is modified in this diff: D19328 


REPOSITORY
  R216 Syntax Highlighting

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

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


D19328: MustacheJS: also highlight template files, fix syntax and improve support for Handlebars

2019-02-26 Thread Nibaldo González
nibags closed this revision.

REPOSITORY
  R216 Syntax Highlighting

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

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


KDE CI: Frameworks » baloo » kf5-qt5 SUSEQt5.12 - Build # 21 - Still Unstable!

2019-02-26 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/baloo/job/kf5-qt5%20SUSEQt5.12/21/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Wed, 27 Feb 2019 06:18:34 +
 Build duration:
11 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5Baloo-5.56.0.xmlcompat_reports/KF5Baloo_compat_report.htmllogs/KF5Baloo/5.56.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.autotests Failed: 0 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 4 test(s)Name: projectroot.autotests.unit Failed: 2 test(s), Passed: 31 test(s), Skipped: 0 test(s), Total: 33 test(s)Failed: projectroot.autotests.unit.file.filewatchtestFailed: projectroot.autotests.unit.file.kinotifytest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report43%
(10/23)60%
(100/168)60%
(100/168)57%
(5383/9501)40%
(2126/5250)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.benchmarks100%
(2/2)100%
(2/2)100%
(58/58)100%
(22/22)autotests.integration100%
(3/3)100%
(3/3)92%
(373/406)76%
(168/220)autotests.unit.codecs100%
(3/3)100%
(3/3)100%
(70/70)64%
(23/36)autotests.unit.engine100%
(17/17)100%
(17/17)100%
(752/752)55%
(212/382)autotests.unit.file91%
(10/11)91%
(10/11)88%
(794/905)48%
(256/530)autotests.unit.lib67%
(4/6)67%
(4/6)87%
(358/411)43%
(80/184)src.codecs100%
(5/5)100%
(5/5)89%
(130/146)76%
(35/46)src.engine95%
(35/37)95%
(35/37)81%
(1732/2146)60%
(688/1151)src.file39%
(15/38)39%
(15/38)39%
(652/1666)37%
(387/1058)src.file.extractor0%
(0/6)0%
(0/6)0%
(0/182)0%
(0/76)src.kioslaves.kded0%
(0/1)0%
(0/1)0%
(0/38)0%
(0/42)src.kioslaves.search0%
(0/1)0%
(0/1)0%
(0/105)0%
(0/32)src.kioslaves.tags0%
(0/1)0%
(0/1)0%
(0/273)0%
(0/235)src.kioslaves.timeline0%
(0/2)0%
(0/2)0%
(0/211)0%
(0/119)src.lib55%
(6/11)55%
(6/11)49%
(464/945)45%
(255/573)src.qml0%
(0/2)0%
(0/2)0%
(0/69)0%
 

D19367: [RFC]SearchBar: Don't block GUI when enter incremental pattern

2019-02-26 Thread loh tar
loh.tar added a comment.


  To my surprise are two test fail.
  SearchBarTest::testFindNextIncremental()
  SearchBarTest::testSetMatchCaseIncremental()
  Doing the same by hand looks OK...hm

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

To: loh.tar, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann


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

2019-02-26 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/baloo/job/kf5-qt5%20SUSEQt5.10/28/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Wed, 27 Feb 2019 06:18:34 +
 Build duration:
3 min 32 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5Baloo-5.56.0.xmlcompat_reports/KF5Baloo_compat_report.htmllogs/KF5Baloo/5.56.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.autotests Failed: 0 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 4 test(s)Name: projectroot.autotests.unit Failed: 1 test(s), Passed: 32 test(s), Skipped: 0 test(s), Total: 33 test(s)Failed: projectroot.autotests.unit.file.filewatchtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report43%
(10/23)60%
(100/168)60%
(100/168)57%
(5405/9501)41%
(2141/5250)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.benchmarks100%
(2/2)100%
(2/2)100%
(58/58)100%
(22/22)autotests.integration100%
(3/3)100%
(3/3)92%
(373/406)76%
(168/220)autotests.unit.codecs100%
(3/3)100%
(3/3)100%
(70/70)64%
(23/36)autotests.unit.engine100%
(17/17)100%
(17/17)100%
(752/752)55%
(212/382)autotests.unit.file91%
(10/11)91%
(10/11)90%
(816/905)51%
(270/530)autotests.unit.lib67%
(4/6)67%
(4/6)87%
(358/411)43%
(80/184)src.codecs100%
(5/5)100%
(5/5)89%
(130/146)76%
(35/46)src.engine95%
(35/37)95%
(35/37)81%
(1732/2146)60%
(688/1151)src.file39%
(15/38)39%
(15/38)39%
(652/1666)37%
(388/1058)src.file.extractor0%
(0/6)0%
(0/6)0%
(0/182)0%
(0/76)src.kioslaves.kded0%
(0/1)0%
(0/1)0%
(0/38)0%
(0/42)src.kioslaves.search0%
(0/1)0%
(0/1)0%
(0/105)0%
(0/32)src.kioslaves.tags0%
(0/1)0%
(0/1)0%
(0/273)0%
(0/235)src.kioslaves.timeline0%
(0/2)0%
(0/2)0%
(0/211)0%
(0/119)src.lib55%
(6/11)55%
(6/11)49%
(464/945)45%
(255/573)src.qml0%
(0/2)0%
(0/2)0%
(0/69)0%
(0/20)src.qml.experimental0%
  

D19317: compile without foreach

2019-02-26 Thread Laurent Montel
mlaurent updated this revision to Diff 52700.
mlaurent added a comment.


  Use auto as requested

REPOSITORY
  R287 KImageFormats

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19317?vs=52559&id=52700

BRANCH
  compile_without_foreach (branched from master)

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

AFFECTED FILES
  CMakeLists.txt
  autotests/readtest.cpp
  autotests/writetest.cpp
  src/imageformats/pic.cpp
  tests/imageconverter.cpp
  tests/imagedump.cpp

To: mlaurent, dfaure
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns


D19317: compile without foreach

2019-02-26 Thread Laurent Montel
mlaurent added inline comments.

INLINE COMMENTS

> apol wrote in pic.cpp:391
> Probably could do a splitRef here

it seems that we can't use it.
As it will return a list of QStringRef
and we want to use "simplified() on a QStringRef which doesn't exist

REPOSITORY
  R287 KImageFormats

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

To: mlaurent, dfaure
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns


KDE CI: Frameworks » kpty » kf5-qt5 FreeBSDQt5.12 - Build # 8 - Still Unstable!

2019-02-26 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kpty/job/kf5-qt5%20FreeBSDQt5.12/8/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Wed, 27 Feb 2019 05:51:58 +
 Build duration:
1 min 17 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: projectroot.autotests.kptyprocesstest

D19319: make it compile without foreach

2019-02-26 Thread Laurent Montel
mlaurent updated this revision to Diff 52699.
mlaurent added a comment.


  Remove not necessary change

REPOSITORY
  R291 KPty

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19319?vs=52561&id=52699

BRANCH
  compile_without_foreach (branched from master)

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

AFFECTED FILES
  CMakeLists.txt

To: mlaurent, dfaure
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns


D12336: Replace several Q_ASSERTs with proper checks

2019-02-26 Thread Stefan Brüns
bruns added a comment.


  In D12336#333853 , @apol wrote:
  
  > If it was silently corrupting the DB maybe the right solution would have 
been to integrate baloo properly with KCrash.
  >  Like @sitter did in https://phabricator.kde.org/D15573.
  
  
  **Silently** - it does not crash, garbage data ends up in the database for 
(mostly) unknown reasons.
  
  Only known reason for zero IDs are races due to renames and similar, where 
e.g. a file is added with some path, and the parent does not exist microseconds 
later (at least not under the previous name).

REPOSITORY
  R293 Baloo

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

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


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

2019-02-26 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.12/38/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Wed, 27 Feb 2019 01:34:32 +
 Build duration:
6 min 29 sec and counting
   JUnit Tests
  Name: projectroot Failed: 5 test(s), Passed: 47 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_dropjobtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestFailed: projectroot.autotests.kiowidgets_kurifiltertestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: projectroot.src.ioslaves.trash.tests.testtrashName: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

KDE CI: Frameworks » kdesu » kf5-qt5 FreeBSDQt5.12 - Build # 9 - Still Unstable!

2019-02-26 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kdesu/job/kf5-qt5%20FreeBSDQt5.12/9/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Wed, 27 Feb 2019 01:40:37 +
 Build duration:
43 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: projectroot.autotests.kdesutest

D19324: Add code-oss icon

2019-02-26 Thread Ariel AxionL
axionl edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: axionl, #vdg
Cc: ndavis, rooty, ngraham, kde-frameworks-devel, michaelh, bruns


D19324: Add code-oss icon

2019-02-26 Thread Ariel AxionL
axionl updated this revision to Diff 52696.
axionl added a comment.


  - Merge branch 'master' into code-oss
  
  1. Updating D19324 : Add code-oss icon #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create
  
  Try to fix the layer and shadows.

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19324?vs=52588&id=52696

BRANCH
  code-oss (branched from master)

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

AFFECTED FILES
  icons-dark/apps/48/code-oss.svg
  icons-dark/apps/48/code.svg
  icons/apps/48/code-oss.svg
  icons/apps/48/code.svg

To: axionl, #vdg
Cc: ndavis, rooty, ngraham, kde-frameworks-devel, michaelh, bruns


D19380: # Enter a commit message. # # Changes: # # icons-dark/apps/48/code.svg # icons/apps/48/code.svg

2019-02-26 Thread Ariel AxionL
axionl abandoned this revision.

REPOSITORY
  R266 Breeze Icons

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

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


D19380: # Enter a commit message. # # Changes: # # icons-dark/apps/48/code.svg # icons/apps/48/code.svg

2019-02-26 Thread Stefan Brüns
bruns added a comment.


  Just "Abandom" it, at the very bottom of the page ("Add Action...").

REPOSITORY
  R266 Breeze Icons

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

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


D18915: Fix batchrename changing extension to lower case

2019-02-26 Thread Stefan Brüns
bruns added inline comments.

INLINE COMMENTS

> batchrenamejob.cpp:160
>  
> +//The function QMimeDatabase.suffixForFileName always converts the extension 
> to lower case
> +//This function is a wrapper which avoids the coversion to lower case

Space after `//`

> batchrenamejob.cpp:161
> +//The function QMimeDatabase.suffixForFileName always converts the extension 
> to lower case
> +//This function is a wrapper which avoids the coversion to lower case
> +QString BatchRenameJobPrivate::GetFileExtension(QUrl url)

helper, not wrapper

> batchrenamejob.cpp:161
> +//The function QMimeDatabase.suffixForFileName always converts the extension 
> to lower case
> +//This function is a wrapper which avoids the coversion to lower case
> +QString BatchRenameJobPrivate::GetFileExtension(QUrl url)

`co_n_version`

> batchrenamejob.cpp:165
> +QMimeDatabase db;
> +const size_t extensionLen = 
> db.suffixForFileName(url.toDisplayString()).length();
> + 

I think you can use `QUrl::filename()`, and you should do it from the calling 
code.

REPOSITORY
  R241 KIO

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

To: cfoster, #dolphin, #frameworks, abalaji
Cc: bruns, ngraham, elvisangelaccio, chinmoyr, kde-frameworks-devel, michaelh


D18826: Rewrite the taglib extractor to use the generic PropertyMap interface

2019-02-26 Thread James Smith
smithjd added inline comments.

INLINE COMMENTS

> astippich wrote in taglibextractor.cpp:195
> I wanted to do so first, but that will require to also put the PropertyMap 
> into the extractAsfTags method, which I think is not worth it.

lstASF = asfTags->attribute("WM/Writer");
...

The existing code does this.

REPOSITORY
  R286 KFileMetaData

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

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


D19380: # Enter a commit message. # # Changes: # # icons-dark/apps/48/code.svg # icons/apps/48/code.svg

2019-02-26 Thread Ariel AxionL
axionl added a comment.


  I make a mistake to create a new revision, please delete this one.

REPOSITORY
  R266 Breeze Icons

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

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


D19380: # Enter a commit message. # # Changes: # # icons-dark/apps/48/code.svg # icons/apps/48/code.svg

2019-02-26 Thread Ariel AxionL
axionl created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
axionl requested review of this revision.

REVISION SUMMARY
  Tyr to fix the layer and the shadow

REPOSITORY
  R266 Breeze Icons

BRANCH
  code-oss

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

AFFECTED FILES
  icons-dark/apps/48/code.svg
  icons/apps/48/code.svg

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


D18915: Fix batchrename changing extension to lower case

2019-02-26 Thread cfoster
cfoster updated this revision to Diff 52691.
cfoster added a comment.


  
  
  - Add function for getting extension All unit tests pass Manually tested 
invalid extensions. Files with no extensions and files with upper and lower 
case extensions

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18915?vs=52180&id=52691

BRANCH
  arcpatch-D18915

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

AFFECTED FILES
  src/core/batchrenamejob.cpp

To: cfoster, #dolphin, #frameworks, abalaji
Cc: bruns, ngraham, elvisangelaccio, chinmoyr, kde-frameworks-devel, michaelh


D19371: Tags ioslave: Do not crash for the "tags:/" URL

2019-02-26 Thread Nathaniel Graham
ngraham accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R293 Baloo

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

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


D19328: MustacheJS: also highlight template files, fix syntax and improve support for Handlebars

2019-02-26 Thread Christoph Cullmann
cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.


  No objections from my side.
  Thanks.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  update-mustache

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

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


T3689: Add abi compliance checker to CI

2019-02-26 Thread Sandro Knauß
knauss added a revision: D19377: check-abi: refactor script.

TASK DETAIL
  https://phabricator.kde.org/T3689

To: knauss
Cc: danders, davidedmundson, dfaure, kde-frameworks-devel, bcooksley, sysadmin, 
scarlettclark, aacid, knauss, alexeymin, kaning, blazquez


T3689: Add abi compliance checker to CI

2019-02-26 Thread Sandro Knauß
knauss added a revision: D19376: remove unnessary include..

TASK DETAIL
  https://phabricator.kde.org/T3689

To: knauss
Cc: danders, davidedmundson, dfaure, kde-frameworks-devel, bcooksley, sysadmin, 
scarlettclark, aacid, knauss, alexeymin, kaning, blazquez


T3689: Add abi compliance checker to CI

2019-02-26 Thread Sandro Knauß
knauss added a revision: D19375: check-abi: make it possible to break the 
build..

TASK DETAIL
  https://phabricator.kde.org/T3689

To: knauss
Cc: danders, davidedmundson, dfaure, kde-frameworks-devel, bcooksley, sysadmin, 
scarlettclark, aacid, knauss, alexeymin, kaning, blazquez


T3689: Add abi compliance checker to CI

2019-02-26 Thread Sandro Knauß
knauss added a revision: D19374: create-abi-dump: set logging for 
paramiko.transport to WARNING.

TASK DETAIL
  https://phabricator.kde.org/T3689

To: knauss
Cc: danders, davidedmundson, dfaure, kde-frameworks-devel, bcooksley, sysadmin, 
scarlettclark, aacid, knauss, alexeymin, kaning, blazquez


D19371: Tags ioslave: Do not crash for the "tags:/" URL

2019-02-26 Thread Jonathan Marten
marten created this revision.
marten added a reviewer: Baloo.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
marten requested review of this revision.

REVISION SUMMARY
  Bug https://bugs.kde.org/show_bug.cgi?id=400594 was fixed by 
https://commits.kde.org/baloo/f4dd3f7bab790734b47a31c70fbb68297d4d3062
  which checks for the URL starting with "tags://" and considering this to be 
an invalid URL.  This is fine, but the problem is that if the URL "tags:/" is
  passed in the string is not long enough and QString::at() asserts.  This is 
seen as a crash of the ioslave every time the right click menu is popped up
  in Konqueror (on any sort of view), or from the command line by doing 
"kioclient5 ls tags:/":
  
  kio_tags(11869) unknown: ASSERT: "uint(i) < uint(size())" in file 
/usr/include/qt5/QtCore/qstring.h, line 934
  
  #10 0x7f3d402fe0bc in qt_assert(char const*, char const*, int) () from 
/usr/lib64/libQt5Core.so.5
  #11 0x7f3d36eec337 in QString::at (i=6, this=0x7ffd4531d8a8) at 
/usr/include/qt5/QtCore/qstring.h:934
  #12 Baloo::TagsProtocol::parseUrl (this=this@entry=0x7ffd4531da60, url=..., 
flags=...) at baloo/baloo/src/kioslaves/tags/kio_tags.cpp:299
  #13 0x7f3d36eedda6 in Baloo::TagsProtocol::listDir (this=0x7ffd4531da60, 
url=...) at baloo/baloo/src/kioslaves/tags/kio_tags.cpp:59
  #14 0x7f3d3663e1de in KIO::SlaveBase::dispatch (this=0x7ffd4531da70, 
command=71, data=...) at kio/src/core/slavebase.cpp:1176
  #15 0x7f3d3663f78e in KIO::SlaveBase::dispatchLoop (this=0x7ffd4531da70) 
at kio/src/core/slavebase.cpp:325
  #16 0x7f3d36ef1943 in kdemain (argc=, argv=) at baloo/baloo/src/kioslaves/tags/kio_tags.cpp:486
  
  The QString::at() needs to be guarded by a length check.

TEST PLAN
  Built Baloo with this change, verified that no crash happens when using the 
URL "tags:/"

REPOSITORY
  R293 Baloo

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

AFFECTED FILES
  src/kioslaves/tags/kio_tags.cpp

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


D19087: Add standalone conversion functions for PropertyMap to Json and vice versa

2019-02-26 Thread Alexander Stippich
astippich added a comment.


  I get the feeling that we need to discuss how the PropertyMap should look 
like and how we achieve a seamless transition in a backwards compatible way.
  I have slowly been working to resolve T8196 
, which is why I am so insisting. This is 
the last missing piece.
  Currently, the clients expect a list in the map when there are multiple 
values, probably since the inception of baloo. All other entries are simply 
ignored. If the map is changed to having multiple entries with duplicate keys, 
then we have to adjust all clients and make sure that all possible combinations 
of applications and frameworks continue to work. 
  Additionally, baloo's serialization/deserialization does currently alter the 
structure of the map. Multiple values with the same key will be merged into a 
list afterwards. This causes issues in baloo-widgets for example, which breaks 
for multiple properties when baloo indexing is off. Baloo-widget then uses KFM 
directly, which results in a different PropertyMap. 
  Considering this, my plan was that to insert multiple values as single entry 
containing a list. Once all extractors are converted, the merging of multiple 
values into lists, currently done before serialization in baloo, will be 
removed. This yields a backwards compatible way of fixing the issue with 
multiple entries (That is also what D17302  
is for). This demands that the same list is retrieved after deserialization, 
with no merged values, e.g. additional entries in the list, during the 
serialization.
  
  So there are a few things that I would like to consider:
  
  - The same PropertyMap is generated either by baloo or KFM directly. Keeping 
the PropertyMap the same is only possible to a certain extend with JSON, e.g. 
with ints becoming doubles etc...
  - Extractors will insert multiple entries as a list.
  - Multiple extractors for the same mime type may insert duplicate entries.  
This would  allow clients to easily differentiate between data from multiple 
extractors in the future.

INLINE COMMENTS

> bruns wrote in propertydata.cpp:80
> > All clients currently rely on having string lists (**or variantlists**)
> 
> Actually, string lists make the code more complicated, as the code already 
> has to handle variant lists. There are no `QDoublelists`, `QIntlists` ... 
> baloo-widgets treats a `QVariantList` with string entries exactly like a 
> `QStringlist`, and dolphin uses baloo-widgets.
> 
> > It is also what is advertised in KFileMetaData.
> 
> KFileMetaData does not specify in any way how values for repeated properties 
> are retuned. It explicitly allows calling ExtractionResult::add() with the 
> same property multiple times, but that's it. PropertyMap is just a QMap<> 
> typedef, and does not specify any behavior. For me, it seems much more 
> natural to expect multiple individual entries for the same key, which should 
> be retrieved with QMap::values().
> 
> Currently, baloo-widgets 
> (https://cgit.kde.org/baloo-widgets.git/tree/src/filemetadatawidget.cpp#n152) 
>  only uses `data[key]`, i.e. it only retrieves the first value, but fixing 
> this is a single line change (i.e. replace `data[key]` by 
> `QVariant(data.values(key))`).
> 
> > Having it as string lists also makes the handling of them so much easier. 
> > It can be handled with one call to QVariant::toStringList().join().
> 
> `QVariant::toStringList()` also works on `QVariantLists` - 
> https://doc.qt.io/qt-5/qvariant.html#toStringList

> Actually, string lists make the code more complicated, as the code already 
> has to handle variant lists. There are no QDoublelists, QIntlists ... baloo- 
> widgets treats a QVariantList with string entries exactly like a QStringlist, 
> and dolphin uses baloo-widgets.

Dolphin has its own handling for the detailed view and does not use 
baloo-widgets there.

I think here is actually a misunderstanding, by supporting string lists I meant 
that if one inserts a list, one also gets the same list after deserialization. 
I actually do not care if it is a string list before and a variant list 
afterwards, that is how the current code already works.

> KFileMetaData does not specify in any way how values for repeated properties 
> are retuned. It explicitly allows calling ExtractionResult::add() with the 
> same property multiple times, but that's it. PropertyMap is just a QMap<> 
> typedef, and does not specify any behavior. For me, it seems much more 
> natural to expect multiple individual entries for the same key, which should 
> be retrieved with QMap::values().

You are right, ExtractionResult explicitly states that multiple calls are 
allowed. IMHO this should be more specified to "can be called multiple times 
from different extractors".

> Currently, baloo-widgets 
> (https://cgit.kde.org/baloo-widgets.git/tree/src/filemetadatawidget.cpp#n152) 
> only uses data[key], i.e. it only retrieves the first v

D18826: Rewrite the taglib extractor to use the generic PropertyMap interface

2019-02-26 Thread Alexander Stippich
astippich updated this revision to Diff 52679.
astippich added a comment.


  - remove now unnecessary include

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18826?vs=52675&id=52679

BRANCH
  rewrite_taglib_extractor

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

AFFECTED FILES
  src/extractors/taglibextractor.cpp
  src/extractors/taglibextractor.h

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


D18826: Rewrite the taglib extractor to use the generic PropertyMap interface

2019-02-26 Thread Alexander Stippich
astippich marked an inline comment as done.
astippich added inline comments.

INLINE COMMENTS

> smithjd wrote in taglibextractor.cpp:195
> Can this be moved into the asf-specific extractions?

I wanted to do so first, but that will require to also put the PropertyMap into 
the extractAsfTags method, which I think is not worth it.

REPOSITORY
  R286 KFileMetaData

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

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


D18826: Rewrite the taglib extractor to use the generic PropertyMap interface

2019-02-26 Thread Alexander Stippich
astippich updated this revision to Diff 52675.
astippich added a comment.


  - return early if map is empty

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18826?vs=51128&id=52675

BRANCH
  rewrite_taglib_extractor

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

AFFECTED FILES
  src/extractors/taglibextractor.cpp
  src/extractors/taglibextractor.h

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


D16579: Remove support for non-standard APE tag field names

2019-02-26 Thread Alexander Stippich
astippich added a comment.


  In D16579#418256 , @smithjd wrote:
  
  > > Anyway, most of this special handling of mimetypes goes away with D18826 
, where TagLib will do the conversion 
automatically.
  >
  > The test files should reflect the most common tag fieldnames; 'albumartist' 
is used in the APEv2 test files instead of 'album artist'.
  
  
  I have no problem with only adjusting the test files, but please make sure 
that the new files do not break the embeddimagedatatest.

REPOSITORY
  R286 KFileMetaData

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

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


D19367: [RFC]SearchBar: Don't block GUI when enter incremental pattern

2019-02-26 Thread loh tar
loh.tar updated this revision to Diff 52669.
loh.tar added a comment.


  - Fix my test chunkSize to some real value

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19367?vs=52660&id=52669

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

AFFECTED FILES
  src/search/katesearchbar.cpp
  src/search/katesearchbar.h

To: loh.tar, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann


D18793: Handle text completion with block selection mode

2019-02-26 Thread loh tar
loh.tar added a comment.


  Fix this patch also Bug 382213 ? - do not do auto-brackets when block mode is 
enabled
  Well, bad title/request. I would expect that each line get the bracket.

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, #ktexteditor, cullmann, dhaumann, #kdevelop, mwolff
Cc: loh.tar, mwolff, kde-frameworks-devel, kwrite-devel, #ktexteditor, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann


D19367: [RFC]SearchBar: Don't block GUI when enter incremental pattern

2019-02-26 Thread loh tar
loh.tar created this revision.
loh.tar added a reviewer: KTextEditor.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
loh.tar requested review of this revision.

REVISION SUMMARY
  When the document is very big and there is no early match while you enter the 
  search pattern the GUI hangs as long the search is running after each key 
  stroke. This patch break the document in smaller chunks to search to return 
fast 
  enough to the event loop to update user input.
  
  - Disable next/prev buttons when not match
  
  BUG:339337

TEST PLAN
  (No)Issues;
  
  - There may a range added where fromPos = toPos. Happens when start from 
first or last position of the doc. For my taste is that better than to check 
every time that very rare case
  
  Potential TODOs:
  
  - See code
  - Merge selectRange2 into indicateMatch
  - There is a status (KSqueezedTextLabel) set in indicateMatch, but I have 
never seen this hint. Tinker the UI file make it visible. Not so bad. Do anyone 
knows why this is still there? May that be better to use instead of the fancy 
in-view-hint which is sometimes annoying because it hides the text? Then Ctrl-H 
would only use fancy in-view-hint when no bar is visible. PowerUi would need 
such label too.

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/search/katesearchbar.cpp
  src/search/katesearchbar.h

To: loh.tar, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann


D18793: Handle text completion with block selection mode

2019-02-26 Thread Ahmad Samir
ahmadsamir added a comment.


  I changed the diff to make replaceText handle inserting the word completion 
text.
  
  Is this a new behaviour? it exactly matches what users expect when they type 
something in block selection mode.
  
  Also why would this be opt-in? if you type something in block selection is 
get replicated on all lines, so it follows that word completion should do the 
same thing...
  
  Also note that the completion-with-remove-tail bits will have to stay in 
executeCompletion code, because we iterate over each line to find the tail, 
there won't be a removeText/replaceText that fits all the tails...
  
  As for a unit test, I'll look into that as I haven't written one before.

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, #ktexteditor, cullmann, dhaumann, #kdevelop, mwolff
Cc: mwolff, kde-frameworks-devel, kwrite-devel, #ktexteditor, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann


D18793: Handle text completion with block selection mode

2019-02-26 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 52658.
ahmadsamir added a comment.


  Change replaceText to handle inserting word completion with block selection 
mode

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18793?vs=51051&id=52658

BRANCH
  block-selection-completion-2 (branched from master)

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

AFFECTED FILES
  src/completion/katewordcompletion.cpp
  src/document/katedocument.cpp

To: ahmadsamir, #ktexteditor, cullmann, dhaumann, #kdevelop, mwolff
Cc: mwolff, kde-frameworks-devel, kwrite-devel, #ktexteditor, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann


D19080: [WIP] Make file overwrite a bit safer

2019-02-26 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

BRANCH
  safe-overwrite

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

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


D19080: [WIP] Make file overwrite a bit safer

2019-02-26 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.


  Brilliant, works great!

REPOSITORY
  R241 KIO

BRANCH
  safe-overwrite

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

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


D19027: Fix "Invalid URL: QUrl("some.txt")" warnings in Save dialog

2019-02-26 Thread Peter Wu
Lekensteyn added a comment.


  Ping?

REPOSITORY
  R241 KIO

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

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


D19016: Update Android toolchain files to reality

2019-02-26 Thread Johnny Jazeix
jjazeix added a comment.


  In D19016#420246 , @apol wrote:
  
  > Applications can still override it, note it's a default.
  >  Also note this: 
https://developer.android.com/distribute/best-practices/develop/target-sdk
  
  
  Ok, good for me :).
  Thanks for the link!

REPOSITORY
  R240 Extra CMake Modules

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

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


D19332: KProcess compile fix for obsolete QProcess members in 5.13

2019-02-26 Thread Jonathan Marten
marten added inline comments.

INLINE COMMENTS

> apol wrote in kprocess.h:332
> Also should check for QT_DISABLE_DEPRECATED_BEFORE no?

Do you mean conditionalise instead on #if !QT_DEPRECATED_SINCE(5, 13), so that 
if the functions are not disabled in Qt then they are hidden here?  That would 
work, although QT_DEPRECATED_SINCE is not public API or defined in the Qt 
documentation.

REPOSITORY
  R244 KCoreAddons

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

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


D19170: Fix crash while moving files

2019-02-26 Thread David Hallas
hallas added inline comments.

INLINE COMMENTS

> filecopyjob.cpp:495
> +} else if (job == d->m_chmodJob) {
> +if (d->m_delJob) {
> +d->m_delJob->kill(Quietly);

Should we also set d->m_chmodJob to nullptr here, just like the m_putJob 
handling branch?

> filecopyjob.cpp:500
> +} else if (job == d->m_delJob) {
> +if (d->m_chmodJob) {
> +d->m_chmodJob->kill(Quietly);

Should we also set d->m_delJob to nullptr here, just like the m_putJob handling 
branch?

REPOSITORY
  R241 KIO

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

To: hallas, #frameworks, elvisangelaccio, dfaure
Cc: cfeck, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D19170: Fix crash while moving files

2019-02-26 Thread David Hallas
hallas updated this revision to Diff 52643.
hallas added a comment.


  Updated fix.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19170?vs=52240&id=52643

BRANCH
  fix_crash_while_moving_files (branched from master)

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

AFFECTED FILES
  src/core/filecopyjob.cpp

To: hallas, #frameworks, elvisangelaccio, dfaure
Cc: cfeck, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D19170: Fix crash while moving files

2019-02-26 Thread David Hallas
hallas added a comment.


  In D19170#418234 , @dfaure wrote:
  
  > I think it's an unwanted fact that the chmod job runs in parallel with the 
del job.
  >  A "return" after creating the chmod job would fix all this.
  >
  > But of course it should be possible to have two concurrent subjobs (one on 
dest, one on src), this whole issue just makes me realize that it makes the 
error handling more tricky ;-)
  >
  > If we want to keep these two running in parallel, we need to fix 
FileCopyJob to kill the running subjob when emitting the error from the other 
one.
  >  Would this fix it?
  >  http://www.davidfaure.fr/2019/filecopyjob.cpp.diff
  
  
  Hi @dfaure  I was actually thinking of the same fix ;)
  
  I have updated the diff with your suggested fix, but I have also added the 
"reverse" case, so if the chmod reports an error and we have a del job running, 
kill the del job. If the del job reports an error and we have a chmod job 
running, kill the chmod job. This pattern of error handling is actually the 
same as with the getjob/putjob. I have tested the fix and can see that it 
solved the problem!

REPOSITORY
  R241 KIO

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

To: hallas, #frameworks, elvisangelaccio, dfaure
Cc: cfeck, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D19338: New location for KNSRC files

2019-02-26 Thread Aleix Pol Gonzalez
apol added a comment.


  +1 cool stuff!
  
  this opens the possibility for files being both in /usr and /etc, can you 
make sure hell doesn't break loose when that happens?
  
  Was these files look up was delegated entirely to KConfig?

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, apol, ngraham, fvogt
Cc: kde-frameworks-devel, #knewstuff, michaelh, ZrenBot, ngraham, bruns


D19332: KProcess compile fix for obsolete QProcess members in 5.13

2019-02-26 Thread Aleix Pol Gonzalez
apol added inline comments.

INLINE COMMENTS

> kprocess.h:332
>  // hide those
> +#if (QT_VERSION < QT_VERSION_CHECK(5, 13, 0))
>  using QProcess::setReadChannelMode;

Also should check for QT_DISABLE_DEPRECATED_BEFORE no?

REPOSITORY
  R244 KCoreAddons

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

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


D19016: Update Android toolchain files to reality

2019-02-26 Thread Aleix Pol Gonzalez
apol added a comment.


  Applications can still override it, note it's a default.
  Also note this: 
https://developer.android.com/distribute/best-practices/develop/target-sdk

REPOSITORY
  R240 Extra CMake Modules

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

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


D19319: make it compile without foreach

2019-02-26 Thread Aleix Pol Gonzalez
apol added a comment.


  Looks odd, it's not necessary any changes?

INLINE COMMENTS

> kpty.cpp:88
>  public:
> -virtual void setupChildProcess() override
> +void setupChildProcess() override
>  {

Unrelated? Commit separately.

REPOSITORY
  R291 KPty

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

To: mlaurent, dfaure
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns


D19317: compile without foreach

2019-02-26 Thread Aleix Pol Gonzalez
apol added inline comments.

INLINE COMMENTS

> pic.cpp:391
>  m_description.clear();
> -QStringList entries = 
> value.toString().split(QStringLiteral("\n\n"));
> -Q_FOREACH(const QString entry, entries) {
> +const QStringList entries = 
> value.toString().split(QStringLiteral("\n\n"));
> +for (const QString &entry : entries) {

Probably could do a splitRef here

> imageconverter.cpp:67
>  out << "Input formats:\n";
> -foreach (const QByteArray &fmt, 
> QImageReader::supportedImageFormats()) {
> +const QList lstReaderSupportedFormats = 
> QImageReader::supportedImageFormats();
> +for (const QByteArray &fmt : lstReaderSupportedFormats) {

I'd use auto in this cases, otherwise you're creating a dependency to the type.

REPOSITORY
  R287 KImageFormats

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

To: mlaurent, dfaure
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns


D19339: Add option to change drive label to places panel context menu

2019-02-26 Thread Chinmoy Ranjan Pradhan
chinmoyr planned changes to this revision.

REPOSITORY
  R318 Dolphin

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

To: chinmoyr, #dolphin, #frameworks
Cc: broulik, ngraham, bruns, kfm-devel, alexde, feverfew, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, mikesomov


D19339: Add option to change drive label to places panel context menu

2019-02-26 Thread Kai Uwe Broulik
broulik added a comment.


  Also, this should probably go into Solid StorageVolume with proper backend 
abstraction

REPOSITORY
  R318 Dolphin

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

To: chinmoyr, #dolphin, #frameworks
Cc: broulik, ngraham, bruns, kfm-devel, alexde, feverfew, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, mikesomov


D19339: Add option to change drive label to places panel context menu

2019-02-26 Thread Nathaniel Graham
ngraham added a comment.


  Yeah, it would be nice to get this into KIO's own Places Panel model instead 
so it can be done from any KDE app with a Places Panel. +1 for the feature 
though!
  
  Also screenshots are appreciated. :) 
https://community.kde.org/Infrastructure/Phabricator#Include_some_screenshots

REPOSITORY
  R318 Dolphin

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

To: chinmoyr, #dolphin, #frameworks
Cc: ngraham, bruns, kfm-devel, alexde, feverfew, spoorun, navarromorales, 
firef, andrebarros, emmanuelp, mikesomov


D19339: Add option to change drive label to places panel context menu

2019-02-26 Thread Stefan Brüns
bruns added a comment.


  This definitely should not be implemented in dolphin ...

REPOSITORY
  R318 Dolphin

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

To: chinmoyr, #dolphin, #frameworks
Cc: bruns, kfm-devel, alexde, feverfew, spoorun, navarromorales, firef, 
andrebarros, emmanuelp, mikesomov


D19335: [kdoctools] make it compile without foreach

2019-02-26 Thread Laurent Montel
mlaurent added a comment.


  oops indeed. I landed it and forgot to git pull :)
  
  I will close it now :)

REPOSITORY
  R238 KDocTools

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

To: mlaurent, dfaure
Cc: ltoscano, kde-frameworks-devel, kde-doc-english, gennad, michaelh, ngraham, 
bruns, skadinna


D19335: [kdoctools] make it compile without foreach

2019-02-26 Thread Laurent Montel
mlaurent abandoned this revision.

REPOSITORY
  R238 KDocTools

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

To: mlaurent, dfaure
Cc: ltoscano, kde-frameworks-devel, kde-doc-english, gennad, michaelh, ngraham, 
bruns, skadinna


D19338: New location for KNSRC files

2019-02-26 Thread Dan Leinir Turthra Jensen
leinir added a dependent revision: D19340: Search new knsrc locations.

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, apol, ngraham, fvogt
Cc: kde-frameworks-devel, #knewstuff, michaelh, ZrenBot, ngraham, bruns


D19339: Add option to change drive label to places panel context menu

2019-02-26 Thread Chinmoy Ranjan Pradhan
chinmoyr created this revision.
chinmoyr added reviewers: Dolphin, Frameworks.
Herald added a project: Dolphin.
Herald added a subscriber: kfm-devel.
chinmoyr requested review of this revision.

REVISION SUMMARY
  Uses org.freedesktop.UDisks2 dbus service for changing drive label.

REPOSITORY
  R318 Dolphin

BRANCH
  change-label

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

AFFECTED FILES
  src/panels/places/placesitemmodel.cpp
  src/panels/places/placesitemmodel.h
  src/panels/places/placespanel.cpp

To: chinmoyr, #dolphin, #frameworks
Cc: kfm-devel, alexde, feverfew, spoorun, navarromorales, firef, andrebarros, 
emmanuelp, mikesomov


D19338: New location for KNSRC files

2019-02-26 Thread Dan Leinir Turthra Jensen
leinir created this revision.
leinir added reviewers: KNewStuff, apol, ngraham, fvogt.
leinir added a project: KNewStuff.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  This introduces a cmake variable in KNewStuffCore which contains a 
KDEInstallDirs based location for our knsrc files, and further adds 
functionality to fetch search locations from KNewStuff.
  
  A fallback is currently included, as we have to be able to work with older 
software. We also unfortunately cannot properly mark anything as deprecated, as 
there previously was no API for this, but rather a basic instruction to just 
install knsrc files in the system configuration directory (instructions which 
previously not even a part of the API docs, which this patch also addresses). 
The code was written with future deprecation in mind, and we can at that time 
simply remove the configuration fallback options and the functionality it holds.
  
  Rationale:
  This change is being made on request from various distributions, as the 
current install location for knsrc files is causing a great many issues during 
the distribution update cycle which means we are left with stale knsrc files in 
various places. This in turn causes a great many issues, particularly for 
Discover which has suffered crashes and other strange side effects when 
attempting to use these stale files (some of which attempt to access servers 
which almost-but-not-quite respond). Discover has workarounds for some of 
these, but it is becoming more than a little awkward, and being able to manage 
this a little more tightly would come in very handy indeed.

REPOSITORY
  R304 KNewStuff

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

AFFECTED FILES
  KF5NewStuffCoreConfig.cmake.in
  README.md
  src/core/engine.cpp
  src/core/engine.h

To: leinir, #knewstuff, apol, ngraham, fvogt
Cc: kde-frameworks-devel, #knewstuff, michaelh, ZrenBot, ngraham, bruns


D19326: [Kconfig] Compile without foreach

2019-02-26 Thread Laurent Montel
mlaurent retitled this revision from "Compile without foreach" to "[Kconfig] 
Compile without foreach".

REPOSITORY
  R237 KConfig

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

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


D19335: [kdoctools] make it compile without foreach

2019-02-26 Thread Luigi Toscano
ltoscano added a comment.


  Isn't it a duplicate of D19271  ?

REPOSITORY
  R238 KDocTools

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

To: mlaurent, dfaure
Cc: ltoscano, kde-frameworks-devel, kde-doc-english, gennad, michaelh, ngraham, 
bruns, skadinna


D19336: [kguiaddons] make it compile without foreach

2019-02-26 Thread Laurent Montel
mlaurent created this revision.
mlaurent added a reviewer: dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
mlaurent requested review of this revision.

REVISION SUMMARY
  compile without foreach

REPOSITORY
  R273 KGuiAddons

BRANCH
  make_it_compile_without_foreach (branched from master)

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

AFFECTED FILES
  CMakeLists.txt
  src/colors/kcolorcollection.cpp

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


D19335: [kdoctools] make it compile without foreach

2019-02-26 Thread Laurent Montel
mlaurent created this revision.
mlaurent added a reviewer: dfaure.
Herald added projects: Frameworks, Documentation.
Herald added subscribers: kde-doc-english, kde-frameworks-devel.
mlaurent requested review of this revision.

REVISION SUMMARY
  compile without foreach

REPOSITORY
  R238 KDocTools

BRANCH
  make_it_compile_without_foreach (branched from master)

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

AFFECTED FILES
  CMakeLists.txt
  src/xslt.cpp

To: mlaurent, dfaure
Cc: kde-frameworks-devel, kde-doc-english, gennad, michaelh, ngraham, bruns, 
skadinna


D19332: KProcess compile fix for obsolete QProcess members in 5.13

2019-02-26 Thread Jonathan Marten
marten created this revision.
marten added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
marten requested review of this revision.

REVISION SUMMARY
  The functions QProcess::setReadChannelMode() and QProcess::readChannelMode() 
are obsolete, but were not specially treated in any way in earlier Qt versions. 
 In 5.13 they are actually disabled in the header files, using 
QT_DEPRECATED_SINCE(5, 13).  KDE PIM optimistically sets 
QT_DISABLE_DEPRECATED_BEFORE=0x06 (since May 2017), so the build fails 
because these functions are no longer available:
  
  /usr/include/KF5/KCoreAddons/kprocess.h:332: error: no members matching 
'QProcess::setReadChannelMode' in 'class QProcess'
  /usr/include/KF5/KCoreAddons/kprocess.h:333: error: no members matching 
'QProcess::readChannelMode' in 'class QProcess'
  
  This does not affect the remainder of Frameworks or Plasma (yet), because 
they do not use this QT_DISABLE_DEPRECATED_BEFORE setting.
  
  This change makes the use of these two members conditional on the Qt version 
(5.12 or earlier).

TEST PLAN
  Built kcoreaddons with this change, checked that applications using this 
header build correctly without the above errors.

REPOSITORY
  R244 KCoreAddons

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

AFFECTED FILES
  src/lib/io/kprocess.h

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


D19324: Add code-oss icon

2019-02-26 Thread Noah Davis
ndavis added a comment.


  Much better. Now for some questions and suggestions.
  
  Why does the icon use green on the top of the logo? The light theme version 
doesn't look bad, but isn't the official logo all blue? In general, we try to 
preserve the original branding.
  
  Why is the dark theme version greener than the light theme? Normally we try 
to make color icons the same for light and dark themes.
  
  It would be good to turn those shadows into gradients with `` as the 
color for the bottom right of the shadow. The darker shadow should expand to 
cover the whole area to the bottom left of the right side of the logo. Here's 
what I mean: 
  F6637009: Screenshot_20190226_052530.png 

  F6637011: Screenshot_20190226_052559.png 

  
  The parts of the logo in this icon are layered differently from the official 
logo: 
https://user-images.githubusercontent.com/49339/32078472-5053adea-baa7-11e7-9034-519002f12ac7.png
  
  The right edge of the right side of the logo is a bit too dark relative to 
the inner edge.
  
  If you're having trouble finding the right blues to make the logo look 3D, 
try using the shadows to separate the layers like I did in the screenshots 
above.
  
  I'll have more to say later.

REPOSITORY
  R266 Breeze Icons

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

To: axionl, #vdg
Cc: ndavis, rooty, ngraham, kde-frameworks-devel, michaelh, bruns


D19324: Add code-oss icon

2019-02-26 Thread Ariel AxionL
axionl updated this revision to Diff 52588.
axionl added a comment.


  - Merge branch 'master' into code-oss
  
  1. Updating D19324 : Add code-oss icon #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create
  
  fixed color details.

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19324?vs=52587&id=52588

BRANCH
  code-oss (branched from master)

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

AFFECTED FILES
  icons-dark/apps/48/code-oss.svg
  icons-dark/apps/48/code.svg
  icons/apps/48/code-oss.svg
  icons/apps/48/code.svg

To: axionl, #vdg
Cc: ndavis, rooty, ngraham, kde-frameworks-devel, michaelh, bruns


D19324: Add code-oss icon

2019-02-26 Thread Ariel AxionL
axionl edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: axionl, #vdg
Cc: ndavis, rooty, ngraham, kde-frameworks-devel, michaelh, bruns


D19324: Add code-oss icon

2019-02-26 Thread Ariel AxionL
axionl edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: axionl, #vdg
Cc: ndavis, rooty, ngraham, kde-frameworks-devel, michaelh, bruns


D19324: Add code-oss icon

2019-02-26 Thread Ariel AxionL
axionl updated this revision to Diff 52587.
axionl added a comment.


  - Merge branch 'master' into code-oss
  
  1. Updating D19324 : Add code-oss icon #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create
  
  Redraw by inkscape.

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19324?vs=52581&id=52587

BRANCH
  code-oss (branched from master)

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

AFFECTED FILES
  icons-dark/apps/48/code-oss.svg
  icons-dark/apps/48/code.svg
  icons/apps/48/code-oss.svg
  icons/apps/48/code.svg

To: axionl, #vdg
Cc: ndavis, rooty, ngraham, kde-frameworks-devel, michaelh, bruns


D19080: [WIP] Make file overwrite a bit safer

2019-02-26 Thread David Faure
dfaure accepted this revision.
dfaure added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> file_unix.cpp:414
> +if (::unlink(_dest_backup.constData()) == -1) {
> +qCWarning(KIO_FILE) << "Couldn't remove original dest" <<  
> _dest_backup;
> +}

printing strerror(errno) here would help debugging why it failed

> file_unix.cpp:418
> +if (::rename(_dest.constData(), _dest_backup.constData()) == -1) {
> +qCWarning(KIO_FILE) << "Couldn't rename" << _dest << "to" << 
> _dest_backup;
> +}

printing strerror(errno) here would make sense too

REPOSITORY
  R241 KIO

BRANCH
  safe-overwrite

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

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


D19080: [WIP] Make file overwrite a bit safer

2019-02-26 Thread David Faure
dfaure added a comment.


  And don't forget to remove the [WIP] from your commit log ;-)

REPOSITORY
  R241 KIO

BRANCH
  safe-overwrite

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

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


D19328: MustacheJS: also highlight template files, fix syntax and improve support for Handlebars

2019-02-26 Thread Nibaldo González
nibags created this revision.
nibags added reviewers: Framework: Syntax Highlighting, cullmann, dhaumann.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
nibags requested review of this revision.

REVISION SUMMARY
  The Mustache highlighting has been modified, so as to also highlight template 
files and make it more complete (previously, this highlighter was only used 
within HTML highlighting, in the