D6624: do not crash qaccessible by causing a resize in a resize event

2017-07-11 Thread Harald Sitter
sitter added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kcharselect.cpp:251 > QSignalBlocker blockResize(this) ? Block inhibits signal emission/slot calling. That is not what we want. We want the signals to run, just not on the same call chain as the resize event. REPOSITORY

kaboutlicense api extension ::spdxId()?

2017-07-11 Thread Harald Sitter
Hola I was wondering if anyone had an opinion on extending kaboutlicense with a ::spdxId() instance method returning the license's spdx id [1]. Use case at hand is kpackagetool, which maps X-KDE-PluginInfo-License of (e.g.) plasma applets to appstream metadata. Appstream however uses the standard

D6624: do not crash qaccessible by causing a resize in a resize event

2017-07-11 Thread Harald Sitter
sitter added a comment. In https://phabricator.kde.org/D6624#124077, @cfeck wrote: > Reading your description on the referenced QTBUG, does it help to use a compare with previous m_columns in KCharSelectItemModel ::setColumnCount() before doing the emit dance? It probably does. T

D6604: add a metainfo.yaml to make ECM a proper framework

2017-07-11 Thread Harald Sitter
sitter added a comment. In https://phabricator.kde.org/D6604#124228, @ochurlaud wrote: > At the time I thought about merging it but we would have to re-document everything in doxygen , which would be completely different from what upstream cmake does. > > The ECM page is already link

Re: kaboutlicense api extension ::spdxId()?

2017-07-12 Thread Harald Sitter
On Tue, Jul 11, 2017 at 2:17 PM, Sebastian Kügler wrote: > It does parse "and later", it's indicated by the + sign, but it's not > reflected in the enum, GPLv2+ would be mapped to GPLv2, so you're right, our > current system is lacking in that regard (but could be extended, although I > don't know

D6604: add a metainfo.yaml to make ECM a proper framework

2017-07-13 Thread Harald Sitter
sitter updated this revision to Diff 16626. sitter added a comment. add a readme I've tried for like 2 hours to wire this into the existing docs build and get away with one README. Alas, no luck. REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-13 Thread Harald Sitter
sitter created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY External software (e.g. appstream) uses the standardized SPDX license identifiers. Seeing as they are specified and ours are not it is usefu

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-13 Thread Harald Sitter
sitter updated this revision to Diff 16633. sitter added a comment. forgot to sort out BIC There is one theoretical BIC with the License ctor which has a new param. I am not sure we care about this given the ctor is private. Objections welcome but I think in the past we decied to not car

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-13 Thread Harald Sitter
sitter updated this revision to Diff 16634. sitter added a comment. add missing since tag REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6672?vs=16633&id=16634 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6672 AFFECTED FILES autot

Re: kaboutlicense api extension ::spdxId()?

2017-07-13 Thread Harald Sitter
On Tue, Jul 11, 2017 at 2:17 PM, Sebastian Kügler wrote: > On dinsdag 11 juli 2017 13:41:17 CEST Harald Sitter wrote: >> I was wondering if anyone had an opinion on extending kaboutlicense >> with a ::spdxId() instance method returning the license's spdx id [1]. >&

D6674: use an initializer list instead of calling insert a million times

2017-07-13 Thread Harald Sitter
sitter created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY looks nicer, is probably also more optimized, and allows us to tag the dict as const. TEST PLAN builds and kaboutdatatest passes REPOSIT

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-13 Thread Harald Sitter
sitter added a reviewer: mpyne. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D6672 To: sitter, sebas, mpyne Cc: #frameworks

D6674: use an initializer list instead of calling insert a million times

2017-07-13 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R244:30b735056fa2: use an initializer list instead of calling insert a million times (authored by sitter). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D6674?vs=16635&id=16638#toc REPOSITORY R24

D6604: add a metainfo.yaml to make ECM a proper framework

2017-07-13 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R240:5f6aa58b8067: add a metainfo.yaml to make ECM a proper framework (authored by sitter). REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6604?vs=16626&id=1

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-13 Thread Harald Sitter
sitter added a comment. For convenience reasons it may actually be prudent to expose both `spdxID` and `orLater` publicly. Case in point right now appstream license information seem to be semi-expressions... they have the spdx ID followed by plus sign notation but compound expressions use lo

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-13 Thread Harald Sitter
sitter added a comment. In https://phabricator.kde.org/D6672#124995, @sitter wrote: > For convenience reasons it may actually be prudent to expose both `spdxID` and `orLater` publicly. Case in point right now appstream license information seem to be semi-expressions... they have the spdx

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-14 Thread Harald Sitter
sitter updated this revision to Diff 16694. sitter added a comment. BC constructor addition + change to enum for orlater as suggested + switch param orders to take license, then restriction, then kaboutdata kaboutdata looks and feels idiomatically like a qobject parent which also is

D6700: disable compiler warning on testing deprecated function

2017-07-14 Thread Harald Sitter
sitter added a reviewer: mpyne. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D6700 To: sitter, mpyne Cc: #frameworks

D6700: disable compiler warning on testing deprecated function

2017-07-14 Thread Harald Sitter
sitter created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. TEST PLAN builds and doesn't warn anymore REPOSITORY R244 KCoreAddons BRANCH no-testwarning REVISION DETAIL https://phabricator.kde.org/D6700 AFFECTE

D6700: disable compiler warning on testing deprecated function

2017-07-17 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R244:0c33e313d5e2: disable compiler warning on testing deprecated function (authored by sitter). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6700?vs=16696&id=1681

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-17 Thread Harald Sitter
sitter updated this revision to Diff 16813. sitter marked an inline comment as done. sitter added a comment. - change "partial" Private ctor to delegate to "full" ctor so we don't have repetitive init lists - fix Private cctor to copy version restriction properly - adjust copy test to as

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-17 Thread Harald Sitter
sitter updated this revision to Diff 16814. sitter added a comment. rebase REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6672?vs=16813&id=16814 BRANCH spdx REVISION DETAIL https://phabricator.kde.org/D6672 AFFECTED FILES autotests/kaboutdatate

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-17 Thread Harald Sitter
sitter marked 3 inline comments as done. sitter added inline comments. INLINE COMMENTS > mpyne wrote in kaboutdata.cpp:166 > Need a setter here for `_versionRestriction` I am changing this one to delegate to the other ctor actually. Not much point > mpyne wrote in kaboutdata.cpp:206 > We use `O

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-17 Thread Harald Sitter
sitter marked 2 inline comments as done. sitter added a comment. Oh, actually. Maybe we could/should make `KAboutLicense::KAboutLicense(const KAboutData *aboutData)` delegate to the "full" public ctor. Then we can drop `Private(const KAboutData *aboutData);` entirely. Like so KAbo

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-17 Thread Harald Sitter
sitter added a comment. Ah drat. We could still get rid of the second private ctor though, by moving the defaults from the private to the public and calling the full private ctor: KAboutLicense::KAboutLicense(const KAboutData *aboutData) : d(new Private(Unknown, OnlyThisVersion,

D6624: do not crash qaccessible by causing a resize in a resize event

2017-07-18 Thread Harald Sitter
sitter added a comment. In https://phabricator.kde.org/D6624#126465, @cfeck wrote: > > It probably does. > > Were you able to test? I would prefer the simpler patch. I cannot test it, because my system does not have accessibility enabled. Yes, I did not manage to crash it that

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-18 Thread Harald Sitter
sitter updated this revision to Diff 16858. sitter added a comment. - do not use ctor delegation, can't use that in kf5 yet - eliminate the "partial" private ctor, instead call the full private ctor from the partial public ctors. this results in defaults being implemented in the public ctor

D6624: do not crash qaccessible by causing a resize in a resize event

2017-07-18 Thread Harald Sitter
sitter added a comment. In https://phabricator.kde.org/D6624#126503, @cfeck wrote: > If you are sure that your fix is "correct", then please remove the comment. From reading it, it looks like a workaround for a Qt bug. It is a workaround. It crashes because the life time of the qa

D6624: do not crash qaccessible by causing a resize in a resize event

2017-07-18 Thread Harald Sitter
sitter added a comment. In https://phabricator.kde.org/D6624#126506, @cfeck wrote: > Let's say someone fixes the referenced Qt bug, and we are at a point requiring that Qt version anyway. Are you going to remove the workaround? Anyone who happens to stumble upon it can. REPOSITOR

D6624: do not crash qaccessible by causing a resize in a resize event

2017-07-18 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R236:ca40063c4e49: do not crash qaccessible by causing a resize in a resize event (authored by sitter). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D6624?vs=16502&id=16860#toc REPOSITORY R236 K

D6767: do not keep constructing new selectionmodels

2017-07-18 Thread Harald Sitter
sitter created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY simply change its underlying model after it has been initialized once. ideally I suppose instead of changing the model at all we should h

D6767: simplify setContents by letting Qt do more of the work

2017-07-18 Thread Harald Sitter
sitter updated this revision to Diff 16865. sitter retitled this revision from "do not keep constructing new selectionmodels" to "simplify setContents by letting Qt do more of the work". sitter edited the summary of this revision. sitter added a comment. did some more digging with input from D

D6672: add KAboutLicense::spdx and introduce orLater qualification

2017-07-19 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R244:107a7fd1a3c0: add KAboutLicense::spdx and introduce orLater qualification (authored by sitter). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6672?vs=16858&id=

D6794: assert the testpackage appstream data validates

2017-07-20 Thread Harald Sitter
sitter created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY helps us prevent invalid desktop->appstream conversion TEST PLAN depends on https://phabricator.kde.org/D6793, with that the tests pass RE

D6793: adopt new KAboutLicense::spdx

2017-07-20 Thread Harald Sitter
sitter added a reviewer: apol. REPOSITORY R290 KPackage REVISION DETAIL https://phabricator.kde.org/D6793 To: sitter, sebas, apol Cc: #frameworks

D6793: adopt new KAboutLicense::spdx

2017-07-20 Thread Harald Sitter
sitter created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY Previously we'd use the verbatim License key of the kpackage desktop file as license in appstream. This is however incorrect since appstream

D6793: adopt new KAboutLicense::spdx

2017-07-20 Thread Harald Sitter
sitter added a dependent revision: D6794: assert the testpackage appstream data validates. REPOSITORY R290 KPackage REVISION DETAIL https://phabricator.kde.org/D6793 To: sitter, sebas, apol Cc: #frameworks

D6793: adopt new KAboutLicense::spdx

2017-07-23 Thread Harald Sitter
sitter added a comment. Ping? REPOSITORY R290 KPackage REVISION DETAIL https://phabricator.kde.org/D6793 To: sitter, sebas, apol Cc: #frameworks

D6793: adopt new KAboutLicense::spdx

2017-07-23 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R290:f2127b122bd1: adopt new KAboutLicense::spdx (authored by sitter). REPOSITORY R290 KPackage CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6793?vs=16935&id=17078 REVISION DETAIL https://

D6794: assert the testpackage appstream data validates

2017-07-23 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R290:cd4ec70f3539: assert the testpackage appstream data validates (authored by sitter). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D6794?vs=16936&id=17079#toc REPOSITORY R290 KPackage CHANGE

D7069: kpackagetool can now write appstream data to a file

2017-08-02 Thread Harald Sitter
sitter created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY Previously kpackagetool used STDOUT as output for the appstream xml data it generated. This has various problems: - cannot print arbitr

D7069: kpackagetool can now write appstream data to a file

2017-08-03 Thread Harald Sitter
sitter marked an inline comment as done. sitter added inline comments. INLINE COMMENTS > apol wrote in kpackagetool.cpp:473 > Why the `? true : false`? lol, brain fart that. REPOSITORY R290 KPackage BRANCH master REVISION DETAIL https://phabricator.kde.org/D7069 To: sitter, apol, sebas

D7069: kpackagetool can now write appstream data to a file

2017-08-03 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. sitter marked an inline comment as done. Closed by commit R290:ef15ef0889d2: kpackagetool can now write appstream data to a file (authored by sitter). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D7069?vs=17556&id=

D7008: Don't export internal helper executables

2017-08-08 Thread Harald Sitter
sitter added subscribers: habacker, sandsmark, sitter. sitter added a comment. As per the if below the proposed change it is used for cross compiling. Subscribing original author and committer. (kind of off topic: This code should really be changed to use the `KF5_HOST_TOOLING` we us

D7008: Don't export internal helper executables

2017-08-08 Thread Harald Sitter
sitter added a comment. In https://phabricator.kde.org/D7008#133644, @sitter wrote: > https://phabricator.kde.org/source/kconfig/browse/master/KF5ConfigConfig.cmake.in;023e3ecfe985e09f786134fc28793d24383998f8$11 when not xcompiling the targets are fairly useless and don't need to get im

D7094: Include a module for finding qml imports as runtime dependencies

2017-08-08 Thread Harald Sitter
sitter added a comment. qmlplugindump not being found needs to be handled somehow. Other than that only minor nitpicks. (as always I'd also be more confident if it had a test case ;)) INLINE COMMENTS > ECMFindQMLModule.cmake.in:30 > + > +execute_process(COMMAND qmlplugindump "@MODULE_NA

D7008: Don't export internal helper executables

2017-08-08 Thread Harald Sitter
sitter added a comment. In https://phabricator.kde.org/D7008#133764, @vkrause wrote: > I assume installation is done for cross-compilation, yes. For Yocto this wouldn't even be necessary though, we manually inject the host executables into the -dev packages there, for this it only matter

D7008: Don't export internal helper executables

2017-08-08 Thread Harald Sitter
sitter added a subscriber: apol. sitter added a comment. In https://phabricator.kde.org/D7008#133756, @habacker wrote: > In https://phabricator.kde.org/D7008#133755, @sitter wrote: > > > In https://phabricator.kde.org/D7008#133644, @sitter wrote: > > > > > https://phabricator.kde.

D7094: Include a module for finding qml imports as runtime dependencies

2017-08-09 Thread Harald Sitter
sitter added inline comments. INLINE COMMENTS > apol wrote in ECMFindQMLModule.cmake.in:30 > Doing find_program for now. The right fix would be to change Qt to export the > `qmlplugindump` target. You still need to do "something" if qmlplugindump isn't found. Print a `message(WARNING` and/or `

D7094: Include a module for finding qml imports as runtime dependencies

2017-08-09 Thread Harald Sitter
sitter accepted this revision. sitter added a comment. This revision is now accepted and ready to land. Oh it's gorgeous! REPOSITORY R240 Extra CMake Modules BRANCH master REVISION DETAIL https://phabricator.kde.org/D7094 To: apol, #build_system, #frameworks, sitter Cc: dfaure, aacid

D7216: refactor kpackagetool away from stringy options

2017-08-09 Thread Harald Sitter
sitter created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY A notable advantage of qcommandlineoption is that using the objects the compiler makes sure that everyone is talking about the same option.

relocatable kdoctools

2017-08-22 Thread Harald Sitter
Ahoy ahoy I've just stumbled upon a rather puzzling situation with kdoctools. It has code branching to turn its assets relocatable [1] (i.e. resolve paths relative rather than hardcode their location). Now the weird bit about this is that it is only used on windows. The reason this puzzles me is

Re: relocatable kdoctools

2017-08-22 Thread Harald Sitter
On Tue, Aug 22, 2017 at 11:32 AM, Ben Cooksley wrote: > On Tue, Aug 22, 2017 at 9:25 PM, Harald Sitter wrote: >> Ahoy ahoy > > Hi Harald, > >> >> I've just stumbled upon a rather puzzling situation with kdoctools. It >> has code branching to turn its asse

D7462: [kiocore] assert that klauncher is running or not

2017-08-22 Thread Harald Sitter
sitter added a comment. LGTM. Also can confirm this fixes the original problem of defunct kfileopen dialogs. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7462 To: bshah, #frameworks, sitter, dfaure

Re: relocatable kdoctools

2017-08-23 Thread Harald Sitter
On Tue, Aug 22, 2017 at 10:18 PM, Luigi Toscano wrote: > Harald Sitter wrote: >> Ahoy ahoy >> >> I've just stumbled upon a rather puzzling situation with kdoctools. It >> has code branching to turn its assets relocatable [1] (i.e. resolve >> paths relati

Re: relocatable kdoctools

2017-08-23 Thread Harald Sitter
On Tue, Aug 22, 2017 at 10:47 PM, Ralf Habacker wrote: > Hi, > > I'm using the following patch to cross build kdoctools for windows > > https://build.opensuse.org/package/view_file/home:rhabacker:branches:windows:mingw:win32:KF536/mingw32-kdoctools/0001-Generate-xml-files-containing-relative-pathe

D7462: [kiocore] assert that klauncher is running or not

2017-08-23 Thread Harald Sitter
sitter added a comment. You committed `(... && getuid() != reply)` rather than `||`. We'll want a fork in either scenario, so this should be `||` as suggested by David. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7462 To: bshah, #frameworks, sitter, dfaure

Re: relocatable kdoctools

2017-08-29 Thread Harald Sitter
On Thu, Aug 24, 2017 at 3:20 PM, Ralf Habacker wrote: > 10. The conclusion of 9. is that it might also be a solution on windows > to patch only all generated files after 3. and before 4. Hm, I may be missing a piece of the puzzle in my mind, but it appears to me this should be working with the cm

Re: relocatable kdoctools

2017-08-29 Thread Harald Sitter
On Sun, Aug 27, 2017 at 8:37 PM, Luigi Toscano wrote: > Even without the if branch it would be still complicated. Ah! I think I get the complexity now. At build time we need to run our tools already and so the dtd/xsl need to be able to resolve the paths, hence the initial configure_file with abs

D6767: simplify setContents by letting Qt do more of the work

2017-08-30 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R236:c0b452358397: simplify setContents by letting Qt do more of the work (authored by sitter). REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6767?vs=16865&id=18

D7216: refactor kpackagetool away from stringy options

2017-09-01 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R290:9597930a4596: refactor kpackagetool away from stringy options (authored by sitter). REPOSITORY R290 KPackage CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7216?vs=17926&id=19045 REVISION

D7430: Add unit test

2017-09-01 Thread Harald Sitter
sitter added a reviewer: dfaure. sitter added a subscriber: Frameworks. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7430 To: chinmoyr, dfaure Cc: #frameworks

Re: qqc2-desktop-style as framework

2017-09-11 Thread Harald Sitter
On Mon, Sep 11, 2017 at 4:16 PM, Marco Martin wrote: > On Sun, Sep 10, 2017 at 3:41 PM, David Faure wrote: >> Sounds OK to me, get it moved to frameworks/ so it can be included in the >> next >> release. >> >> I notice there are no unittests (but that's always a bit hard for pure-gui >> stuff).

Re: qqc2-desktop-style as framework

2017-09-12 Thread Harald Sitter
Mh > set(CMAKE_LIBRARY_OUTPUT_DIRECTORY I don't think that will do for production. "This property specifies the directory into which library target files should be built. Multi-configuration generators (VS, Xcode) append a per-configuration subdirectory to the specified directory." I *think* the

D5032: drop examples dir

2017-09-27 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R308:5ea3c8759505: drop examples dir (authored by sitter). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D5032?vs=12428&id=19968#toc REPOSITORY R308 KRunner CHANGES SINCE LAST UPDATE https://ph

Re: Frameworks release schedule?

2017-10-25 Thread Harald Sitter
On Wed, Oct 25, 2017 at 10:35 AM, laurent Montel wrote: > Hi, > I send this email to kde-frameworks-devel > > It's the correct ML for having an answer about it > > Regards. > > Le mercredi 25 octobre 2017, 09:29:58 CEST Valorie Zimmerman a écrit : >> Hello folks, with my Kubuntu release manager ha

D8824: port from deprecated KAuthorized::authorizeKAction to authorizeAction

2017-11-14 Thread Harald Sitter
sitter created this revision. sitter added a reviewer: Frameworks. Restricted Application added a project: Frameworks. REVISION SUMMARY authorizeKAction is internally calling authorizeAction, so simply switch the function calls to the new name fixes excessive deprecation warnings getting

D8825: do not show edit bookmarks action if keditbookmarks is not installed

2017-11-14 Thread Harald Sitter
sitter created this revision. sitter added a reviewer: Frameworks. Restricted Application added a project: Frameworks. REVISION SUMMARY keditbookmarks lives in applications, making it very likely that it is not installed. this does already raise an error window explaining that the binary is

D8825: do not show edit bookmarks action if keditbookmarks is not installed

2017-11-15 Thread Harald Sitter
sitter added inline comments. INLINE COMMENTS > apol wrote in kbookmarkmenu_p.h:42 > Why not in the cpp file? It's used in 2 difference cpps. REPOSITORY R294 KBookmarks REVISION DETAIL https://phabricator.kde.org/D8825 To: sitter, #frameworks Cc: apol

D8824: port from deprecated KAuthorized::authorizeKAction to authorizeAction

2017-11-16 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R294:378808a575d7: port from deprecated KAuthorized::authorizeKAction to authorizeAction (authored by sitter). REPOSITORY R294 KBookmarks CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8824?vs=

D8825: do not show edit bookmarks action if keditbookmarks is not installed

2017-11-16 Thread Harald Sitter
sitter updated this revision to Diff 22443. sitter added a comment. rebased REPOSITORY R294 KBookmarks CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8825?vs=22379&id=22443 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8825 AFFECTED FILES src/kbookmarkmanager

D8848: do not install development tool to aggregate desktop files

2017-11-16 Thread Harald Sitter
sitter created this revision. sitter added reviewers: Frameworks, whiting, gregormi. Restricted Application added a project: Frameworks. REVISION SUMMARY it's not useful in production REPOSITORY R304 KNewStuff BRANCH master REVISION DETAIL https://phabricator.kde.org/D8848 AFFECTED FIL

D8848: do not install development tool to aggregate desktop files

2017-11-17 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R304:f4f245325cc0: do not install development tool to aggregate desktop files (authored by sitter). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8848?vs=22469&id=225

D8825: do not show edit bookmarks action if keditbookmarks is not installed

2017-11-22 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R294:5dffdee21f20: do not show edit bookmarks action if keditbookmarks is not installed (authored by sitter). REPOSITORY R294 KBookmarks CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8825?vs=2

Re: Frameworks and pkgconfig files

2017-11-29 Thread Harald Sitter
On Wed, Nov 29, 2017 at 9:43 AM, Christophe Giboudeaux wrote: > Hello, > > in https://bugs.kde.org/386933, the bug reporter mentions the pkgconfig file > name shall match the library one. I cannot find any such requirement. It is *suggested* to have one config per lib with the config name matchin

D9240: [RunnerManager] Use QThread::idealThreadCount() instead of going through Solid

2017-12-07 Thread Harald Sitter
sitter added a comment. It seems to me the only reason we have custom code to set the max count is because of that `maxThreads` config entry. An entry for which I can't see any UI backing, so it's borderline usless to begin with. The qMin then destroys any remaining use that entry may have h

D9240: [RunnerManager] Don't mess with ThreadWeaver thread count

2017-12-07 Thread Harald Sitter
sitter accepted this revision. sitter added a comment. I love it REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D9240 To: broulik, #plasma, #frameworks, davidedmundson, sitter Cc: sitter, davidedmundson, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali

D9295: do not treat ts-pmap-compile as exectuable

2017-12-12 Thread Harald Sitter
sitter created this revision. sitter added a reviewer: ilic. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY - strip shebang - make -x the script is run through add_custom_command with the explicit python exeuta

D9295: do not treat ts-pmap-compile as exectuable

2017-12-12 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R249:54147ad2fc2d: do not treat ts-pmap-compile as exectuable (authored by sitter). REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9295?vs=23802&id=23803 REVISION DETAIL

D9299: Introduce INSTALL_PREFIX_SCRIPT to easily set up prefixes

2017-12-12 Thread Harald Sitter
sitter requested changes to this revision. sitter added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > KDEInstallDirs.cmake:699 > +if(INSTALL_PREFIX_SCRIPT) > +file(WRITE ${CMAKE_BINARY_DIR}/prefix.sh " > +export XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:$XD

D9299: Introduce INSTALL_PREFIX_SCRIPT to easily set up prefixes

2017-12-14 Thread Harald Sitter
sitter accepted this revision. sitter added a comment. This revision is now accepted and ready to land. I currently can't do a testbuild, but the code looks good now. If we want to iterate on the concept further I think we should do it separately, no sense holding up this diff from landin

D7008: Don't export internal helper executables

2018-01-04 Thread Harald Sitter
sitter added a comment. In https://phabricator.kde.org/D7008#185897, @apol wrote: > Or as @dfaure says, don't install at all, because I don't see who uses this. Or remove altogether?🔥 The helpers are used by sonnet at build time, so to cross compile sonnet you need the helpers of

Re: duplicate icon in breeze-icons

2018-01-10 Thread Harald Sitter
Whatever's going on with that output Oo On Fri, Jan 5, 2018 at 10:13 AM, David Faure wrote: > Hello Andreas, > > I'm emailing you since you committed some gnumeric icons to breeze-icons. > > CI fails because it says that > /icons/actions/24/gnumeric-formulaguru.svg > and > /icons/actions/24/gnume

D9781: do not incorrectly split spaces in paths of duplicate results

2018-01-10 Thread Harald Sitter
sitter created this revision. sitter added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. sitter requested review of this revision. REVISION SUMMARY use kcoreaddons to split fdupes output. fdupes outputs `/space\ i

Re: duplicate icon in breeze-icons

2018-01-10 Thread Harald Sitter
https://phabricator.kde.org/D9781 On Wed, Jan 10, 2018 at 10:06 AM, Harald Sitter wrote: > Whatever's going on with that output Oo > > On Fri, Jan 5, 2018 at 10:13 AM, David Faure wrote: >> Hello Andreas, >> >> I'm emailing you since you committed some gnu

D9781: do not incorrectly split spaces in paths of duplicate results

2018-01-10 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R266:08ddec40bf89: do not incorrectly split spaces in paths of duplicate results (authored by sitter). REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9781?vs=25061&

D9807: autotests: do not incorrectly split spaces in paths of duplicate results

2018-01-10 Thread Harald Sitter
sitter accepted this revision. This revision is now accepted and ready to land. BRANCH master REVISION DETAIL https://phabricator.kde.org/D9807 To: dfaure, sitter, apol Cc: cfeck, #frameworks

D9888: Make kdesu work when PWD is /usr/bin

2018-01-15 Thread Harald Sitter
sitter accepted this revision. sitter added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > suprocess.cpp:122 > > -QByteArray command; > -if (d->superUserCommand == QLatin1String("sudo")) { > -command = __PATH_SUDO; > -} else { > -

D10020: add a workaround to mark snap bundle mounts as hidden

2018-01-22 Thread Harald Sitter
sitter created this revision. sitter added a reviewer: apol. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. sitter requested review of this revision. REVISION SUMMARY snap bundles are loop mounted into the file system, the more bundles

D10020: add a workaround to mark snap bundle mounts as hidden

2018-01-22 Thread Harald Sitter
sitter abandoned this revision. sitter added a comment. Ah. I like his patch better. Thanks REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D10020 To: sitter, apol Cc: bshah, broulik, #frameworks

D10044: actually install the new animations context

2018-01-23 Thread Harald Sitter
sitter created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. sitter requested review of this revision. REVISION SUMMARY https://phabricator.kde.org/R266:495829b2b0fea458bfa01a996d23181d254e3521 introduced a new context

D10046: unset maintainer

2018-01-23 Thread Harald Sitter
sitter created this revision. sitter added a reviewer: apol. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. sitter requested review of this revision. REVISION SUMMARY Alex isn't really active right now, so untag him as maintainer and le

D10044: actually install the new animations context

2018-01-23 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R266:c6fcf84e1ac8: actually install the new animations context (authored by sitter). REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10044?vs=25807&id=25810 REVISIO

D10046: unset maintainer

2018-01-24 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. Closed by commit R245:978676ee9a20: unset maintainer (authored by sitter). REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10046?vs=25809&id=25870 REVISION DETAIL https://phabricator.kde.

D10716: handle wrong password when using sudo which asks for another password

2018-02-21 Thread Harald Sitter
sitter added a comment. As mentioned on IRC I think this would benefit greatly from some unit testing as that line check is dangerously close to requiring mental gymnastics to read. AT a glance, all that's needed is writing stub sudo/su helpers and a test asserting outcome of `SuProcess('us0

D10716: handle wrong password when using sudo which asks for another password

2018-02-26 Thread Harald Sitter
sitter added a comment. In D10716#212174 , @jriddell wrote: > I fear that creating a stub replacement wouldn't be necessarily a reliable recreation. I am not sure what you mean. If you create a stub from what you observe right now then t

D10716: handle wrong password when using sudo which asks for another password

2018-03-05 Thread Harald Sitter
sitter added a comment. Ping REPOSITORY R299 KDESu REVISION DETAIL https://phabricator.kde.org/D10716 To: jriddell, sitter Cc: #frameworks, michaelh

D10776: Make it possible to generate po files in parallel

2018-03-05 Thread Harald Sitter
sitter added a comment. As discussed on telegram: the proposed change makes it a different problem. Now instead of running serial all the time, it'd fork bomb all the time (peaks at 3671 forks for plasma-desktop 5.12 when run on a fairly fast system with SSD). Also, missing adjustment t

Re: Fallback Icon Theme

2018-03-05 Thread Harald Sitter
On Sun, Mar 4, 2018 at 7:30 PM, Martin Kostolný wrote: > If there is a better way of solving our situation, please tell us :). Someone needs to file bug reports against adwaita so the missing icons can get added. That is the proper solution to an incomplete icon set. It's a game of pick your poi

D10776: Make it possible to generate po files in parallel

2018-03-05 Thread Harald Sitter
sitter added a comment. Ingenious code! You left some debug clutter behind though. Also, the if i statement is inconsistent between the two files `if (i EQUAL ${numberOfProcesses})` vs. `if (i GREATER ${numberOfProcesses})` INLINE COMMENTS > build-pofiles.cmake:45 > +

<    1   2   3   4   5   6   7   8   9   10   >