Re: Retiring

[Kevin Funk]

On Wednesday, 13 January 2021 23:05:50 CET Milian Wolff wrote:
> On Mittwoch, 13. Januar 2021 21:57:13 CET Christoph Feck wrote:
> > Hello developers,
> > 
> > my personal situation allows for much less time for KDE
> > related work, at least during the Corona times.
> > 
> > I would like to retire as soon as possible from these
> > responsibilities:
> > - bug triaging
> > - release service
> > - KIconTheme maintainer
> > - KPlotting maintainer
> > - KWidgetAddons maintainer
> > 
> > For bugzilla, I see many new and old contributors doing awesome
> > work with triaging, so departing here shouldn't be a big issue.
> > I hope someone can continue checking responses to NEEDINFO bugs
> > and REPORTED bugs that didn't get a reply within about two weeks.
> > 
> > Regarding the release service work, I have made sure the load
> > isn't all back to Albert. For future release work, Heiko Becker
> > volunteered to help, but maybe another hand would be awesome, too.
> > 
> > For the frameworks modules, I initially assumed each module
> > must have a separate maintainer, so I volunteered. Today I
> > see many modules still just community-maintained, and I hope
> > the community can take over maintainance. Not that I did any
> > relevant work here in the recent years...
> > 
> > I might eventually continue to prepare some patches, so please
> > keep all my accounts intact, but only de-list me as the maintainer.
> 
> Thanks a lot Christoph for all your work! As I myself has had the pleasure
> of quasi-retiring but never quite leaving - may the same happen to you ;-)
> I.e.: Take your well earned break and come back whenever it suites you
> again!

+100. I can relate to that :)

Thanks for all the enduring work you did for KDE, Christoph! Come back when 
time permits it again!

Cheers,
Kevin


> Cheers


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: CuteHMI in kdereview

[Kevin Funk]

On Thursday, 13 February 2020 11:00:27 CET Michal Policht wrote:
> Yeah, I neglected translations a bit... I am going to implement adequate
> Qbs module for extracting translations.

Heya,

> When it comes to Qbs, it's not dead. Community has taken over the
> project, there's active development and recent version was released just
> 44 days ago.
> 
> We migrated from QMake to Qbs, when it was still supported by Qt Company
> and promoted as official build system for Qt 6. Thus we assumed Qbs is
> the future. We've found that Qbs has some issues (like every software),
> but in overal it's very capable and powerful piece of software. It also
> provides much faster builds on Windows. I wish more people would give it
> a try before burying the project, to at least see the potential. With
> something like Qbs we could create a build framework with reusable
> components, so that each KDE subproject could benefit from it and become
> naturally integrated.

I think you're beating a dead horse here. The ship has sailed. 

You'll be missing out on quite a bit of tooling which is implemented in KDE's 
Extra CMake Modules framework: 
  https://github.com/KDE/extra-cmake-modules
  (part of that is all the translation handling)

There's also no support for building QBS projects on KDE's CI:
  https://build.kde.org

> It may be hard to accomplish with CMake.

What exactly? I mean it's all there already.

> Regards,

PS: Qt's CMake-based build system just got merged into qtbase dev branch a few 
days ago.

Regards,
Kevin


> Michal
> 
> > El dilluns, 3 de febrer de 2020, a les 17:57:24 CET, Michal Policht va 
escriure:
> >> Hello there,
> >> 
> >> CuteHMI (https://cutehmi.kde.org/) has been moved to kdereview.
> > 
> > It has no Messages.sh for translation extraction.
> > 
> > Any particular reason you're using a dead build system none of our
> > projects uses?
> > 
> > Cheers,
> > 
> >   Albert
> >> 
> >> CuteHMI is meant to be a set of tools and components that help one to
> >> create QML-based HMI/SCADA software.
> >> 
> >> The project has been started few years ago, because I couldn't find any
> >> open-source, QML-based HMI/SCADA framework I could put my things into.
> >> 
> >> Regards
> >> 
> >> Michal Policht


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

D26394: ECMGeneratePriFile: Fix static configurations

[Kevin Funk]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:fafbc8cec665: ECMGeneratePriFile: Fix static 
configurations (authored by kfunk).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26394?vs=72687=75155

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

AFFECTED FILES
  modules/ECMGeneratePriFile.cmake

To: kfunk, dfaure, winterz, vkrause, apol
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, GB_2, bencreasy, 
michaelh, ngraham, bruns

D26364: SlaveBase::dispatchLoop: Fix timeout calculation

[Kevin Funk]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:27271405f408: SlaveBase::dispatchLoop: Fix timeout 
calculation (authored by Julien Goodwin jgood...@studio442.com.au, 
committed by kfunk).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26364?vs=72610=73129

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

AFFECTED FILES
  src/core/slavebase.cpp

To: kfunk, chinmoyr, davidedmundson, dfaure, broulik
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D26394: ECMGeneratePriFile: Fix static configurations

[Kevin Funk]

kfunk added reviewers: winterz, vkrause.

REPOSITORY
  R240 Extra CMake Modules

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

To: kfunk, dfaure, winterz, vkrause
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D26394: ECMGeneratePriFile: Fix static configurations

[Kevin Funk]

kfunk created this revision.
kfunk added a reviewer: dfaure.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
kfunk requested review of this revision.

REVISION SUMMARY
  Populate module_config with staticlib. This is needed for Qt 5.12, as
  Makefiles contain the full path to the library instead of just the base
  name. QMake needs to be aware of the build type. This issue was found in
  KDStateMachineEditor's .pri files.
  
  Before this patch the linker tried to link against .so files even for
  static libraries.
  
  Note: Probably not very relevenat to KDE Frameworks (since it's all
  about shared libraries, but I'd like to keep the original
  ECMGeneratePriFile version up-to-date)
  
  Compare:
  
% cat kdsme-qmake-test.pro
QT += KDSMEDebugInterfaceSource

!qtHaveModule(KDSMEDebugInterfaceSource): warning("Library not found")

SOURCES += main.cpp

% qmake --version
QMake version 3.1
Using Qt version 5.9.8 in /home/kfunk/devel/build/qt5.9/qtbase/lib
% qmake .
% make
...
g++ -Wl,-rpath,/home/kfunk/devel/build/qt5.9/qtbase/lib ...  -L.../lib 
-lkdstatemachineeditor_debuginterfacesource ...

% make clean

% env-qt5.12
% qmake --version
QMake version 3.1
Using Qt version 5.12.5 in /home/kfunk/devel/build/qt5.12/qtbase/lib

% qmake .
% make
...
g++ -Wl,-rpath,/home/kfunk/devel/build/qt5.12/qtbase/lib ... 
.../lib/libkdstatemachineeditor_debuginterfacesource.a ...

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  modules/ECMGeneratePriFile.cmake

To: kfunk, dfaure
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D26364: SlaveBase::dispatchLoop: Fix timeout calculation

[Kevin Funk]

kfunk added a reviewer: broulik.

REPOSITORY
  R241 KIO

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

To: kfunk, chinmoyr, davidedmundson, dfaure, broulik
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26364: SlaveBase::dispatchLoop: Fix timeout calculation

[Kevin Funk]

kfunk added reviewers: chinmoyr, davidedmundson, dfaure.

REPOSITORY
  R241 KIO

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

To: kfunk, chinmoyr, davidedmundson, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26364: SlaveBase::dispatchLoop: Fix timeout calculation

[Kevin Funk]

kfunk created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
kfunk requested review of this revision.

REVISION SUMMARY
  Old version of the code:
  
ms = qMax(d->nextTimeout.elapsed() - d->nextTimeoutMsecs, 1);
  
  ... will mean the sleep is for as long as the timer has run *minus* the
  intended duration, so if nextTimeoutMsecs is ever set and the timer just
  started this becomes very negative, and 1ms is the result.
  
  Inverting the subtraction:
  
ms = qMax(d->nextTimeoutMsecs - d->nextTimeout.elapsed(), 1);
  
  Means sleeping for the remaining time, and so far my CPU seems much
  happier, with my KIO-HTTP using apps looking fine.
  
  BUG: 392768

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/core/slavebase.cpp

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

Re: KDiff3 craft setup

[Kevin Funk]

On Tuesday, 5 March 2019 21:11:11 CET Albert Astals Cid wrote:
> El dimarts, 19 de febrer de 2019, a les 7:35:58 CET, Kevin Funk va escriure:
> > On Monday, 18 February 2019 17:06:25 CET Michael Reeves wrote:
> > > https://download.kde.org/stable/applications/18.12.1/src/kdiff3-18.12.1.
> > > tar. xz
> > > 
> > > Get some one tell me how to change where it's trying to download from.
> > > KDiff3 is not part of applications and doesn't follow the same
> > > versioning.
> > 
> > Heya,
> > 
> > Could you please reconsider that decision and check whether it's not more
> > worthwhile making kdiff3 part of KDE Applications? It will save you (as
> > the
> > maintainer) and others (distribution packagers) a major headache.
> > 
> > You'll be responsible for releasing kdiff3 now and in the future if you
> > choose to do your own release schedule. Let me just say: It's not
> > something which is particular entertaining in the long-term. Your KDiff3
> > involvement will get less eventually, and then someone else needs to take
> > over releasing it -- if it's part of the KDE Apps cycle it'll be done
> > automatically, no matter what.
> > 
> > KDiff3 is not the type of application which needs its own release cycle,
> > IMO, it's too small & "undynamic" [1] for that.
> 
> Just answering now because i did ignore an email named "KDiff3 craft setup"
> since i don't know anything about craft and it seems now this is being used
> as some kind of agreement that KDiff3 should be moved to KDE Applications.
> 
> Personally given KDiff3 has not had any release on its own for a long time I
> would very much prefer to get a few releases on its own.
> 
> This way new features/fixes can be released sooner if needed and not tied to
> the more strict KDE Applications schedule.

Well, I dont agree, but choose your pain :)

This might be true for the very first few releases, but after that you'd be 
better off just taking part of the KDE Apps cycle. But well, I tried 
convincing people.

/me walks away after almost ruining kdevelop.git due to releaseme script usage 
(which accidentally pushed tons of stale local tags [1]) and manually scp'ing 
tarballs to KDE FTP and now waiting for mirrors to pick up the changes. Some 
wasted hours which are not productive at all and which I'd never experience if 
there'd be an automatism in place doing all this for me. Just saying.

Regards,
Kevin


[1] https://phabricator.kde.org/D19578

 
> There's also the matter of http://kdiff3.sourceforge.net/ being
> outdated/wrong. Do we have a plan to fix that?
> 
> Cheers,
>   Albert
> 
> > Regards,
> > Kevin
> > 
> > 
> > [1] "Undynamic" in a sense that we're likely not going to see drastic UI
> > changes on weekly basis which need to get out to users ASAP. At least for
> > kdiff3 I'd rather have a conservative approach in that regard, since it's
> > a
> > complex tool by definition.


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: Gitlab Evaluation & Migration

[Kevin Funk]

On Monday, 25 February 2019 09:12:47 CET Eike Hein wrote:
> On 2/25/19 4:31 AM, Martin Flöser wrote:
> > Changing the tooling will not solve any of the contribution problems.
> 
> I'm aware of several projects who would like to incubate with KDE.org
> (e.g. Kaidan and Spectral, both new-breed Kirigami apps with new
> contributor ecosystems) but are waiting for the outcome of this
> decision, because they believe being part of KDE wouldn't be worth the
> cost of losing access to contributors that Phabricator imposes to them.
> 
> It's difficult to get hard data on this, of course. From Gnome I've been
> told their contributon numbers from new contributors have seen a massive
> uptick since adopting GitLab, however their infra pre-GitLab was (to me)
> worse than Phabricator.
> 
> It's unclear to me what exactly would happen for us. However, it seems
> clear to me that the world outside of the existing cadre of KDE
> contribtors has a consensus on this: When the news about the GitLab eval
> leaked to Reddit and other venues, "finally I'll be able to submit my
> patch" was a recurring theme.
> 
> One thing that strikes me is that KDE upstream relations have usually
> been a defining trait of our tooling decisions. We've adopted things
> like gitolite, CMake and even Qt based on having an upstream to talk to.
> This is currently not the case with Phabricator, but it is the case with
> GitLab. I'm hesistant of saying it's awesome just yet (there's some
> features in the GitLab Enterprise Edition we would like GitLab to move
> to the Community Edition for us, and it's not been decided there yet),
> but they've done regular calls with us, opened a master ticket to track
> our account with them, and have multiple times expressed an interest in
> attending KDE's upcoming Onboarding sprint. This is very nice.
> 
> It's also worth noting that we're the only big FOSS player using
> Phabricator at the moment. To contribute to KDE, people have to learn
> Phabricator. If they've already contributed to freedesktop, Gnome, or
> hundreds of other projects, they've already learned GitLab.
> 
> My personal take is this: I'm used to Phabricator and fine with it. It
> doesn't impede my work. But I think it would be a mistake to make this
> decision based on what I'm fine with, because I'm equally able to adjust
> to GitLab and be fine with that. I'd rather make this decision based on
> what people who aren't KDE contributors yet find attractive, and that
> seems overwhelmingly in GitLab's favor from everything I've read and
> heard. New contributors showing up would affect me a lot more than
> having to adjust to typing a different command does.
> 
> I also have reason to believe this delta between "I'm fine with it" and
> "Others want X" is only going to increase based on the relative
> development speeds of Phabricator and GitLab. The latter seems more
> likely to keep up with the state of the art currently. I'd like KDE to
> be on board with the state of the art.

Heya,

+1 on everything Eike said. I think adopting GitLab will be a huge benefit for 
KDE.

Some two-digit million amount of projects hosted on GitLab+GitHub+BitBucket 
(systems which all share the same "workflow" principles) cannot be wrong. At 
least there are /tons/ of developers out there which are familiar with them by 
heart, which makes it super easy for them to contribute to KDE, in theory.

Even after having worked a couple of years with Phabricator's review system it 
stills feels alien to me, mostly b/c it does not integrate that well with Git. 
I always get slight shivers when having to work with it. I'm contributing to 
several other projects on either GitLab, GitHub, BitBucket, and the whole 
experience is simply better. I think there's no doubt about that.

The move to yet another system is ambitious but let's rather do it sooner than 
later. Looking forward to GitLab adoption across the board!

Thanks for the effort, guys!

Cheers,
Kevin


> Cheers,
> Eike


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: KDiff3 craft setup

[Kevin Funk]

On Monday, 18 February 2019 17:06:25 CET Michael Reeves wrote:
> https://download.kde.org/stable/applications/18.12.1/src/kdiff3-18.12.1.tar.
> xz
> 
> Get some one tell me how to change where it's trying to download from.
> KDiff3 is not part of applications and doesn't follow the same versioning.

Hey,

Anyhow, to help you out on that regard:

You'll want to check out craft-blueprints-kde.git, and there find the kdiff3 
subfolder.

You'll probably need to add a separate version.ini file there with custom 
versions/urls. See e.g. kmymoney/version.ini as example.

Regards,
Kevin



-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: KDiff3 craft setup

[Kevin Funk]

On Monday, 18 February 2019 17:06:25 CET Michael Reeves wrote:
> https://download.kde.org/stable/applications/18.12.1/src/kdiff3-18.12.1.tar.
> xz
> 
> Get some one tell me how to change where it's trying to download from.
> KDiff3 is not part of applications and doesn't follow the same versioning.

Heya,

Could you please reconsider that decision and check whether it's not more 
worthwhile making kdiff3 part of KDE Applications? It will save you (as the 
maintainer) and others (distribution packagers) a major headache.

You'll be responsible for releasing kdiff3 now and in the future if you choose 
to do your own release schedule. Let me just say: It's not something which is 
particular entertaining in the long-term. Your KDiff3 involvement will get 
less eventually, and then someone else needs to take over releasing it -- if 
it's part of the KDE Apps cycle it'll be done automatically, no matter what.

KDiff3 is not the type of application which needs its own release cycle, IMO, 
it's too small & "undynamic" [1] for that.

Regards,
Kevin


[1] "Undynamic" in a sense that we're likely not going to see drastic UI 
changes on weekly basis which need to get out to users ASAP. At least for 
kdiff3 I'd rather have a conservative approach in that regard, since it's a 
complex tool by definition.


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

D17325: Fix leak in kemoticons

[Kevin Funk]

kfunk updated this revision to Diff 46787.
kfunk added a comment.


  Use QSharedPointer

REPOSITORY
  R301 KEmoticons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17325?vs=46765=46787

BRANCH
  master

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

AFFECTED FILES
  src/core/kemoticonstheme.cpp

To: kfunk
Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

D17325: Fix leak in kemoticons

[Kevin Funk]

kfunk added a comment.


  Not entirely sure about whether this is the right fix for this. Can someone 
check why this was commented before?
  
  The leak is fixed with this patch. Unit tests still work, too.
  
  And it kind of makes sense to have the `KEmoticonsTheme` class own its 
provider.

REPOSITORY
  R301 KEmoticons

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

To: kfunk
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17325: Fix leak in kemoticons

[Kevin Funk]

kfunk created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
kfunk requested review of this revision.

REVISION SUMMARY
  ASAN trace:
  Indirect leak of 64 byte(s) in 2 object(s) allocated from:
  
#0 0x52a000 in operator new(unsigned long) 
(/home/kfunk/devel/install/kf5/bin/kmail+0x52a000)
#1 0x7fbac897e192 in 
QList::node_construct(QList::Node*,
 KEmoticonsProvider::Emoticon const&) 
/home/kfunk/devel/build/qt5.11/qtbase/include/QtCore/../../../../../src/qt5.11/qtbase/src/corelib/tools/qlist.h:435:65
#2 0x7fbac897d184 in 
QList::append(KEmoticonsProvider::Emoticon 
const&) 
/home/kfunk/devel/build/qt5.11/qtbase/include/QtCore/../../../../../src/qt5.11/qtbase/src/corelib/tools/qlist.h:584:13
#3 0x7fbac897c59c in KEmoticonsProvider::addIndexItem(QString const&, 
QStringList const&) 
/home/kfunk/devel/src/kf5/kemoticons/src/core/kemoticonsprovider.cpp:162:39
#4 0x7fba8a15f7b5 in KdeEmoticons::loadTheme(QString const&) 
/home/kfunk/devel/src/kf5/kemoticons/src/providers/kde/kde_emoticons.cpp:183:13
#5 0x7fbac896ceb7 in KEmoticonsPrivate::loadTheme(QString 
const&)::$_6::operator()(QString const&, QString const&, KEmoticonsProvider*) 
const /home/kfunk/devel/src/kf5/kemoticons/src/core/kemoticons.cpp:126:19
#6 0x7fbac896cca1 in KEmoticonsPrivate::loadTheme(QString const&) 
/home/kfunk/devel/src/kf5/kemoticons/src/core/kemoticons.cpp:151:24
#7 0x7fbac896d24f in KEmoticons::theme(QString const&) const 
/home/kfunk/devel/src/kf5/kemoticons/src/core/kemoticons.cpp:181:15
#8 0x7fbac896d1ab in KEmoticons::theme() const 
/home/kfunk/devel/src/kf5/kemoticons/src/core/kemoticons.cpp:171:12
#9 0x7fba8a2e25bb in KTextToHTMLEmoticons::parseEmoticons(QString const&, 
bool, QStringList const&) 
/home/kfunk/devel/src/kf5/kemoticons/src/integrationplugin/ktexttohtml.cpp:43:24
...

REPOSITORY
  R301 KEmoticons

BRANCH
  master

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

AFFECTED FILES
  src/core/kemoticonstheme.cpp

To: kfunk
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17031: Fix a few memory leaksASAN: Fix leak in AppletQuickItem

[Kevin Funk]

kfunk added a comment.


  Yep, those are in fact separate commits.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: kfunk, mart
Cc: mart, kde-frameworks-devel, michaelh, ngraham, bruns

D17031: Fix a few memory leaksASAN: Fix leak in AppletQuickItem

[Kevin Funk]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:421e1c01266e: ASAN: Fix memory leak in CalendarPlugin 
(authored by kfunk).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D17031?vs=45849=45891#toc

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17031?vs=45849=45891

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

AFFECTED FILES
  src/declarativeimports/calendar/calendarplugin.cpp

To: kfunk, mart
Cc: mart, kde-frameworks-devel, michaelh, ngraham, bruns

D17031: Fix a few memory leaksASAN: Fix leak in AppletQuickItem

[Kevin Funk]

kfunk created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
kfunk requested review of this revision.

REVISION SUMMARY
  Direct leak of 3960 byte(s) in 15 object(s) allocated from:
  
#0 0x544cc0 in operator new(unsigned long) 
(/home/kfunk/devel/install/kf5/bin/plasmashell+0x544cc0)
#1 0x7f0dd8a3e2bd in 
PlasmaQuick::AppletQuickItem::AppletQuickItem(Plasma::Applet*, QQuickItem*) 
/home/kfunk/devel/src/kf5/plasma-framework/src/plasmaquick/appletquickitem.cpp:472:9
#2 0x7f0dcb7b365b in 
AppletInterface::AppletInterface(DeclarativeAppletScript*, QList 
const&, QQuickItem*) 
/home/kfunk/devel/src/kf5/plasma-framework/src/scriptengines/qml/plasmoid/appletinterface.cpp:50:7
#3 0x7f0dcb7aede1 in DeclarativeAppletScript::init() 
/home/kfunk/devel/src/kf5/plasma-framework/src/scriptengines/qml/plasmoid/declarativeappletscript.cpp:87:27
...
  
  ASAN: Fix memory leak in Corona
  
  KPackagePrivate::internalPackage already existed, the re-assignment to a
  new value causes a memory leak.
  
  Trace:
  Indirect leak of 112 byte(s) in 1 object(s) allocated from:
  
#0 0x544cc0 in operator new(unsigned long) 
(/home/kfunk/devel/install/kf5/bin/plasmashell+0x544cc0)
#1 0x7f905829163f in 
KPackage::Package::Package(KPackage::PackageStructure*) 
/home/kfunk/devel/src/kf5/kpackage/src/kpackage/package.cpp:51:9
#2 0x7f9058fad786 in Plasma::Package::Package(Plasma::PackageStructure*) 
/home/kfunk/devel/src/kf5/plasma-framework/src/plasma/package.cpp:66:34
#3 0x7f9058f14dee in Plasma::Corona::package() const 
/home/kfunk/devel/src/kf5/plasma-framework/src/plasma/corona.cpp:78:13
#4 0x5d9eb9 in ShellCorona::ShellCorona(QObject*) 
/home/kfunk/devel/src/kf5/plasma-workspace/shell/shellcorona.cpp:132:70
#5 0x65c31d in ShellManager::loadHandlers() 
/home/kfunk/devel/src/kf5/plasma-workspace/shell/shellmanager.cpp:93:21
  
  ASAN: Fix memory leak in DataSource
  
  Trace:
  Direct leak of 216 byte(s) in 27 object(s) allocated from:
  
#0 0x544cc0 in operator new(unsigned long) 
(/home/kfunk/devel/install/kf5/bin/plasmashell+0x544cc0)
#1 0x7fb2e9cd77d9 in Plasma::DataSource::setEngine(QString const&) 
/home/kfunk/devel/src/kf5/plasma-framework/src/declarativeimports/core/datasource.cpp:93:28
#2 0x7fb2e9d5c8f1 in Plasma::DataSource::qt_static_metacall(QObject*, 
QMetaObject::Call, int, void**) 
/home/kfunk/devel/build/kf5/plasma-framework/src/declarativeimports/core/corebindingsplugin_autogen/EWIEGA46WW/moc_datasource.cpp:330:21
  
  ASAN: Fix memory leak in CalendarPlugin
  
  Trace:
  Indirect leak of 24 byte(s) in 1 object(s) allocated from:
  
#0 0x508a17 in __interceptor_malloc 
(/home/kfunk/devel/install/kf5/bin/plasmashell+0x508a17)
#1 0x7fcf92aa6230 in QHashData::allocateNode(int) 
/home/kfunk/devel/src/qt5.11/qtbase/src/corelib/tools/qhash.cpp:479:79
#2 0x7fcf7a851d10 in QHash::detach() 
/home/kfunk/devel/build/qt5.11/qtbase/include/QtCore/../../../../../src/qt5.11/qtbase/src/corelib/tools/qhash.h:275:51
#3 0x7fcf7a8519c9 in QHash::insert(int const&, QByteArray 
const&) 
/home/kfunk/devel/build/qt5.11/qtbase/include/QtCore/../../../../../src/qt5.11/qtbase/src/corelib/tools/qhash.h:769:5
#4 0x7fcf7a859736 in 
EventPluginsModel::EventPluginsModel(EventPluginsManager*) 
/home/kfunk/devel/src/kf5/plasma-framework/src/declarativeimports/calendar/eventpluginsmanager.cpp:42:17
#5 0x7fcf7a854c55 in EventPluginsManager::EventPluginsManager(QObject*) 
/home/kfunk/devel/src/kf5/plasma-framework/src/declarativeimports/calendar/eventpluginsmanager.cpp:185:19
...

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

AFFECTED FILES
  src/declarativeimports/calendar/calendarplugin.cpp
  src/declarativeimports/core/datasource.cpp
  src/declarativeimports/core/datasource.h
  src/plasma/corona.cpp
  src/plasmaquick/appletquickitem.cpp

To: kfunk
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16894: [ECM] use a macro to add compiler flags conditionally

[Kevin Funk]

kfunk requested changes to this revision.
kfunk added a comment.
This revision now requires changes to proceed.


  I don't like the hiding of the if-branches as argument to macro. We shouldn't 
to this as it makes the code harder to understand.
  
  The general issue with the existing code here is: Statements like 
`CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT CMAKE_CXX_COMPILER_VERSION 
VERSION_LESS 3.5` do not make sense. If we check the compiler version, then we 
need to differentiate between Clang and AppleClang (cf. something like 
https://github.com/Microsoft/LightGBM/blob/master/CMakeLists.txt#L20).
  
  Thus these places need to be turned into:
  
...
if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_VERSION 
VERSION_LESS "3.8")
  ...
endif()
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" AND 
CMAKE_CXX_COMPILER_VERSION VERSION_LESS "8.1.0")
  ...
endif()
  
  If you're unsure in which version of AppleClang a certain feature was 
introduced then:
  
...
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" AND 
CMAKE_CXX_COMPILER_VERSION VERSION_LESS "8.1.0")
  add_compile_flag_if_supported(...)
endif()
  
  Please check out the functions provided in: 
kdevelop.git:cmake/modules/KDevelopMacrosInternal.cmake
  
#   add_compile_flag_if_supported( [CXX_ONLY])
#   add_target_compile_flag_if_supported( 
 )
  
  They are more clean than your implementation, I think it would make sense to 
actually add them to KDECompilerSettings.cmake and prefix them with `kde_`.

INLINE COMMENTS

> KDECompilerSettings.cmake:202
> +# is expected to support _FLAG.
> +if (${ARGN})
> +if(APPLE AND CMAKE_CXX_COMPILER_ID MATCHES "Clang")

This "let's abuse ${ARGN} as code to be evaluated later" is pretty ugly. The 
previous version (with the if-statements at  the caller side) is way more clean 
and understandable.

> KDECompilerSettings.cmake:203
> +if (${ARGN})
> +if(APPLE AND CMAKE_CXX_COMPILER_ID MATCHES "Clang")
> +# Clang on APPLE needs verification because of Apple's

I don't really understand why this branch is needed. The macro name name 
suggests it does the compile check on all compilers. Again very misleading.

REPOSITORY
  R240 Extra CMake Modules

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

To: rjvbb, #build_system, kfunk
Cc: kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system, 
michaelh, ngraham, bruns

D15002: Allow to install syntax files instead of having them in a resource

[Kevin Funk]

kfunk added inline comments.

INLINE COMMENTS

> CMakeLists.txt:3
>  macro(generate_php_syntax_definition targetFile srcFile)
> -add_custom_command(
> -OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${targetFile}
> -COMMAND ${PERL_EXECUTABLE} 
> ${CMAKE_CURRENT_SOURCE_DIR}/generators/generate-php.pl < 
> ${CMAKE_CURRENT_SOURCE_DIR}/syntax/${srcFile} > 
> ${CMAKE_CURRENT_BINARY_DIR}/${targetFile}
> -DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/generators/generate-php.pl 
> ${CMAKE_CURRENT_SOURCE_DIR}/syntax/${srcFile}
> -)
> +execute_process(COMMAND ${CMAKE_COMMAND} -E make_directory 
> ${CMAKE_CURRENT_BINARY_DIR}/syntax)
> +execute_process(COMMAND ${PERL_EXECUTABLE} 
> ${CMAKE_CURRENT_SOURCE_DIR}/generators/generate-php.pl

Note: The Ninja generator doesn't like this:

  CMake Warning (dev):
Policy CMP0058 is not set: Ninja requires custom command byproducts to be
explicit.  Run "cmake --help-policy CMP0058" for policy details.  Use the
cmake_policy command to set the policy and suppress this warning.
  
This project specifies custom command DEPENDS on files in the build tree
that are not specified as the OUTPUT or BYPRODUCTS of any
add_custom_command or add_custom_target:
  
 data/syntax/css-php.xml
 data/syntax/html-php.xml
 data/syntax/javascript-php.xml
  
For compatibility with versions of CMake that did not have the BYPRODUCTS
option, CMake is generating phony rules for such files to convince 'ninja'
to build.
  
Project authors should add the missing BYPRODUCTS or OUTPUT options to the
custom commands that produce these files.
  This warning is for project developers.  Use -Wno-dev to suppress it.

There must be a way to implement this feature and keep using 
`add_custom_command`...? Using `execute_process()` is usually a bad sign (tm). 
Note that these commands will be executed each CMake run(!)

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kfunk, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, 
bruns, demsking, cullmann, sars

D14987: hunspell: Restore build with hunspell <=v1.5.0

[Kevin Funk]

This revision was automatically updated to reflect the committed changes.
Closed by commit R246:a18b3810a18d: hunspell: Restore build with hunspell 
=v1.5.0 (authored by kfunk).

REPOSITORY
  R246 Sonnet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14987?vs=40190=40200

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

AFFECTED FILES
  src/plugins/hunspell/CMakeLists.txt
  src/plugins/hunspell/config-hunspell.h.cmake
  src/plugins/hunspell/hunspelldict.cpp

To: kfunk, rjvbb, dfaure, mlaurent
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14987: hunspell: Restore build with hunspell <=v1.5.0

[Kevin Funk]

kfunk added inline comments.

INLINE COMMENTS

> dfaure wrote in hunspelldict.cpp:130
> lst.reserve(nbWord);

Note: That's just old/copied code. Unlikely to be called on recent distros 
anyway.

REPOSITORY
  R246 Sonnet

BRANCH
  master

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

To: kfunk, rjvbb, dfaure, mlaurent
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14987: hunspell: Restore build with hunspell <=v1.5.0

[Kevin Funk]

kfunk added reviewers: rjvbb, dfaure, mlaurent.

REPOSITORY
  R246 Sonnet

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

To: kfunk, rjvbb, dfaure, mlaurent
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14987: hunspell: Restore build with hunspell <=v1.5.0

[Kevin Funk]

kfunk created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
kfunk requested review of this revision.

REVISION SUMMARY
  Commit 0a96acf251baa5c9dd042d093ab2bf8fcee10502 
 
broke compatibility with
  hunspell versions earlier than v1.5.1. v1.5.1 is was released Nov 2016,
  thus only two years old.
  
  This patch restores the build with hunspell versions provided on more
  conservative platforms (e.g. CentOS 6.8, which provides hunspell v1.2.x)

REPOSITORY
  R246 Sonnet

BRANCH
  master

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

AFFECTED FILES
  src/plugins/hunspell/CMakeLists.txt
  src/plugins/hunspell/config-hunspell.h.cmake
  src/plugins/hunspell/hunspelldict.cpp

To: kfunk
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D11193: Sonnet : use current hunspell API

[Kevin Funk]

kfunk added a comment.


  Please see concerns on 
https://phabricator.kde.org/R246:0a96acf251baa5c9dd042d093ab2bf8fcee10502

REPOSITORY
  R246 Sonnet

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

To: rjvbb, #frameworks, dfaure, mlaurent, vkrause
Cc: kfunk, kde-frameworks-devel, michaelh, ngraham, bruns

Re: vnc client keyboard mapping

[Kevin Funk]

On Tuesday, 31 July 2018 09:02:49 CEST Stéphane Ancelot wrote:
> Hi,
> 
> I don't manage to have my local client keyboard mapping when I connect
> to vnc server (x11vnc) in a kde plasma system.
> 
> Are there some settings needed in order to make it recognized ?

Are you maybe running into this problem here?
  
https://unix.stackexchange.com/questions/346107/keyboard-mapping-wrong-only-in-specific-applications-under-tightvnc

I suggest trying out TurboVNC or TigerVNC as VNCServer. Using TigerVNC solved 
this issue for me -- it's very easy to install on a Linux system.
  (see https://github.com/TigerVNC/tigervnc/releases)

Regards,
Kevin


> Is it supported in KDE plasma (that works fine in other systems.)
> 
> Regards,
> 
> S.Ancelot


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

D9408: extractors: Hide warnings from system headers

[Kevin Funk]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:b680861aa2ed: extractors: Hide warnings from system 
headers (authored by kfunk).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9408?vs=33189=33191

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

AFFECTED FILES
  src/extractors/CMakeLists.txt

To: kfunk, mgallien
Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, bruns

D9408: extractors: Hide warnings from system headers

[Kevin Funk]

kfunk updated this revision to Diff 33189.
kfunk added a comment.


  Add PRIVATE

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9408?vs=33121=33189

BRANCH
  master

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

AFFECTED FILES
  src/extractors/CMakeLists.txt

To: kfunk, mgallien
Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, bruns

Re: kfilemetadata compile failure

[Kevin Funk]

On Wednesday, 25 April 2018 14:34:58 CEST Jonathan Riddell wrote:
> kfilemetadata does not compile in KDE neon from git master currently
> 
> /workspace/build/src/extractors/ffmpegextractor.cpp:97:15: error:
> ‘AVCodecParameters’ does not name a type
> 12:27:35  const AVCodecParameters* codec = stream->codecpar;
> 
> https://build.neon.kde.org/job/xenial_unstable_kde_kfilemetadata_bin_amd64/1
> 45/console

Raised a concern on the resp. Phab Diff (to ping the responsible people):
  https://phabricator.kde.org/R286:037208a787e0c2412ab616ff1573c323a2346d2d

Regards,
Kevin
 
> It may be expecting a newer version of ffmpeg
> 
> Jonathan


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

D9408: extractors: Hide warnings from system headers

[Kevin Funk]

kfunk added a reviewer: mgallien.

REPOSITORY
  R286 KFileMetaData

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

To: kfunk, mgallien
Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, bruns

D9408: extractors: Hide warnings from system headers

[Kevin Funk]

kfunk updated this revision to Diff 33121.
kfunk added a comment.
Restricted Application added a project: Baloo.


  Rebased

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9408?vs=24101=33121

BRANCH
  master

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

AFFECTED FILES
  src/extractors/CMakeLists.txt

To: kfunk
Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, bruns

D12112: Fix compiler warning under Clang

[Kevin Funk]

This revision was automatically updated to reflect the committed changes.
Closed by commit R244:53f44072c70a: Fix compiler warning under Clang (authored 
by kfunk).

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12112?vs=31888=31914

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

AFFECTED FILES
  src/lib/caching/kshareddatacache_p.h

To: kfunk, apol
Cc: apol, #frameworks, michaelh, ngraham, bruns

D12112: Fix compiler warning under Clang

[Kevin Funk]

kfunk updated this revision to Diff 31888.
kfunk added a comment.


  Address concerns. I knew why I put this up for review. /me grabs another 
coffee...

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12112?vs=31878=31888

BRANCH
  master

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

AFFECTED FILES
  src/lib/caching/kshareddatacache_p.h

To: kfunk, apol
Cc: apol, #frameworks, michaelh, ngraham, bruns

D12112: Fix compiler warning under Clang

[Kevin Funk]

kfunk created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
kfunk requested review of this revision.

REVISION SUMMARY
  Issue:
  In file included from .../kcoreaddons/src/lib/caching/kshareddatacache.cpp:25:
  .../kcoreaddons/src/lib/caching/kshareddatacache_p.h:169:47: warning: unknown 
attribute 'artificial' ignored [-Wunknown-attributes]
  
__attribute__((always_inline, gnu_inline, artificial))
  ^

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

AFFECTED FILES
  src/lib/caching/kshareddatacache_p.h

To: kfunk
Cc: #frameworks, michaelh, ngraham, bruns

D11610: clang-tidy: modernize-use-default-member-init run

[Kevin Funk]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:40d801599549: clang-tidy: 
modernize-use-default-member-init run (authored by kfunk).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11610?vs=30305=31852

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

AFFECTED FILES
  autotests/src/plaintextsearch_test.cpp
  autotests/src/plaintextsearch_test.h
  autotests/src/scriptdocument_test.cpp
  autotests/src/scriptdocument_test.h
  autotests/src/vimode/emulatedcommandbar.cpp
  autotests/src/vimode/emulatedcommandbar.h
  src/buffer/katetexthistory.h
  src/buffer/katetextline.cpp
  src/buffer/katetextline.h
  src/completion/katecompletionmodel.cpp
  src/completion/katecompletionmodel.h
  src/completion/katecompletionwidget.h
  src/dialogs/kateconfigpage.cpp
  src/dialogs/kateconfigpage.h
  src/document/katedocument.cpp
  src/document/katedocument.h
  src/include/ktexteditor/configinterface.h
  src/include/ktexteditor/cursor.h
  src/include/ktexteditor/markinterface.h
  src/include/ktexteditor/modificationinterface.h
  src/include/ktexteditor/movinginterface.h
  src/include/ktexteditor/movingrangefeedback.h
  src/include/ktexteditor/sessionconfiginterface.h
  src/include/ktexteditor/texthintinterface.h
  src/mode/katemodemanager.h
  src/printing/printpainter.cpp
  src/render/katerenderrange.cpp
  src/render/katerenderrange.h
  src/render/katetextlayout.cpp
  src/render/katetextlayout.h
  src/schema/katecolortreewidget.h
  src/script/kateindentscript.h
  src/script/katescript.h
  src/spellcheck/prefixstore.cpp
  src/spellcheck/prefixstore.h
  src/spellcheck/spellcheckbar.cpp
  src/syntax/katehighlight.h
  src/syntax/katesyntaxmanager.cpp
  src/syntax/katesyntaxmanager.h
  src/utils/codecompletionmodel.cpp
  src/utils/configinterface.cpp
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h
  src/utils/ktexteditor.cpp
  src/utils/movinginterface.cpp
  src/utils/movingrangefeedback.cpp
  src/vimode/modes/modebase.h

To: kfunk, dhaumann
Cc: dhaumann, #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, 
cullmann, sars

D11948: [KFileWidget] Hardcode example user name

[Kevin Funk]

kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.


  Makes sense to me. Go for it.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, kfunk
Cc: kfunk, elvisangelaccio, michaelh, ngraham, bruns

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

[Kevin Funk]

kfunk resigned from this revision.

REPOSITORY
  R293 Baloo

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

To: tcberner, #freebsd, poboiko, bruns
Cc: bruns, adridg, kfunk, #frameworks, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, alexeymin

Re: Asking for recommendations on how to add a feature to KTextEditor

[Kevin Funk]

On Monday, 9 April 2018 13:13:46 CEST Aleix Pol wrote:
> On Mon, Apr 9, 2018 at 12:49 PM, Michal Srb <michal...@gmail.com> wrote:
> > Hi,
> > 
> > I would like to ask for hints/recommendations on how to implement a
> > feature for KTextEditor.
> > 
> > My ultimate goal is to make a plugin for KDevelop that will show some
> > additional information in the code itself. For example names of
> > function arguments at call site. Something like this in intellij:
> > https://d3nmt5vlzunoa1.cloudfront.net/idea/files/2016/09/Screen-Shot-2016-> 
> > > 09-27-at-10.29.15.png

Heya Michal,

That indeed would be super helpful to have, for a variety of use-cases.

+1 on the idea for the feature!

Can't help a lot with implementing this correctly in KTextEditor, though. The 
Kate people should chime in there :)

Cheers,
Kevin

> > For this I would need a way to render something in the text, affecting
> > the layout of the line. It would be nice to be able to place any
> > general widget or image into the line, but even plain text would be
> > sufficient. It must behave differently than the "real" text around it,
> > such that it can not be edited, cursor skips it, copying does not copy
> > it, etc.
> > 
> > So far I thought of adding something like `AnnotationViewInterface`,
> > which would allow to add this kind of immutable text on line+column
> > position. Then in `KateRenderer` it could take these into
> > consideration when layouting the line and render it in. It would be
> > visible in the text, but never become a real part of the text.
> > 
> > Would you recommend some other approach, or is there perhaps a way to
> > do it without modifying KTextEditor?
> 
> Including kwrite-devel where many kate developers live. :)
> Looking forward to seeing your feature implemented in KDevelop! :D
> 
> Aleix


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: Unable to compile KWidgetsAddons

[Kevin Funk]

On Thursday, 29 March 2018 15:53:25 CEST Nate Graham wrote:
> In trying to test out a KWidgetsAddons patch, I find that I'm unable to
> compile it on KDE Neon dev unstable the code due to a CMake error:
> 
> 
> CMake Error in autotests/CMakeLists.txt:
>No known features for CXX compiler
> 
>"GNU"
> 
>version 5.4.0.
> 
> Full output available at https://paste.kde.org/piyy4wsnl

Heya,

FYI: The compile-feature [1] is used by the CMake Config files installed by 
Qt. For Qt it is used to figure out when and how to pass the i.e. the resp. `-
std=...` flag for GNU-like compilers for users of Qt.

Usually you get that kind of error in case you're trying to use an unsupported 
compiler for which CMake doesn't know the availability of the C++ features. A 
typical example is Android GCC.

I'm surprised you get that error by a standard GCC...? Would be nice to figure 
out what's going wrong. Maybe `cmake --trace` or `cmake --debug-output` can 
help.

Please check the following link for a work-around:
  https://stackoverflow.com/a/40256862/592636

[1] https://cmake.org/cmake/help/v3.8/manual/cmake-compile-features.7.html

Regards,
Kevin

> My g++ version looks okay:
> 
> $ g++ --version
> g++ (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
> Copyright (C) 2015 Free Software Foundation, Inc.
> 
> 
> Nate


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

D10739: Remove deprecated cmake code

[Kevin Funk]

kfunk accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R246 Sonnet

BRANCH
  master

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

To: vonreth, kfunk, dfaure, vkrause, mlaurent
Cc: apol, #frameworks, michaelh, ngraham

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

[Kevin Funk]

kfunk requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R240 Extra CMake Modules

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

To: rjvbb, #frameworks, #build_system, cgilles, kfunk
Cc: thiago, kfunk, michaelh, ngraham

D11610: clang-tidy: modernize-use-default-member-init run

[Kevin Funk]

kfunk added a comment.


  I'll wait until v5.45 got branched off. No need to hurry with this.

REPOSITORY
  R39 KTextEditor

BRANCH
  master

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

To: kfunk, dhaumann
Cc: dhaumann, #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, 
sars

D11610: clang-tidy: modernize-use-default-member-init run

[Kevin Funk]

kfunk edited the summary of this revision.
kfunk edited the test plan for this revision.

REPOSITORY
  R39 KTextEditor

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

To: kfunk
Cc: #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, sars, 
dhaumann

D11610: clang-tidy: modernize-use-default-member-init run

[Kevin Funk]

kfunk created this revision.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added a subscriber: Frameworks.
kfunk requested review of this revision.

REPOSITORY
  R39 KTextEditor

BRANCH
  master

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

AFFECTED FILES
  autotests/src/plaintextsearch_test.cpp
  autotests/src/plaintextsearch_test.h
  autotests/src/scriptdocument_test.cpp
  autotests/src/scriptdocument_test.h
  autotests/src/vimode/emulatedcommandbar.cpp
  autotests/src/vimode/emulatedcommandbar.h
  src/buffer/katetexthistory.h
  src/buffer/katetextline.cpp
  src/buffer/katetextline.h
  src/completion/katecompletionmodel.cpp
  src/completion/katecompletionmodel.h
  src/completion/katecompletionwidget.h
  src/dialogs/kateconfigpage.cpp
  src/dialogs/kateconfigpage.h
  src/document/katedocument.cpp
  src/document/katedocument.h
  src/include/ktexteditor/configinterface.h
  src/include/ktexteditor/cursor.h
  src/include/ktexteditor/markinterface.h
  src/include/ktexteditor/modificationinterface.h
  src/include/ktexteditor/movinginterface.h
  src/include/ktexteditor/movingrangefeedback.h
  src/include/ktexteditor/sessionconfiginterface.h
  src/include/ktexteditor/texthintinterface.h
  src/mode/katemodemanager.h
  src/printing/printpainter.cpp
  src/render/katerenderrange.cpp
  src/render/katerenderrange.h
  src/render/katetextlayout.cpp
  src/render/katetextlayout.h
  src/schema/katecolortreewidget.h
  src/script/kateindentscript.h
  src/script/katescript.h
  src/spellcheck/prefixstore.cpp
  src/spellcheck/prefixstore.h
  src/spellcheck/spellcheckbar.cpp
  src/syntax/katehighlight.h
  src/syntax/katesyntaxmanager.cpp
  src/syntax/katesyntaxmanager.h
  src/utils/codecompletionmodel.cpp
  src/utils/configinterface.cpp
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h
  src/utils/ktexteditor.cpp
  src/utils/movinginterface.cpp
  src/utils/movingrangefeedback.cpp
  src/vimode/modes/modebase.h

To: kfunk
Cc: #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, sars, 
dhaumann

D11278: [KateCompletionWidget] Create configuration interface on demand

[Kevin Funk]

kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.


  In D11278#224484 , @kfunk wrote:
  
  > Looks like you don't need the member at all? Otherwise late-init wouldn't 
work this way.
  >
  > `m_configWidget` seems only used in `showConfig()`. Let's remove the member 
altogether?
  
  
  Ah, disregard my comment.  If `showConfig()` is invoked multiple times then 
my approach would be slower.

REPOSITORY
  R39 KTextEditor

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

To: broulik, #ktexteditor, dhaumann, kfunk
Cc: kfunk, #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, 
sars, dhaumann

D11278: [KateCompletionWidget] Create configuration interface on demand

[Kevin Funk]

kfunk added a comment.


  Looks like you don't need the member at all? Otherwise late-init wouldn't 
work this way.
  
  `m_configWidget` seems only used in `showConfig()`. Let's remove the member 
altogether?

REPOSITORY
  R39 KTextEditor

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

To: broulik, #ktexteditor, dhaumann
Cc: kfunk, #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, 
sars, dhaumann

Re: Converting KDE4 projects to KF5

[Kevin Funk]

On Wednesday, 21 February 2018 14:42:20 CET Robin Atwood wrote:
> I have several old projects that really needed to be updated to KF5. Is
> there a handy guide to doing the conversion?

Heya Robin,

note: wrong mailing list. This is the mailing list dealing with KDevelop, the 
IDE :)

I've CC'd the correct mailing list now.

Anyway, for porting your applications to KF5, check out:
  http://www.proli.net/2014/06/21/porting-your-project-to-qt5kf5/

I'd especially pay attention to:
  http://community.kde.org/Frameworks/Porting_Notes

And I'd strongly recommend you to use the porting scripts here:
  git://anongit.kde.org/kde-dev-scripts, subdirectory kf5

Happy porting!

Regards,
Kevin

> Thanks
> Robin


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: KDirListerCache destroy comment in kcoredirlister.cpp

[Kevin Funk]

On Monday, 29 January 2018 11:32:56 CET René J.V. Bertin wrote:
> Hi,
> 
> Researching an issue I'm seeing (hang/crash on exit) I found the following
> vestigial out-commented code the KCoreDirListerCache ctor in
> kcoredirlister.cpp:

FYI: Note to the mailing list, before more precious developer time is being 
spent on this here:

There are two more follow-up posts, just a few more hours younger, related to 
this issue on qt-interest:
  http://lists.qt-project.org/pipermail/interest/2018-January/029182.html
  (rather unrelated, but is also hinting at said problem...)
  http://lists.qt-project.org/pipermail/interest/2018-January/029194.html
  (asks about help for debugging crashes during destruction of QRegExpEngine)

Regards,
Kevin

> // Probably not needed in KF5 anymore:
> // The use of KUrl::url() in ~DirItem (sendSignal) crashes if the static
> for QRegExpEngine got deleted already, // so we need to destroy the
> KCoreDirListerCache before that.
> //qAddPostRoutine(kDirListerCache.destroy);
> 
> WHat's clear is that this no longer works: there is no `destroy` method in
> whatever kDirListerCache's real class is.
> 
> Git blame is of little help here; can anyone remember why the explicit
> pro-active delete was deemed unnecessary, and what the exact symptoms were
> when this was not done in KDE4?
> 
> FWIW, the crash (or hang) happens during QRegExpEngine take-down when I have
> used a KDE file dialog in a "pure Qt" application via a mechanism built on
> code taken from the plasma integration plugin. It does not happen in KF5
> applications like kate or kdialog. As far as I know those applications
> obtain the KDE file dialog from the platform plugin too. The difference
> must be in the order in which certain things are deleted during the global
> destruction phase, but which and how to restore the proper order. (The
> Audacious audioplayer had a similar issue on exit in its Qt5 version, which
> I traced to the QApplication instance being deleted via atexit; a lucky
> shot in the dark that makes a lot of sense in retrospect.)
> 
> Thanks for any pointers,
> R.


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: Importing kdiff3 - was - Re: Aw: Re: KDE inclusion

[Kevin Funk]

On Friday, 26 January 2018 08:27:55 CET Kevin Ottens wrote:
> Hello,
> 
> On Thursday, 25 January 2018 22:35:37 CET Albert Astals Cid wrote:
> > El dijous, 25 de gener de 2018, a les 9:01:10 CET, Kevin Ottens va 
escriure:
> > > On Thursday, 25 January 2018 00:08:03 CET Albert Astals Cid wrote:
> > > > As I did with the last person that also was confused and annoyed by
> > > > all
> > > > this burocracy, just ask me any question you may have.
> > > 
> > > Oh come on... the bad mean bureaucracy argument now. Wanna look at the
> > > Eclipse incubation process? Or the Apache one?
> > 
> > Can I use the "if all your friends jump from a balcony will you do it"
> > defense?
> 
> Not really since my point was more that what we have in place is very very
> far from bureaucracy not that we should replicate other's bureaucracy. If
> you want an example of bureaucracy look at those friends who jump from a
> balcony. ;-)
> > > Seriously it's just about having a person already within the community
> > > making sure the new project needs get catered to and also making sure
> > > the
> > > new project is on the right path to put in place rules and procedures
> > > compatible with the rest of the community (and the Manifesto).
> > 
> > But how do you find that person? You're just an 'outsider', how do you
> > find
> > a random person to be your incubator guy? Because as it happens, it's the
> > second time in a month or something that i have to volunteer.
> 
> Ah! That is interesting feedback. You're correct that we're currently
> assuming that someone will step in to do that and that there's enough of us
> and that we're responsible enough to do that when we see something we're
> interested in.
> 
> Personally for kdiff3, I'd have expected Kevin Funk to end up doing it,
> indeed he was first responder with "I'd love to see kdiff3 being adopted by
> KDE again". To me he sounded like a perfect sponsor.

Heya,

Sorry that wasn't clear from my mail.

Several reasons I'd be rather not do it: I'm reaaally lacking time and focus 
to go through the whole process at this point; and adding to that I'm not 
familiar with the incubation process myself either...

I just expressed that I'd love to see it happen, in general. Somehow. :)

Regards,
Kevin
 
> I'd like to see that fixed. Right now, I'm not sure how, but if you're the
> only one indeed caring about new projects getting in, we have a more general
> community problem, it's just that the incubator makes it visible...
> > I think it's much easier if we had guidelines and the rest was just "ask
> > in
> > kde-devel mailing list if you have further questions",
> 
> It'd be easier, but not better. Because then it's no different than "ask the
> GitHub support if you have further questions", and it's not what it's
> about.
> 
> In my previous email I mentioned this is *also* for the "sponsor" to touch
> base with the joining project to verify it's getting into fruition to *be* a
> KDE project (which is not just about having a repository on our
> infrastructure)... I know, pesky people and culture thing.
> 
> > and sure if you find a dedicated person for you, great, but requiring it
> > feels weird, and also makes it for less scalability, as an example I
> > already have an email from Michael that was sent only to me but anyone
> > else in this list would have been able to answer, but he had to wait at
> > least 14 hours for me to have time to answer it.
> 
> Maybe that needs to be made clearer in the wiki? I'd expect the sponsor to
> push the involved persons to ask these type of questions on public mailing
> lists indeed. One on one discussions are likely to happen but they must be
> the minority of the communication going on. The sponsor in that case is the
> fail safe mechanism to make sure an answer indeed happens in those public
> forums or trying to solve the case if no answer happened for some reason.
> 
> Regards.


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: Importing kdiff3 - was - Re: Aw: Re: KDE inclusion

[Kevin Funk]

On Wednesday, 24 January 2018 16:24:10 CET Michael Reeves wrote:
> A little confused on where to start with this sponsor thing.

Huh? 'sponsor thing'? :)

Care to elaborate what you mean?

Regards,
Kevin

> On Jan 19, 2018 3:52 PM, "Kevin Funk" <kf...@kde.org> wrote:
> > On Wednesday, 17 January 2018 21:40:49 CET Michael Reeves wrote:
> > > Yes I use this myself so I won't simply abandon it. That said I work in
> > > mostly in Unix world and could use help with the windows specific
> > > testing
> > > although I should be able to setup a build envirionment to do basic
> > > functionality testing.
> > 
> > Heya,
> > 
> > For that please join #kde-windows and get in touch with kfunk (me) or
> > TheOneRing (Hannah von Reth) there.
> > 
> > We could guide setting up the development environment.
> > 
> > We use Craft to build Qt/KDE projects on Windows, read more here:
> >   https://community.kde.org/Craft
> > 
> > Cheers,
> > Kevin
> > 
> > > I'll have look at what needs done. Currently working
> > > on getting merging in changes from Thomas as needed.  Just got through
> > > bringing it up to date with Joachim's code. Real fun since it's still
> > 
> > kde4.
> > 
> > > On Jan 15, 2018 5:47 PM, "Albert Astals Cid" <aa...@kde.org> wrote:
> > > 
> > > El dimarts, 9 de gener de 2018, a les 20:06:36 CET, Michael Reeves va
> > > 
> > > escriure:
> > > > I have a version of kdiff3 that I ported to kf5. I like to what build
> > > > requirements kf5 as a whole has. Also what would be the process for
> > 
> > being
> > 
> > > > considered for inclusion in kde?
> > > 
> > > So now that we have Joachim's blessing to use the name, the process is
> > > basically outlined at https://community.kde.org/
> > 
> > Incubator/Incubation_Process
> > 
> > > but it's basically importing the project into our git and then making
> > 
> > sure
> > 
> > > it
> > > generally follows our rules & procedures.
> > > 
> > > I understand you want to continue maintaining the new kdiff3 and you are
> > 
> > not
> > 
> > > just "dumping it" into us, right?
> > > 
> > > 
> > > Cheers,
> > > 
> > >   Albert
> > > 
> > > El divendres, 12 de gener de 2018, a les 1:21:02 CET, Joachim Eibl va
> > > 
> > > escriure:
> > > > Hi,
> > > > 
> > > > You have my blessing to use the name KDiff3.
> > 
> > --
> > Kevin Funk | kf...@kde.org | http://kfunk.org


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: Importing kdiff3 - was - Re: Aw: Re: KDE inclusion

[Kevin Funk]

On Wednesday, 17 January 2018 21:40:49 CET Michael Reeves wrote:
> Yes I use this myself so I won't simply abandon it. That said I work in
> mostly in Unix world and could use help with the windows specific testing
> although I should be able to setup a build envirionment to do basic
> functionality testing.

Heya,

For that please join #kde-windows and get in touch with kfunk (me) or 
TheOneRing (Hannah von Reth) there.

We could guide setting up the development environment. 

We use Craft to build Qt/KDE projects on Windows, read more here: 
  https://community.kde.org/Craft

Cheers,
Kevin

> I'll have look at what needs done. Currently working
> on getting merging in changes from Thomas as needed.  Just got through
> bringing it up to date with Joachim's code. Real fun since it's still kde4.
> 
> On Jan 15, 2018 5:47 PM, "Albert Astals Cid" <aa...@kde.org> wrote:
> 
> El dimarts, 9 de gener de 2018, a les 20:06:36 CET, Michael Reeves va
> 
> escriure:
> > I have a version of kdiff3 that I ported to kf5. I like to what build
> > requirements kf5 as a whole has. Also what would be the process for being
> > considered for inclusion in kde?
> 
> So now that we have Joachim's blessing to use the name, the process is
> basically outlined at https://community.kde.org/Incubator/Incubation_Process
> but it's basically importing the project into our git and then making sure
> it
> generally follows our rules & procedures.
> 
> I understand you want to continue maintaining the new kdiff3 and you are not
> just "dumping it" into us, right?
> 
> 
> Cheers,
>   Albert
> 
> El divendres, 12 de gener de 2018, a les 1:21:02 CET, Joachim Eibl va
> 
> escriure:
> > Hi,
> > 
> > You have my blessing to use the name KDiff3.


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

D9766: Use at least the requested width for the argument hint tree

[Kevin Funk]

kfunk added a comment.


  Whoops. Sorry for accidentally reopening it to begin with.

REPOSITORY
  R39 KTextEditor

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

To: mwolff, #kdevelop, #kate
Cc: dhaumann, kfunk, #frameworks, michaelh, kevinapavew, ngraham, demsking, 
cullmann, sars

D9766: Use at least the requested width for the argument hint tree

[Kevin Funk]

kfunk reopened this revision.
kfunk added a comment.


  LGTM.
  
  Just a had a quick look at the appearance of the popup before vs. after the 
patch when doing code completion inside the a function call parameter list. 
Looks more consisten now, thanks!

REPOSITORY
  R39 KTextEditor

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

To: mwolff, #kdevelop, #kate
Cc: dhaumann, kfunk, #frameworks, michaelh, kevinapavew, ngraham, demsking, 
cullmann, sars

Re: KDE inclusion

[Kevin Funk]

On Tuesday, 9 January 2018 21:06:36 CET Michael Reeves wrote:
> I have a version of kdiff3 that I ported to kf5. I like to what build
> requirements kf5 as a whole has. Also what would be the process for being
> considered for inclusion in kde?

Heya,

Note: kdiff3 right now is hosted & developed on SourceForge.

I'd love to see kdiff3 being adopted by KDE again (it former was KDE extragear 
if I understood correctly). kdiff3 is a super useful tool -- and right now 
development has stalled a bit.

Talked to Joachim (the original author) a few weeks ago, where he stated he 
just doesn't have the time maintaining it anymore, really. I've CC'd Joachim 
so he can tell us whether he's okay with having kdiff3 developed further under 
the KDE umbrella.

I don't really know the process of having it integrated either. I'll leave 
that to others.

Kudos for doing the KF5 port!

Regards,
Kevin


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

D9760: Fix kded dbus name in solid-networking howto

[Kevin Funk]

kfunk accepted this revision.

REPOSITORY
  R239 KDELibs4Support

BRANCH
  master

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

To: habacker, kfunk
Cc: kfunk, #frameworks

D9760: Fix kded dbus name in solid-networking howto

[Kevin Funk]

kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.


  //src/solid-networkstatus/DESIGN// also still references `org.kde.kded`. 
Please update, too.
  
  Rest LGTM

REPOSITORY
  R239 KDELibs4Support

BRANCH
  master

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

To: habacker, kfunk
Cc: kfunk, #frameworks

D9750: ExpandingWidgetModel: find the right-most column based on location

[Kevin Funk]

kfunk added a comment.


  LGTM, but I'll let someone else approve.

REPOSITORY
  R39 KTextEditor

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

To: mwolff, #kate
Cc: kfunk, #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, 
sars, dhaumann

D9749: Simplify code: return early to reduce indentation depth

[Kevin Funk]

kfunk accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R39 KTextEditor

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

To: mwolff, #kate, kfunk
Cc: #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, sars, 
dhaumann

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Kevin Funk]

kfunk added a comment.


  In https://phabricator.kde.org/D9423#181460, @habacker wrote:
  
  > In https://phabricator.kde.org/D9423#181442, @kfunk wrote:
  >
  > > And I agree. There shouldn't be a need to use two different input files. 
That's the whole point of `configure_file(...)`: the interpolation of values 
happens *inside* the input files.
  >
  >
  > This would be possible by using in the service file something like this
  >
  > [D-BUS Service]
  >  Name=org.kde.kiod5
  >  Exec=@SOME_PREFIX@kiod5
  
  
  Sounds good.
  
  In the top-level CMakeLists.txt maybe:
  
if(WIN32)
 set(DBUS_LIBEXECDIR)
else()
 set(DBUS_LIBEXECDIR ${KDE_INSTALL_FULL_LIBEXECDIR}/kf5/)
endif()
  
  ... and then use that everywhere needed.

REPOSITORY
  R241 KIO

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

To: habacker, dfaure, bcooksley, kfunk
Cc: kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Kevin Funk]

kfunk added a comment.


  @dfaure Opinions?

REPOSITORY
  R241 KIO

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

To: habacker, dfaure, bcooksley, kfunk
Cc: kfunk, broulik, #frameworks

D9424: Fix 'Exec line in kglobalaccel5 service file must not have any path prefix on Windows'

[Kevin Funk]

kfunk added a comment.


  Please let's discuss on https://phabricator.kde.org/D9423 exclusively. This 
is basically fixing the same issue in a different repository.

REPOSITORY
  R268 KGlobalAccel

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

To: habacker, dfaure, bcooksley, apol
Cc: kfunk, vonreth, #windows, apol, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Kevin Funk]

kfunk requested changes to this revision.
kfunk added a comment.
This revision now requires changes to proceed.


  And I agree. There shouldn't be a need to use two different input files. 
That's the whole point of `configure_file(...)` the interpolation of values 
happens *inside* the input files.

REPOSITORY
  R241 KIO

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

To: habacker, dfaure, bcooksley, kfunk
Cc: kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Kevin Funk]

kfunk added a comment.


  Could you please squash the commits and just keep this review for all the 
changes in KIO? Doesn't make sense to have different Phab Diffs for that (makes 
it harder to review, harder to revert in case, etc.).

REPOSITORY
  R241 KIO

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

To: habacker, dfaure, bcooksley
Cc: kfunk, broulik, #frameworks

D9398: Fix TagLib detection and build on Windows

[Kevin Funk]

This revision was automatically updated to reflect the committed changes.
Closed by commit R320:5e6c6fe0c7a1: Fix TagLib detection and build on Windows 
(authored by kfunk).

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9398?vs=24075=24104

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

AFFECTED FILES
  cmake/FindTaglib.cmake
  thumbnail/audiocreator.cpp

To: leinir, #frameworks, kfunk
Cc: vonreth

D9408: extractors: Hide warnings from system headers

[Kevin Funk]

kfunk created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  Also prefer kde_target_enable_exceptions() over the function enabling
  exceptions globally.

REPOSITORY
  R286 KFileMetaData

BRANCH
  master

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

AFFECTED FILES
  src/extractors/CMakeLists.txt

To: kfunk
Cc: #frameworks

D9398: Fix TagLib detection and build on Windows

[Kevin Funk]

kfunk accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R320 KIO Extras

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

To: leinir, #frameworks, kfunk
Cc: vonreth

D9299: Introduce INSTALL_PREFIX_SCRIPT to easily set up prefixes

[Kevin Funk]

kfunk added a comment.


  Sorry for being late, but I didn't have time to follow up on this one.
  
  My concerns:
  
  - If you have that option `ON` and all frameworks install into the same 
prefix, `prefix.sh` will be overwritten. => Bad.
  - I still don't fully see how to adopt the use case apol mentioned: Say each 
Framework installs into its own unique prefix. => Ok.
- But how are you supposed to source each `prefix.sh`? There must be script 
to do this? Where's the documentation then?

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, sitter
Cc: kfunk, bcooksley, ngraham, sitter, cgiboudeaux, #build_system

D9321: Fix another CMake CMP0071 warning

[Kevin Funk]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:fb80c0549aed: Fix another CMake CMP0071 warning (authored 
by kfunk).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9321?vs=23887=23896

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

AFFECTED FILES
  data/CMakeLists.txt

To: kfunk, mlaurent, dfaure
Cc: #frameworks

D9321: Fix another CMake CMP0071 warning

[Kevin Funk]

kfunk added reviewers: mlaurent, dfaure.

REPOSITORY
  R216 Syntax Highlighting

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

To: kfunk, mlaurent, dfaure
Cc: #frameworks

D9321: Fix another CMake CMP0071 warning

[Kevin Funk]

kfunk created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
CMake Warning (dev) in data/CMakeLists.txt:
  Policy CMP0071 is not set: Let AUTOMOC and AUTOUIC process GENERATED 
files.
  Run "cmake --help-policy CMP0071" for policy details.  Use the 
cmake_policy
  command to set the policy and suppress this warning.

  For compatibility, CMake is excluding the GENERATED source file(s):


"/home/kfunk/devel/src/kf5/syntax-highlighting/build/data/qrc_syntax-data.cpp"

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

AFFECTED FILES
  data/CMakeLists.txt

To: kfunk
Cc: #frameworks

D9277: Remove cmake warning about generating moc file

[Kevin Funk]

kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.


  Yes. Makes sense to me.

INLINE COMMENTS

> KF5ConfigMacros.cmake:98
>  
> +   set_property(SOURCE ${_header_FILE} PROPERTY SKIP_AUTOMOC TRUE)  # 
> don't run automoc on this file
> +   set_property(SOURCE ${_src_FILE} PROPERTY SKIP_AUTOMOC TRUE)  # don't 
> run automoc on this file

Use `set_source_files_properties(...)`  instead, cf. 
https://phabricator.kde.org/R249:6e3b708435cc7747ea83a3afea4647c2d48b

REPOSITORY
  R237 KConfig

BRANCH
  remove_cmake_warning

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

To: mlaurent, kfunk, dfaure
Cc: mpyne, apol, aacid, #frameworks

D9299: Introduce INSTALL_PREFIX_SCRIPT to easily set up prefixes

[Kevin Funk]

kfunk added a comment.


  In https://phabricator.kde.org/D9299#179036, @cgiboudeaux wrote:
  
  > In https://phabricator.kde.org/D9299#179035, @cgiboudeaux wrote:
  >
  > > In https://phabricator.kde.org/D9299#179032, @kfunk wrote:
  > >
  > > > If we'd name this file somewhat less generic then it could be even 
installed by default, no?
  > > >
  > > > I had the scheme of  the QNX setup script in my mind: 
https://github.com/acklinr/qnx660/blob/master/qnx660-env.sh
  > > >
  > > > Thus: Maybe rename prefix.sh to say 'ecm-env.sh' and install the file 
by default?
  > >
  > >
  > > Files overwritting each others is a no-go for packagers. +1 for a less 
generic name however.
  >
  >
  > Adding files in /usr directly, will also be rejected.
  >
  > Now that I had a closer look: KDEInstallDirs only defines some paths, I 
don't think this change shall be part of this file. What about moving it to its 
own file and sourcing it instead ?
  
  
  I agree. KDEInstallDirs.cmake seems to be wrong location for this 
functionality.
  
  What I'm envisioning is a ecm-env.sh-like script which gets installed into 
`$PREFIX/bin` as soon as you install ECM.
  
  Pseudo-cmake code:
  
configure_file(${CMAKE_SOURCE_DIR}/ecm-env.sh 
${CMAKE_BINARY_DIR}/ecm-env.sh)
install(FILES ${CMAKE_BINARY_DIR}/ecm-env.sh DESTINATION ${BIN_INSTALL_DIR})

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, sitter
Cc: kfunk, bcooksley, ngraham, sitter, cgiboudeaux, #build_system

D9299: Introduce INSTALL_PREFIX_SCRIPT to easily set up prefixes

[Kevin Funk]

kfunk added inline comments.

INLINE COMMENTS

> sitter wrote in KDEInstallDirs.cmake:699
> From a style perspective, I'd suggest having the prefix.sh live somewhere in 
> the installed ECM tree and get copied, rather than maintained as a glorified 
> heredoc in the cmake code. That's just a suggestion though.

Definitely, +1.

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, sitter
Cc: kfunk, bcooksley, ngraham, sitter, cgiboudeaux, #build_system

D9299: Introduce INSTALL_PREFIX_SCRIPT to easily set up prefixes

[Kevin Funk]

kfunk added a comment.


  If we'd name this file somewhat less generic then it could be even installed 
by default, no?
  
  I had the scheme of  the QNX setup script in my mind: 
https://github.com/acklinr/qnx660/blob/master/qnx660-env.sh
  
  Thus: Maybe rename prefix.sh to say 'ecm-env.sh' and install the file by 
default?

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, sitter
Cc: kfunk, bcooksley, ngraham, sitter, cgiboudeaux, #build_system

D9182: return nullptr -> return {} for QFlags

[Kevin Funk]

kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.


  For the record, see comment https://phabricator.kde.org/D3987#174960

REPOSITORY
  R287 KImageFormats

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

To: mkoller, kfunk, kossebau
Cc: #frameworks

D9118: ki18n cmake macros: Mark UIC-generated .h files to skip AUTOMOC by default

[Kevin Funk]

kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.


  This shouldn't create any troubles with earlier CMake versions.
  
  Also note that we set the same properties unconditionally in newer Qt5 CMake 
macros: https://codereview.qt-project.org/#/c/207327

INLINE COMMENTS

> KF5I18NMacros.cmake:56
>)
> +  set_property(SOURCE ${_header} PROPERTY SKIP_AUTOMOC ON)
>list(APPEND ${_sources} ${_header})

`set_source_files_properties(${_header} ...)` is probably the more canonical 
way to do this.

REPOSITORY
  R249 KI18n

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

To: mpyne, #frameworks, #build_system, #localization, kfunk
Cc: aacid

D3987: Use nullptr in all Frameworks (just diff in KIO shown here)

[Kevin Funk]

kfunk added a comment.


  In https://phabricator.kde.org/D3987#174960, @mkoller wrote:
  
  > I just found out that this change introduced wrong replacements.
  
  
  It did not, the changes are correct, though they're probably not what 
everyone expects.
  
  Please read through the comments in this Diff, this issue has been discussed 
in length already.
  
  tl;dr: Proposed resolution of this "problem": I'd replace `return nullptr` in 
your cases with `return {}` or just leave it as-is.
  
  > The changes in frameworks/kimageformats/src/imageformats like the following 
are incorrect, since the return type is not a pointer but an enum flags type, 
so it should still be 0, not nullptr.
  > 
  > @@ -337,10 +337,10 @@ QImageIOPlugin::Capabilities 
EPSPlugin::capabilities(QIODevice *device, const QB
  > 
  >   return Capabilities(CanRead | CanWrite);
  >   }
  >   if (!format.isEmpty()) {
  > 
  > - return 0; +return nullptr; } if (!device->isOpen()) {
  > - return 0; +return nullptr; }
  > 
  >   Capabilities cap;

REPOSITORY
  R280 Prison

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

To: kfunk, #frameworks, dfaure, kossebau
Cc: mkoller, skelly, kossebau, dfaure, graesslin

D8979: ecm_add_test: Use proper path sep on Windows

[Kevin Funk]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:64915e0291cd: ecm_add_test: Use proper path sep on 
Windows (authored by kfunk).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8979?vs=22863=22869

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

AFFECTED FILES
  modules/ECMAddTests.cmake

To: kfunk, dfaure, cgiboudeaux, bcooksley
Cc: #windows, #frameworks, #build_system

D8979: ecm_add_test: Use proper path sep on Windows

[Kevin Funk]

kfunk added a reviewer: cgiboudeaux.

REPOSITORY
  R240 Extra CMake Modules

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

To: kfunk, dfaure, cgiboudeaux
Cc: #frameworks, #build_system

D8979: ecm_add_test: Use proper path sep on Windows

[Kevin Funk]

kfunk added a reviewer: bcooksley.

REPOSITORY
  R240 Extra CMake Modules

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

To: kfunk, dfaure, cgiboudeaux, bcooksley
Cc: #windows, #frameworks, #build_system

D8979: ecm_add_test: Use proper path sep on Windows

[Kevin Funk]

kfunk added a subscriber: Windows.

REPOSITORY
  R240 Extra CMake Modules

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

To: kfunk, dfaure, cgiboudeaux
Cc: #windows, #frameworks, #build_system

D8979: ecm_add_test: Use proper path sep on Windows

[Kevin Funk]

kfunk created this revision.
kfunk added a reviewer: dfaure.
Restricted Application added projects: Frameworks, Build System.
Restricted Application added subscribers: Build System, Frameworks.

REVISION SUMMARY
  We need to separate paths in environment variables
  with a semincolon (;) on Windows.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  modules/ECMAddTests.cmake

To: kfunk, dfaure
Cc: #frameworks, #build_system

D8976: Remove redundant map look-ups

[Kevin Funk]

kfunk accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R282 NetworkManagerQt

BRANCH
  master

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

To: apol, #frameworks, jgrulich, kfunk

Re: releaseme new requirement: non-conflicting files on case-insensitive FS/OS

[Kevin Funk]

On Tuesday, 21 November 2017 14:24:04 CET Felix Miata wrote:
> Sven Brauch composed on 2017-11-21 12:25 (UTC+0100):
> > Felix Miata wrote:
> >> Case sensitive filesystems are one of the most annoying things about
> >> FOSS.
> > 
> > I'd count that as a win.
> 
> InsaniTY
> INSaniTy
> InSAnItY
> inSANity
> INsANItY
> INSANiTY
> iNSaNiTy
> InsAnItY
> 
> Boss: Worker, please get me the insanity file.
> 
> Worker: which one Boss?
> 
> Boss: The one spelled I N S A N I T Y!
> 
> Worker: There's at least 8 spelled that way!
> 
> Boss: How is that possible?
> 
> Worker: A & a sound the same, B & b sound the same, C & c sound the same, ad
> nauseum. There's no rhyme or reason for the case differences. It's
> anarchistic.

*yawn*.

Really, that discussion is completely moot.

-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

D8923: Offer QWindow API for KJobWidgets:: decorators

[Kevin Funk]

kfunk added inline comments.

INLINE COMMENTS

> apol wrote in kjobwidgets.cpp:67
> Note I didn't change any code, I just moved it around.
> 
> Removing the cast still seems to work, so I'll do that.

> Note I didn't change any code, I just moved it around.

Whoops, sorry. Didn't notice.

REPOSITORY
  R288 KJobWidgets

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

To: apol, #frameworks
Cc: kfunk

D8923: Offer QWindow API for KJobWidgets:: decorators

[Kevin Funk]

kfunk added inline comments.

INLINE COMMENTS

> kjobwidgets.cpp:67
> +if (window) {
> +job->setProperty("window-id", 
> QVariant::fromValue(qptrdiff(window->winId(;
> +}

`WId` is a `quintptr`: `src/gui/kernel/qwindowdefs.h:99:typedef 
QT_PREPEND_NAMESPACE(quintptr) WId;`

Besides: `QVariant::fromValue(window->winId())` should work just fine, or?

REPOSITORY
  R288 KJobWidgets

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

To: apol, #frameworks
Cc: kfunk

Re: releaseme new requirement: non-conflicting files on case-insensitive FS/OS

[Kevin Funk]

On Monday, 20 November 2017 16:50:58 CET Harald Sitter wrote:
> Guten Abend Everyone!
> 
> To aid with use of our tarballs on Windows and case-insensitive file
> systems, releaseme now makes sure [1] that the tarballs it generates
> are safe to use in such contexts.

Thanks for helping out on that front! 

Regards,
Kevin

> As this is a major problem for pretty much every source tarball as it
> prevents people from doing Windows builds and/or unpack on exfat/fat32
> this is considered a fatal flaw in the tarball. To that end if you use
> releaseme you may want to run tarme now so your next release doesn't
> get held up by case-insensitive file conflicts.
> 
> If you have any questions please do ask.
> 
> [1]
> https://phabricator.kde.org/R572:0936ab0e02551fe6d3e8ad07b97cd7d7d3d26838
> 
> HS


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: How is symbol visibility set in KF5 and KDE?

[Kevin Funk]

On Wednesday, 15 November 2017 15:54:17 CET Shaheed Haque wrote:
> Hi all,
> 
> I just realised that the Python binding effort is not setting the default
> visibility for symbols using the -fvisibility=xxx option when processing
> the header files [1]. Of course I can see the export macros set by the
> likes of attica_exports.h, but I don't see where the compiler default is
> set. Can somebody kindly point that out?

It's configured in extra-cmake-modules.git:

kde-modules/KDECompilerSettings.cmake
223:set(CMAKE_C_VISIBILITY_PRESET hidden)
224:set(CMAKE_CXX_VISIBILITY_PRESET hidden)
225:set(CMAKE_VISIBILITY_INLINES_HIDDEN 1)

Regards,
Kevin

> Thanks, Shaheed
> 
> [1] I'm also a bit mystified by the fact that I am deliberately querying
> CMake for the COMPILE_FLAGS to use, but I have not seen -fvisibility
> anywhere...


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

Re: How is symbol visibility set in KF5 and KDE?

[Kevin Funk]

On Wednesday, 15 November 2017 15:54:17 CET Shaheed Haque wrote:
> Hi all,
> 
> I just realised that the Python binding effort is not setting the default
> visibility for symbols using the -fvisibility=xxx option when processing
> the header files [1]. Of course I can see the export macros set by the
> likes of attica_exports.h, but I don't see where the compiler default is
> set. Can somebody kindly point that out?

It's configured in extra-cmake-modules.git:

kde-modules/KDECompilerSettings.cmake
223:set(CMAKE_C_VISIBILITY_PRESET hidden)
224:set(CMAKE_CXX_VISIBILITY_PRESET hidden)
225:set(CMAKE_VISIBILITY_INLINES_HIDDEN 1)

Regards,
Kevin

> Thanks, Shaheed
> 
> [1] I'm also a bit mystified by the fact that I am deliberately querying
> CMake for the COMPILE_FLAGS to use, but I have not seen -fvisibility
> anywhere...


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

D8810: Do not look for kioslave binary in applicationDirPath (#386859)

[Kevin Funk]

kfunk requested changes to this revision.
kfunk added a comment.
This revision now requires changes to proceed.


  Just chiming in here, without having a final solution for your problem: This 
search path has been added for a reason -- to find a kioslave.exe next to the 
application binary in application bundles on macOS/Windows.
  
  See:
  https://git.reviewboard.kde.org/r/125778/
  
  CC'ing Christoph.

REPOSITORY
  R241 KIO

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

To: kkofler, #frameworks, kfunk
Cc: kfunk

D8810: Do not look for kioslave binary in applicationDirPath (#386859)

[Kevin Funk]

kfunk added a reviewer: cullmann.

REPOSITORY
  R241 KIO

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

To: kkofler, #frameworks, kfunk, cullmann
Cc: kfunk

Re: liquidshell in kdereview

[Kevin Funk]

On Monday, 6 November 2017 10:03:05 CET Tomaz Canabrava wrote:
> On Mon, Nov 6, 2017 at 8:31 AM, Martin Koller <kol...@aon.at> wrote:
> > On Montag, 6. November 2017 01:57:32 CET Aleix Pol wrote:
> > > On Fri, Nov 3, 2017 at 9:30 PM, Martin Koller <kol...@aon.at> wrote:
> > > > Hi all,
> > > > 
> > > > I'd like to announce an application I've implemented over the last few
> > 
> > weeks - liquidshell
> > 
> > > > liquidshell is a replacement for plasmashell
> > > 
> > > Hi Martin,
> > > I'm a bit confused, who is liquidshell for?
> > 
> > Whoever does not like plasmashell, for whatever reason.
> > My reasons are mentioned in the README or the announcement mail.
> > Free software is about choice, no ?
> 
> It is, of course. Then, it's also about non-fragmentation, and you know,
> human resources are spare.
> I know it's your time and your project and I will never tell you what to
> do,
> but considering that there's lxqt which  exists basically for the same
> 'whatever reasons', why not help them instead of creating yet another
> desktop shell?

So much +1.

I'm really sad to see more fragmentation in the DE space instead of finding 
people investing their time helping existing projects, even more so if there's 
one aiming for the same goal under the KDE umbrella.

You're free to work on whatever you like to of course, but to me this sounds 
like wasted effort. Your good incentives would be better spent with joining up 
with others aiming for the same (LxQt for instance).

Hate to be the party pooper here, but I'm just not sure this kind of 
fragmentation helps KDE in the long-term, where we really have a hard time 
finding contributors for the majority of our *existing* projects.

Regards,
Kevin

> > Best regards/Schöne Grüße
> > 
> > Martin
> > A: Because it breaks the logical sequence of discussion
> > Q: Why is top posting bad?
> > 
> > ()  ascii ribbon campaign - against html e-mail
> > /\- against proprietary attachments
> > 
> > Geschenkideen, Accessoires, Seifen, Kulinarisches: www.lillehus.at


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.

D8577: Fix: Missing dependencies for ktexeditor autotests

[Kevin Funk]

kfunk added a comment.


  In https://phabricator.kde.org/D8577#164815, @cullmann wrote:
  
  > Could you just remove the problematic includes and try to compile without 
the dependency, IMHO I see no use of QtScript stuff.
  
  
  Done with: 
https://phabricator.kde.org/R39:631b1447c97f1d81de2e81d2556474f0bc3109dd

REPOSITORY
  R39 KTextEditor

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

To: hgoebel, #ktexteditor, dfaure
Cc: kfunk, cullmann, dhaumann, #frameworks, kevinapavew, demsking, sars

D8544: KTextEditor : avoiding QML crashes

[Kevin Funk]

kfunk added inline comments.

INLINE COMMENTS

> kateglobal.cpp:104
> +// disable the QML JIT compiler as a protection against an unknown bug
> +// in Qt's V4 engine which can provoke a crash in certain of our scripts.
> +qputenv("QV4_FORCE_INTERPRETER", QByteArrayLiteral("1"));

Please add a reference to the bug reports (Qt JIRA, KDE Bugzilla) here.

> kateglobal.cpp:106
> +qputenv("QV4_FORCE_INTERPRETER", QByteArrayLiteral("1"));
> +qCDebug(LOG_KTE) << "QV4_FORCE_INTERPRETER set to" << 
> qgetenv("QV4_FORCE_INTERPRETER");
> +#endif

Minor: "... set to 1" would be sufficient. You're setting it yourself right 
before, so you can presume it is set (=> no need for `qgetenv(...)`.

REPOSITORY
  R39 KTextEditor

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

To: rjvbb, #frameworks
Cc: kfunk, dhaumann, cullmann, kde-frameworks-devel, kevinapavew, demsking, 
head7, sars

D8437: KWidgetsAddons : more compact password dialog

[Kevin Funk]

kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.


  Looks like it :)

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, #frameworks, kfunk, ngraham
Cc: cfeck, ngraham

D8437: KWidgetsAddons : more compact password dialog

[Kevin Funk]

kfunk requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, #frameworks, kfunk
Cc: cfeck, ngraham

D7401: Solid/Mac : fleshing out the skeleton IOKit backend (WIP)

[Kevin Funk]

kfunk requested changes to this revision.
kfunk added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> iokitdevice.cpp:307
> +default:
> +vendor = QString();
> +break;

Why not `return` here as well? For consistency.

You can end the function with

  Q_UNREACHABLE();
  return {};

Very common pattern.

> iokitdevice.cpp:463
>  {
> -return (type == Solid::DeviceInterface::GenericInterface
> -|| type == d->type);
> +bool ret = false;
> +switch (type) {

Returning inside the case statements would make this code clearer as well.

> iokitdeviceinterface.h:51
>  IOKitDevice *m_device;
> +IOKitDevice *copy;
>  };

`m_` prefix missing. Would call it `m_deviceCopy`.

> rjvbb wrote in iokitstorage.cpp:36
> Did you check that these are indeed pointers? ;)

Yes. And they are, no?

> rjvbb wrote in iokitstorage.cpp:75
> I don't get it, sorry, can you explain in more words how you'd want to see 
> this changed? If I remove this extra ctor, I can no longer call 
> `IOKitStorage(this).vendor()` in `IOKitDevice::vendor()` without extra code 
> that's also going to add noise.
> 
> I get a warning when I remove the const attribute from 
> `IOKitDevice::vendor()`, which suggests that I'd no longer be reimplementing 
> a virtual method but adding a method instead.
> 
> All these "extra" ctors hand off the pointer to a "const this" to 
> `DeviceInterface` which finally makes a deep copy. I find that cleaner than 
> creating a deep copy of `this` everywhere needed and ensuring it gets 
> deallocated (even via QPointers).
> Unusual doesn't mean wrong (we're on Mac here, after all ^^)

Okay, well, leave it like that.

I'm running out of time to properly explain how I'd envision this code to be 
like in a perfect world.

> iokitstorage.cpp:73
> +
> +const IOKitDevice *m_device;
> +DASessionRef daSession;

Inconsistent member naming. Some with `m_` prefix, some without. Choose one 
style. Private classes' members usually live without the `m_` prefix, but we 
don't mind them being around (in KDE land)

> iokitstorage.h:43
> +
> +const QString vendor();
> +const QString product();

Remove `const` from return type, but make the method `const`.

> iokitstorageaccess.cpp:56
> +
> +const IOKitDevice *m_device;
> +DASessionRef daSession;

Dito, inconsistent member naming.

> iokitvolume.cpp:70
> +
> +const IOKitDevice *m_device;
> +DASessionRef daSession;

Dito, inconsistent member naming.

And given that I've noticed that three times now, this again leads to the 
conclusion this is very repetitive code amongst . 
`{IOKitStorageAccess,IOKitVolume,IOKitStorage}::Private`.

Maybe there should be helper class for accessing a `CFDictionary` instead which 
all these classes use?

I'm not trying to piss you off René, but this is slightly sloppy coding which 
easy for the initial writer to do, but will bite us any time in the future when 
someone unfamiliar with the code needs to do fixes to this code and potentially 
fixes only one copy of these snippets. Please think about your architecture 
more carefully.

> iokitvolume.h:45
> +
> +virtual bool isIgnored() const Q_DECL_OVERRIDE;
> +virtual Solid::StorageVolume::UsageType usage() const Q_DECL_OVERRIDE;

No `virtual` needed if there's already a `Q_DECL_OVERRIDE` or `override`.

REPOSITORY
  R245 Solid

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

To: rjvbb, #frameworks, kfunk
Cc: kfunk, anthonyfieroni, cgilles, kde-mac

D7401: Solid/Mac : fleshing out the skeleton IOKit backend (WIP)

[Kevin Funk]

kfunk added inline comments.

INLINE COMMENTS

> iokitopticaldrive.cpp:27
> +
> +#ifdef EJECT_USING_DISKARBITRATION
> +// for QCFType:

Where's that defined via the build system?

REPOSITORY
  R245 Solid

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

To: rjvbb, #frameworks, kfunk
Cc: kfunk, anthonyfieroni, cgilles, kde-mac

D7401: Solid/Mac : fleshing out the skeleton IOKit backend (WIP)

[Kevin Funk]

kfunk requested changes to this revision.
kfunk added a comment.
This revision now requires changes to proceed.


  This definitely needs another revision. Please fix the outstanding issues.

INLINE COMMENTS

> iokitblock.h:43
> +
> +virtual int deviceMajor() const;
> +virtual int deviceMinor() const;

Here and below: Missing `Q_DECL_OVERRIDE`

> iokitdevice.cpp:151
> +size_t size = 0;
> +if (sysctlbyname("hw.model", NULL, , NULL, 0) == 0 && size > 0) {
> +model = new char [size];

Use `nullptr` everywhere

> iokitdevice.cpp:154
> +if (sysctlbyname("hw.model", model, , NULL, 0) == 0) {
> +qModel= QLatin1String(model);
> +}

Style: Use `qModel = ...`

> iokitdevice.cpp:156
> +}
> +delete model;
> +}

`new`/`delete` mismatch. Use `delete[]`

> iokitdevice.cpp:167
> +{
> +type << Solid::DeviceInterface::Unknown;
> +}

Initialize at class member decl

> iokitdevice.cpp:171
> +{
> +if (parent) {
> +delete parent;

That's *very* odd style. Why does a private class delete its public 
counterpart? I've never seen this.

> iokitdevice.cpp:197
> +type = typesFromEntry(entry, properties, mainType);
> +// if (udi.contains(QStringLiteral("IOBD")) || 
> udi.contains(QStringLiteral("BD PX"))) {
> +// qWarning() << "Solid: BlueRay entry" << entry << "mainType=" << 
> mainType << "typeList:" << type

Don't leave commented code around. Either enable this code paths properly via 
categorized logging or remove it altogether.

> iokitopticaldrive.cpp:211
> +}
> +delete ioDVDServices;
> +d = new Private(device, devCharMap);

Create on `ioDVDServices` on the stack to begin with?

> iokitopticaldrive.cpp:333
> +{
> +QList speeds;
> +return speeds;

`return {}`

> iokitopticaldrive.cpp:354
> +return true;
> +} else {
> +emit ejectDone(Solid::ErrorType::OperationFailed, QVariant(), 
> m_device->udi());

Coding style: Would turn around this two branches (and remove the else part) to 
make the intended meaning more clear:

Pseudo code:

  if (errorCase) {
return ...;
  }
  
  // normal case, code flow continues
  return ...;

That's usually the common style

> iokitprocessor.cpp:84
> +
> +const QString Processor::vendor()
> +{

Remove `const`

> iokitprocessor.cpp:87
> +QString qVendor = QString();
> +char *vendor = NULL;
> +size_t size = 0;

Way too much error prone `new`/`delete` logic on naked char arrays. Every 
invocation here is wrong (causing undefined behavior) due to the 
`new[]`/delete` mismatch mentioned above.

Factor that out reading into a `QString` into a separate function, cf. 
https://github.com/trueos/lumina/blob/master/src-qt5/core/libLumina/LuminaOS-DragonFly.cpp#L29
 and use it everywhere instead.

> iokitprocessor.cpp:99
>  
> +const QString Processor::product()
> +{

Remove `const`

> iokitstorage.cpp:36
> +{
> +daRef = 0;
> +daDict = 0;

Here and one line below: Could be initialized at decl again.

> iokitstorage.cpp:36
> +{
> +daRef = 0;
> +daDict = 0;

`nullptr`

> iokitstorage.cpp:51
> +CFRelease(daDict);
> +daDict = 0;
> +}

`nullptr`, etc. pp.

> iokitstorage.cpp:75
> +
> +IOKitStorage::IOKitStorage(const IOKitDevice *device)
> +: Block(device)

Please just remove the constructors taking a `const IOKitDevice *device` and 
adapt uses (just use `IOKitDevice *device` everywhere). It's unusual for 
pointer types to be `const` in such contexts. It just adds noise.

> iokitstorageaccess.cpp:41
> +} else {
> +daRef = 0;
> +}

Here and below: `nullptr`

> iokitstorageaccess.cpp:91
> +{
> +// mount points can change (but can they between invocations of 
> filePath()?)
> +if (d->daRef) {

Early-return would reduce the indentation level here [1].

  if (errorCase)
// return early
return {};
  }
  
  // normal case
  // ...

[1] 
https://softwareengineering.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement

> iokitstorageaccess.cpp:132
> +return false;
> +} else {
> +// TODO?

Coding style: Eliminate `else` branch, just `return in function-level scope.

> iokitstorageaccess.cpp:142
> +return false;
> +} else {
> +// TODO?

Dito

> iokitvolume.cpp:42
> +} else {
> +daRef = 0;
> +}

Here and below: `nullptr`

> iokitvolume.cpp:121
> +if (dict) {
> +// qWarning() << Q_FUNC_INFO;
> +// CFShow(dict);

Remove commented code.

> iokitvolume.cpp:177
> +if (d->daRef) {
> +CFDictionaryRef dict = DADiskCopyDescription(d->daRef);
> +if (dict) {

That's a lot of copy-pasted code amongst `fsType`, `label`, `vendor`, `product, 
`description`. I'm sure that can be done better.

> iokitvolume.h:45
> +
> +virtual bool isIgnored() const;
> +virtual