D23579: WIP: port ftp slave to new error reporting system

2019-10-08 Thread David Faure
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> ftp.cpp:2153
> +const auto storResult = ftpOpenCommand("stor", dest, '?', 
> ERR_CANNOT_WRITE, offset);
> +qDebug() << "!!! storresult" << storResult;
> +if (!storResult.success) {

remember to remove this

> ftp.cpp:2465
> +if (QT_CLOSE(iCopyFile) == 0 && !result.success) {
> +// If Closing the file failed but there isn't an error yet, switch
> +// into an error!

s/Closing/closing/

> sitter wrote in ftp.cpp:696
> I know what you mean, I originally started out with consts but moved away 
> because it was getting fairly repetitive and in the end the constness offers 
> little, the objects are fairly "ephemeral" much like errno.
> 
> The result handling is indeed very "wordy", but I think there is value to be 
> had in the expressiveness there:
> A Result is not really a binary affair. Yes it is failure or success, but if 
> it is a failure the developer **must** make a conscious choice to not use the 
> context information of. If Result was implicitly usable as a bool we'd not 
> force this conscious choice, opening us up for mistakes further down the line 
> when everyone has forgotten if ftpSendCmd needs/deserves result handling... 
> or worse yet, looking at old code it won't be obvious if the author ignored 
> the result context by accident or intentionally.
> 
> So, I would stay away from treating a Result like a bool.
> 
> To get to the heart of the issue though: I actually think ftpSendCmd probably 
> shouldn't even return a Result but rather a bool. Probably 99% of all callers 
> entirely ignore the Result context and treat it as a bool, which seems like a 
> strong indication that the return type is unnecessarily complex. Should I go 
> ahead with making it bool?

Forcing to use a local variable isn't forcing to test the error code, though. 
So I'm not convinced by this argument. But yes, let's go for your suggestion, 
ftpSendCmd should return a bool, good compromise.

The only error codes it currently returns are "unsupported action" (unlikely 
case of programmer error) and "cannot login". So false obviously means "cannot 
login" anyway. Not sure all callers of ftpSendCmd realized this, though :)

> dfaure wrote in ftp.cpp:801
> Oh... it's a recursive call... wow, tricky.
> 
> Yeah, that means this method returns false even if the nested method managed 
> to login. Very confusing, and probably wrong...

How about adding a check for the ret val?

> dfaure wrote in ftp.cpp:2032
> ?

`finished(Q_FUNC_INFO)` looks like a local hack?

> sitter wrote in ftp.cpp:2363
> I do need to declare it though, and by design a Result cannot have a "null" 
> state, so I either need to initialize a failure or pass here. Do you have a 
> better suggestion?

Ah ok, never mind then.

> sitter wrote in ftp.cpp:2596
> Since you mention it... where even is the eventloop for any of this?
> SlaveBase has no event processing, neither does the FTP slave. I get the 
> distinct feeling that the entire auth code never worked, or at least not 
> since kf5. To that end I'd actually be in favor of removing it since no one 
> complained (or I cannot find any bug report anyway).
> 
> From some limited testing your assessment would be mostly correct though. 
> When trying to waitForConnected there'd be a 
> ProxyAuthenticationRequiredError. Which we can handle somewhat awkwardly by 
> querying the info from the user and then setting a newly "enhanced" proxy 
> QNetworkProxy::setApplicationProxy before creating a new socket I guess.
> 
> I'm not too excited about fixing proxy auth support no one apparently needs.
> 
> Originally introduced without review as well:
> https://git.reviewboard.kde.org/r/102923/

The (restricted) event loop is within waitFor*. It's not a full event loop, but 
it processes socket events, which in turn leads to that proxy handling.

REPOSITORY
  R241 KIO

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

To: sitter, dfaure
Cc: anthonyfieroni, dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns


D23789: Add ECMGenerateExportHeaders, for improved handling of deprecated API

2019-10-08 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> ECMGenerateExportHeader.cmake:269
> +# cannot be used to reactivate the declaration, and ``Foo::doWhat`` will not
> +# have been built into the library binary.
> +#

Really? Isn't doWhat excluded only if EXCLUDE_DEPRECATED_BEFORE_AND_AT is set 
to 5.12.0?
With 5.0.0 it's still available, no?

I'm also surprised this paragraph talks about doWhat but not doBar, they both 
get disabled together, right?

> ECMGenerateExportHeader.cmake:276
> +# to the version before and at which all deprecated API has been excluded 
> from
> +# the build,
> +# Even more when building against other libraries from the same group "Bar" 
> and

comma followed by uppercase letter is unusual grammar

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, #build_system
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns


D21356: Port to ECMAddQmlModule, add plugins.qmltypes files

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau abandoned this revision.
kossebau added a comment.


  Not sure of usefulness (see D20984#543937 
), so discarding myself. Happy to 
see someone else take over though if they feel this improves things.

REPOSITORY
  R296 KDeclarative

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

To: kossebau, #plasma, apol
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D21356: Port to ECMAddQmlModule, add plugins.qmltypes files

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau added a dependency: D20984: Add ECMAddQmlModule.

REPOSITORY
  R296 KDeclarative

BRANCH
  porttoECMInstallQmlModules

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

To: kossebau, #plasma, apol
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D20984: Add ECMAddQmlModule

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau added a dependent revision: D21356: Port to ECMAddQmlModule, add 
plugins.qmltypes files.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #plasma, apol
Cc: apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns


D21344: Port to ECMAddQmlModule

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau abandoned this revision.
kossebau added a comment.


  Not sure of usefulness (see D20984#543937 
), so discarding myself. Happy to 
see someone else take over though if they feel this improves things.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: kossebau, #plasma, apol
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D21344: Port to ECMAddQmlModule

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau added a dependency: D20984: Add ECMAddQmlModule.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  porttoECMInstallQmlModules

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

To: kossebau, #plasma, apol
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D20984: Add ECMAddQmlModule

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau added a dependent revision: D21344: Port to ECMAddQmlModule.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #plasma, apol
Cc: apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns


D20984: Add ECMAddQmlModule

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau abandoned this revision.
kossebau added a comment.


  @apol Thanks again for your review work.
  
  I am still a bit unsure about the usefulness and completeness of this macro, 
even more as I would not call myself a QML expert who has seen all corners. And 
given the lack of feedback from more QML using developers here, I fear this 
macro might also have a chance of being done because it could be, not because 
it improves developer experience of those who would use it.
  
  Myself I currently have no use for it in projects I contribute to. So to not 
add a macro which only bloats the size of ECM and adds more maintenance burden, 
I for now abandon this code. If someone else still is interested, please do not 
hesitate to take over.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #plasma, apol
Cc: apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns


D23789: Add ECMGenerateExportHeaders, for improved handling of deprecated API

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau added a comment.


  So, given the lack of further change proposals or objections, I would proceed 
to push this in the next days (Thurday evening or Friday morning), to have 3 
weeks of pre-5.64 real world testing by CI and people running from git master.
  See also 
https://mail.kde.org/pipermail/kde-frameworks-devel/2019-October/094708.html

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, #build_system
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns


D23789: Add ECMGenerateExportHeaders, for improved handling of deprecated API

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 67523.
kossebau added a comment.


  - add missing group defaulting for warning settings, got lost in rebase before

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23789?vs=67324=67523

BRANCH
  addgenerateexportheader

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

AFFECTED FILES
  docs/module/ECMGenerateExportHeader.rst
  modules/ECMGenerateExportHeader.cmake
  tests/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/consumer/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/consumer/main.cpp
  
tests/ECMGenerateExportHeaderTest/consumer/testAPI_DISABLE_DEPRECATED_BEFORE_AND_AT.cmake
  tests/ECMGenerateExportHeaderTest/consumer/testAPI_NO_DEPRECATED.cmake
  tests/ECMGenerateExportHeaderTest/format_version/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/format_version/main.cpp
  tests/ECMGenerateExportHeaderTest/library/CMakeLists.txt
  tests/ECMGenerateExportHeaderTest/library/library.cpp
  tests/ECMGenerateExportHeaderTest/library/library.hpp
  tests/ECMGenerateExportHeaderTest/library/main.cpp

To: kossebau, #frameworks, #build_system
Cc: chehrlic, dfaure, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns


D24372: Compile without deprecated foreach

2019-10-08 Thread Ahmad Samir
ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in job.cpp:181
> You need a local const var to hold the result of the subjobs() method call.
> 
> (repeats)

IIUC, subjobs() returns a const QList &, do we still need a local const var?
https://api.kde.org/frameworks/kcoreaddons/html/classKCompositeJob.html#aaec8d9b05c7c4194c5ba121d43f2997e

> dfaure wrote in ktcpsocket.cpp:729
> or just iterate over `ciphers`, which is already const

Yep.

> dfaure wrote in scheduler.cpp:214
> qAsConst not needed, this method is const

(... and m_runningJobs is a member var).

> dfaure wrote in scheduler.cpp:377
> Did you try enabling this to make sure your ported code compiles?

Yes, I did. (I, like everyone else, hate to be embarrassed, so I always make 
sure it builds and passes unittests whenever I change anything except maybe 
comments :)).

> dfaure wrote in dropjob.cpp:270
> qAsConst

m_urls is declared const in DropJobPrivate: 
https://cgit.kde.org/kio.git/tree/src/widgets/dropjob.cpp#n142

> dfaure wrote in kfileitemdelegate.cpp:233
> not needed, method is const and informationList is a member

"member" is what made that concept finally click in my head; (I kept thinking 
calling begin() on a qt container won't call the const overload, but it will if 
the container is a member and the this pointer is a pointer to const).

REPOSITORY
  R241 KIO

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

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

2019-10-08 Thread Kevin Ottens
ervin added inline comments.

INLINE COMMENTS

> davidedmundson wrote in kcoreconfigskeleton.cpp:140
> Do we need to make this
> 
>   if (d->mIsDefaultImpl){
>  return d->mIsDefaultImpl();
>   }
>   return false;
> 
> and initialize mIsDefaultImpl to nullptr
> 
> so that it doesn't crash if someone subclasses KConfigSkeletonItem directly 
> and doesn't implement this?

Initializing to nullptr wouldn't help, you would get the same behavior than 
with the default constructor.

It means it doesn't crash but raises a std::bad_function_call exception, which 
I think is fine... it's the next best thing to a pure virtual, but it's caught 
at runtime. I don't think we can do better than this in the current situation.

> davidedmundson wrote in kcoreconfigskeleton.h:218
> Is this an ABI break?
> 
> :/

*gasp* you're right, how stupid of me... we can't merge that...

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart
Cc: kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns


D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

2019-10-08 Thread Kevin Ottens
ervin updated this revision to Diff 67522.
ervin added a comment.


  5.63 -> 5.64

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24494?vs=67507=67522

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

AFFECTED FILES
  autotests/kconfigskeletontest.cpp
  src/core/kcoreconfigskeleton.cpp
  src/core/kcoreconfigskeleton.h
  src/core/kcoreconfigskeleton_p.h

To: ervin, #plasma, #frameworks, dfaure, mart
Cc: kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns


D24493: Add draw-highlight action icons

2019-10-08 Thread Noah Davis
ndavis accepted this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-highlight-icons (branched from master)

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

To: trickyricky26, #vdg, ngraham, ndavis
Cc: ngraham, ndavis, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau added inline comments.

INLINE COMMENTS

> kcoreconfigskeleton.h:216
> + *
> + * @since 5.63
> + */

-> 5.64

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart
Cc: kossebau, davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns


D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

2019-10-08 Thread David Edmundson
davidedmundson added a comment.


  Looks good to me

INLINE COMMENTS

> kcoreconfigskeleton.cpp:140
> +{
> +return d->mIsDefaultImpl();
> +}

Do we need to make this

  if (d->mIsDefaultImpl){
 return d->mIsDefaultImpl();
  }
  return false;

and initialize mIsDefaultImpl to nullptr

so that it doesn't crash if someone subclasses KConfigSkeletonItem directly and 
doesn't implement this?

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns


D24490: Make kconfig_compiler generate ctors with the optional parent arg

2019-10-08 Thread Kevin Ottens
ervin updated this revision to Diff 67516.
ervin added a comment.


  Address Friedrich's comments, I went for hiding the feature behind a flag 
after all, it was just less complexity added to the compiler in the end

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24490?vs=67490=67516

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

AFFECTED FILES
  autotests/kconfig_compiler/test1.cpp.ref
  autotests/kconfig_compiler/test1.h.ref
  autotests/kconfig_compiler/test1.kcfgc
  autotests/kconfig_compiler/test10.cpp.ref
  autotests/kconfig_compiler/test11.cpp.ref
  autotests/kconfig_compiler/test11.h.ref
  autotests/kconfig_compiler/test11.kcfgc
  autotests/kconfig_compiler/test11a.cpp.ref
  autotests/kconfig_compiler/test12.cpp.ref
  autotests/kconfig_compiler/test13.cpp.ref
  autotests/kconfig_compiler/test2.cpp.ref
  autotests/kconfig_compiler/test3.cpp.ref
  autotests/kconfig_compiler/test3a.cpp.ref
  autotests/kconfig_compiler/test4.cpp.ref
  autotests/kconfig_compiler/test5.cpp.ref
  autotests/kconfig_compiler/test5.h.ref
  autotests/kconfig_compiler/test5.kcfgc
  autotests/kconfig_compiler/test6.cpp.ref
  autotests/kconfig_compiler/test7.cpp.ref
  autotests/kconfig_compiler/test8a.cpp.ref
  autotests/kconfig_compiler/test8a.h.ref
  autotests/kconfig_compiler/test8a.kcfgc
  autotests/kconfig_compiler/test8b.cpp.ref
  autotests/kconfig_compiler/test8c.cpp.ref
  autotests/kconfig_compiler/test8c.h.ref
  autotests/kconfig_compiler/test8c.kcfgc
  autotests/kconfig_compiler/test9.cpp.ref
  autotests/kconfig_compiler/test_dpointer.cpp.ref
  autotests/kconfig_compiler/test_notifiers.cpp.ref
  autotests/kconfig_compiler/test_qdebugcategory.cpp.ref
  autotests/kconfig_compiler/test_signal.cpp.ref
  autotests/kconfig_compiler/test_translation_kde.cpp.ref
  autotests/kconfig_compiler/test_translation_kde_domain.cpp.ref
  autotests/kconfig_compiler/test_translation_qt.cpp.ref
  src/kconfig_compiler/kconfig_compiler.cpp

To: ervin, #plasma, #frameworks, dfaure, mart, apol
Cc: kossebau, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns


D24453: [RFC] Unify style of new Kirigami.ListSectionHeader and CategoryDrawer

2019-10-08 Thread Carl Schwan
ognarb added inline comments.

INLINE COMMENTS

> kcategorydrawer.cpp:88
> +textRect.setTop(textRect.top() + 4);
> +textRect.setLeft(textRect.left() + 8);
>  textRect.setHeight(fontMetrics.height());

Instead of using magic number, I would create a constant named padding.

REPOSITORY
  R276 KItemViews

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

To: davidre, #frameworks, #vdg, #konversation, #kexi, #kde_edu, #kde_pim, 
#kpublictransport, #amarok
Cc: ognarb, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D24490: Make kconfig_compiler generate ctors with the optional parent arg

2019-10-08 Thread Kevin Ottens
ervin added a comment.


  In D24490#543810 , @kossebau wrote:
  
  > In D24490#543807 , @ervin wrote:
  >
  > > In D24490#543724 , @kossebau 
wrote:
  > >
  > > > How does this effect the BIC of the generated classes? Consumers might 
have exposed the generated classes in public API. Any chance this could become 
a opt-in change for KF5 times?
  > >
  > >
  > > To my knowledge it shouldn't, it's like adding an extra constructor 
really.
  >
  >
  > On the source-compatibility layer I would agree it is, but what about the 
API symbol layer (which is what I meant with public API, as in, exported 
visible symbols).
  
  
  Right, the type info will change the symbol indeed. Other option is to add an 
extra ctor instead.

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart, apol
Cc: kossebau, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns


D24372: Compile without deprecated foreach

2019-10-08 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 67510.
ahmadsamir marked 7 inline comments as done.
ahmadsamir added a comment.


  - User more descriptive var names other than list2
  - qAsConst isn't needed if the method is const and the container is a member 
var

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24372?vs=67217=67510

BRANCH
  ahmad/foreach-urifilters2 (branched from master)

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

AFFECTED FILES
  src/core/batchrenamejob.cpp
  src/core/connection.cpp
  src/core/copyjob.cpp
  src/core/desktopexecparser.cpp
  src/core/job.cpp
  src/core/kprotocolinfofactory.cpp
  src/core/kprotocolmanager.cpp
  src/core/ksslcertificatemanager.cpp
  src/core/ktcpsocket.cpp
  src/core/scheduler.cpp
  src/core/tcpslavebase.cpp
  src/urifilters/ikws/ikwsopts.cpp
  src/urifilters/ikws/searchproviderdlg.cpp
  src/urifilters/shorturi/kshorturifilter.cpp
  src/widgets/accessmanager.cpp
  src/widgets/clipboardupdater.cpp
  src/widgets/delegateanimationhandler.cpp
  src/widgets/dropjob.cpp
  src/widgets/kdesktopfileactions.cpp
  src/widgets/kdirmodel.cpp
  src/widgets/kfileitemactions.cpp
  src/widgets/kfileitemdelegate.cpp
  src/widgets/kpropertiesdialog.cpp
  src/widgets/krun_win.cpp
  src/widgets/ksslcertificatebox.cpp
  src/widgets/ksslinfodialog.cpp
  src/widgets/kurifiltersearchprovideractions.cpp
  src/widgets/previewjob.cpp
  src/widgets/sslui.cpp

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24490: Make kconfig_compiler generate ctors with the optional parent arg

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau added a comment.


  In D24490#543807 , @ervin wrote:
  
  > In D24490#543724 , @kossebau 
wrote:
  >
  > > How does this effect the BIC of the generated classes? Consumers might 
have exposed the generated classes in public API. Any chance this could become 
a opt-in change for KF5 times?
  >
  >
  > To my knowledge it shouldn't, it's like adding an extra constructor really.
  
  
  On the source-compatibility layer I would agree it is, but what about the API 
symbol layer (which is what I meant with public API, as in, exported visible 
symbols).

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart, apol
Cc: kossebau, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns


D24490: Make kconfig_compiler generate ctors with the optional parent arg

2019-10-08 Thread Kevin Ottens
ervin added a comment.


  In D24490#543724 , @kossebau wrote:
  
  > How does this effect the BIC of the generated classes? Consumers might have 
exposed the generated classes in public API. Any chance this could become a 
opt-in change for KF5 times?
  
  
  To my knowledge it shouldn't, it's like adding an extra constructor really.

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart, apol
Cc: kossebau, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns


D24415: Add standard icons to support to all entries in QDialogButtonBox

2019-10-08 Thread Björn Feber
This revision was automatically updated to reflect the committed changes.
Closed by commit R252:50593c561899: Add standard icons to support to all 
entries in QDialogButtonBox (authored by GB_2).

REPOSITORY
  R252 Framework Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24415?vs=67336=67508

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

AFFECTED FILES
  src/kstyle/kstyle.cpp

To: GB_2, #frameworks, #vdg, davidedmundson, ngraham
Cc: ndavis, dhaumann, davidedmundson, #vdg, kde-frameworks-devel, #frameworks, 
LeGast00n, GB_2, michaelh, ngraham, bruns


D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

2019-10-08 Thread Kevin Ottens
ervin updated this revision to Diff 67507.
ervin added a comment.


  Second try, without breaking ABI... it's the best workaround I found in the 
current situation, I admit I died a bit inside.

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24494?vs=67496=67507

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

AFFECTED FILES
  autotests/kconfigskeletontest.cpp
  src/core/kcoreconfigskeleton.cpp
  src/core/kcoreconfigskeleton.h
  src/core/kcoreconfigskeleton_p.h

To: ervin, #plasma, #frameworks, dfaure, mart
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns


D24502: Replace recursion in FilteredDirIterator with loop iteration

2019-10-08 Thread Stefan Brüns
bruns created this revision.
bruns added a reviewer: Baloo.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  In case many consecutive files are skipped in a directory, the
  recursive next() implementation recursed for every file, i.e.
  calling itself repeatedly.
  
  Instead, loop over all directory items until either a file or directory
  is found which should be indexed, or there are no more items left.
  
  Move the "shouldIndexHidden" variable out of the loop, as it is
  invariant.

TEST PLAN
  ctest -R filtereddiriterator

REPOSITORY
  R293 Baloo

BRANCH
  iterative_filtereddiriterator

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

AFFECTED FILES
  src/file/filtereddiriterator.cpp

To: bruns, #baloo
Cc: kde-frameworks-devel, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, 
ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams


D24453: [RFC] Unify style of new Kirigami.ListSectionHeader and CategoryDrawer

2019-10-08 Thread Nathaniel Graham
ngraham added a comment.


  LGTM except for the abrupt end on the right side where the scrollbar's area 
begins, but this is a pre-existing problem with all things that butt up against 
a scrollbar in a scrollview, and I think we should add a vertical separator 
line to fix it universally.

REPOSITORY
  R276 KItemViews

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

To: davidre, #frameworks, #vdg, #konversation, #kexi, #kde_edu, #kde_pim, 
#kpublictransport, #amarok
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D24493: Add draw-highlight action icons

2019-10-08 Thread Nathaniel Graham
ngraham added subscribers: ndavis, ngraham.
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  LGTM! @ndavis?

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-highlight-icons (branched from master)

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

To: trickyricky26, #vdg, ngraham
Cc: ngraham, ndavis, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D24493: Add draw-highlight action icons

2019-10-08 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-highlight-icons (branched from master)

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

To: trickyricky26, #vdg, ngraham
Cc: ngraham, ndavis, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D24490: Make kconfig_compiler generate ctors with the optional parent arg

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau added a comment.


  How does this effect the BIC of the generated classes? Consumers might have 
exposed the generated classes in public API. Any chance this could become a 
opt-in change for KF5 times?

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart, apol
Cc: kossebau, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns


D24490: Make kconfig_compiler generate ctors with the optional parent arg

2019-10-08 Thread Aleix Pol Gonzalez
apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.


  Right, makes sense.

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart, apol
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24496: Use ECMGenerateExportHeader to manage deprecated API better

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau created this revision.
kossebau added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  Allows
  
  - projects linking to KConfigCore/Gui to hide deprecated API up to a given 
version or silence deprecation warnings after a given version, using
- -DKCONFIGCORE_DISABLE_DEPRECATED_BEFORE_AND_AT
- -DKCONFIGCORE_NO_DEPRECATED
- -DKCONFIGCORE_DEPRECATED_WARNINGS_SINCE
- -DKCONFIGCORE_NO_DEPRECATED_WARNINGS
- -DKCONFIGGUI_DISABLE_DEPRECATED_BEFORE_AND_AT
- -DKCONFIGGUI_NO_DEPRECATED
- -DKCONFIGGUI_DEPRECATED_WARNINGS_SINCE
- -DKCONFIGGUI_NO_DEPRECATED_WARNINGS
  
  or
- -DKF_DISABLE_DEPRECATED_BEFORE_AND_AT
- -DKF_NO_DEPRECATED
- -DKF_DEPRECATED_WARNINGS_SINCE
- -DKF_NO_DEPRECATED_WARNINGS
  - to build KConfigCore/Gui optionally with deprecated API excluded from the 
build, using "EXCLUDE_DEPRECATED_BEFORE_AND_AT" cmake argument.

TEST PLAN
  Builds with EXCLUDE_DEPRECATED_BEFORE_AND_AT set to 0, 4.0.0, 5.0.0,
  5.11.0, 5.24.0, 5.39.0, 5.42.0, CURRENT.

REPOSITORY
  R237 KConfig

BRANCH
  useECMGenerateExportHeader

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

AFFECTED FILES
  CMakeLists.txt
  autotests/kconfigtest.cpp
  autotests/kdesktopfiletest.cpp
  autotests/kdesktopfiletest.h
  src/core/CMakeLists.txt
  src/core/kauthorized.cpp
  src/core/kauthorized.h
  src/core/kconfig.cpp
  src/core/kconfig.h
  src/core/kconfiggroup.cpp
  src/core/kconfiggroup.h
  src/core/kcoreconfigskeleton.cpp
  src/core/kcoreconfigskeleton.h
  src/core/kdesktopfile.cpp
  src/core/kdesktopfile.h
  src/core/kemailsettings.cpp
  src/core/kemailsettings.h
  src/gui/CMakeLists.txt
  src/gui/kconfiggui.cpp
  src/gui/kconfiggui.h
  src/gui/kconfigloader.cpp
  src/gui/kconfigloader.h
  src/gui/kstandardshortcut.cpp
  src/gui/kstandardshortcut.h

To: kossebau, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24497: Use ECMGenerateExportHeader to manage deprecated API better

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau created this revision.
kossebau added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  Allows
  
  - projects linking to KConfigWidgets to hide deprecated API up to a given 
version or silence deprecation warnings after a given version, using
- -DKCONFIGWIDGETS_DISABLE_DEPRECATED_BEFORE_AND_AT
- -DKCONFIGWIDGETS_NO_DEPRECATED
- -DKCONFIGWIDGETS_DEPRECATED_WARNINGS_SINCE
- -DKCONFIGWIDGETS_NO_DEPRECATED_WARNINGS
  
  or
- -DKF_DISABLE_DEPRECATED_BEFORE_AND_AT
- -DKF_NO_DEPRECATED
- -DKF_DEPRECATED_WARNINGS_SINCE
- -DKF_NO_DEPRECATED_WARNINGS
  - to build KConfigWidgets optionally with deprecated API excluded from the 
build, using "EXCLUDE_DEPRECATED_BEFORE_AND_AT" cmake argument.

TEST PLAN
  Builds with EXCLUDE_DEPRECATED_BEFORE_AND_AT set to 0, 4.0.0, 5.0.0,
  5.23.0, 5.38.0, 5.39.0, CURRENT.

REPOSITORY
  R265 KConfigWidgets

BRANCH
  useECMGenerateExportHeader

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

AFFECTED FILES
  CMakeLists.txt
  autotests/kconfigdialog_unittest.cpp
  src/CMakeLists.txt
  src/kcmodule.cpp
  src/kcmodule.h
  src/kconfigdialogmanager.cpp
  src/kconfigdialogmanager.h
  src/kpastetextaction.cpp
  src/kpastetextaction.h
  src/kstandardaction.cpp
  src/kstandardaction.h
  src/kstandardaction_p.h

To: kossebau, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

2019-10-08 Thread David Edmundson
davidedmundson added a comment.


  In principle +

INLINE COMMENTS

> kcoreconfigskeleton.h:218
> + */
> +virtual bool isDefault() const = 0;
> +

Is this an ABI break?

:/

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns


D24495: Use ECMGenerateExportHeader to manage deprecated API better

2019-10-08 Thread Friedrich W. H. Kossebau
kossebau created this revision.
kossebau added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  Allows
  
  - projects linking to KCompletion to hide deprecated API up to a given 
version or silence deprecation warnings after a given version, using
- -DKCOMPLETION_DISABLE_DEPRECATED_BEFORE_AND_AT
- -DKCOMPLETION_NO_DEPRECATED
- -DKCOMPLETION_DEPRECATED_WARNINGS_SINCE
- -DKCOMPLETION_NO_DEPRECATED_WARNINGS
  
  or
- -DKF_DISABLE_DEPRECATED_BEFORE_AND_AT
- -DKF_NO_DEPRECATED
- -DKF_DEPRECATED_WARNINGS_SINCE
- -DKF_NO_DEPRECATED_WARNINGS
  - to build KCompletion optionally with deprecated API excluded from the 
build, using "EXCLUDE_DEPRECATED_BEFORE_AND_AT" cmake argument.

TEST PLAN
  Builds with EXCLUDE_DEPRECATED_BEFORE_AND_AT set to 0, 4.0.0, 4.5.0,
  5.0.0, 5.46.0, CURRENT.

REPOSITORY
  R284 KCompletion

BRANCH
  useECMGenerateExportHeader

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

AFFECTED FILES
  CMakeLists.txt
  autotests/klineedit_unittest.cpp
  src/CMakeLists.txt
  src/kcombobox.cpp
  src/kcombobox.h
  src/kcompletionbase.h
  src/kcompletionbox.h
  src/klineedit.cpp
  src/klineedit.h
  src/ksortablelist.h

To: kossebau, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24494: Add convenience for defaults/dirty states to KCoreConfigSkeleton

2019-10-08 Thread Kevin Ottens
ervin created this revision.
ervin added reviewers: Plasma, Frameworks, dfaure, mart.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ervin requested review of this revision.

REVISION SUMMARY
  It allows to verify if all the items of the skeleton are in their
  default values or if they hold any value deviating from the latest
  loaded values from KConfig.
  
  We didn't really need this during the KCModule/QtWidgets time since we
  could write KConfigDialogManager just fine without it. But for use with
  QML and aiming at having similar magic in KQuickAddons::ConfigModule
  such convenience functions will be needed.

REPOSITORY
  R237 KConfig

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

AFFECTED FILES
  autotests/kconfigskeletontest.cpp
  src/core/kcoreconfigskeleton.cpp
  src/core/kcoreconfigskeleton.h

To: ervin, #plasma, #frameworks, dfaure, mart
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24430: [KCModuleLoader] Show error when QML fails to load

2019-10-08 Thread Kai Uwe Broulik
broulik closed this revision.
broulik added a comment.


  
https://cgit.kde.org/kcmutils.git/commit/?id=8cea2dacd80565f6d22bf1e5151e5c7be1620eab

REPOSITORY
  R295 KCMUtils

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

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


D24463: Treat "application/x-ms-dos-executable" as executable on all platforms

2019-10-08 Thread Björn Feber
GB_2 added inline comments.

INLINE COMMENTS

> ppeter wrote in executablefileopendialog.cpp:58
>   if (mode == OnlyExecute)
> connect(executeButton, ...)
>   else if (mode == OpenAsExecute)
> ...
> 
> looks better :)

That would be against the common KDE coding style: 
https://community.kde.org/Policies/Kdelibs_Coding_Style#Braces

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: guoyunhe, #frameworks, ngraham, dfaure
Cc: GB_2, ppeter, dfaure, ngraham, broulik, kde-frameworks-devel, LeGast00n, 
michaelh, bruns


KDE CI: Frameworks » kcmutils » kf5-qt5 WindowsMSVCQt5.13 - Build # 17 - Failure!

2019-10-08 Thread CI System
BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks/job/kcmutils/job/kf5-qt5%20WindowsMSVCQt5.13/17/
 Project:
kf5-qt5 WindowsMSVCQt5.13
 Date of build:
Tue, 08 Oct 2019 12:41:23 +
 Build duration:
5 min 7 sec and counting
   CONSOLE OUTPUT
  [...truncated 337 lines...][2019-10-08T12:46:06.863Z]  * QCH, API documentation in QCH format (for e.g. Qt Assistant, Qt Creator & KDevelop)[2019-10-08T12:46:06.863Z] [2019-10-08T12:46:06.863Z] -- The following REQUIRED packages have been found:[2019-10-08T12:46:06.863Z] [2019-10-08T12:46:06.863Z]  * Qt5Gui (required version >= 5.13.0)[2019-10-08T12:46:06.863Z]  * Qt5Widgets[2019-10-08T12:46:06.863Z]  * Qt5DBus[2019-10-08T12:46:06.863Z]  * Qt5Network (required version >= 5.13.0)[2019-10-08T12:46:06.863Z]  * Qt5Qml[2019-10-08T12:46:06.863Z]  * Qt5Quick[2019-10-08T12:46:06.863Z]  * Qt5QuickWidgets[2019-10-08T12:46:06.863Z]  * Qt5 (required version >= 5.11.0)[2019-10-08T12:46:06.863Z]  * KF5ItemViews (required version >= 5.63.0)[2019-10-08T12:46:06.863Z]  * KF5ConfigWidgets (required version >= 5.63.0)[2019-10-08T12:46:06.863Z]  * KF5CoreAddons (required version >= 5.63.0)[2019-10-08T12:46:06.863Z]  * Gettext[2019-10-08T12:46:06.863Z]  * KF5I18n (required version >= 5.63.0)[2019-10-08T12:46:06.863Z]  * KF5IconThemes (required version >= 5.63.0)[2019-10-08T12:46:06.863Z]  * KF5Service (required version >= 5.63.0)[2019-10-08T12:46:06.863Z]  * KF5XmlGui (required version >= 5.63.0)[2019-10-08T12:46:06.863Z]  * ECM (required version >= 1.6.0), Extra CMake Modules., [2019-10-08T12:46:06.863Z]  * KF5Declarative (required version >= 5.63.0)[2019-10-08T12:46:06.863Z]  * Doxygen (required version >= 1.8.13)[2019-10-08T12:46:06.863Z]Needed for API dox QCH file generation[2019-10-08T12:46:06.863Z]  * QHelpGenerator, Part of Qt5 tools[2019-10-08T12:46:06.863Z]Needed for API dox QCH file generation[2019-10-08T12:46:06.863Z] [2019-10-08T12:46:06.863Z] -- Configuring done[2019-10-08T12:46:07.128Z] -- Generating done[2019-10-08T12:46:07.387Z] -- Build files have been written to: C:/CI/workspace/Frameworks/kcmutils/kf5-qt5 WindowsMSVCQt5.13/build[Pipeline] }[Pipeline] // stage[Pipeline] stage[Pipeline] { (Compiling)[Pipeline] bat[2019-10-08T12:46:10.557Z] [2019-10-08T12:46:10.557Z] C:\CI\workspace\Frameworks\kcmutils\kf5-qt5 WindowsMSVCQt5.13>call "C:/Program Files (x86)/Microsoft Visual Studio/2019/Professional/VC/Auxiliary/Build/vcvars64.bat" -vcvars_ver=14.16 [2019-10-08T12:46:10.971Z] **[2019-10-08T12:46:10.971Z] ** Visual Studio 2019 Developer Command Prompt v16.3.0[2019-10-08T12:46:10.971Z] ** Copyright (c) 2019 Microsoft Corporation[2019-10-08T12:46:10.971Z] **[2019-10-08T12:46:21.140Z] [vcvarsall.bat] Environment initialized for: 'x64'[2019-10-08T12:46:21.140Z] [2019-10-08T12:46:21.140Z] jom 1.1.3 - empower your cores[2019-10-08T12:46:21.140Z] [2019-10-08T12:46:21.140Z] jom: parallel job execution disabled for Makefile[2019-10-08T12:46:21.140Z] Scanning dependencies of target KF5KCMUtils_autogen[2019-10-08T12:46:21.140Z] Scanning dependencies of target KF5KCMUtils_QCH[2019-10-08T12:46:21.140Z] [  6%] Automatic MOC for target KF5KCMUtils[2019-10-08T12:46:21.721Z] [ 12%] Built target KF5KCMUtils_autogen[2019-10-08T12:46:21.721Z] [ 12%] Generating src/KF5KCMUtils.qch, src/KF5KCMUtils.tags[2019-10-08T12:46:22.367Z] Scanning dependencies of target KF5KCMUtils[2019-10-08T12:46:23.309Z] [ 81%] Built target KF5KCMUtils_QCH[2019-10-08T12:46:23.309Z] [ 18%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/kcmoduleinfo.cpp.obj[2019-10-08T12:46:23.309Z] [ 25%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/kcmoduleloader.cpp.obj[2019-10-08T12:46:23.309Z] kcmoduleinfo.cpp[2019-10-08T12:46:23.309Z] [ 31%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/kcmoduleqml.cpp.obj[2019-10-08T12:46:23.309Z] [ 37%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/kcmultidialog.cpp.obj[2019-10-08T12:46:23.309Z] [ 43%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/kcmoduleproxy.cpp.obj[2019-10-08T12:46:23.309Z] [ 50%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/kpluginselector.cpp.obj[2019-10-08T12:46:23.309Z] [ 56%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/kcmodulecontainer.cpp.obj[2019-10-08T12:46:23.309Z] [ 62%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/ksettingswidgetadaptor.cpp.obj[2019-10-08T12:46:23.309Z] [ 68%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/ksettings/dispatcher.cpp.obj[2019-10-08T12:46:23.309Z] [ 75%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/ksettings/dialog.cpp.obj[2019-10-08T12:46:23.309Z] [ 81%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/ksettings/pluginpage.cpp.obj[2019-10-08T12:46:23.309Z] [ 87%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/ksettings/componentsdialog.cpp.obj[2019-10-08T12:46:25.879Z] 

D24493: Add draw-highlight action icons

2019-10-08 Thread TrickyRicky
trickyricky26 edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24493: Add draw-highlight action icons

2019-10-08 Thread TrickyRicky
trickyricky26 edited the summary of this revision.
trickyricky26 edited the test plan for this revision.
trickyricky26 added a reviewer: VDG.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


KDE CI: Frameworks » kcmutils » kf5-qt5 SUSEQt5.13 - Build # 24 - Failure!

2019-10-08 Thread CI System
BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks/job/kcmutils/job/kf5-qt5%20SUSEQt5.13/24/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Tue, 08 Oct 2019 12:41:23 +
 Build duration:
3 min 6 sec and counting
   CONSOLE OUTPUT
  [...truncated 289 lines...][2019-10-08T12:44:14.850Z] [2019-10-08T12:44:14.850Z]  * Qt5Gui (required version >= 5.13.1)[2019-10-08T12:44:14.850Z]  * Qt5Network (required version >= 5.13.1)[2019-10-08T12:44:14.850Z]  * Qt5Qml[2019-10-08T12:44:14.850Z]  * Qt5QuickWidgets[2019-10-08T12:44:14.850Z]  * Qt5 (required version >= 5.11.0)[2019-10-08T12:44:14.850Z]  * KF5ItemViews (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * Gettext[2019-10-08T12:44:14.850Z]  * KF5I18n (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * KF5IconThemes (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * KF5Service (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * Qt5DBus (required version >= 5.11.0)[2019-10-08T12:44:14.850Z]  * KF5Auth (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * KF5Codecs (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * Qt5Widgets (required version >= 5.11.0)[2019-10-08T12:44:14.850Z]  * KF5WidgetsAddons (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * KF5ConfigWidgets (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * KF5XmlGui (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * Qt5Quick (required version >= 5.11.0)[2019-10-08T12:44:14.850Z]  * KF5CoreAddons (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * ECM (required version >= 1.6.0), Extra CMake Modules., [2019-10-08T12:44:14.850Z]  * KF5Package (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * KF5Declarative (required version >= 5.63.0)[2019-10-08T12:44:14.850Z]  * Doxygen (required version >= 1.8.13)[2019-10-08T12:44:14.850Z]Needed for API dox QCH file generation[2019-10-08T12:44:14.850Z]  * QHelpGenerator, Part of Qt5 tools[2019-10-08T12:44:14.850Z]Needed for API dox QCH file generation[2019-10-08T12:44:14.850Z]  * Qt5Core[2019-10-08T12:44:14.850Z] [2019-10-08T12:44:14.850Z] -- Configuring done[2019-10-08T12:44:14.850Z] -- Generating done[2019-10-08T12:44:14.850Z] -- Build files have been written to: /home/jenkins/workspace/Frameworks/kcmutils/kf5-qt5 SUSEQt5.13/build[Pipeline] }[Pipeline] // stage[Pipeline] stage[Pipeline] { (Compiling)[Pipeline] sh[2019-10-08T12:44:17.276Z] + python3 -u ci-tooling/helpers/compile-build.py --product Frameworks --project kcmutils --branchGroup kf5-qt5 --platform SUSEQt5.13 --usingInstall /home/jenkins//install-prefix/[2019-10-08T12:44:17.276Z] Scanning dependencies of target KF5ConfigWidgets_QCH[2019-10-08T12:44:17.276Z] Scanning dependencies of target KF5Service_QCH[2019-10-08T12:44:17.276Z] Scanning dependencies of target KF5Auth_QCH[2019-10-08T12:44:17.276Z] Scanning dependencies of target KF5Config_QCH[2019-10-08T12:44:17.276Z] Scanning dependencies of target KF5CoreAddons_QCH[2019-10-08T12:44:17.276Z] Scanning dependencies of target KF5Codecs_QCH[2019-10-08T12:44:17.276Z] [  0%] Built target KF5ConfigWidgets_QCH[2019-10-08T12:44:17.276Z] [  0%] Built target KF5Service_QCH[2019-10-08T12:44:17.276Z] [  0%] Built target KF5Auth_QCH[2019-10-08T12:44:17.276Z] [  0%] Built target KF5CoreAddons_QCH[2019-10-08T12:44:17.276Z] [  0%] Built target KF5Config_QCH[2019-10-08T12:44:17.276Z] [  0%] Built target KF5Codecs_QCH[2019-10-08T12:44:17.276Z] Scanning dependencies of target KF5WidgetsAddons_QCH[2019-10-08T12:44:17.276Z] Scanning dependencies of target KF5KCMUtils_autogen[2019-10-08T12:44:17.276Z] [  6%] Automatic MOC for target KF5KCMUtils[2019-10-08T12:44:17.276Z] [  6%] Built target KF5WidgetsAddons_QCH[2019-10-08T12:44:17.276Z] Scanning dependencies of target KF5KCMUtils_QCH[2019-10-08T12:44:17.276Z] [ 12%] Generating src/KF5KCMUtils.qch, src/KF5KCMUtils.tags[2019-10-08T12:44:19.616Z] [ 12%] Built target KF5KCMUtils_autogen[2019-10-08T12:44:20.665Z] Building up file structure...[2019-10-08T12:44:20.665Z] Insert custom filters...[2019-10-08T12:44:20.665Z] Insert help data for filter section (1 of 1)...[2019-10-08T12:44:20.665Z] Insert files...[2019-10-08T12:44:20.665Z] Insert contents...[2019-10-08T12:44:20.665Z] Insert indices...[2019-10-08T12:44:20.665Z] Documentation successfully generated.[2019-10-08T12:44:20.665Z] [ 12%] Built target KF5KCMUtils_QCH[2019-10-08T12:44:20.665Z] Scanning dependencies of target KF5KCMUtils[2019-10-08T12:44:20.665Z] [ 18%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/KF5KCMUtils_autogen/mocs_compilation.cpp.o[2019-10-08T12:44:20.665Z] [ 25%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/kcmoduleinfo.cpp.o[2019-10-08T12:44:20.665Z] [ 31%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/kcmoduleloader.cpp.o[2019-10-08T12:44:20.665Z] [ 37%] Building CXX object src/CMakeFiles/KF5KCMUtils.dir/kcmultidialog.cpp.o[2019-10-08T12:44:20.665Z] [ 43%] Building CXX object 

D24493: Add draw-highlight action icons

2019-10-08 Thread TrickyRicky
trickyricky26 created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
trickyricky26 requested review of this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-highlight-icons (branched from master)

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

AFFECTED FILES
  icons-dark/actions/16/draw-highlight.svg
  icons-dark/actions/22/draw-highlight.svg
  icons/actions/16/draw-highlight.svg
  icons/actions/22/draw-highlight.svg

To: trickyricky26
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24492: [FilteredDirIterator] Reduce stack pressure

2019-10-08 Thread Kai Krakow
hurikhan77 added a comment.


  I think we can also assume that using possibly unlimited recursion is also 
not good practice.
  
  Let me change that into a proper loop.

REPOSITORY
  R293 Baloo

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

To: hurikhan77, #baloo
Cc: cullmann, shubham, kde-frameworks-devel, lots0logs, LeGast00n, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams


D24492: [FilteredDirIterator] Reduce stack pressure

2019-10-08 Thread Christoph Cullmann
cullmann added a comment.


  Yes, I think you want the concept of a loop with continue.

REPOSITORY
  R293 Baloo

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

To: hurikhan77, #baloo
Cc: cullmann, shubham, kde-frameworks-devel, lots0logs, LeGast00n, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams


D24463: Treat "application/x-ms-dos-executable" as executable on all platforms

2019-10-08 Thread Yi-Jyun Pan
ppeter added inline comments.

INLINE COMMENTS

> executablefileopendialog.cpp:58
> +
> +if (mode == OnlyExecute) {
> +connect(executeButton, ::clicked, 
> [=]{done(ExecuteFile);});

if (mode == OnlyExecute)
connect(executeButton, ...)
  else if (mode == OpenAsExecute)
...

looks better :)

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: guoyunhe, #frameworks, ngraham, dfaure
Cc: ppeter, dfaure, ngraham, broulik, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns


D24490: Make kconfig_compiler generate ctors with the optional parent arg

2019-10-08 Thread Kevin Ottens
ervin added inline comments.

INLINE COMMENTS

> apol wrote in test_notifiers.cpp.ref:7
> Maybe it would be better to use the parent argument in KConfigSkeleton?
> explicit KConfigSkeleton(const QString  = QString(), QObject 
> *parent = nullptr);

It's what I attempted first, the problem was that it will create compatibility 
issues with existing code. From the generator perspective KConfigSkeleton might 
not be the base class and since we never enforced the parent chaining 
constructor before they might not have it. The tests using MyPrefs as base 
class exhibit that exact problem.

That's why I ended up falling back on setParent... it doesn't have my 
preference but at least now we won't force the use of setParent in user code 
(even though we use it internally).

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24429: [ConfigModule] Expose mainUi component status and error string

2019-10-08 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R296:2c2942cee9ec: [ConfigModule] Expose mainUi component 
status and error string (authored by broulik).

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24429?vs=67355=67494

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

AFFECTED FILES
  src/quickaddons/configmodule.cpp
  src/quickaddons/configmodule.h

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


D24492: [FilteredDirIterator] Reduce stack pressure

2019-10-08 Thread Shubham
shubham added a comment.


  It is not a good practice to "goto" in C++

REPOSITORY
  R293 Baloo

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

To: hurikhan77, #baloo
Cc: shubham, kde-frameworks-devel, lots0logs, LeGast00n, fbampaloukas, GB_2, 
domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams


D24490: Make kconfig_compiler generate ctors with the optional parent arg

2019-10-08 Thread Aleix Pol Gonzalez
apol added a comment.


  +1 in spirit.

INLINE COMMENTS

> test_notifiers.cpp.ref:7
> +TestNotifiers::TestNotifiers( int Number, QObject *parent )
>: KConfigSkeleton( QStringLiteral( "test7rc" ) )
>, mParamNumber(Number)

Maybe it would be better to use the parent argument in KConfigSkeleton?
explicit KConfigSkeleton(const QString  = QString(), QObject *parent 
= nullptr);

REPOSITORY
  R237 KConfig

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

To: ervin, #plasma, #frameworks, dfaure, mart
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24492: [FilteredDirIterator] Reduce stack pressure

2019-10-08 Thread Kai Krakow
hurikhan77 created this revision.
hurikhan77 added a reviewer: Baloo.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
hurikhan77 requested review of this revision.

REVISION SUMMARY
  Let's not call ourselves recursively in next() when the currently iterated 
file is going to be skipped.

REPOSITORY
  R293 Baloo

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

AFFECTED FILES
  src/file/filtereddiriterator.cpp

To: hurikhan77, #baloo
Cc: kde-frameworks-devel, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, 
ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams


D24430: [KCModuleLoader] Show error when QML fails to load

2019-10-08 Thread Marco Martin
mart accepted this revision.

REPOSITORY
  R295 KCMUtils

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

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


D24429: [ConfigModule] Expose mainUi component status and error string

2019-10-08 Thread Marco Martin
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R296 KDeclarative

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

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


D24490: Make kconfig_compiler generate ctors with the optional parent arg

2019-10-08 Thread Kevin Ottens
ervin created this revision.
ervin added reviewers: Plasma, Frameworks, dfaure, mart.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ervin requested review of this revision.

REPOSITORY
  R237 KConfig

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

AFFECTED FILES
  autotests/kconfig_compiler/test1.cpp.ref
  autotests/kconfig_compiler/test1.h.ref
  autotests/kconfig_compiler/test10.cpp.ref
  autotests/kconfig_compiler/test10.h.ref
  autotests/kconfig_compiler/test11.cpp.ref
  autotests/kconfig_compiler/test11.h.ref
  autotests/kconfig_compiler/test11a.cpp.ref
  autotests/kconfig_compiler/test11a.h.ref
  autotests/kconfig_compiler/test12.cpp.ref
  autotests/kconfig_compiler/test12.h.ref
  autotests/kconfig_compiler/test13.cpp.ref
  autotests/kconfig_compiler/test13.h.ref
  autotests/kconfig_compiler/test2.cpp.ref
  autotests/kconfig_compiler/test2.h.ref
  autotests/kconfig_compiler/test3.cpp.ref
  autotests/kconfig_compiler/test3.h.ref
  autotests/kconfig_compiler/test3a.cpp.ref
  autotests/kconfig_compiler/test3a.h.ref
  autotests/kconfig_compiler/test4.cpp.ref
  autotests/kconfig_compiler/test4.h.ref
  autotests/kconfig_compiler/test5.cpp.ref
  autotests/kconfig_compiler/test5.h.ref
  autotests/kconfig_compiler/test6.cpp.ref
  autotests/kconfig_compiler/test6.h.ref
  autotests/kconfig_compiler/test7.cpp.ref
  autotests/kconfig_compiler/test7.h.ref
  autotests/kconfig_compiler/test8a.cpp.ref
  autotests/kconfig_compiler/test8a.h.ref
  autotests/kconfig_compiler/test8b.cpp.ref
  autotests/kconfig_compiler/test8b.h.ref
  autotests/kconfig_compiler/test8c.cpp.ref
  autotests/kconfig_compiler/test8c.h.ref
  autotests/kconfig_compiler/test9.cpp.ref
  autotests/kconfig_compiler/test9.h.ref
  autotests/kconfig_compiler/test_dpointer.cpp.ref
  autotests/kconfig_compiler/test_dpointer.h.ref
  autotests/kconfig_compiler/test_notifiers.cpp.ref
  autotests/kconfig_compiler/test_notifiers.h.ref
  autotests/kconfig_compiler/test_qdebugcategory.cpp.ref
  autotests/kconfig_compiler/test_qdebugcategory.h.ref
  autotests/kconfig_compiler/test_signal.cpp.ref
  autotests/kconfig_compiler/test_signal.h.ref
  autotests/kconfig_compiler/test_translation_kde.cpp.ref
  autotests/kconfig_compiler/test_translation_kde.h.ref
  autotests/kconfig_compiler/test_translation_kde_domain.cpp.ref
  autotests/kconfig_compiler/test_translation_kde_domain.h.ref
  autotests/kconfig_compiler/test_translation_qt.cpp.ref
  autotests/kconfig_compiler/test_translation_qt.h.ref
  src/kconfig_compiler/kconfig_compiler.cpp

To: ervin, #plasma, #frameworks, dfaure, mart
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24489: KAutosaveFile not respecting maximum filename length

2019-10-08 Thread Jean-Baptiste Mardelle
mardelle created this revision.
mardelle added a reviewer: Frameworks.
mardelle added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
mardelle requested review of this revision.

REVISION SUMMARY
  There are 2 different issues in current code regarding maximum filename 
length:
  
  1- We use FILENAME_MAX which is defined as 4096, while most filesystems have 
a max length of 256. Replacing FILENAME_MAX with NAME_MAX fixes this first 
problem (could not test on Windows if it works)
  
  2- We are calculating the maximum length on the UTF-8 string, then encoding 
to percent encoding. This can result in longer strings since single characters 
will be replaced by a percent string. So in some situations, we end up with a 
string longer than allowed. Doing the percent encoding before length 
calculation fixes the problem.
  
  This fixes the follwing bug: https://bugs.kde.org/show_bug.cgi?id=412519

TEST PLAN
  Bug is fixed with the changes

REPOSITORY
  R244 KCoreAddons

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

AFFECTED FILES
  src/lib/io/kautosavefile.cpp

To: mardelle, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24487: fix password error condition handling of smb mount

2019-10-08 Thread Harald Sitter
sitter created this revision.
sitter added a reviewer: dfaure.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  this was previously wrong in two ways:
  
  1. errors from the password check would previously be ignored but still end 
in early termination of the mount command despite that command not having 
finished (successfully)
  2. since the password check is always run we may not actually require auth 
data in which case it is perfectly reasonable for the user to cancel the auth 
request (the user experience sucks, but there's not much we can do within the 
special command I think)
  
  to solve both issues special now exists in error when there was an error
  forwarded out of checkpassword (e.g. kiod is broken) BUT NOT when that
  error is that the user canceled the auth query.
  
  no auth info being provided is already supported later on in the actual
  mount code.
  
  testing code for posterity
  
#define KIO_ARGS QByteArray packedArgs; QDataStream stream( , 
QIODevice::WriteOnly ); stream

KIO_ARGS << int(1)
 << QString("HOST/PATH/") // remotepath
 << QString("/LOCAL/MNT") // mountpoint
;
auto job = KIO::special(QUrl("smb://HOST/PATH/"), packedArgs);

TEST PLAN
  broken kiod
  ===
  
  - run job
  - get error about broken kiod
  
  cancel with working kiod
  
  
  - run job
  - get error about smbmount not being installed (bc it hasn't been a thing 
since the 2000's ;))

REPOSITORY
  R320 KIO Extras

BRANCH
  Applications/19.08

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

AFFECTED FILES
  smb/kio_smb_mount.cpp

To: sitter, dfaure
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D24387: improve error reporting for when kpasswdserver is unreachable

2019-10-08 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes.
Closed by commit R320:5a8705548fae: improve error reporting for when 
kpasswdserver is unreachable (authored by sitter).

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24387?vs=67256=67482

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

AFFECTED FILES
  smb/kio_smb.h
  smb/kio_smb_auth.cpp
  smb/kio_smb_browse.cpp
  smb/kio_smb_mount.cpp

To: sitter, #frameworks, dfaure
Cc: ngraham, kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov


D24443: Add a plugin system

2019-10-08 Thread Daniel Vrátil
dvratil added a comment.


  - I would add `name` property to the `CalendarEntry`
  - I would add `color` property to the `CalendarEntry`, so that calendar that 
is e.g. green in KOrganizer would show up green in the Plasma applet, too.
  
  The Akonadi plugin for this will need to expose a list of multiple calendars, 
otherwise the user would only be able to toggle all-or-nothing, but not to 
toggle individual calendars. In Akonadi, we can have a tree of calendar 
folders, but realistically what you usually see is only the account folder with 
calendar subfolders, without any deeper nesting. I wonder if maybe we could 
have some system of "groups", so e.g. all different plugins that provide some 
holidays plugins would make the calendars members of "holidays" group, the 
Akonadi calendars could be groupped by the account name they belong to...this 
could be then shown nicely in the (QML) UI.

INLINE COMMENTS

> calendarentry.h:28
> +
> +class Q_DECL_EXPORT CalendarEntry : public QObject
> +{

I'm confused, does this represent a single calendar or  event? To me a calendar 
entry is an event. I realize `Calendar` cannot be used, but I think it should 
be a better name, or maybe be a nested class in `CalendarPlugin` (if you can 
nest QObjects, I actually don't know...).

> calendarentry.h:28
> +
> +class Q_DECL_EXPORT CalendarEntry : public QObject
> +{

Should this be declared in the `KCalendarCore` namespace?

> calendarentry.h:48
> +CalendarEntry(const QString , const QString , const 
> QString , CalendarType type, KCalendarCore::Calendar::Ptr calendar);
> +~CalendarEntry();
> +

`override` (overrides QObject's dtor)

> calendarentry.h:57
> +public Q_SLOTS:
> +void sync();
> +

I would move `sync()` from here to be a virtual method on the plugin - 
`sync(const CalendarEntry::Ptr &)`. The implementations would reimplement it to 
handle sync, which feels cleaner than having to connect to `syncRequested()` 
signal on each calendar that the plugin owns, and it decouples data (calendar) 
from logic (plugin).

> calendarentry.h:63
> +private:
> +CalendarEntryPrivate *d;
> +};

Use `std::unique_ptr const d` for automatic memory 
management.

> calendarplugin.h:31
> +public:
> +CalendarPlugin(QObject *parent, const QVariantList args);
> +

`const QVariantList `

> calendarplugin.h:33
> +
> +virtual std::vector calendars() const;
> +

I'd go for QVector, here.

> calendarplugin.h:33
> +
> +virtual std::vector calendars() const;
> +

Should it be a pure virtual function? There's no point in implementing a 
`CalendarPlugin` if you don't reimplement this method...

> calendarplugin.h:39
> +private:
> +void *d;
> +};

Unused?

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

To: nicolasfella, #frameworks, #plasma, #kde_pim
Cc: vkrause, dvratil, davidedmundson, dhaumann


D24453: [RFC] Unify style of new Kirigami.ListSectionHeader and CategoryDrawer

2019-10-08 Thread David Redondo
davidre added a reviewer: Amarok.

REPOSITORY
  R276 KItemViews

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

To: davidre, #frameworks, #vdg, #konversation, #kexi, #kde_edu, #kde_pim, 
#kpublictransport, #amarok
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D24387: improve error reporting for when kpasswdserver is unreachable

2019-10-08 Thread Harald Sitter
sitter added inline comments.

INLINE COMMENTS

> dfaure wrote in kio_smb_mount.cpp:72
> (pre-existing: shouldn't this call `error` rather than `finished`?)

Ah yes, I was thinking the same thing but didn't manage to find a reasonable 
way to use this feature so it's hard to test. I'll write my own demo app I 
guess.

REPOSITORY
  R320 KIO Extras

BRANCH
  Applications/19.08

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

To: sitter, #frameworks, dfaure
Cc: ngraham, kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov


D24453: [RFC] Unify style of new Kirigami.ListSectionHeader and CategoryDrawer

2019-10-08 Thread David Redondo
davidre added reviewers: Konversation, KEXI, KDE Edu, KDE PIM, KPublicTransport.
davidre added a comment.


  Looking at lxr other users include (not exhaustive, I may have missed some)
  Konversation F7546131: Screenshot_20191008_105439.png 

  Kexi (don't have it installed),
  Amarok,
   KPluginSelector (example where it is used?), 
  KTouch F7546240: Screenshot_20191008_56.png 
,
  some pimExamples,
  nepomuk,
  and public transport (optionally).

REPOSITORY
  R276 KItemViews

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

To: davidre, #frameworks, #vdg, #konversation, #kexi, #kde_edu, #kde_pim, 
#kpublictransport
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D24453: [RFC] Unify style of new Kirigami.ListSectionHeader and CategoryDrawer

2019-10-08 Thread David Redondo
davidre edited the test plan for this revision.

REPOSITORY
  R276 KItemViews

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

To: davidre, #frameworks, #vdg
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D24453: [RFC] Unify style of new Kirigami.ListSectionHeader and CategoryDrawer

2019-10-08 Thread David Redondo
davidre updated this revision to Diff 67480.
davidre edited the summary of this revision.
davidre added a comment.


  Set categoryspacing to 0

REPOSITORY
  R276 KItemViews

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24453?vs=67408=67480

BRANCH
  newcategorystyle (branched from master)

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

AFFECTED FILES
  src/kcategorizedview.cpp
  src/kcategorydrawer.cpp

To: davidre, #frameworks, #vdg
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns


D24443: Add a plugin system

2019-10-08 Thread Volker Krause
vkrause added a reviewer: KDE PIM.

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

To: nicolasfella, #frameworks, #plasma, #kde_pim
Cc: vkrause, dvratil, davidedmundson, dhaumann


D24443: Add a plugin system

2019-10-08 Thread Volker Krause
vkrause added a comment.


  In D24443#543140 , @vkrause wrote:
  
  > - Is KDeclarative is the right place for this? It's a module on the way out 
in KF6, it is hard to build for Android due to its dependency chain (which 
isn't even needed for this), while at the same time forcing ABI stability on 
this basically immediately without much chance for battle testing this.
  
  
  Uh, where did I get the idea from that this is in KDeclarative? So that part 
of the comment is obviously nonsense. The concern about committing to ABI too 
quickly remains though.

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

To: nicolasfella, #frameworks, #plasma
Cc: vkrause, dvratil, davidedmundson, dhaumann