D25117: file ioslave: stop copying as soon as the ioslave is killed

meven updated this revision to Diff 74543. meven added a comment. Add an env variable KIOSLAVE_FILE_ENABLE_TESTMODE to slow down copy file operations to make tests reliable REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25117?vs=74542=74543 BRANCH

D26131: Braces around for, break early.

ervin added a comment. In D26131#602149 , @dfaure wrote: > No, never const QChar&. > QChar is a short int. Indeed. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26131 To: tcanabrava, ervin, dfaure Cc:

D25117: file ioslave: stop copying as soon as the ioslave is killed

meven added inline comments. INLINE COMMENTS > jobtest.cpp:2077 > +} else { > +QVERIFY(QFile::exists(destToCheck)); > +} This is not right. It might hide a bug. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25117 To: meven, #frameworks, dfaure, ngraham,

D25117: file ioslave: stop copying as soon as the ioslave is killed

meven planned changes to this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25117 To: meven, #frameworks, dfaure, ngraham, apol Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25117: file ioslave: stop copying as soon as the ioslave is killed

meven added a comment. In D25117#602227 , @meven wrote: > Fix test I was missing the case when the dest file existed before copying. I think this is ready for landing. REPOSITORY R241 KIO REVISION DETAIL

D25117: file ioslave: stop copying as soon as the ioslave is killed

meven updated this revision to Diff 74542. meven marked 6 inline comments as done. meven added a comment. Fix test REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25117?vs=72832=74542 BRANCH arcpatch-D25117 REVISION DETAIL

KDE CI: Frameworks » kdelibs4support » kf5-qt5 FreeBSDQt5.13 - Build # 57 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/kdelibs4support/job/kf5-qt5%20FreeBSDQt5.13/57/ Project: kf5-qt5 FreeBSDQt5.13 Date of build: Wed, 29 Jan 2020 04:33:40 + Build duration: 5 min 25 sec and counting JUnit Tests

D26955: Fix background SVG colors and adjust background SVG corners

ndavis abandoned this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D26955 To: ndavis, #vdg, #plasma Cc: niccolove, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D21721: Bring KNewStuffQuick to feature parity with KNewStuff(Widgets)

aacid added a comment. FWIW this broke artikulate since you changed the expected entry for engine: in KNS.ItemsModel REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D21721 To: leinir, #knewstuff, #vdg, #frameworks, ahiemstra Cc: aacid, cfeck, davidedmundson,

D26131: Braces around for, break early.

kossebau added a comment. Getting rid of the inversion might be easier to read for simple humans (as me :) ): const bool isAscii = std::all_of(std::begin(s), std::end(s), [](QChar a) { return a.unicode() <= 127; }); REPOSITORY R237 KConfig REVISION DETAIL

D26868: Move newItem to private method in KConfigSourceGenerator

kossebau added a comment. In D26868#602150 , @dfaure wrote: > There is indeed a QString overload for concatenating QLatin1String, but it will have to be converted char-by-char (from 8 bits to 16 bits), so isn't it faster to concatenate

D26868: Move newItem to private method in KConfigSourceGenerator

dfaure added a comment. There is indeed a QString overload for concatenating QLatin1String, but it will have to be converted char-by-char (from 8 bits to 16 bits), so isn't it faster to concatenate QStringLiterals? REPOSITORY R237 KConfig REVISION DETAIL

D26131: Braces around for, break early.

dfaure added a comment. No, never const QChar&. QChar is a short int. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26131 To: tcanabrava, ervin, dfaure Cc: ervin, patrickelectric, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26957: KateModeMenuList: don't overlap the scroll bar

nibags updated this revision to Diff 74531. nibags added a comment. - Restore parameters REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26957?vs=74479=74531 BRANCH mode-menu-scrollbar-overlap REVISION DETAIL https://phabricator.kde.org/D26957

D26167: Update holidays and add flagdays and namedays for Sweden

This revision was automatically updated to reflect the committed changes. Closed by commit R175:2d9ea97d1cdd: Update holidays and add flagdays and namedays for Sweden (authored by riiga, committed by aacid). REPOSITORY R175 KHolidays CHANGES SINCE LAST UPDATE

T11627: Improve KIO asynchronicity

meven added a comment. In T11627#218943 , @meven wrote: > In T11627#206210 , @broulik wrote: > > > I'm pretty sure there's a tonne of blocking DBus calls being done all over the place during file

D26951: Remove old code that is not used anymore

ervin added a comment. LGTM, better wait for @dfaure opinion though REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D26951 To: davidre, dfaure, ervin, #frameworks Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26133: Enable Auto Save

ervin added a comment. Maybe look at KConfigCompilerSignallingItem? But I don't see another way of doing it than a wrapper approach like KConfigCompilerSignallingItem does... but then that's why ManagedConfigModule assumes GenerateProperties=true in the kcfgc, otherwise we'd have hacked the

D26867: New class: KConfigTypeInformation

ervin added a comment. Not sure if we want it in this patch or later on, but wouldn't it be better to have Param::type and CfgEntry::type be of KConfigTypeInformation type? We would then convert early and avoid all those ctor calls to KConfigTypeInformation spread in the code. I suspect

D26868: Move newItem to private method in KConfigSourceGenerator

kossebau added a comment. And more while at it: for latin1 strings used in concatenation it is recommended to use QLatin1String and for single chars QLatin1Char. QStringLiteral is more expensive here, and QStringBuilder & QString has proper overloads for the concatenations. So you want to

D26868: Move newItem to private method in KConfigSourceGenerator

ervin added inline comments. INLINE COMMENTS > KConfigSourceGenerator.cpp:659 > + > +QString KConfigSourceGenerator::newItem(const CfgEntry* entry, const QString > , const QString& defaultValue, const QString ) > +{ Since we're here, what about fixing the coding style? (same thing for the

D26876: Remove the Enum hack: finish lists with a comma, it's valid c++

ervin added inline comments. INLINE COMMENTS > KConfigHeaderGenerator.cpp:252 > > -// HACK: Use C-Style for add a comma in all but the last element, > -// just to make the source generated code equal to the old one. > -// When we are sure, revert this to a range-based-for and just

D26877: Simplify calls to whitespace() and use it in more places.

ervin added a comment. I'm not sure I like whitespace() returning the stream and then being used. In effect it leads to hiding the stream object making the code more terse indeed but less obvious I think. Maybe it's a question of personal taste, @dfaure any opinion? REPOSITORY R237

D26968: Removed hardcoded colors in background SVGs

niccolove added a comment. In D26968#602035 , @ndavis wrote: > In D26968#602034 , @niccolove wrote: > > > Okay! I'm installing Inkscape 1, for future patches. May I ask you to land this, as I don't

D26968: Removed hardcoded colors in background SVGs

ndavis retitled this revision from "Removed hardcoded values" to "Removed hardcoded colors in background SVGs". REPOSITORY R242 Plasma Framework (Library) BRANCH fix_panel_color REVISION DETAIL https://phabricator.kde.org/D26968 To: niccolove, #vdg, #plasma, ndavis Cc: ndavis, ngraham,

D26968: Removed hardcoded colors in background SVGs

This revision was automatically updated to reflect the committed changes. Closed by commit R242:ffbd186975cf: Removed hardcoded colors in background SVGs (authored by niccolove, committed by ndavis). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE

D26968: Removed hardcoded values

ndavis added a comment. In D26968#602034 , @niccolove wrote: > Okay! I'm installing Inkscape 1, for future patches. May I ask you to land this, as I don't have access rights? Sure. Do you not have a KDE developer account? REPOSITORY

D26968: Removed hardcoded values

niccolove added a comment. Okay! I'm installing Inkscape 1, for future patches. May I ask you to land this, as I don't have access rights? REPOSITORY R242 Plasma Framework (Library) BRANCH fix_panel_color REVISION DETAIL https://phabricator.kde.org/D26968 To: niccolove, #vdg,

D26968: Removed hardcoded values

ndavis added a comment. Anyway, it works as it is, so don't worry about it. REPOSITORY R242 Plasma Framework (Library) BRANCH fix_panel_color REVISION DETAIL https://phabricator.kde.org/D26968 To: niccolove, #vdg, #plasma, ndavis Cc: ndavis, ngraham, kde-frameworks-devel, LeGast00n,

D26968: Removed hardcoded values

ndavis added a comment. In D26968#602031 , @niccolove wrote: > In D26968#601998 , @ndavis wrote: > > > You should remove the color field instead of using `color:currentColor` and

D26968: Removed hardcoded values

niccolove added a comment. In D26968#601998 , @ndavis wrote: > You should remove the color field instead of using `color:currentColor` and `fill:currentColor` I tried to do that, but for some reason the "color:#effeff" automatically

D21466: Recommend rebooting after installing Samba

sitter added inline comments. INLINE COMMENTS > ngraham wrote in sambausershareplugin.h:63 > Yes, but I'd prefer to do that in another patch and port all the inline > messages at once. Ah! There's more. Fine with me then. REPOSITORY R432 File Sharing (Samba) integration REVISION DETAIL

D21466: Recommend rebooting after installing Samba

ngraham added inline comments. INLINE COMMENTS > sitter wrote in sambausershareplugin.h:63 > shouldn't this rather be a KMessageWidget? As I recall we usually use KMWs > for this type of notification. Yes, but I'd prefer to do that in another patch and port all the inline messages at once.

D26968: Removed hardcoded values

ndavis added reviewers: VDG, Plasma. ndavis added a comment. This does have rounded corners and the background is fixed. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D26968 To: niccolove, #vdg, #plasma Cc: ndavis, ngraham, kde-frameworks-devel,

D26968: Removed hardcoded values

ndavis accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH fix_panel_color REVISION DETAIL https://phabricator.kde.org/D26968 To: niccolove, #vdg, #plasma, ndavis Cc: ndavis, ngraham, kde-frameworks-devel, LeGast00n,

D26961: Be more helpfully verbose when we can't start an action

ngraham accepted this revision. REPOSITORY R283 KAuth BRANCH master REVISION DETAIL https://phabricator.kde.org/D26961 To: davidedmundson, tcanabrava, ngraham Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26968: Removed hardcoded values

ndavis added a comment. You should remove the color field instead of using `color:currentColor` and `fill:currentColor` REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D26968 To: niccolove Cc: ndavis, ngraham, kde-frameworks-devel, LeGast00n,

D26957: KateModeMenuList: don't overlap the scroll bar

ngraham added a comment. JFYI Kate has moved to GitLab and most patches are now submitted at https://invent.kde.org/kde/kate. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D26957 To: nibags, cullmann, #ktexteditor Cc: ngraham, anthonyfieroni, kwrite-devel,

D26955: Fix background SVG colors and adjust background SVG corners

ndavis added a comment. In D26955#601984 , @niccolove wrote: > Might the problems of not-rounded corners here be caused from the "adjust background SVG corners" part by any chance? I tried using the same corners as the shadow patch, but

KDE CI: Frameworks » extra-cmake-modules » kf5-qt5 SUSEQt5.12 - Build # 90 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/extra-cmake-modules/job/kf5-qt5%20SUSEQt5.12/90/ Project: kf5-qt5 SUSEQt5.12 Date of build: Tue, 28 Jan 2020 16:26:57 + Build duration: 3 min 59 sec and counting JUnit Tests Name:

KDE CI: Frameworks » extra-cmake-modules » kf5-qt5 SUSEQt5.13 - Build # 56 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/extra-cmake-modules/job/kf5-qt5%20SUSEQt5.13/56/ Project: kf5-qt5 SUSEQt5.13 Date of build: Tue, 28 Jan 2020 16:26:58 + Build duration: 3 min 16 sec and counting JUnit Tests Name:

D26969: fix type namespace requirement

sitter created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. sitter requested review of this revision. REVISION SUMMARY it was hard to find in all the text anyway. and it's an entirely pointless requirement. we can easily determine the

D26573: Add missing Import Env Variable

This revision was automatically updated to reflect the committed changes. Closed by commit R240:ee0531749057: Add missing Import Env Variable (authored by tcanabrava). REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26573?vs=74512=74513 REVISION

D26955: Fix background SVG colors and adjust background SVG corners

niccolove added a comment. Might the problems of not-rounded corners here be caused from the "adjust background SVG corners" part by any chance? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D26955 To: ndavis, #vdg, #plasma Cc: niccolove,

D26573: Add missing Import Env Variable

tcanabrava updated this revision to Diff 74512. tcanabrava added a comment. - Fix Variable REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26573?vs=73237=74512 BRANCH add_missing_env_var REVISION DETAIL https://phabricator.kde.org/D26573

D26968: Removed hardcoded values

niccolove created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. niccolove requested review of this revision. REVISION SUMMARY Alternative and more basic patch to D26955 , this aims to fix the background

D26131: Braces around for, break early.

ervin added a comment. Does it apply on top of your refactoring? Also the description looks wrong now. INLINE COMMENTS > kconfig_compiler.cpp:179 > { > -bool isAscii = true; > -for (int i = s.length(); i--;) > -if (s[i].unicode() > 127) { > -isAscii = false; > -

D26573: Add missing Import Env Variable

apol added a comment. I don't really understand why this is needed, I've never needed that at least. Having @davidedmundson's patch would be interesting too, although we might want to support earlier versions that won't get the patch. REPOSITORY R240 Extra CMake Modules BRANCH

D26573: Add missing Import Env Variable

davidedmundson added a comment. I'm now affected by this :( I think this shouldn't be needed, but I have no objections to merging this either. https://codereview.qt-project.org/c/qt/qtquickcontrols2/+/288238 REPOSITORY R240 Extra CMake Modules BRANCH add_missing_env_var

Re: SPDX KAboutLicense

On Tue, Jan 28, 2020 at 3:06 PM Aleix Pol wrote: > > On Tue, Jan 28, 2020 at 12:29 PM Harald Sitter wrote: > > > > Hey! > > > > For general interest, and so this thought doesn't only exist in my > > head: As a follow-up to us embracing SPDX for code files it would make > > a whole lot of sense

Re: SPDX KAboutLicense

On Tue, Jan 28, 2020 at 12:29 PM Harald Sitter wrote: > > Hey! > > For general interest, and so this thought doesn't only exist in my > head: As a follow-up to us embracing SPDX for code files it would make > a whole lot of sense sense to move KAboutLicense away from our custom > text format (see

D26957: KateModeMenuList: don't overlap the scroll bar

anthonyfieroni added a comment. Don't remove default parameter, it'll break other users. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D26957 To: nibags, cullmann, #ktexteditor Cc: anthonyfieroni, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, GB_2,

D21466: Recommend rebooting after installing Samba

sitter added a comment. random comment: since we install through polkit the distro should deal with this. polkit can report whether a reboot is necessary, and discover has support for that, so in theory if a distro/polkit backend reports it as necessary (because they added a new group), it

D26961: Be more helpfully verbose when we can't start an action

tcanabrava accepted this revision. This revision is now accepted and ready to land. REPOSITORY R283 KAuth BRANCH master REVISION DETAIL https://phabricator.kde.org/D26961 To: davidedmundson, tcanabrava Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26960: RFC: [KWindowShadows] Check for X connection

This revision was automatically updated to reflect the committed changes. Closed by commit R278:21d67e31f718: [KWindowShadows] Check for X connection (authored by broulik). REPOSITORY R278 KWindowSystem CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26960?vs=74486=74489 REVISION

D26961: Be more helpfully verbose when we can't start an action

davidedmundson created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. davidedmundson requested review of this revision. REVISION SUMMARY If we show what action it tried to start someone can run some more useful diagnostics. TEST PLAN

D26960: RFC: [KWindowShadows] Check for X connection

davidedmundson accepted this revision. davidedmundson added a comment. Ok, panel shadows is a singleton. That explains things. REPOSITORY R278 KWindowSystem REVISION DETAIL https://phabricator.kde.org/D26960 To: broulik, #plasma, zzag, davidedmundson Cc: davidedmundson,

D26960: RFC: [KWindowShadows] Check for X connection

davidedmundson added a comment. Heh, I'm going to quote a conversation with you: David Edmundson, [17.01.20 10:07] would you say it's always wrong to have a QWindow outlive a QApplication? Kai Uwe, [17.01.20 10:08] I would say yes, why? David Edmundson, [17.01.20

D26960: RFC: [KWindowShadows] Check for X connection

zzag accepted this revision. This revision is now accepted and ready to land. REPOSITORY R278 KWindowSystem REVISION DETAIL https://phabricator.kde.org/D26960 To: broulik, #plasma, zzag Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26960: RFC: [KWindowShadows] Check for X connection

broulik created this revision. broulik added reviewers: Plasma, zzag. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. broulik requested review of this revision. REVISION SUMMARY On teardown of plasmashell the shared pointer with shadows is unref'd quite

SPDX KAboutLicense

Hey! For general interest, and so this thought doesn't only exist in my head: As a follow-up to us embracing SPDX for code files it would make a whole lot of sense sense to move KAboutLicense away from our custom text format (see KAboutLicense::byKeyword) to SPDX as well. The text format is

D26858: Provide an implementation for the tablet interface

zzag accepted this revision. zzag added a subscriber: davidedmundson. zzag added a comment. This revision is now accepted and ready to land. Code-wise, this change looks good. I don't have a tablet device to test this patch so you may want to wait for a +1 from @davidedmundson. INLINE

D26758: Fix the size and pixel alignment of checkboxes and radiobuttons

gvgeo added a comment. Plasma theme explorer is fine. The difference is that it uses plasmaComponents, while D26271 uses plasmaComponents**3**. About plasmaComponents: There is some bug with the checkbox and the Label. Fails to take the correct

D26957: KateModeMenuList: don't overlap the scroll bar

nibags edited the summary of this revision. nibags added reviewers: cullmann, KTextEditor. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D26957 To: nibags, cullmann, #ktexteditor Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, GB_2, domson, michaelh,

D26957: KateModeMenuList: don't overlap the scroll bar

nibags created this revision. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. nibags requested review of this revision. REPOSITORY R39 KTextEditor BRANCH mode-menu-scrollbar-overlap REVISION DETAIL https://phabricator.kde.org/D26957