D21783: [WIP]Show more details in warning dialog shown before starting a privileged operation

2019-12-06 Thread Chinmoy Ranjan Pradhan
chinmoyr updated this revision to Diff 71041. chinmoyr added a comment. Rebased, compiled and tested. Changes work as expected. Double checked file_unix.cpp. Would be a shame if it happens yet again. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D25789: Correctly report if baloo_file is unavailable

2019-12-06 Thread Stefan BrĂ¼ns
bruns added inline comments. INLINE COMMENTS > monitor.cpp:67 > m_balooRunning = false; > QDBusServiceWatcher* balooWatcher = new > QDBusServiceWatcher(m_scheduler->service(), > m_bus, The watcher should be

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

2019-12-06 Thread David Faure
dfaure added a comment. In D23384#571455 , @sitter wrote: > The opposite extreme is to always pass when X-KDE-Protocols is set and assume that the applications are actually working correctly (e.g. vlc ought to talk to kiod/KPasswdServerClient

D25682: [WIP] add initial wsdiscovery support

2019-12-06 Thread Ben Cooksley
bcooksley added a comment. With regards to the Docker/Gitlab CI part, please use the images under kdeorg/ on Dockerhub rather than personally maintained images as the wider community has no access to your namespace on Gitlab.com (Additionally please note that Fedora is not permitted to

D25698: New class KApplicationTrader, to replace KMimeTypeTrader and KServiceTypeTrader

2019-12-06 Thread David Faure
dfaure added a comment. Feedback on the API question would be welcome. Also, see the discovery in T12256 . I'm thinking of renaming this class to KServiceTrader and add a queryByServiceType to it. REPOSITORY R309 KService REVISION DETAIL

D25682: [WIP] add initial wsdiscovery support

2019-12-06 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I guess you were expecting a higher-level review, but I don't know anything about these protocols. I'm glad to see my KDSoap library being useful in KDE too though :-)

D25798: Deprecated allowAsDefault

2019-12-06 Thread Friedrich W. H. Kossebau
kossebau added inline comments. INLINE COMMENTS > kservice.cpp:982 > > +#if KSERVICE_ENABLE_DEPRECATED_SINCE(5, 65) > bool KService::allowAsDefault() const This should be BUILD, not ENABLE. *BUILD* macros are controlled by the EXCLUDE_DEPRECATED_BEFORE_AND_AT value, which iis what is used

D25793: Rename "Configure Shortcuts" to "Configure Keyboard Shortcuts"

2019-12-06 Thread Luigi Toscano
ltoscano added a comment. Oh sorry, I missed that. It's the kind of information I wouldn't expect in the commit message. REPOSITORY R265 KConfigWidgets BRANCH configure-keyboard-shortcuts (branched from master) REVISION DETAIL https://phabricator.kde.org/D25793 To: ngraham, #vdg,

D25793: Rename "Configure Shortcuts" to "Configure Keyboard Shortcuts"

2019-12-06 Thread Nathaniel Graham
ngraham added a comment. Yep, in fact I mentioned this in the description section of the patch: > If accepted, will wait until after tagging to land it so as not to break the string freeze. REPOSITORY R265 KConfigWidgets BRANCH configure-keyboard-shortcuts (branched from master)

D25798: Deprecated allowAsDefault

2019-12-06 Thread Nicolas Fella
nicolasfella created this revision. nicolasfella added a reviewer: Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. nicolasfella requested review of this revision. REVISION SUMMARY Execute upon T12309 . The

D25793: Rename "Configure Shortcuts" to "Configure Keyboard Shortcuts"

2019-12-06 Thread Luigi Toscano
ltoscano added a comment. Please commit it after the commit for the new Frameworks is made (so probably from Sunday onwards). REPOSITORY R265 KConfigWidgets BRANCH configure-keyboard-shortcuts (branched from master) REVISION DETAIL https://phabricator.kde.org/D25793 To: ngraham,

D21783: [WIP]Show more details in warning dialog shown before starting a privileged operation

2019-12-06 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I'm concerned that you didn't compile this (because of dependency issues, from what I gather), which means it's not tested either. I tried to compile it but it doesn't

D25340: Added background colors to active and inactive icon view

2019-12-06 Thread Nathaniel Graham
ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. I guess I should change my status to accepted given that I think this is good enough and already an improvement. But I think we can do even better, @niccolove. :) REPOSITORY R242

D25794: ArraySourceTest: Use QTEST_GUILESS_MAIN

2019-12-06 Thread David Edmundson
davidedmundson accepted this revision. This revision is now accepted and ready to land. BRANCH master REVISION DETAIL https://phabricator.kde.org/D25794 To: heikobecker, #frameworks, ahiemstra, davidedmundson

D25795: Rename kf5quickcharts_example to kquickcharts_example

2019-12-06 Thread David Edmundson
davidedmundson accepted this revision. This revision is now accepted and ready to land. BRANCH master REVISION DETAIL https://phabricator.kde.org/D25795 To: heikobecker, #frameworks, ahiemstra, davidedmundson

D24489: KAutosaveFile not respecting maximum filename length

2019-12-06 Thread David Faure
dfaure added a comment. I see. This answers my question about why two merge requests -- no problem, keep them separate, but commit the fix before the unittest [this is so that bisecting never ends up in the situation where unittests are broken] You can also ignore my suggestion about

D25795: Rename kf5quickcharts_example to kquickcharts_example

2019-12-06 Thread Heiko Becker
heikobecker created this revision. heikobecker added reviewers: Frameworks, ahiemstra. heikobecker requested review of this revision. REVISION SUMMARY Matching the project name. TEST PLAN exmaple is installed under the new name, runs fine BRANCH master REVISION DETAIL

D25794: ArraySourceTest: Use QTEST_GUILESS_MAIN

2019-12-06 Thread Heiko Becker
heikobecker created this revision. heikobecker added reviewers: Frameworks, ahiemstra. heikobecker requested review of this revision. REVISION SUMMARY Allows the test to run without a display server. TEST PLAN Builds, test still passes BRANCH master REVISION DETAIL

D25340: Added background colors to active and inactive icon view

2019-12-06 Thread Filip Fila
filipf added a comment. In D25340#563400 , @ndavis wrote: > This diff is against commit 467d721cc96258b54048c0dd1508d16e03c0cd55, which isn't in git master. Do I actually need that commit for this patch to work? A similar issue still

D25793: Rename "Configure Shortcuts" to "Configure Keyboard Shortcuts"

2019-12-06 Thread Noah Davis
ndavis accepted this revision. ndavis added a comment. This revision is now accepted and ready to land. Welp, there's nothing objectively wrong with making this patch. LGTM REPOSITORY R265 KConfigWidgets BRANCH configure-keyboard-shortcuts (branched from master) REVISION DETAIL

D25793: Rename "Configure Shortcuts" to "Configure Keyboard Shortcuts"

2019-12-06 Thread Nathaniel Graham
ngraham added a comment. There was a bug report about it that had some people agreeing with it. I think it makes a bit of sense because yes, this dialog is indeed only about keyboard shortcuts, and at least to my ears, the phrase "keyboard shortcuts" instantly connotes what this is about,

D25793: Rename "Configure Shortcuts" to "Configure Keyboard Shortcuts"

2019-12-06 Thread Noah Davis
ndavis added a comment. This doesn't seem wrong, but why is it needed? Do people get confused about the type of shortcuts? Are there non-keyboard shortcuts? If there are, would we put their configuration menu under a different menu option? REPOSITORY R265 KConfigWidgets REVISION DETAIL

D25793: Rename "Configure Shortcuts" to "Configure Keyboard Shortcuts"

2019-12-06 Thread Nathaniel Graham
ngraham created this revision. ngraham added a reviewer: VDG. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ngraham requested review of this revision. REVISION SUMMARY This is a bit clearer, and reinforces the keyboard icon used for the menu item.

D25791: Fix writeFlags with KConfigCompilerSignallingItem

2019-12-06 Thread David Edmundson
davidedmundson added a dependent revision: D25792: Add notifiers to workspace options kcfg. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D25791 To: davidedmundson, ervin Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25791: Fix writeFlags with KConfigCompilerSignallingItem

2019-12-06 Thread David Edmundson
davidedmundson created this revision. davidedmundson added a reviewer: ervin. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. davidedmundson requested review of this revision. REVISION SUMMARY KConfigCompilerSignallingItem both inherits KConfigSkeletonItem

D25340: Added background colors to active and inactive icon view

2019-12-06 Thread Nathaniel Graham
ngraham added a comment. Can we push this forward? I just triaged a bunch of bugs and found that https://bugs.kde.org/show_bug.cgi?id=370465 now has five duplicates. There seems to be quite a bit of demand for this. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL

D24489: KAutosaveFile not respecting maximum filename length

2019-12-06 Thread Jean-Baptiste Mardelle
mardelle added a comment. Oh no don't misunderstand me. I am very glad that someone else stepped up! I have more than enough work on my plate with Kdenlive so please help :) Hopefully David can give us some hints about the best way to move on! REPOSITORY R244 KCoreAddons REVISION DETAIL

D25788: Initialise QML monitor values

2019-12-06 Thread Nathaniel Graham
ngraham accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH master REVISION DETAIL https://phabricator.kde.org/D25788 To: davidedmundson, ngraham Cc: kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2,

D25789: Correctly report if baloo_file is unavailable

2019-12-06 Thread Nathaniel Graham
ngraham added a comment. Maybe instead of "Unavailable", it could say "Not Running"? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D25789 To: davidedmundson Cc: ngraham, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson,

T10262: Integrate KIO Slaves into file system using FUSE gateway

2019-12-06 Thread Nathaniel Graham
ngraham closed this task as "Resolved". ngraham added a comment. This is currently in progress and I don't think we need this task open anymore. The patches are scattered across various bits of infrastructure so it won't be useful for linking them here. TASK DETAIL

D21783: [WIP]Show more details in warning dialog shown before starting a privileged operation

2019-12-06 Thread Chinmoy Ranjan Pradhan
chinmoyr added inline comments. INLINE COMMENTS > dfaure wrote in slaveinterface_p.h:56 > Please explain how, for when the time comes (in case you're not around to do > it). Removing each of its occurence (of which there are 3) and the surrounding `if`. I will add comments where its needed

D21783: [WIP]Show more details in warning dialog shown before starting a privileged operation

2019-12-06 Thread Chinmoy Ranjan Pradhan
chinmoyr updated this revision to Diff 71025. chinmoyr marked 10 inline comments as done. chinmoyr added a comment. Addressed the issues. Annotated code to be removed in KF6. My build env is outdated so I couldn't compile the new depracation macro. Copied as it is. REPOSITORY R241 KIO

D25789: Correctly report if baloo_file is unavailable

2019-12-06 Thread David Edmundson
davidedmundson created this revision. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. davidedmundson requested review of this revision. REVISION SUMMARY Add a new state. Watch for service unregistration as well as registration. REPOSITORY

D24489: KAutosaveFile not respecting maximum filename length

2019-12-06 Thread Ahmad Samir
ahmadsamir added a comment. In D24489#573152 , @mardelle wrote: > The unittest is in another diff because it's a different author I guess. [...] I didn't mean to step on your toes; feel free to disregard the unit test I wrote, or reuse

D25788: Initialise QML monitor values

2019-12-06 Thread David Edmundson
davidedmundson created this revision. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. davidedmundson requested review of this revision. REVISION SUMMARY m_indexerState would not be initialised and m_totalFiles would be garbage values if

D25296: [RFC] Fix Display Configuration icon margins

2019-12-06 Thread Noah Davis
ndavis added a comment. In D25296#563291 , @ngraham wrote: > Never mind, I wasn't deleting the cache files properly after rebuilding. When I do that, the monochrome icons don't get used at all and it reverts to the colorful one: > F778:

D25753: EBN extra-cmake-modules transport cleanup

2019-12-06 Thread John Hayes
jhayes added a comment. The two links I know of are: http://0pointer.de/lennart/projects/libcanberra (works as is in file but wont work as https) http://www.x86-64.org/documentation/abi.pdf (http://www.x86-64.org is dead) REPOSITORY R240 Extra CMake Modules REVISION DETAIL

D25682: [WIP] add initial wsdiscovery support

2019-12-06 Thread Nathaniel Graham
ngraham added reviewers: dfaure, Frameworks, Dolphin. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D25682 To: sitter, dfaure, #frameworks, #dolphin Cc: ngraham, caspermeijn, davidedmundson, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n,

D25296: [RFC] Fix Display Configuration icon margins

2019-12-06 Thread Nathaniel Graham
ngraham added a subscriber: trickyricky26. ngraham added a comment. @ndavis or @trickyricky26, could I ping you on this? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D25296 To: ngraham, #vdg, ndavis Cc: trickyricky26, kde-frameworks-devel,

D25149: Add a new template for KCMs

2019-12-06 Thread Tomaz Canabrava
tcanabrava marked 2 inline comments as done. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D25149 To: tcanabrava, #plasma, #frameworks, mart, ervin Cc: #plasma, GB_2, yurchor, davidedmundson, ognarb, ervin, kde-frameworks-devel, LeGast00n,

D25149: Add a new template for KCMs

2019-12-06 Thread Tomaz Canabrava
tcanabrava marked 6 inline comments as done. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D25149 To: tcanabrava, #plasma, #frameworks, mart, ervin Cc: #plasma, GB_2, yurchor, davidedmundson, ognarb, ervin, kde-frameworks-devel, LeGast00n,

D25149: Add a new template for KCMs

2019-12-06 Thread Tomaz Canabrava
tcanabrava updated this revision to Diff 71009. tcanabrava marked an inline comment as done. tcanabrava added a comment. - Fix indentation REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25149?vs=69942=71009 BRANCH arcpatch-D25149

D24489: KAutosaveFile not respecting maximum filename length

2019-12-06 Thread Jean-Baptiste Mardelle
mardelle added a comment. The unittest is in another diff because it's a different author I guess. I can process the comments but looking closer I found 2 other important issues in this class: 1. When using KAutoSaveFile::staleFiles(filename) to check if there is an existing stale file

D25753: EBN extra-cmake-modules transport cleanup

2019-12-06 Thread Christophe Giboudeaux
cgiboudeaux added a comment. In D25753#572894 , @winterz wrote: > please send me a list of urls that don't have https: and I'll add them to the whitelist The x86-64.org domain is dead. I'm not sure about what could be used instead.

D25682: [WIP] add initial wsdiscovery support

2019-12-06 Thread Casper Meijn
caspermeijn added a comment. In D25682#570849 , @sitter wrote: > In D25682#570845 , @davidedmundson wrote: > > > Why do we need to mirror this dsoap-ws-discovery-client lib that seems to be copied

D21783: [WIP]Show more details in warning dialog shown before starting a privileged operation

2019-12-06 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > slavebase.cpp:1525 > + > +#ifndef KIOCORE_NO_DEPRECATED > +PrivilegeOperationStatus SlaveBase::requestPrivilegeOperation() `#if

D25767: KAutoSaveFile: add a unit test to check max. filename length

2019-12-06 Thread Dominik Haumann
dhaumann added inline comments. INLINE COMMENTS > dfaure wrote in kautosavefiletest.cpp:98 > Let's hope none of the supported OSes have a smaller PATH_MAX :) > FreeBSD, Windows, macOS, who knows what limits they have. What I wonder is: PATH_MAX etc are #defines, right? Since on Windows there is