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 https://phabricator.kde.org/D21783?vs=710

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 insta

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 to

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 b

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 https://

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 :-) INLINE

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 a

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, nda

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) RE

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 KServ

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, #vdg

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 cleanly

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 Plasm

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 f

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 https://phabricato

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 https://phabricator.k

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 exis

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 https:/

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, wh

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. BUG

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 an

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 https://phab

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, doms

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, ashap

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 https://phabricator.kde.

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 to

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 R29

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 baloo_fi

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: S

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 https://p

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, Mr

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, LeGast

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, The-Feren-

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, The-Feren-

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&id=71009 BRANCH arcpatch-D25149 R

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. REPO

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 f

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 KIOCORE_BUILD_DEPRECATED_SINCE

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