D12592: KFileWidget: Prevent places panel width from growing 1px iteratively

[Henrik Fehlauer]

rkflx added a comment.


  Sorry for the wait, writing the commit message for this one and the alignment 
patch meant going through a //lot// of notes. Who'd knew that after the 
off-by-one error in Spectacle which only happened in a VM there could be even 
more difficult problems :D

REPOSITORY
  R241 KIO

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

To: rkflx, #frameworks
Cc: michaelh, bruns

D12591: KFileWidget: Provide faster access to the icon position setting

[Henrik Fehlauer]

rkflx added a comment.


  If this gets approved, I'll do the same change to K3b for consistency, which 
was the only other app showing the submenu I could find on https://lxr.kde.org.

REPOSITORY
  R241 KIO

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

To: rkflx, #frameworks
Cc: michaelh, bruns

D12592: KFileWidget: Prevent places panel width from growing 1px iteratively

[Henrik Fehlauer]

rkflx added a dependent revision: D12594: KFileWidget: Perfectly align filename 
widget with icon view.

REPOSITORY
  R241 KIO

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

To: rkflx, #frameworks
Cc: michaelh, bruns

D12594: KFileWidget: Perfectly align filename widget with icon view

[Henrik Fehlauer]

rkflx created this revision.
rkflx added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.
rkflx requested review of this revision.

REVISION SUMMARY
  Since the redesign of the file dialog for KDE4, the left edges of the
  line edits below the icon view were supposed to be aligned to the left
  edge of the icon view, while in KDE3 the naming labels were aligned
  instead.
  
  However, in practice that never worked out perfectly, there was always a
  small displacement. While hardly noticable in KDE4's widget styles, in
  particular for Breeze in KF5 with its small splitter handle width and
  larger layout spacing this became much more annoying to look at.
  
  This can be fixed by taking the splitter handle width as well as the
  grid layout spacing into account. While the patch seems straightforward,
  making the patch work with all styles was actually suprisingly
  difficult, not only because of the bug in the dependent patch.
  
  Depends on D12592 
  
  Ref T8552 
  
  FIXED-IN: 5.46

TEST PLAN
  `kdialog --getopenfilename`, check alignment with KMag for
  different widget styles (e.g. Breeze, Oxygen, Windows and Fusion) after
  startup, and after moving the splitter, resizing the dialog and toggling
  the places panel.
  
  Before:
  F5826739: KIO-alignment-before.png 
  
  After:
  F5826740: KIO-alignment-after.png 

REPOSITORY
  R241 KIO

BRANCH
  fix-alignment (branched from master)

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

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp

To: rkflx, #frameworks
Cc: michaelh, bruns

D12592: KFileWidget: Prevent places panel width from growing 1px iteratively

[Henrik Fehlauer]

rkflx added a dependent revision: D12593: KFileWidget: Save places panel width 
also after hiding panel.

REPOSITORY
  R241 KIO

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

To: rkflx, #frameworks
Cc: michaelh, bruns

D12593: KFileWidget: Save places panel width also after hiding panel

[Henrik Fehlauer]

rkflx created this revision.
rkflx added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.
rkflx requested review of this revision.

REVISION SUMMARY
  In certain situations the user-set width of the places panel was not
  remembered.
  
  As `placesViewWidth` is initialized by `configGroup.readEntry` and
  updated in reaction to user actions, it makes sense to also save the
  same variable back to the config file, instead of looking directly at
  the `placesViewSplitter`.
  
  Depends on D12592 
  
  Ref T8552 
  
  FIXED-IN: 5.46

TEST PLAN
  `kdialog --getopenfilename`, change width of places panel, hide places
  panel with [F9], close dialog by clicking on Cancel, reopen
  dialog, close dialog a second time, reopen dialog, press [F9] again
  and check that previously set width is restored.

REPOSITORY
  R241 KIO

BRANCH
  fix-hidden-places-width-saving (branched from master)

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

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp

To: rkflx, #frameworks
Cc: michaelh, bruns

D12592: KFileWidget: Prevent places panel width from growing 1px iteratively

[Henrik Fehlauer]

rkflx created this revision.
rkflx added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.
rkflx requested review of this revision.

REVISION SUMMARY
  cc00bbb22010 
 
tried to prevent the places panel from reopening with a
  width reduced by 1px compared to the previous invocation of the file
  dialog.
  
  While the patch worked on the surface when it was applied, the actual
  reasoning was wrong: The difference was not because of "extra resize()
  events", but because of a mismatch between what was read from the config
  (`placesViewWidth`) and what was written to the config (`sizes[0]`).
  
  The actual reason `placesViewWidth` and `sizes[0]` are different is
  because of a previous error: It is assumed that the splitter sizes sum
  up to `availableWidth`, while in reality this width also contains the
  width of the splitter handle. Upon `setSizes`, Qt equally takes away
  width from both sides of the splitter to make everything fit.
  
  While for the Oxygen and Fusion styles (3px and 4px splitter handle
  width, respectively) an adjustment of 1px did the trick, for Breeze with
  its splitter handle width of only 1px this did not work anymore due to
  rounding errors, it resulted in the config entry incrementing by 1px for
  every invocation of the dialog.
  
  To fix the problem for every style, we simply take the width of the
  splitter handle into account and revert the previous fix. While we are
  at it, we refactor to make everything more DRY.
  
  Ref T8552 
  
  FIXED-IN: 5.46

TEST PLAN
  Setup monitoring with `watch grep Speedbar ~/.config/kdeglobals`.
  Repeatedly open `kdialog --getopenfilename`, click on Cancel and
  check that the config entry does not change. Test for multiple widget
  styles (e.g. Breeze, Oxygen, Windows and Fusion), as well as for hiding
  the places panel and manually moving the splitter to both ends.

REPOSITORY
  R241 KIO

BRANCH
  fix-places-width-growth (branched from master)

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

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp

To: rkflx, #frameworks
Cc: michaelh, bruns

D12591: KFileWidget: Provide faster access to the icon position setting

[Henrik Fehlauer]

rkflx created this revision.
rkflx added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.
rkflx requested review of this revision.

REVISION SUMMARY
  The file dialog allows to move the file name labels from next to the
  icon to right under the icon for Short View. However, having to go in
  a deeply nested Settings button > Icon Position > Above File Name
  submenu makes it both hard to discover and cumbersome to use. Ideally
  this setting could be accessed directly from the menu for good usability.
  
  Making it easy to change back settings is especially important in light
  of D12326 , which will enable Icons Above 
File Name by default.
  
  We linearize the submenu entries from the submenu to the top-level menu,
  which works just fine as there are only two entries, increasing the
  overall menu size by only one item while removing an entire submenu.
  
  While the submenu itself is not advertised in the API docs, it was
  publicly accessible, and therefore is kept for compatiblity reasons
  (although with a slight change in wording).
  
  Ref T8552 
  
  FIXED-IN: 5.47

TEST PLAN
  `kdialog --getopenfilename`, switch view modes via the settings
  button, check that the icon position settings are only enabled for
  View > Short View and still work properly.
  
  Before:
  F5826732: KIO-icon-position-before.png 
  
  After:
  F5826733: KIO-icon-position-after.png 

REPOSITORY
  R241 KIO

BRANCH
  icon-position-usability (branched from master)

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kfilewidget.cpp

To: rkflx, #frameworks
Cc: michaelh, bruns

D12590: KFileWidget: Disable zoom buttons once reached minimum or maximum

[Henrik Fehlauer]

rkflx created this revision.
rkflx added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.
rkflx requested review of this revision.

REVISION SUMMARY
  In Gwenview the zoom buttons will be disabled when clicking on it would
  not result in any further change of zoom level, e.g. Zoom In is
  disabled for the largest icon size. In general it is recommended to
  disable unavailable actions.
  
  This patch adds the functionality to the file dialog too.
  
  Ref T8552 
  
  FIXED-IN: 5.46

TEST PLAN
  `kdialog --getopenfilename`, change icon size to minimum and maximum by
  sliding, mouse wheeling, pressing buttons and using the keyboard, and
  check that buttons are disabling and enabling properly.
  
  F5826731: KIO-disable-zoom-buttons.webm 

REPOSITORY
  R241 KIO

BRANCH
  disable-zoom-buttons (branched from master)

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

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp

To: rkflx, #frameworks
Cc: michaelh, bruns

D12588: KFileWidget: Set minimum size for zoom slider

[Henrik Fehlauer]

rkflx created this revision.
rkflx added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.
rkflx requested review of this revision.

REVISION SUMMARY
  When choosing a very narrow width for the file dialog, the zoom slider
  for the icon size would get squished so that only the handle would be
  visible and the slider could not be used anymore to change zoom levels
  in any way.
  
  This can be avoided by setting a minimum size. 40px are smaller than the
  default size, but still allow to operate the slider comfortably.
  
  The minimal width of the dialog is unchanged, as before the toolbar will
  show the elision button once elements start overflowing.
  
  Ref T8552 
  
  FIXED-IN: 5.46

TEST PLAN
  `kdialog --getopenfilename`, change window size and observe zoom slider.
  
  F5826728: KIO-slider-min-size.webm 

REPOSITORY
  R241 KIO

BRANCH
  slider-min-size (branched from master)

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

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp

To: rkflx, #frameworks
Cc: michaelh, bruns

KDE CI: Frameworks purpose kf5-qt5 SUSEQt5.10 - Build # 36 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20purpose%20kf5-qt5%20SUSEQt5.10/36/
 Project:
Frameworks purpose kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 29 Apr 2018 03:23:13 +
 Build duration:
23 min and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 3 test(s)Failed: TestSuite.alternativesmodeltestFailed: TestSuite.menutest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report25%
(5/20)30%
(17/56)30%
(17/56)18%
(387/2102)18%
(227/1250)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(4/4)100%
(4/4)85%
(132/155)52%
(97/188)src100%
(8/8)100%
(8/8)61%
(183/298)43%
(101/237)src.externalprocess0%
(0/2)0%
(0/2)0%
(0/142)0%
(0/94)src.plugins.email0%
(0/1)0%
(0/1)0%
(0/57)0%
(0/32)src.plugins.imgur0%
(0/2)0%
(0/2)0%
(0/188)0%
(0/78)src.plugins.kdeconnect0%
(0/1)0%
(0/1)0%
(0/27)0%
(0/12)src.plugins.ktp-sendfile0%
(0/1)0%
(0/1)0%
(0/30)0%
(0/16)src.plugins.nextcloud0%
(0/3)0%
(0/3)0%
(0/81)0%
(0/40)src.plugins.pastebin0%
(0/1)0%
(0/1)0%
(0/56)0%
(0/36)src.plugins.phabricator0%
(0/3)0%
(0/3)0%
(0/220)0%
(0/90)src.plugins.phabricator.quick0%
(0/6)0%
(0/6)0%
(0/86)0%
(0/59)src.plugins.phabricator.tests0%
(0/1)0%
(0/1)0%
(0/60)0%
(0/28)src.plugins.reviewboard0%
(0/3)0%
(0/3)0%
(0/242)0%
(0/80)src.plugins.reviewboard.quick0%
(0/8)0%
(0/8)0%
(0/159)0%
(0/100)src.plugins.saveas100%
(1/1)100%
(1/1)38%
(20/52)50%
(24/48)src.plugins.youtube0%
(0/5)0%
(0/5)0%
(0/122)0%
(0/44)src.quick100%
(2/2)100%
(2/2)100%
(9/9)100%
(0/0)src.widgets100%
(2/2)100%
(2/2)88%
(43/49)50%

KDE CI: Frameworks purpose kf5-qt5 SUSEQt5.9 - Build # 12 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20purpose%20kf5-qt5%20SUSEQt5.9/12/
 Project:
Frameworks purpose kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 29 Apr 2018 03:23:13 +
 Build duration:
19 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 3 test(s)Failed: TestSuite.alternativesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report25%
(5/20)30%
(17/56)30%
(17/56)22%
(459/2109)22%
(277/1250)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(4/4)100%
(4/4)93%
(144/155)59%
(111/188)src100%
(8/8)100%
(8/8)75%
(227/303)55%
(130/237)src.externalprocess0%
(0/2)0%
(0/2)0%
(0/142)0%
(0/94)src.plugins.email0%
(0/1)0%
(0/1)0%
(0/57)0%
(0/32)src.plugins.imgur0%
(0/2)0%
(0/2)0%
(0/188)0%
(0/78)src.plugins.kdeconnect0%
(0/1)0%
(0/1)0%
(0/27)0%
(0/12)src.plugins.ktp-sendfile0%
(0/1)0%
(0/1)0%
(0/30)0%
(0/16)src.plugins.nextcloud0%
(0/3)0%
(0/3)0%
(0/81)0%
(0/40)src.plugins.pastebin0%
(0/1)0%
(0/1)0%
(0/56)0%
(0/36)src.plugins.phabricator0%
(0/3)0%
(0/3)0%
(0/220)0%
(0/90)src.plugins.phabricator.quick0%
(0/6)0%
(0/6)0%
(0/86)0%
(0/59)src.plugins.phabricator.tests0%
(0/1)0%
(0/1)0%
(0/60)0%
(0/28)src.plugins.reviewboard0%
(0/3)0%
(0/3)0%
(0/242)0%
(0/80)src.plugins.reviewboard.quick0%
(0/8)0%
(0/8)0%
(0/159)0%
(0/100)src.plugins.saveas100%
(1/1)100%
(1/1)58%
(31/53)63%
(30/48)src.plugins.youtube0%
(0/5)0%
(0/5)0%
(0/122)0%
(0/44)src.quick100%
(2/2)100%
(2/2)100%
(9/9)100%
(0/0)src.widgets100%
(2/2)100%
(2/2)96%
(48/50)60%
   

KDE CI: Frameworks plasma-framework kf5-qt5 FreeBSDQt5.9 - Build # 122 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20FreeBSDQt5.9/122/
 Project:
Frameworks plasma-framework kf5-qt5 FreeBSDQt5.9
 Date of build:
Sun, 29 Apr 2018 03:23:08 +
 Build duration:
9 min 32 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 13 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: TestSuite.plasma-packagestructuretest

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20SUSEQt5.10/147/
 Project:
Frameworks plasma-framework kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 29 Apr 2018 03:23:08 +
 Build duration:
8 min 49 sec and counting
   JUnit Tests
  Name: (root) Failed: 7 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 15 test(s)Failed: TestSuite.dialognativetestFailed: TestSuite.plasma-configmodeltestFailed: TestSuite.plasma-dialogqmltestFailed: TestSuite.plasma-fallbackpackagetestFailed: TestSuite.plasma-iconitemtestFailed: TestSuite.plasma-packagestructuretestFailed: TestSuite.plasma-storagetest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report33%
(6/18)35%
(55/159)35%
(55/159)26%
(3558/13515)18%
(1978/10693)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests85%
(22/26)85%
(22/26)53%
(609/1140)28%
(421/1492)src.declarativeimports.calendar0%
(0/11)0%
(0/11)0%
(0/452)0%
(0/241)src.declarativeimports.core22%
(4/18)22%
(4/18)11%
(253/2316)7%
(102/1554)src.declarativeimports.plasmacomponents0%
(0/9)0%
(0/9)0%
(0/525)0%
(0/214)src.declarativeimports.plasmaextracomponents0%
(0/5)0%
(0/5)0%
(0/44)0%
(0/27)src.declarativeimports.platformcomponents0%
(0/4)0%
(0/4)0%
(0/60)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/15)0%
(0/4)src.plasma55%
(12/22)55%
(12/22)41%
(1442/3532)28%
(827/2965)src.plasma.packagestructure0%
(0/7)0%
(0/7)0%
(0/141)0%
(0/14)src.plasma.private46%
(11/24)46%
(11/24)41%
(671/1621)28%
(318/1121)src.plasma.scripting0%
(0/3)0%
(0/3)0%
(0/162)0%
(0/132)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick42%
(5/12)42%
(5/12)27%
(552/2015)17%
(305/1779)src.plasmaquick.private33%
(1/3)33%
(1/3)28%
(31/110)36%
(5/14)src.scriptengines.qml.plasmoid0%
(0/6)0%
(0/6)0%
(0/1179)0%
(0/1058)tests.dpi0%
(0/2)0%
(0/2)0%
(0/22)0%
(0/2)tests.kplugins0%
 

D12538: Always prompt for overwrite on double-click in save dialog

[Henrik Fehlauer]

rkflx added a comment.


  TBH, I don't get what the overwrite and release schedule discussion has to do 
with this patch. All it does is add an additional method to trigger `slotOk()`, 
i.e. instead of clicking on Save, users can simply double-click.
  
  The fact that either the dialog or the app ask before overwriting in response 
to the slot is not changed in any way here. Don't judge a Diff by its title, in 
particular as long as the request to "Please update your summary too" is still 
in state "red" ;)
  
  ---
  
  Regarding what you were discussing: If I understood Peter's analysis in 
D12346  correctly, the API will always 
allow apps to opt-out of the dialog providing the overwrite confirmation, and 
instead add their own. Therefore to me it seems like it is only a matter of 
going through all apps and opt-in to the already existing functionality, i.e. I 
don't see why this would require changes in KIO which would result in 
dependency issues (unless I'm mistaken?).

REPOSITORY
  R241 KIO

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

To: anemeth, #frameworks, #vdg, ngraham, rkflx
Cc: ltoscano, rkflx, broulik, jtamate, ngraham, #frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham planned changes to this revision.

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

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


  Further improvements; always draw the theme background color under the 
KMessageWidget, which handles the case of the theme and active window defining 
different colors.
  
  One remaining issue: the KMessageWidget now has a 1px border around it that 
may be the wrong color.

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12508?vs=33254&id=33265

BRANCH
  arcpatch-D12508

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

AFFECTED FILES
  src/kmessagewidget.cpp

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12355: [API dox] New UI marker @info:placeholder

[Friedrich W . H . Kossebau]

kossebau added a comment.


  Phabricator display of diff looks fine here, both in firefox (logged in) & 
chromium (not logged in). Can you specify "a bit weird"? :)

REPOSITORY
  R249 KI18n

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

To: kossebau, #localization, ilic
Cc: #frameworks, michaelh, bruns

D12355: [API dox] New UI marker @info:placeholder

[Chusslove Illich]

ilic added a comment.


  In D12355#255342 , @ilic wrote:
  
  > The diff is a bit weird, as if it is for the vallusuffix from D12353 
. But looks good in the repository as 
committed.
  
  
  ...or, my browser did something wrong. Or PEBKAC.

REPOSITORY
  R249 KI18n

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

To: kossebau, #localization, ilic
Cc: #frameworks, michaelh, bruns

D12355: [API dox] New UI marker @info:placeholder

[Chusslove Illich]

ilic added a comment.


  The diff is a bit weird, as if it is for the vallusuffix from D12353 
. But looks good in the repository as 
committed.

REPOSITORY
  R249 KI18n

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

To: kossebau, #localization, ilic
Cc: #frameworks, michaelh, bruns

D12355: [API dox] New UI marker @info:placeholder

[Friedrich W . H . Kossebau]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R249:449e6fd556ee: [API dox] New UI marker @info:placeholder 
(authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D12355?vs=32604&id=33262#toc

REPOSITORY
  R249 KI18n

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12355?vs=32604&id=33262

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

AFFECTED FILES
  docs/programmers-guide.md
  src/kuitmarkup.cpp

To: kossebau, #localization, ilic
Cc: #frameworks, michaelh, bruns

D12353: [API dox] New UI marker @item:valuesuffix

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R249:b88bb7e1b8d9: [API dox] New UI marker @item:valuesuffix 
(authored by kossebau).

REPOSITORY
  R249 KI18n

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12353?vs=32605&id=33261

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

AFFECTED FILES
  docs/programmers-guide.md
  src/kuitmarkup.cpp

To: kossebau, #localization, ilic
Cc: aacid, #frameworks, michaelh, bruns

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/237/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Sat, 28 Apr 2018 17:41:27 +
 Build duration:
16 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)67%
(294/442)67%
(294/442)53%
(31605/59762)38%
(18585/49003)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(73/73)100%
(73/73)94%
(8598/9151)48%
(5251/10861)autotests.http100%
(9/9)100%
(9/9)100%
(586/587)59%
(217/368)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(180/198)67%
(63/94)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core84%
(100/119)84%
(100/119)58%
(8369/14391)50%
(4895/9751)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets79%
(31/39)79%
(31/39)49%
(3876/7877)33%
(1635/4940)src.gui100%
(2/2)100%
(2/2)95%
(104/110)77%
(57/74)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(521/1008)42%
(417/1004)src.ioslaves.file.kauth0%
(0/3)0%
(0/3)0%
(0/104)0%
(0/75)src.ioslaves.ftp0%
(0/2)0%
(0/2)0%
(0/1365)0%
(0/1515)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/184)src.ioslaves.http89%
(8/9)89%
(8/9)41%
(1783/4338)35%
(1375/3979)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(631/1333)55%
(649/1174)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/258)8%
(19/242)src.ioslaves.remote.kdedmodule0%
(0/4)0%
(0/4)0%
(0/14)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash64%
(7/11)64%
(7/11)

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.9 - Build # 215 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.9/215/
 Project:
Frameworks kio kf5-qt5 FreeBSDQt5.9
 Date of build:
Sat, 28 Apr 2018 17:41:26 +
 Build duration:
8 min 41 sec and counting
   JUnit Tests
  Name: (root) Failed: 6 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 57 test(s)Failed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiofilewidgets-kfileplacesmodeltestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltest

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/86/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Sat, 28 Apr 2018 17:41:27 +
 Build duration:
4 min 43 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 57 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesmodeltest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)67%
(294/442)67%
(294/442)53%
(31630/59762)38%
(18585/48999)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(73/73)100%
(73/73)94%
(8598/9151)48%
(5253/10861)autotests.http100%
(9/9)100%
(9/9)100%
(586/587)59%
(217/368)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(180/198)67%
(63/94)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core84%
(100/119)84%
(100/119)58%
(8405/14391)50%
(4900/9755)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets79%
(31/39)79%
(31/39)49%
(3877/7877)33%
(1636/4940)src.gui100%
(2/2)100%
(2/2)95%
(104/110)77%
(57/74)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(521/1008)42%
(417/1004)src.ioslaves.file.kauth0%
(0/3)0%
(0/3)0%
(0/104)0%
(0/75)src.ioslaves.ftp0%
(0/2)0%
(0/2)0%
(0/1365)0%
(0/1515)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/247)0%
(0/184)src.ioslaves.http89%
(8/9)89%
(8/9)41%
(1788/4338)35%
(1373/3979)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1333)55%
(648/1174)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/258)8%
(19/242)src.ioslaves.remote.kdedmodule0%
(0/4)0%
(0/4)0%
(0/14)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash64%
(7/11)64%
(7/

D12490: concatPaths: process empty path1 correctly

[Алексей Шилин]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:47340d6a554c: concatPaths: process empty path1 correctly 
(authored by aleksejshilin).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12490?vs=32961&id=33260

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

AFFECTED FILES
  src/pathhelpers_p.h

To: aleksejshilin, #frameworks, anthonyfieroni, dfaure
Cc: michaelh, bruns

D12587: Indentation script for R

[Pierre de Villemereuil]

devillemereuil created this revision.
devillemereuil added reviewers: KTextEditor, RKWard.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added a subscriber: Frameworks.
devillemereuil requested review of this revision.

REVISION SUMMARY
  This is a javascript indentation script for the statistical language R. This 
script is for all software using KTextEditor (e.g. Kate), but more specifically 
for RKWard, which is an IDE for R.
  
  I tested it for corner cases for some time so it should be quite robust, 
although my ability to find corner cases alone is obviously limited by a lack 
of diversity in coding styles.

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/script/data/indentation/r.js

To: devillemereuil, #ktexteditor, #rkward
Cc: #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, 
sars, dhaumann

D11925: Add "SkipSwitcher" to API

[Scott Harvey]

sharvey added a dependent revision: D11926: Add "SkipSwitcher" to API.

REPOSITORY
  R127 KWayland

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

To: sharvey, hein, graesslin
Cc: lbeltrame, rikmills, davidedmundson, #plasma, graesslin, #frameworks, 
michaelh, bruns

D12578: fix some issues reported by clazy

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> CMakeLists.txt:4
>  set(REQUIRED_QT_VERSION 5.8.0)
> -set(KF5_VERSION "5.45.0") # handled by release scripts
> -set(KF5_DEP_VERSION "5.45.0") # handled by release scripts
> +set(KF5_VERSION "5.44.0") # handled by release scripts
> +set(KF5_DEP_VERSION "5.44.0") # handled by release scripts

I don't think this belongs in here

> transaction.cpp:326
>  
> -for (const EngineQuery& q : query.subQueries()) {
> +const auto subQueries = query.subQueries();
> +for (const EngineQuery& q : subQueries) {

Move this one up, before the `query.op() == EngineQuery::Phrase)`

> transaction.cpp:508
>  out << "\tFileNameTermsDB: ";
> -for (const QByteArray& term : terms) {
> +for (const QByteArray& term : fileNameTerms) {
>  out << term << " ";

This is a functional change and should not be hidden between "cosmetical" 
changes

> searchstore.cpp:167
>  
> -for (const Term& t : term.subTerms()) {
> +for (const Term& t : subTerms) {
>  vec << constructQuery(tr, t);

No const here?

REPOSITORY
  R293 Baloo

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

To: mgallien, #baloo, #frameworks
Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

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


  set autoFillBackground, which fixes most client use cases except for when the 
parent widget's palette's background color matches the theme's text color (as 
with a dark Konsole theme on Breeze light)

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12508?vs=33089&id=33254

BRANCH
  arcpatch-D12508

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

AFFECTED FILES
  src/kmessagewidget.cpp

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12371: fix always reproducible crash

[David Faure]

dfaure added a comment.


  "I'm redirected to fish://127.0.0.1/home/jtorres"  is exactly what is 
supposed to happen. If that asserts nowadays, it means there's a regression in 
the redirect handling. Sounds like this has to be fixed first.
  
  QUrl::toString is slowish btw, it has to assemble the string from the 
multiple components of the URLs. Just to test the path, it's overkill.
  
  Anyhow, "if (!u.path().isEmpty()) " is what makes sense, we "just" have to 
fix redirection handling too (I can take a look tomorrow, I have to leave soon 
today)

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks, apol
Cc: anthonyfieroni, apol, michaelh, bruns

D7415: kconfig: kcfg.xsd do not require a kcfgfile

[Allen Winter]

winterz added a comment.


  ping.
  this is still relevant.
  some kconfigxt files don't set the kcfgfile element.
  
  to name a few:
  mailcommon/src/settings/mailcommon.kcfg
  kdepim-runtime/agents/newmailnotifier/newmailnotifieragentsettings.kcfg
  pim-data-exporter/gui/settings/pimsettingexporterglobalconfig.kcfg
  grantlee-editor/grantleethemeeditor/settings/grantleethemeeditor.kcfg
  libksieve/src/ksieveui/settings/sieve-editor.kcfg.cmake
  knotes/src/settings/knotesglobalconfig.kcfg.cmake
  messagelib/messagelist/src/core/settings.kcfg
  
messagelib/messageviewer/src/messageviewerheaderplugins/defaultgrantleeheaderstyleplugin/settings/defaultgrantleeheaderstyleplugin.kcfg
  libgravatar/src/settings/gravatar.kcfg
  pim-sieve-editor/src/settings/sieveeditorglobalconfig.kcfg
  kmail/agents/archivemailagent/settings/archivemailagentsettings.kcfg

REPOSITORY
  R237 KConfig

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

To: winterz
Cc: #frameworks, michaelh, bruns

D12578: fix some issues reported by clazy

[Matthieu Gallien]

mgallien created this revision.
mgallien added reviewers: Baloo, Frameworks.
Restricted Application added projects: Frameworks, Baloo.
mgallien requested review of this revision.

REVISION SUMMARY
  fix some issues reported by clazy

TEST PLAN
  tests are OK

REPOSITORY
  R293 Baloo

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  autotests/unit/file/pendingfilequeuetest.cpp
  src/engine/transaction.cpp
  src/file/fileindexerconfig.cpp
  src/file/fileindexscheduler.cpp
  src/file/firstrunindexer.cpp
  src/file/indexcleaner.cpp
  src/file/modifiedfileindexer.cpp
  src/file/newfileindexer.cpp
  src/file/xattrindexer.cpp
  src/kioslaves/kded/baloosearchmodule.cpp
  src/lib/query.cpp
  src/lib/searchstore.cpp
  src/lib/taglistjob.cpp
  src/lib/term.cpp

To: mgallien, #baloo, #frameworks
Cc: ashaposhnikov, michaelh, astippich, spoorun, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a comment.


  I'm working on setting the background color within the KMessageWidget itself, 
which should make these implementation issues in individual apps non-blockers.

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D4911: add Baloo DBus signals for moved or removed files

[Matthieu Gallien]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:ef0e268c6577: add Baloo DBus signals for moved or removed 
files (authored by mgallien).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D4911?vs=14953&id=33246#toc

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4911?vs=14953&id=33246

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

AFFECTED FILES
  autotests/unit/file/CMakeLists.txt
  autotests/unit/file/filewatchtest.cpp
  autotests/unit/file/metadatamovertest.cpp
  autotests/unit/file/metadatamovertest.h
  src/CMakeLists.txt
  src/engine/transaction.cpp
  src/engine/transaction.h
  src/file/CMakeLists.txt
  src/file/filewatch.cpp
  src/file/filewatch.h
  src/file/mainhub.cpp
  src/file/mainhub.h
  src/file/metadatamover.cpp
  src/file/metadatamover.h
  src/file/org.kde.BalooWatcherApplication.xml

To: mgallien, vhanda, dfaure, michaelh, #baloo
Cc: ngraham, cullmann, apol, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, bruns

D12538: Always prompt for overwrite on double-click in save dialog

[Nathaniel Graham]

ngraham added a comment.


  It will always work properly, it's just a question of how users will be 
annoyed. :) FWIW right now with the status quo, Kate users are faced with two 
annoyances:
  
  - Double-clicking on a file doesn't trigger overwrite
  - Once the overwrite confirmation dialog appears, the Save dialog has already 
disappeared, so if you change your mind, you need to manually open it again.
  
  This KIO patch removes annoyance #1, and the as-yet-nonexistent Kate patch 
will remove annoyance #2

REPOSITORY
  R241 KIO

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

To: anemeth, #frameworks, #vdg, ngraham, rkflx
Cc: ltoscano, rkflx, broulik, jtamate, ngraham, #frameworks, michaelh, bruns

D12538: Always prompt for overwrite on double-click in save dialog

[Luigi Toscano]

ltoscano added a comment.


  Whatever happens, just consider that it's always possible (and it will 
happen) for users to use a newer KIO with an older Kate; so regardless of the 
Kate patch, this case should work properly.

REPOSITORY
  R241 KIO

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

To: anemeth, #frameworks, #vdg, ngraham, rkflx
Cc: ltoscano, rkflx, broulik, jtamate, ngraham, #frameworks, michaelh, bruns

D12538: Always prompt for overwrite on double-click in save dialog

[Nathaniel Graham]

ngraham added a comment.


  In D12538#255184 , @ltoscano wrote:
  
  > In D12538#255183 , @ngraham 
wrote:
  >
  > > In D12538#255101 , @ltoscano 
wrote:
  > >
  > > > Does it work if a newer KIO is used with an older version of those 
applications?
  > >
  > >
  > > In Kate's case, it overrides the default save dialog to not handle 
overwrite, just using a newer KIO wouldn't help and we would definitely want to 
ship that fix before rolling out the KIO change.
  >
  >
  > If I understand it correctly if a the current Kate (or other applications 
handling the overwrite case) is used with KIO patched with this change, then it 
will still work because Kate will handle this feature as before.
  
  
  Correct. It will just be a bit annoying in the case that you didn't mean to 
overwrite, because the "are you sure" dialog will only appear //after//the save 
dialog has disappeared, so you'll need to open it again.
  
  > So a patched KIO should be released before Kate and then Kate should be 
fixed and depend on this newer version. Is all of this correct?
  
  My preference would be to release a patched Kate first, if we think that's 
appropriate for an 18.04.1 point release. Here are our options, along with what 
will happen for each case:
  
  - **Release patched KIO before patched Kate**: Double-clicking on a file 
triggers overwrite in Kate itself, so the dialog will have annoyingly gone away
  - **Release patched Kate before patched KIO**: No drawbacks, but users don't 
get the desirable prompt-overwrite-from-the-save-dialog-on-double-click behavior
  - **Release both simultaneously** (mostly impossible due to different release 
schedules): users-double-clicking on a file in Kate's Save dialog get the 
prompt above the dialog, and canceling it humanely keeps the dialog open.

REPOSITORY
  R241 KIO

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

To: anemeth, #frameworks, #vdg, ngraham, rkflx
Cc: ltoscano, rkflx, broulik, jtamate, ngraham, #frameworks, michaelh, bruns

D12538: Always prompt for overwrite on double-click in save dialog

[Luigi Toscano]

ltoscano added a comment.


  In D12538#255183 , @ngraham wrote:
  
  > In D12538#255101 , @ltoscano 
wrote:
  >
  > > Does it work if a newer KIO is used with an older version of those 
applications?
  >
  >
  > In Kate's case, it overrides the default save dialog to not handle 
overwrite, just using a newer KIO wouldn't help and we would definitely want to 
ship that fix before rolling out the KIO change.
  
  
  If I understand it correctly if a the current Kate (or other applications 
handling the overwrite case) is used with KIO patched with this change, then it 
will still work because Kate will handle this feature as before. So a patched 
KIO should be released before Kate and then Kate should be fixed and depend on 
this newer version. Is all of this correct?

REPOSITORY
  R241 KIO

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

To: anemeth, #frameworks, #vdg, ngraham, rkflx
Cc: ltoscano, rkflx, broulik, jtamate, ngraham, #frameworks, michaelh, bruns

D12538: Always prompt for overwrite on double-click in save dialog

[Nathaniel Graham]

ngraham added a comment.


  In D12538#255101 , @ltoscano wrote:
  
  > In D12538#255048 , @ngraham 
wrote:
  >
  > > We still need to fix Kate's save dialog before we can land this, because 
right now it handles the overwrite case itself rather than delegating to the 
dialog like most KDE apps do. And we should do a sweep of other KDE apps that 
also have this legacy behavior left over from before the save dialog handled 
overwrite itself.
  >
  >
  > Does it work if a newer KIO is used with an older version of those 
applications?
  
  
  In Kate's case, it overrides the default save dialog to not handle overwrite, 
just using a newer KIO wouldn't help and we would definitely want to ship that 
fix before rolling out the KIO change.

REPOSITORY
  R241 KIO

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

To: anemeth, #frameworks, #vdg, ngraham, rkflx
Cc: ltoscano, rkflx, broulik, jtamate, ngraham, #frameworks, michaelh, bruns

D11925: Add "SkipSwitcher" to API

[Scott Harvey]

sharvey added a comment.


  In D11925#255082 , @lbeltrame 
wrote:
  
  > For now I reverted the commit in kwin to allow building. Once this review 
is merged, it can be re-instated.
  
  
  Thank you for reverting the commit. The one - and only - reason I committed 
it was because it was flagged as approved and ready to land. I apologize for 
the chaos. I'll work with my reviewers to straighten the rest of this out.

REPOSITORY
  R127 KWayland

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

To: sharvey, hein, graesslin
Cc: lbeltrame, rikmills, davidedmundson, #plasma, graesslin, #frameworks, 
michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Nathaniel Graham]

ngraham added a comment.


  In D12508#255158 , @jnoack wrote:
  
  > Imo KMessageWidget should not rely on being placed where there's enough 
contrast. The transparency just doesn't work in the case of Konsole.
  
  
  Hmm, I see your point. Let me see what I can do with it...

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D12371: fix always reproducible crash

[Jaime Torres Amate]

jtamate updated this revision to Diff 33242.
jtamate edited the summary of this revision.
jtamate added a comment.


  If I use if (!u.path().isEmpty()) or if (!u.path().isEmpty() && 
!u.path().endsWith('/')), 
  as soon as I enter the url fish://127.0.0.1 (without trailing /), I'm 
redirected to fish://127.0.0.1/home/jtorres (that didn't happen before), and 
after finishing reading the directory, assertion.
  
kf5.kio.core: Internal error: job is listing 
QUrl("fish://127.0.0.1/home/jtorres") but directoryData says no listers are 
currently listing  "fish://127.0.0.1/home/jtorres"
kf5.kio.core.dirlister: Items in use:
kf5.kio.core.dirlister: "fish://127.0.0.1/home/jtorres" URL: 
QUrl("fish://127.0.0.1/home/jtorres") rootItem: 
QUrl("fish://127.0.0.1/home/jtorres") autoUpdates refcount: 1 complete: true 
"with 1010 items."
kf5.kio.core.dirlister: Directory data:
kf5.kio.core.dirlister:"fish://127.0.0.1/home/jtorres" 0 listers: ""
kf5.kio.core.dirlister:"fish://127.0.0.1/home/jtorres" 1 holders: " 
0x3279dd0"
kf5.kio.core.dirlister: Jobs:
kf5.kio.core.dirlister: Items in cache:
kf5.kio.core.dirlister: 
"file:///virtual/kde5/5kde/build/frameworks/kio" rootItem: 
"file:///virtual/kde5/5kde/build/frameworks/kio" with 16 items.
ASSERT: "!dirData.listersCurrentlyListing.isEmpty()" in file 
/g/5kde/frameworks/kio/src/core/kcoredirlister.cpp, line 1208


#9  QMessageLogger::fatal (this=this@entry=0x7fff05486530, 
msg=msg@entry=0x7f36ba7ddeb0 "ASSERT: \"%s\" in file %s, line %d") at 
global/qlogging.cpp:816
#10 0x7f36ba548e86 in qt_assert 
(assertion=assertion@entry=0x7f36beb7b410 
"!dirData.listersCurrentlyListing.isEmpty()", file=file@entry=0x7f36beb7b0b0 
"/g/5kde/frameworks/kio/src/core/kcoredirlister.cpp", line=line@entry=1208) at 
global/qglobal.cpp:3123
#11 0x7f36beb40230 in KCoreDirListerCache::slotEntries 
(this=0x7f36bedaf420 <(anonymous 
namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, job=, entries=...) at /g/5kde/frameworks/kio/src/core/kcoredirlister.cpp:1208
...
#18 0x7f36beae500d in KIO::ListJob::entries (this=this@entry=0x324dd20, 
_t1=, _t1@entry=0x324dd20, _t2=...) at 
/virtual/kde5/5kde/build/frameworks/kio/src/core/KF5KIOCore_autogen/include/moc_listjob.cpp:232
#19 0x7f36beae6ce2 in KIO::ListJobPrivate::slotListEntries 
(this=0x37bd250, list=...) at /g/5kde/frameworks/kio/src/core/listjob.cpp:154
#20 0x7f36beae7052 in KIO::ListJobPrivateoperator() (list=..., __closure=) at 
/g/5kde/frameworks/kio/src/core/listjob.cpp:288
  
  Checking for the slash in the string representation of the url, not only in 
the path part, works for me.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12371?vs=33211&id=33242

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

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp

To: jtamate, dfaure, #frameworks, apol
Cc: anthonyfieroni, apol, michaelh, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Joshua Noack]

jnoack added a comment.


  Imo KMessageWidget should not rely on being placed where there's enough 
contrast. The transparency just doesn't work in the case of Konsole.
  Either Konsole needs to move the widget outside the terminal painting area 
which looks complicated to achieve or the message widget should provide some API
  which apps can use in cases where the background has not enough contrast.
  
  In D12508#253854 , @cfeck wrote:
  
  > Regarding the Konsole isse, it may not be the only application expecting a 
window background. Maybe some autoFillBackground helps, see 
http://doc.qt.io/qt-5/qwidget.html#autoFillBackground-prop
  
  
  I just tried setting autoFillBackground: The background is not animated, so 
it breaks the animation of the message widget. Also it is painted black. I 
guess those things could be fixed within the KMessageWidget.
  
  The question is what makes more sense, fixing each application where the 
message widget looks broken or fixing the message widget to look not broken. 🤔

REPOSITORY
  R236 KWidgetsAddons

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

To: ngraham, hein, #plasma, #vdg, elvisangelaccio, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, bruns

D11282: less expensive findByUrl in KCoreDirListerCache

[David Faure]

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


  This definitely needs a benchmark. (use Q_BENCHMARK in a qtestlib-based 
unittest)

INLINE COMMENTS

> mwolff wrote in kcoredirlister.cpp:852
> is it OK that you operate on a copy of the item here? the item in the hash 
> won't be modified, is that on purpose?

That was the case already hmm but not in my version of kio. It looks like 
this patch is on top of other patches.

REPOSITORY
  R241 KIO

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

To: jtamate, #frameworks, dfaure
Cc: mwolff, michaelh, bruns

D11604: kdirlistertest doesn't fail at random

[David Faure]

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


  I'm all for making tests more stable, but changing what the test is testing, 
sounds dangerous to me (it might hide bugs in the cases that the test intended 
to be testing). Sometimes it's necessary to change what we're testing, if we're 
testing something unreliable, but e.g. creating one file should and must 
reliably trigger an update, we need to make sure of that.

INLINE COMMENTS

> kdirlistertest.cpp:183
>  // This test assumes testOpenUrl was run before. So m_dirLister is holding 
> the items already.
> +// This test creates 1000 files in the temp directory
>  void KDirListerTest::testNewItems()

That's testing a different use case. Isn't there a risk that this hides a bug 
when a single file is created by the user (which is more common than 1000...)?

> kdirlistertest.cpp:293
>  
> -QTest::qWait(1000); // We need a 1s timestamp difference on the dir, 
> otherwise FAM won't notice anything.
> +QTest::qWait(4000); // We need a 4s timestamp difference on the dir, 
> otherwise FAM won't notice enough.
>  

Why 4s? The comment sounds like it has to be 4 exactly, but I guess this is 
just "safety"? That's a lot, it makes the test quite slow. (Rule of thumb is 
that a unittest should last less than 5s)

> kdirlistertest.cpp:502
> +QMimeDatabase db;
> +const QMimeType htmlType = 
> db.mimeTypeForUrl(QUrl(QLatin1String("file://index.html")));
> +

This URL is incorrect, it's missing a slash, you're referring to a hostname of 
"index.html" here.

But if you fix it, how then could this ever be anything else than text/html? I 
don't see the point of this indirection.

REPOSITORY
  R241 KIO

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

To: jtamate, #frameworks, dfaure
Cc: apol, michaelh, bruns

D12002: Check if group < LastGroup, as KIconEffect doesn't handle UserGroup anyway

[David Faure]

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

REPOSITORY
  R302 KIconThemes

BRANCH
  fixWarning

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

To: fabiank, mart, dfaure, mlaurent
Cc: #frameworks, michaelh, bruns

D4911: add Baloo DBus signals for moved or removed files

[David Faure]

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


  My objections no longer apply, thanks for the redesign for more performance.

REPOSITORY
  R293 Baloo

BRANCH
  master

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

To: mgallien, vhanda, dfaure, michaelh, #baloo
Cc: ngraham, cullmann, apol, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, bruns

D10363: [KIO] Add support for XDG_TEMPLATES_DIR in KNewFileMenu

[David Faure]

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


  Hello, sorry for the delay in my review, I failed to react to the 
notification and then it scrolled out of view :/

INLINE COMMENTS

> knewfilemenu.cpp:642
>  QSet seenTexts;
> +QString lastSeenText;
>  // these shall be put at special positions

That's not a (user-visible) text, that's a path, please rename the variable to 
lastTemplatePath or something like that.

> knewfilemenu.cpp:902
> +QStringList installedTemplates = { 
> QStandardPaths::locateAll(QStandardPaths::GenericDataLocation, 
> QStringLiteral("templates"), QStandardPaths::LocateDirectory) };
> +// Qt does not provide an easy way to recieve the xdg dir for templates 
> so we have to find it on our own
> +#ifdef Q_OS_UNIX

typo: receive

> knewfilemenu.cpp:977
> +key.prepend('2');
> +} else if (file.startsWith(QDir::homePath())) {
>  key.prepend('1');

can you swap the conditions so that this code still is ordered 0, 1, 2, 3? 
Makes it more readable (since it will match the resulting menu order)

But actually, shouldn't we still have New Folder and New Text File at the top 
before everything else?

> mmustac wrote in knewfilemenu.cpp:904
> It is because we need to get rid of the " before and at the end of the path.

Ah I see, OK.

REPOSITORY
  R241 KIO

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

To: mmustac, #frameworks, dfaure
Cc: broulik, ngraham, michaelh, bruns

D12371: fix always reproducible crash

[David Faure]

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

INLINE COMMENTS

> kfilewidget.cpp:1523
>  // append '/' if needed: url combo does not add it
>  // tokenize() expects it because uses KUrl::setFileName()
>  QUrl u(url);

This needs to be updated btw, to "because it uses 
QUrl::adjusted(QUrl::RemoveFilename)", but I think the idea remains the same.

> kfilewidget.cpp:1525
>  QUrl u(url);
> -if (!u.path().endsWith('/')) {
> +if (u.isLocalFile() && !u.path().endsWith('/')) {
>  u.setPath(u.path() + '/');

I don't think this is correct, we still need that code to be called for ftp:// 
and similar.

I think this needs to be if (!u.path().isEmpty()) so that only smb:// and 
ftp://host (which is supposed to redirect to the home dir) are untouched, and 
everything else gets a trailing slash.

Thanks for the investigation.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks, apol
Cc: anthonyfieroni, apol, michaelh, bruns

D7534: [KUrlNavigator] Emit tabRequested when path in path selector menu is middle-clicked

[David Faure]

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


  (even better would be to unittest this).

REPOSITORY
  R241 KIO

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

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

D11511: [KPropertiesDialog] Support X-KDE-Protocols

[David Faure]

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

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, dfaure
Cc: michaelh, bruns

D12490: concatPaths: process empty path1 correctly

[David Faure]

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


  OK, so concatPaths returns absolute paths if path1 is absolute, and relative 
paths if path1 is relative. Makes sense, but should be documented.

REPOSITORY
  R241 KIO

BRANCH
  fix_concatPaths

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

To: aleksejshilin, #frameworks, anthonyfieroni, dfaure
Cc: michaelh, bruns

D12538: Always prompt for overwrite on double-click in save dialog

[Luigi Toscano]

ltoscano added a comment.


  In D12538#255048 , @ngraham wrote:
  
  > We still need to fix Kate's save dialog before we can land this, because 
right now it handles the overwrite case itself rather than delegating to the 
dialog like most KDE apps do. And we should do a sweep of other KDE apps that 
also have this legacy behavior left over from before the save dialog handled 
overwrite itself.
  
  
  Does it work if a newer KIO is used with an older version of those 
applications?

REPOSITORY
  R241 KIO

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

To: anemeth, #frameworks, #vdg, ngraham, rkflx
Cc: ltoscano, rkflx, broulik, jtamate, ngraham, #frameworks, michaelh, bruns

D11925: Add "SkipSwitcher" to API

[Luca Beltrame]

lbeltrame added a comment.


  For now I reverted the commit in kwin to allow building. Once this review is 
merged, it can be re-instated.

REPOSITORY
  R127 KWayland

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

To: sharvey, hein, graesslin
Cc: lbeltrame, rikmills, davidedmundson, #plasma, graesslin, #frameworks, 
michaelh, bruns