Warning: KPluginInfo::property(X-KDE-PluginInfo-Name) is deprecated

2015-02-21 Thread Marco Martin 
Hi all,
As you may have noticed, right now starting plasma is a big spam of
the following error:
Calling KPluginInfo::property(X-KDE-PluginInfo-Name) is deprecated,
use KPluginInfo::pluginName() in /whatever/plugin.so instead.

i tried to see where it happens, and seems it's in
ktradeparsetree.cpp , line 30
QVariant ParseContext::property(const QString _key) const

I don't think this properly fixable, since from the stack trace it
seems an appropriate use.. i see two ways to fix it:

1) in ParseContext::property stuf a very long if.. else.. that makes
it call the proper KPluginInfo::correctAccessor() .. but is ugly and
slows it down

2) since this is an appropriate use, consider it not wrong anymore,
and just get rid of the warning.

Opinions? ideas?

--
Marco Martin

Re: Warning: KPluginInfo::property(X-KDE-PluginInfo-Name) is deprecated

2015-02-21 Thread Sebastian Kügler 
On Saturday, February 21, 2015 11:02:48 Marco Martin wrote:
 As you may have noticed, right now starting plasma is a big spam of
 the following error:
 Calling KPluginInfo::property(X-KDE-PluginInfo-Name) is deprecated,
 use KPluginInfo::pluginName() in /whatever/plugin.so instead.
 
 i tried to see where it happens, and seems it's in
 ktradeparsetree.cpp , line 30
 QVariant ParseContext::property(const QString _key) const
 
 I don't think this properly fixable, since from the stack trace it
 seems an appropriate use.. i see two ways to fix it:
 
 1) in ParseContext::property stuf a very long if.. else.. that makes
 it call the proper KPluginInfo::correctAccessor() .. but is ugly and
 slows it down
 
 2) since this is an appropriate use, consider it not wrong anymore,
 and just get rid of the warning.
 
 Opinions? ideas?

I've looked at it, too, since the warnings shown are just burying any useful 
information during the startup of Plasmashell. I think the warning is rather 
useless, and I'd just remove it, so 2).

Cheers,
-- 
sebas

Sebastian Kügler|http://vizZzion.org| http://kde.org

Re: Warning: KPluginInfo::property(X-KDE-PluginInfo-Name) is deprecated

2015-02-21 Thread Marco Martin 
On Sat, Feb 21, 2015 at 1:34 PM, Alexander Richardson
arichardson@gmail.com wrote:
 and then we could also have something like
 KServiceTypeTrader::findPlugin(serviceType, name) that expands to
 KServiceTypeTrader::self()-query(serviceType, [](const  KPluginInfo info) {
  return info.pluginName() == name;
 }

 I have been meaning to add this for quite a while, but I am really
 busy at the moment.

So we would maintain such a warning, but start to port users to that
new api instead?

--
Marco Martin

Re: Warning: KPluginInfo::property(X-KDE-PluginInfo-Name) is deprecated

2015-02-21 Thread Marco Martin 
On Sat, Feb 21, 2015 at 3:43 PM, Marco Martin notm...@gmail.com wrote:
 On Sat, Feb 21, 2015 at 1:34 PM, Alexander Richardson
 arichardson@gmail.com wrote:
 and then we could also have something like
 KServiceTypeTrader::findPlugin(serviceType, name) that expands to
 KServiceTypeTrader::self()-query(serviceType, [](const  KPluginInfo info) {
  return info.pluginName() == name;
 }

anyways, absolutely +1 on this new function, I like the idea a lot :)

--
Marco Martin

Re: Warning: KPluginInfo::property(X-KDE-PluginInfo-Name) is deprecated

2015-02-21 Thread Sebastian Kügler 
On Saturday, February 21, 2015 17:19:02 Alexander Richardson wrote:
 2015-02-21 14:43 GMT+00:00 Marco Martin notm...@gmail.com:
  On Sat, Feb 21, 2015 at 1:34 PM, Alexander Richardson
  
  arichardson@gmail.com wrote:
  and then we could also have something like
  KServiceTypeTrader::findPlugin(serviceType, name) that expands to
  KServiceTypeTrader::self()-query(serviceType, [](const  KPluginInfo
  info) {
   return info.pluginName() == name;
 
  }
  
  I have been meaning to add this for quite a while, but I am really
  busy at the moment.
  
  So we would maintain such a warning, but start to port users to that
  new api instead?
 
 Yes, that would be my suggestion. I won't be able to work on it before
 Thursday though. Ideally there shouldn't be many changes required to
 make it work for KMimeTypeTrader aswell, so then in KF5 we can get rid
 of all the generated parsing code.

That's completely fine. We're not too much in a hurry, it's not a showstopper 
by any means, just some janitorial work to make Plasma a bit easier to debug. 
Currently, about 2/3 of its console output consists of these messages.

Thanks, Alex!
-- 
sebas

Sebastian Kügler|http://vizZzion.org| http://kde.org

Re: Warning: KPluginInfo::property(X-KDE-PluginInfo-Name) is deprecated

2015-02-21 Thread Alexander Richardson 
2015-02-21 10:02 GMT+00:00 Marco Martin notm...@gmail.com:
 Hi all,
 As you may have noticed, right now starting plasma is a big spam of
 the following error:
 Calling KPluginInfo::property(X-KDE-PluginInfo-Name) is deprecated,
 use KPluginInfo::pluginName() in /whatever/plugin.so instead.

 i tried to see where it happens, and seems it's in
 ktradeparsetree.cpp , line 30
 QVariant ParseContext::property(const QString _key) const

 I don't think this properly fixable, since from the stack trace it
 seems an appropriate use.. i see two ways to fix it:

 1) in ParseContext::property stuf a very long if.. else.. that makes
 it call the proper KPluginInfo::correctAccessor() .. but is ugly and
 slows it down

 2) since this is an appropriate use, consider it not wrong anymore,
 and just get rid of the warning.

 Opinions? ideas?
I guess most of these would result from a call to
KServiceTypeTrader::self()-query(KMyApp/Plugin,
[X-KDE-PluginInfo-Name] = Foo).
My suggestion would be to add an overload to
KServiceTypeTrader::query() that takes a std::functionbool(const
KPluginInfo) instead of the constraints string.
Ideally we could then deprecate the string based version and use the
std::function version everywhere since that should be safer (and
faster).

KServiceTypeTrader::self()-query(KMyApp/Plugin, [](const
KPluginInfo info) {
 return info.property(X-KMyApp-InterfaceVersion).toInt()  15;
}
instead of
KServiceTypeTrader::self()-query(KMyApp/Plugin,
[X-KMyApp-InterfaceVersion]  15);

and then we could also have something like
KServiceTypeTrader::findPlugin(serviceType, name) that expands to
KServiceTypeTrader::self()-query(serviceType, [](const  KPluginInfo info) {
 return info.pluginName() == name;
}

I have been meaning to add this for quite a while, but I am really
busy at the moment.

Alex

Re: Warning: KPluginInfo::property(X-KDE-PluginInfo-Name) is deprecated

2015-02-21 Thread Alexander Richardson 
2015-02-21 14:43 GMT+00:00 Marco Martin notm...@gmail.com:
 On Sat, Feb 21, 2015 at 1:34 PM, Alexander Richardson
 arichardson@gmail.com wrote:
 and then we could also have something like
 KServiceTypeTrader::findPlugin(serviceType, name) that expands to
 KServiceTypeTrader::self()-query(serviceType, [](const  KPluginInfo info) {
  return info.pluginName() == name;
 }

 I have been meaning to add this for quite a while, but I am really
 busy at the moment.

 So we would maintain such a warning, but start to port users to that
 new api instead?

Yes, that would be my suggestion. I won't be able to work on it before
Thursday though. Ideally there shouldn't be many changes required to
make it work for KMimeTypeTrader aswell, so then in KF5 we can get rid
of all the generated parsing code.

Alex