Re: Review Request 121094: KCoreAddons: add KTextToHTML class for plaintext -> HTML conversion

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121094/#review71266
---

Ship it!


Good by me :) Two small notes below, but not commit blockers imo.


src/lib/text/ktexttohtml_p.h
<https://git.reviewboard.kde.org/r/121094/#comment49767>

Comment is not accurate/relevant anymore



src/lib/text/ktexttohtml_p.h
<https://git.reviewboard.kde.org/r/121094/#comment49766>

Comment is not accurate/relevant anymore


- Aaron J. Seigo


On Dec. 2, 2014, 4:57 p.m., Daniel Vrátil wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121094/
> ---
> 
> (Updated Dec. 2, 2014, 4:57 p.m.)
> 
> 
> Review request for KDE Frameworks and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> This class comes from KPimUtils::LinkLocator from kdepimlibs.git. This class 
> takes plaintext string, and converts it to HTML while perserving spacing, 
> converting URLs and email addresses into  links and interpreting 
> highligting markup (*bold*, _underline_, /italic/) and wrapping it to 
> corresponding HTML tags. It can also convert text emoticons into  
> smileys.
> 
> The last - emoticons conversion - depends on KEmoticons, which we can't use 
> directly from KCoreAddons, so I moved the actual KEmoticons code into 
> KEmoticonsIntegration plugin in KEmoticons. There is a dummy implementation 
> provided by default in KCoreAddons that is loaded when the 
> KEmoticonsIntegration plugin is not available, or when the application is 
> only a QCoreApplication (since KEmoticons require QGuiApplication, so we must 
> not load the plugin). The dummy implementation does nothing and returns the 
> original string unaltered.
> 
> The reason for moving the class from KPimUtils is that KPimUtils is going to 
> die, and while we could make this internal to KDE PIM, I think this is a 
> rather useful code, that can be used by many applications that need to deal 
> with 3rd party text data and want to provide user-friendly clickable links.
> 
> If you compare this with the original KPimUtils::LinkLocator you find, that I 
> removed all public API except for the convertToHTML method. This is because 
> the remaining methods (getUrl, getEmailAddress) depend on the actual state of 
> the LinkLocator object, but LinkLocator provides no API for step-by-step 
> parsing, so they were actually useless for public use. For that reason I 
> decided to only publish the convertToHTML method, and hide the remaining 
> methods as internal methods of the KTextToHTMLHelper (useful for 
> unit-testing).
> 
> This patch also contains a unit-test from KPimUtils.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt d5a5feb 
>   autotests/ktexttohtmltest.h PRE-CREATION 
>   autotests/ktexttohtmltest.cpp PRE-CREATION 
>   src/lib/CMakeLists.txt 3e055a5 
>   src/lib/CMakeLists.txt.orig PRE-CREATION 
>   src/lib/text/.ktexttohtml.cpp.kate-swp PRE-CREATION 
>   src/lib/text/ktexttohtml.h PRE-CREATION 
>   src/lib/text/ktexttohtml.cpp PRE-CREATION 
>   src/lib/text/ktexttohtml_p.h PRE-CREATION 
>   src/lib/text/ktexttohtmlemoticonsinterface.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/121094/diff/
> 
> 
> Testing
> ---
> 
> Unit test passes
> 
> 
> Thanks,
> 
> Daniel Vrátil
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 121118: KEmoticons: Add a KEmoticonsIntegrationPlugin for KTextToHTML from KCoreAddons

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121118/#review71148
---

Ship it!


Depending on the addition of 121094 first ...

- Aaron J. Seigo


On Nov. 25, 2014, 9:39 a.m., Daniel Vrátil wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121118/
> ---
> 
> (Updated Nov. 25, 2014, 9:39 a.m.)
> 
> 
> Review request for KDE Frameworks, Albert Astals Cid and David Faure.
> 
> 
> Repository: kemoticons
> 
> 
> Description
> ---
> 
> This patch is related to /r/121094, which moves KTextToHTML conversion 
> utility from KPimUtils to KCoreAddons. Since KCoreAddons can't depend on 
> KEmoticons needed for smileys conversion, I created a plugin in KEmoticons 
> that implements the interface from KCoreAddons. This is based on the 
> FrameworkIntegrationPlugin.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 1550bfe 
>   autotests/CMakeLists.txt 29b7eb6 
>   autotests/ktexttohtmlplugintest.cpp PRE-CREATION 
>   src/CMakeLists.txt 7b10087 
>   src/integrationplugin/CMakeLists.txt PRE-CREATION 
>   src/integrationplugin/kemoticonsintegrationplugin.h PRE-CREATION 
>   src/integrationplugin/kemoticonsintegrationplugin.cpp PRE-CREATION 
>   src/integrationplugin/ktexttohtml.h PRE-CREATION 
>   src/integrationplugin/ktexttohtml.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/121118/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Vrátil
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 121094: KCoreAddons: add KTextToHTML class for plaintext -> HTML conversion

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121094/#review71147
---



src/lib/text/ktexttohtml_p.h
<https://git.reviewboard.kde.org/r/121094/#comment49706>

this does not seem to be used by the unit tests, so does not need to be 
exported?



src/lib/text/ktexttohtml_p.h
<https://git.reviewboard.kde.org/r/121094/#comment49707>

would it be possible to just pull this file directly into the unit test 
that uses it rather than export the symbol?

every user of the library pays a startup cost for ever exported symbol, and 
it also increases the interface surface of the library even if these classes 
are theoretically "private"


- Aaron J. Seigo


On Nov. 18, 2014, 1:37 p.m., Daniel Vrátil wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121094/
> ---
> 
> (Updated Nov. 18, 2014, 1:37 p.m.)
> 
> 
> Review request for KDE Frameworks and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> This class comes from KPimUtils::LinkLocator from kdepimlibs.git. This class 
> takes plaintext string, and converts it to HTML while perserving spacing, 
> converting URLs and email addresses into  links and interpreting 
> highligting markup (*bold*, _underline_, /italic/) and wrapping it to 
> corresponding HTML tags. It can also convert text emoticons into  
> smileys.
> 
> The last - emoticons conversion - depends on KEmoticons, which we can't use 
> directly from KCoreAddons, so I moved the actual KEmoticons code into 
> FrameworkIntegration plugin. There is a dummy implementation provided by 
> default in KCoreAddons that is loaded when the FrameworkIntegration plugin is 
> not available, or when the application is only a QCoreApplication (since 
> KEmoticons require QGuiApplication, so we must not load the plugin). The 
> dummy implementation does nothing and returns the original string unaltered.
> 
> The reason for moving the class from KPimUtils is that KPimUtils is going to 
> die, and while we could make this internal to KDE PIM, I think this is a 
> rather useful code, that can be used by many applications that need to deal 
> with 3rd party text data and want to provide user-friendly clickable links.
> 
> If you compare this with the original KPimUtils::LinkLocator you find, that I 
> removed all public API except for the convertToHTML method. This is because 
> the remaining methods (getUrl, getEmailAddress) depend on the actual state of 
> the LinkLocator object, but LinkLocator provides no API for step-by-step 
> parsing, so they were actually useless for public use. For that reason I 
> decided to only publish the convertToHTML method, and hide the remaining 
> methods as internal methods of the KTextToHTMLHelper (useful for 
> unit-testing).
> 
> This patch also contains a unit-test from KPimUtils.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt d5a5feb 
>   autotests/ktexttohtmltest.h PRE-CREATION 
>   autotests/ktexttohtmltest.cpp PRE-CREATION 
>   src/lib/CMakeLists.txt 1dc5627 
>   src/lib/text/ktexttohtml.h PRE-CREATION 
>   src/lib/text/ktexttohtml.cpp PRE-CREATION 
>   src/lib/text/ktexttohtml_p.h PRE-CREATION 
>   src/lib/text/ktexttohtmlemoticonsinterface.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/121094/diff/
> 
> 
> Testing
> ---
> 
> Unit test passes
> 
> 
> Thanks,
> 
> Daniel Vrátil
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120283: make KConfigLoader more predictable by exposing a getter to KConfig's OpenFlags

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120283/
---

(Updated Sept. 19, 2014, 2:21 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Matthew Dawson.


Repository: kconfig


Description
---

When opening a KSharedConfig, the OpenFlags must match to share the underlying 
config object. Otherwise a new KConfig is created and the benefit of using 
KSharedConfig is lost. This was triggering subtle bugs with the use of 
KConfigLoader in plasmashell.

This patchset adddress that issue by exposing the OpenFlags used to open a 
KConfig.

Another approach would be to add another KSharedConfig::openConfig() which 
takes a KConfig object and uses that in its search for a matching pointer 
object, and then use that version of openConfig in KSharedConfig


Diffs
-

  src/core/kconfig.h d7d4b7df8f33e33a389527d47d02fc844b74aed3 
  src/core/kconfig.cpp c6dec43641611bf14c5eb04d3007dd9016bcb3c8 
  src/gui/kconfigloader.cpp 52ac6d17b427a6eb8519699f642759b7ae672b81 

Diff: https://git.reviewboard.kde.org/r/120283/diff/


Testing
---

Used KConfigLoader in startup config of plasmashell and settings are now 
propagated correctly from the shell scripting to the config loader objects.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 120283: make KConfigLoader more predictable by exposing a getter to KConfig's OpenFlags

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120283/
---

Review request for KDE Frameworks and Matthew Dawson.


Repository: kconfig


Description
---

When opening a KSharedConfig, the OpenFlags must match to share the underlying 
config object. Otherwise a new KConfig is created and the benefit of using 
KSharedConfig is lost. This was triggering subtle bugs with the use of 
KConfigLoader in plasmashell.

This patchset adddress that issue by exposing the OpenFlags used to open a 
KConfig.

Another approach would be to add another KSharedConfig::openConfig() which 
takes a KConfig object and uses that in its search for a matching pointer 
object, and then use that version of openConfig if KSharedConfig


Diffs
-

  src/core/kconfig.h d7d4b7df8f33e33a389527d47d02fc844b74aed3 
  src/core/kconfig.cpp c6dec43641611bf14c5eb04d3007dd9016bcb3c8 
  src/gui/kconfigloader.cpp 52ac6d17b427a6eb8519699f642759b7ae672b81 

Diff: https://git.reviewboard.kde.org/r/120283/diff/


Testing
---

Used KConfigLoader in startup config of plasmashell and settings are now 
propagated correctly from the shell scripting to the config loader objects.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120283: make KConfigLoader more predictable by exposing a getter to KConfig's OpenFlags

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120283/
---

(Updated Sept. 19, 2014, 11:51 a.m.)


Review request for KDE Frameworks and Matthew Dawson.


Repository: kconfig


Description (updated)
---

When opening a KSharedConfig, the OpenFlags must match to share the underlying 
config object. Otherwise a new KConfig is created and the benefit of using 
KSharedConfig is lost. This was triggering subtle bugs with the use of 
KConfigLoader in plasmashell.

This patchset adddress that issue by exposing the OpenFlags used to open a 
KConfig.

Another approach would be to add another KSharedConfig::openConfig() which 
takes a KConfig object and uses that in its search for a matching pointer 
object, and then use that version of openConfig in KSharedConfig


Diffs
-

  src/core/kconfig.h d7d4b7df8f33e33a389527d47d02fc844b74aed3 
  src/core/kconfig.cpp c6dec43641611bf14c5eb04d3007dd9016bcb3c8 
  src/gui/kconfigloader.cpp 52ac6d17b427a6eb8519699f642759b7ae672b81 

Diff: https://git.reviewboard.kde.org/r/120283/diff/


Testing
---

Used KConfigLoader in startup config of plasmashell and settings are now 
propagated correctly from the shell scripting to the config loader objects.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120029: introduce the concept of package fallback

[Aaron J. Seigo]

> On Sept. 1, 2014, 4:53 p.m., David Edmundson wrote:
> > I need some the concept explaining, why would a developer set a 
> > fallbackpackage?
> > Is it not always org.kde.breeze.desktop?
> 
> Marco Martin wrote:
> It would usually be the packagestructure setting it, in initPackage(), so 
> setFallbackPackage follows the same pattern as addFileDefinition(), 
> setRequired() and the likes.
> 
> so, for the packages of type Plasma/LookAndFeel it would be 
> org.kde.breeze.desktop
> for packages of types Plasma/Shell, it would be org.kde.desktop
> 
> even tough at some point it will probably have to be something more 
> complicated, like the default lookandfeel one depending from  the current 
> shell, if ever needed, will be feasible.
> 
> David Edmundson wrote:
> Ah, makes sense. Thanks.
> 
> Do you really expect a linked link big enough to cycle? I can't really 
> see it going 3 deep. The cycle check is call cool though :)
> 
> Could you add a warning in setFallbackPackage() if it finds a cycle and 
> no-ops. Could be confusing to debug otherwise.
> 
> Marco Martin wrote:
> added warning..
> i see it very unlikely that it's going to go deep, but better safe than 
> sorry :p
> 
> Aaron J. Seigo wrote:
> "Is it not always org.kde.breeze.desktop?"
> 
> No, because of third party developers. Not every project targets the 
> desktop.
> 
> That said, this applies to any use of Plasma::Package, which is meant to 
> be a generic concept not something just for the limited set of package types 
> plasmashell uses. It is helpful to think of it as a concept on its own rather 
> than in the context of Plasma Desktop. Other Frameworks using applications 
> really ought to be able to use Plasma::Package for their "bundles of data" 
> needs.
> 
> "It would usually be the packagestructure setting it, in initPackage()"
> 
> In the case of Shell and Look & Feel packages I would caution against 
> doing this, as it biases them to Plasma Desktop. The tablet interface in 
> Plasma Active will need its own "root" L&F; falling back to components from 
> breeze.desktop will likely be both an oversight and create undesirable 
> results. It also would mean that every time someone touches a component in 
> breeze.desktop that is used by the tablet UX, they will be required to ensure 
> it remains appropriate on touch. For devices even further afield from desktop 
> than tablets this becomes even more of an issue, and those devices may not 
> want to ship breeze.desktop at all. Of course, there is the alternative which 
> is to just tell system integrators to call their root package 
> "org.kde.breeze.desktop" but I'm not sure that would be desirable.
> 
> While setting this in initPackage is a reasonable stop-gap for now, this 
> really ought to be something that gets defined in e.g. the shell package and 
> set in plasmashell so that it can be tied to the shell. If no fallback is 
> defined in the shell package, fair enough -> no fallback :)
> 
> Marco Martin wrote:
> "While setting this in initPackage is a reasonable stop-gap for now"
> for now it does that, I think it's fine for a first step to get rid of 
> lookandfeelaccess. I would advise to provide a full complete package anyways, 
> soat start wouldn't matter much.
> 
> Then, after this, how to make it depend from the shell? it may depend on 
> the shell kded, but it would depend from quite a lot of boilerplate, also it 
> should stay possible to configure it, so in the config file it would end up 
> with pairs shell/l&f packages i guess.
> 
> Or, to make it a bit more primitive, but still functional for the scope, 
> the fallback could be from a configuration file as well, so integrators could 
> just ship such fine.
> For the shell package instead, the fallback *may* be defined in its 
> metadata desktop file? (not sure this could really work for the l&f one)
> 
> Aaron J. Seigo wrote:
> "how to make it depend from the shell?"
> 
> Looking only at the question of default fallback (not user config) then 
> your suggestion should work:
> 
> "the fallback may be defined in its metadata desktop file?"
> 
> This would indeed allow L&F customizations to define the source theme as 
> its fallback. (We'll ignore dependency resolution..) The default fallback 
> could be defined in the shell package's metadata file (so org.kde.desktop 
> would set that to org.kde.breeze.desktop, e.g.) so L&F without a define

Re: Review Request 120029: introduce the concept of package fallback

[Aaron J. Seigo]

> On Sept. 1, 2014, 4:53 p.m., David Edmundson wrote:
> > I need some the concept explaining, why would a developer set a 
> > fallbackpackage?
> > Is it not always org.kde.breeze.desktop?
> 
> Marco Martin wrote:
> It would usually be the packagestructure setting it, in initPackage(), so 
> setFallbackPackage follows the same pattern as addFileDefinition(), 
> setRequired() and the likes.
> 
> so, for the packages of type Plasma/LookAndFeel it would be 
> org.kde.breeze.desktop
> for packages of types Plasma/Shell, it would be org.kde.desktop
> 
> even tough at some point it will probably have to be something more 
> complicated, like the default lookandfeel one depending from  the current 
> shell, if ever needed, will be feasible.
> 
> David Edmundson wrote:
> Ah, makes sense. Thanks.
> 
> Do you really expect a linked link big enough to cycle? I can't really 
> see it going 3 deep. The cycle check is call cool though :)
> 
> Could you add a warning in setFallbackPackage() if it finds a cycle and 
> no-ops. Could be confusing to debug otherwise.
> 
> Marco Martin wrote:
> added warning..
> i see it very unlikely that it's going to go deep, but better safe than 
> sorry :p
> 
> Aaron J. Seigo wrote:
> "Is it not always org.kde.breeze.desktop?"
> 
> No, because of third party developers. Not every project targets the 
> desktop.
> 
> That said, this applies to any use of Plasma::Package, which is meant to 
> be a generic concept not something just for the limited set of package types 
> plasmashell uses. It is helpful to think of it as a concept on its own rather 
> than in the context of Plasma Desktop. Other Frameworks using applications 
> really ought to be able to use Plasma::Package for their "bundles of data" 
> needs.
> 
> "It would usually be the packagestructure setting it, in initPackage()"
> 
> In the case of Shell and Look & Feel packages I would caution against 
> doing this, as it biases them to Plasma Desktop. The tablet interface in 
> Plasma Active will need its own "root" L&F; falling back to components from 
> breeze.desktop will likely be both an oversight and create undesirable 
> results. It also would mean that every time someone touches a component in 
> breeze.desktop that is used by the tablet UX, they will be required to ensure 
> it remains appropriate on touch. For devices even further afield from desktop 
> than tablets this becomes even more of an issue, and those devices may not 
> want to ship breeze.desktop at all. Of course, there is the alternative which 
> is to just tell system integrators to call their root package 
> "org.kde.breeze.desktop" but I'm not sure that would be desirable.
> 
> While setting this in initPackage is a reasonable stop-gap for now, this 
> really ought to be something that gets defined in e.g. the shell package and 
> set in plasmashell so that it can be tied to the shell. If no fallback is 
> defined in the shell package, fair enough -> no fallback :)
> 
> Marco Martin wrote:
> "While setting this in initPackage is a reasonable stop-gap for now"
> for now it does that, I think it's fine for a first step to get rid of 
> lookandfeelaccess. I would advise to provide a full complete package anyways, 
> soat start wouldn't matter much.
> 
> Then, after this, how to make it depend from the shell? it may depend on 
> the shell kded, but it would depend from quite a lot of boilerplate, also it 
> should stay possible to configure it, so in the config file it would end up 
> with pairs shell/l&f packages i guess.
> 
> Or, to make it a bit more primitive, but still functional for the scope, 
> the fallback could be from a configuration file as well, so integrators could 
> just ship such fine.
> For the shell package instead, the fallback *may* be defined in its 
> metadata desktop file? (not sure this could really work for the l&f one)

"how to make it depend from the shell?"

Looking only at the question of default fallback (not user config) then your 
suggestion should work:

"the fallback may be defined in its metadata desktop file?"

This would indeed allow L&F customizations to define the source theme as its 
fallback. (We'll ignore dependency resolution..) The default fallback could be 
defined in the shell package's metadata file (so org.kde.desktop would set that 
to org.kde.breeze.desktop, e.g.) so L&F without a defined fallback would get 
the default fallback.

For shell packages, I would agree that having complete packages is de

Re: Review Request 120029: introduce the concept of package fallback

[Aaron J. Seigo]

> On Sept. 1, 2014, 4:53 p.m., David Edmundson wrote:
> > I need some the concept explaining, why would a developer set a 
> > fallbackpackage?
> > Is it not always org.kde.breeze.desktop?
> 
> Marco Martin wrote:
> It would usually be the packagestructure setting it, in initPackage(), so 
> setFallbackPackage follows the same pattern as addFileDefinition(), 
> setRequired() and the likes.
> 
> so, for the packages of type Plasma/LookAndFeel it would be 
> org.kde.breeze.desktop
> for packages of types Plasma/Shell, it would be org.kde.desktop
> 
> even tough at some point it will probably have to be something more 
> complicated, like the default lookandfeel one depending from  the current 
> shell, if ever needed, will be feasible.
> 
> David Edmundson wrote:
> Ah, makes sense. Thanks.
> 
> Do you really expect a linked link big enough to cycle? I can't really 
> see it going 3 deep. The cycle check is call cool though :)
> 
> Could you add a warning in setFallbackPackage() if it finds a cycle and 
> no-ops. Could be confusing to debug otherwise.
> 
> Marco Martin wrote:
> added warning..
> i see it very unlikely that it's going to go deep, but better safe than 
> sorry :p

"Is it not always org.kde.breeze.desktop?"

No, because of third party developers. Not every project targets the desktop.

That said, this applies to any use of Plasma::Package, which is meant to be a 
generic concept not something just for the limited set of package types 
plasmashell uses. It is helpful to think of it as a concept on its own rather 
than in the context of Plasma Desktop. Other Frameworks using applications 
really ought to be able to use Plasma::Package for their "bundles of data" 
needs.

"It would usually be the packagestructure setting it, in initPackage()"

In the case of Shell and Look & Feel packages I would caution against doing 
this, as it biases them to Plasma Desktop. The tablet interface in Plasma 
Active will need its own "root" L&F; falling back to components from 
breeze.desktop will likely be both an oversight and create undesirable results. 
It also would mean that every time someone touches a component in 
breeze.desktop that is used by the tablet UX, they will be required to ensure 
it remains appropriate on touch. For devices even further afield from desktop 
than tablets this becomes even more of an issue, and those devices may not want 
to ship breeze.desktop at all. Of course, there is the alternative which is to 
just tell system integrators to call their root package 
"org.kde.breeze.desktop" but I'm not sure that would be desirable.

While setting this in initPackage is a reasonable stop-gap for now, this really 
ought to be something that gets defined in e.g. the shell package and set in 
plasmashell so that it can be tied to the shell. If no fallback is defined in 
the shell package, fair enough -> no fallback :)


- Aaron J.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120029/#review65648
---


On Sept. 1, 2014, 7:04 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120029/
> ---
> 
> (Updated Sept. 1, 2014, 7:04 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> introduces the concept of fallback for packages.
> this will replace the private statically linked class LookAndFeelAccess in 
> workspace and desktop, but be more generic so will be usable for things like 
> the shell package as well.
> The feature has an autotest as well to check it's actually working and 
> doesn't break other stuff.
> 
> the package structures that will use this, will just set a package in their 
> structure::initPackage()
> tough for the user of Package in c++ is possible as well to set the fallback 
> package outside the structure if custm things are neede (it's guarded that 
> cycles don't occur in the fallback chain)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 4e64f38 
>   autotests/data/testfallbackpackage/contents/ui/main.qml PRE-CREATION 
>   autotests/data/testfallbackpackage/metadata.desktop PRE-CREATION 
>   autotests/data/testpackage/contents/ui/otherfile.qml PRE-CREATION 
>   autotests/fallbackpackagetest.h PRE-CREATION 
>   autotests/fallbackpackagetest.cpp PRE-CREATION 
>   src/plasma/data/servicetypes/plasma-shell.desktop e2c83ba 
>   src/plasma/package.h 2c686d7 
>   src/plasma/package.cpp 6ad3321 
>   src/plasma/private/package_p.h d902eb1 
> 
> Diff: https://git.reviewboard.kde.org/r/120029/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

Re: Review Request 119992: PackageStructure plugin loading and tidy up of DataEngien plugin loading

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119992/
---

(Updated Aug. 29, 2014, 3:25 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

This patch removes dead code (ContainmentPackage), clean up DataEngine plugin 
loading and updates PackageStructure plugin loading.

Open question: which plugin loading mechanism is to be used:

a) KServiceTypeTrader
b) KPluginTrader
c) KPlugintTrader with a KServiceTypeTrade fallback
d) KServiceTypeTrader with a KPluginTrader fallback

DataEngine is currently (b), complete with a #define in the header that won't 
work as expected.
In this patch PackageStructure is moved to (b) as well. (There is an open RR 
for a correponding patch set in plasma-workspace).
Everything else seems to use (a).

Some plugins are not used in Plasma 5 due to changes with QML, and I guess that 
some of this confusion is due to the decreased use of some of these plugins. A 
decision needs to be made, however about what they should be now and in the 
future.


Diffs
-

  src/plasma/pluginloader.cpp d2ba5ca2d3a96fe6f1ce26be41df3b0954b924df 
  src/plasma/private/packages.cpp 5eb6f0021392257634dfd958c940b2945989e48b 
  src/plasma/private/packages_p.h 0833a4ed1b5704efffccade5e52589878e8b4957 
  src/plasma/package.cpp 6ad332167bb83c2f794f9f5d059e9f369ad33841 
  src/plasma/packagestructure.h fb32c22a6e7df1528b3d7a5b30b94c60a85a93e3 

Diff: https://git.reviewboard.kde.org/r/119992/diff/


Testing
---

Ran a Plasma Desktop 5 session with these changes.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 119988: Package structure cleanups

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119988/
---

(Updated Aug. 29, 2014, 2:20 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

This patch set does the following:

* tidy up the data engine plugin loading code
* make PackageStructure plugins use the json method as with DataEngines
* remove ShellPackage; it moves to live with plasmashell (review #119989)

The goal here is to get rid of the plasmaquick library as much as possible. It 
was unnecessary in the first place since PackageStructure supports plugins. The 
only potentially controversial change here is to move PackageStructure to use 
the json-based plugin loading. That seems to be the more modern approach, but 
plugin loading in libplasma is currently a mix of the old and the new. As 
PackageStructure changed API in plasma-framework, meaning any existing plugins 
from 4.x would need updating anyways, this seems a safe enough change to make 
as it should impact exactly zero plugins out there currently.


Diffs
-

  src/plasma/packagestructure.h fb32c22a6e7df1528b3d7a5b30b94c60a85a93e3 
  src/plasma/pluginloader.cpp d2ba5ca2d3a96fe6f1ce26be41df3b0954b924df 
  src/plasma/private/packages.cpp 5eb6f0021392257634dfd958c940b2945989e48b 
  src/plasma/private/packages_p.h 0833a4ed1b5704efffccade5e52589878e8b4957 
  src/plasmaquick/CMakeLists.txt 1ed7c67efcba0e6dbef1ff929b176090786503de 
  src/plasmaquick/private/packages.h 7498832d0537611903c13e544db6486bab163dd3 
  src/plasmaquick/private/packages.cpp 52758482230d271712e4bb3b6d33f8fdeaa848a8 
  src/plasmaquick/shellpluginloader.h 6c56e5f7b269c3af7587a58cbe104468a2c679c4 
  src/plasmaquick/shellpluginloader.cpp 
2824760e6f64a694bd14b46d2f80151304e3e4d3 
  src/plasma/dataengine.h d87a6f8361c892a249374a5c9da7e84836195156 
  src/plasma/package.cpp 6ad332167bb83c2f794f9f5d059e9f369ad33841 

Diff: https://git.reviewboard.kde.org/r/119988/diff/


Testing
---

Ran a full Plasma Desktop session.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 119992: PackageStructure plugin loading and tidy up of DataEngien plugin loading

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119992/
---

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

This patch removes dead code (ContainmentPackage), clean up DataEngine plugin 
loading and updates PackageStructure plugin loading.

Open question: which plugin loading mechanism is to be used:

a) KServiceTypeTrader
b) KPluginTrader
c) KPlugintTrader with a KServiceTypeTrade fallback
d) KServiceTypeTrader with a KPluginTrader fallback

DataEngine is currently (b), complete with a #define in the header that won't 
work as expected.
In this patch PackageStructure is moved to (b) as well. (There is an open RR 
for a correponding patch set in plasma-workspace).
Everything else seems to use (a).

Some plugins are not used in Plasma 5 due to changes with QML, and I guess that 
some of this confusion is due to the decreased use of some of these plugins. A 
decision needs to be made, however about what they should be now and in the 
future.


Diffs
-

  src/plasma/pluginloader.cpp d2ba5ca2d3a96fe6f1ce26be41df3b0954b924df 
  src/plasma/private/packages.cpp 5eb6f0021392257634dfd958c940b2945989e48b 
  src/plasma/private/packages_p.h 0833a4ed1b5704efffccade5e52589878e8b4957 
  src/plasma/package.cpp 6ad332167bb83c2f794f9f5d059e9f369ad33841 
  src/plasma/packagestructure.h fb32c22a6e7df1528b3d7a5b30b94c60a85a93e3 

Diff: https://git.reviewboard.kde.org/r/119992/diff/


Testing
---

Ran a Plasma Desktop 5 session with these changes.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 119988: Package structure cleanups

[Aaron J. Seigo]

> On Aug. 29, 2014, noon, David Edmundson wrote:
> > src/plasma/packagestructure.h, line 99
> > <https://git.reviewboard.kde.org/r/119988/diff/1/?file=308188#file308188line99>
> >
> > would removing a #define count as a SIC?
> 
> Aaron J. Seigo wrote:
> Yes, but I don't think it maters for two reasons:
> 
> 1) DataEngine also had both forms but only one actually currently works; 
> so it is on the face of it *broken* .. removing that is the same as this 
> change at the end of the day
> 2) There are no Plasma 5 PackageStructure plugins out there except the 
> share dataengine in plasma-workspace (which itself was done in a very odd 
> manner)
> 
> Neither reason is particularly *good* from the perspective of strict 
> policy adherence, but they demonstrate that it is a harmless violation. May 
> as well get it right imho.
> 
> I'd add a third consideration as well: It is quite evident that 
> plasma-framework was release before it was ready. The plugin loading is 
> entirely inconsistent with DataEngines using JSON and the Qt5 plugin loading 
> an the rest of the plugins not. Some of the plugins are obviously not being 
> used at all due to changes brought about with QML2 and that probably 
> contributed to the lack of attention. However, PluginLoader quite obviously 
> went through a partial refactoring that was never completed. It's a little 
> brash to commit to source and binary compatibility when things are in that 
> shape.
> 
> Hrvoje Senjan wrote:
> >Yes, but I don't think it maters for two reasons
> 
> Can users of Plasma 5.0.x can expect a broken desktop once KF5 5.2 is out 
> (if this patch goes in)?
> 
> David Edmundson wrote:
> Whilst I agree that the frameworks situation sucks, I'm not sure we have 
> a choice.
> 
> Frameworks 5.2 releases independently of plasma-workspace. At which point 
> there is a point in time where one can't compile plasma-workspace.
> We could make it go via a no-op that does nothing.
> 
> Martin Gräßlin wrote:
> if one doesn't recompile it should still work, shouldn't it? Otherwise 
> I'd suggest to wrap the define in a ifndef NO_DEPRECATED block to keep SIC 
> (even if it means that we carry deprecated stuff for the time being, but 
> after all that's the point of deprecation).
> 
> Aaron J. Seigo wrote:
> "Can users of Plasma 5.0.x can expect a broken desktop once KF5 5.2 is 
> out (if this patch goes in)?"
> 
> Not due to the issue you are responding to, no.
> 
> The real issue for breakeage is the use of plasmaquick in Workspaces 
> which violates the contract between Frameworks and Workspaces (due to being 
> differnet products) by using a library in workspaces from framweorks that 
> does not have a binary compatibility guarantee and which has no installed 
> headers. I am at a loss as to how this could have shipped in this fashion.
> 
> One possibility is to ship libplasmaquick as-is (with the 
> PackageStructures duplicated and ShellPluginLoader) and simply remove its 
> usage from Plasma Workspace (as already done in 119989), and then at some 
> later point remove it from libplasmaquick and simply require a certain 
> version # of Frameworks for Plasma Desktop. Requiring a minium v# of 
> libraries would be a pretty normal state of affairs.
> 
> If the intention is to allow Frameworks 5.2 to be used with Plasma 
> Desktop 5.0, then that is probably the only realistic option. In which case 
> the Plasma team ought to just install the headers for libplasmaquick and make 
> it official. There is no point to requiring binary compatibility and trying 
> to pretend a library is private.
> 
> "At which point there is a point in time where one can't compile 
> plasma-workspace."
> 
> An alternative is to state compatible versions of Frameworks with 
> plasma-workspace. Either way, it doesn't resolve the DataEngine situation 
> where it may be SC but certainly is not functional.
> 
> Currently relatively few people are using Plasma 5; I would urge the 
> Plasma team to come up with a long-term solution before that changes. 
> Carrying the impact of these decisions for 5+ years after Plasma 5 actually 
> has users is .. well .. it's up to you.
> 
> If it is decided not to merge this patchset, that's fine by me. I can 
> probably re-work the related plasma-workspace patch set to use the older 
> KService based plugin loading. It would just be nice to know *which* path 
> (JSON or .desktop files) is being taken.
> 
> Aaron J. Seigo wrote:
> &q

Re: Review Request 119988: Package structure cleanups

[Aaron J. Seigo]

> On Aug. 29, 2014, noon, David Edmundson wrote:
> > src/plasma/packagestructure.h, line 99
> > <https://git.reviewboard.kde.org/r/119988/diff/1/?file=308188#file308188line99>
> >
> > would removing a #define count as a SIC?
> 
> Aaron J. Seigo wrote:
> Yes, but I don't think it maters for two reasons:
> 
> 1) DataEngine also had both forms but only one actually currently works; 
> so it is on the face of it *broken* .. removing that is the same as this 
> change at the end of the day
> 2) There are no Plasma 5 PackageStructure plugins out there except the 
> share dataengine in plasma-workspace (which itself was done in a very odd 
> manner)
> 
> Neither reason is particularly *good* from the perspective of strict 
> policy adherence, but they demonstrate that it is a harmless violation. May 
> as well get it right imho.
> 
> I'd add a third consideration as well: It is quite evident that 
> plasma-framework was release before it was ready. The plugin loading is 
> entirely inconsistent with DataEngines using JSON and the Qt5 plugin loading 
> an the rest of the plugins not. Some of the plugins are obviously not being 
> used at all due to changes brought about with QML2 and that probably 
> contributed to the lack of attention. However, PluginLoader quite obviously 
> went through a partial refactoring that was never completed. It's a little 
> brash to commit to source and binary compatibility when things are in that 
> shape.
> 
> Hrvoje Senjan wrote:
> >Yes, but I don't think it maters for two reasons
> 
> Can users of Plasma 5.0.x can expect a broken desktop once KF5 5.2 is out 
> (if this patch goes in)?
> 
> David Edmundson wrote:
> Whilst I agree that the frameworks situation sucks, I'm not sure we have 
> a choice.
> 
> Frameworks 5.2 releases independently of plasma-workspace. At which point 
> there is a point in time where one can't compile plasma-workspace.
> We could make it go via a no-op that does nothing.
> 
> Martin Gräßlin wrote:
> if one doesn't recompile it should still work, shouldn't it? Otherwise 
> I'd suggest to wrap the define in a ifndef NO_DEPRECATED block to keep SIC 
> (even if it means that we carry deprecated stuff for the time being, but 
> after all that's the point of deprecation).
> 
> Aaron J. Seigo wrote:
> "Can users of Plasma 5.0.x can expect a broken desktop once KF5 5.2 is 
> out (if this patch goes in)?"
> 
> Not due to the issue you are responding to, no.
> 
> The real issue for breakeage is the use of plasmaquick in Workspaces 
> which violates the contract between Frameworks and Workspaces (due to being 
> differnet products) by using a library in workspaces from framweorks that 
> does not have a binary compatibility guarantee and which has no installed 
> headers. I am at a loss as to how this could have shipped in this fashion.
> 
> One possibility is to ship libplasmaquick as-is (with the 
> PackageStructures duplicated and ShellPluginLoader) and simply remove its 
> usage from Plasma Workspace (as already done in 119989), and then at some 
> later point remove it from libplasmaquick and simply require a certain 
> version # of Frameworks for Plasma Desktop. Requiring a minium v# of 
> libraries would be a pretty normal state of affairs.
> 
> If the intention is to allow Frameworks 5.2 to be used with Plasma 
> Desktop 5.0, then that is probably the only realistic option. In which case 
> the Plasma team ought to just install the headers for libplasmaquick and make 
> it official. There is no point to requiring binary compatibility and trying 
> to pretend a library is private.
> 
> "At which point there is a point in time where one can't compile 
> plasma-workspace."
> 
> An alternative is to state compatible versions of Frameworks with 
> plasma-workspace. Either way, it doesn't resolve the DataEngine situation 
> where it may be SC but certainly is not functional.
> 
> Currently relatively few people are using Plasma 5; I would urge the 
> Plasma team to come up with a long-term solution before that changes. 
> Carrying the impact of these decisions for 5+ years after Plasma 5 actually 
> has users is .. well .. it's up to you.
> 
> If it is decided not to merge this patchset, that's fine by me. I can 
> probably re-work the related plasma-workspace patch set to use the older 
> KService based plugin loading. It would just be nice to know *which* path 
> (JSON or .desktop files) is being taken.

"Otherwise I'd suggest to wrap 

Re: Review Request 119988: Package structure cleanups

[Aaron J. Seigo]

> On Aug. 29, 2014, noon, David Edmundson wrote:
> > src/plasma/packagestructure.h, line 99
> > <https://git.reviewboard.kde.org/r/119988/diff/1/?file=308188#file308188line99>
> >
> > would removing a #define count as a SIC?
> 
> Aaron J. Seigo wrote:
> Yes, but I don't think it maters for two reasons:
> 
> 1) DataEngine also had both forms but only one actually currently works; 
> so it is on the face of it *broken* .. removing that is the same as this 
> change at the end of the day
> 2) There are no Plasma 5 PackageStructure plugins out there except the 
> share dataengine in plasma-workspace (which itself was done in a very odd 
> manner)
> 
> Neither reason is particularly *good* from the perspective of strict 
> policy adherence, but they demonstrate that it is a harmless violation. May 
> as well get it right imho.
> 
> I'd add a third consideration as well: It is quite evident that 
> plasma-framework was release before it was ready. The plugin loading is 
> entirely inconsistent with DataEngines using JSON and the Qt5 plugin loading 
> an the rest of the plugins not. Some of the plugins are obviously not being 
> used at all due to changes brought about with QML2 and that probably 
> contributed to the lack of attention. However, PluginLoader quite obviously 
> went through a partial refactoring that was never completed. It's a little 
> brash to commit to source and binary compatibility when things are in that 
> shape.
> 
> Hrvoje Senjan wrote:
> >Yes, but I don't think it maters for two reasons
> 
> Can users of Plasma 5.0.x can expect a broken desktop once KF5 5.2 is out 
> (if this patch goes in)?
> 
> David Edmundson wrote:
> Whilst I agree that the frameworks situation sucks, I'm not sure we have 
> a choice.
> 
> Frameworks 5.2 releases independently of plasma-workspace. At which point 
> there is a point in time where one can't compile plasma-workspace.
> We could make it go via a no-op that does nothing.
> 
> Martin Gräßlin wrote:
> if one doesn't recompile it should still work, shouldn't it? Otherwise 
> I'd suggest to wrap the define in a ifndef NO_DEPRECATED block to keep SIC 
> (even if it means that we carry deprecated stuff for the time being, but 
> after all that's the point of deprecation).

"Can users of Plasma 5.0.x can expect a broken desktop once KF5 5.2 is out (if 
this patch goes in)?"

Not due to the issue you are responding to, no.

The real issue for breakeage is the use of plasmaquick in Workspaces which 
violates the contract between Frameworks and Workspaces (due to being differnet 
products) by using a library in workspaces from framweorks that does not have a 
binary compatibility guarantee and which has no installed headers. I am at a 
loss as to how this could have shipped in this fashion.

One possibility is to ship libplasmaquick as-is (with the PackageStructures 
duplicated and ShellPluginLoader) and simply remove its usage from Plasma 
Workspace (as already done in 119989), and then at some later point remove it 
from libplasmaquick and simply require a certain version # of Frameworks for 
Plasma Desktop. Requiring a minium v# of libraries would be a pretty normal 
state of affairs.

If the intention is to allow Frameworks 5.2 to be used with Plasma Desktop 5.0, 
then that is probably the only realistic option. In which case the Plasma team 
ought to just install the headers for libplasmaquick and make it official. 
There is no point to requiring binary compatibility and trying to pretend a 
library is private.

"At which point there is a point in time where one can't compile 
plasma-workspace."

An alternative is to state compatible versions of Frameworks with 
plasma-workspace. Either way, it doesn't resolve the DataEngine situation where 
it may be SC but certainly is not functional.

Currently relatively few people are using Plasma 5; I would urge the Plasma 
team to come up with a long-term solution before that changes. Carrying the 
impact of these decisions for 5+ years after Plasma 5 actually has users is .. 
well .. it's up to you.

If it is decided not to merge this patchset, that's fine by me. I can probably 
re-work the related plasma-workspace patch set to use the older KService based 
plugin loading. It would just be nice to know *which* path (JSON or .desktop 
files) is being taken.


- Aaron J.


-------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119988/#review65481
---


On Aug. 29, 2014, 11:51 a.m., Aaron J. Seigo wrote:
> 
> ---

Re: Review Request 119988: Package structure cleanups

[Aaron J. Seigo]

> On Aug. 29, 2014, noon, David Edmundson wrote:
> > src/plasma/packagestructure.h, line 99
> > <https://git.reviewboard.kde.org/r/119988/diff/1/?file=308188#file308188line99>
> >
> > would removing a #define count as a SIC?

Yes, but I don't think it maters for two reasons:

1) DataEngine also had both forms but only one actually currently works; so it 
is on the face of it *broken* .. removing that is the same as this change at 
the end of the day
2) There are no Plasma 5 PackageStructure plugins out there except the share 
dataengine in plasma-workspace (which itself was done in a very odd manner)

Neither reason is particularly *good* from the perspective of strict policy 
adherence, but they demonstrate that it is a harmless violation. May as well 
get it right imho.

I'd add a third consideration as well: It is quite evident that 
plasma-framework was release before it was ready. The plugin loading is 
entirely inconsistent with DataEngines using JSON and the Qt5 plugin loading an 
the rest of the plugins not. Some of the plugins are obviously not being used 
at all due to changes brought about with QML2 and that probably contributed to 
the lack of attention. However, PluginLoader quite obviously went through a 
partial refactoring that was never completed. It's a little brash to commit to 
source and binary compatibility when things are in that shape.


- Aaron J.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119988/#review65481
-------


On Aug. 29, 2014, 11:51 a.m., Aaron J. Seigo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119988/
> ---
> 
> (Updated Aug. 29, 2014, 11:51 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> This patch set does the following:
> 
> * tidy up the data engine plugin loading code
> * make PackageStructure plugins use the json method as with DataEngines
> * remove ShellPackage; it moves to live with plasmashell (review #119989)
> 
> The goal here is to get rid of the plasmaquick library as much as possible. 
> It was unnecessary in the first place since PackageStructure supports 
> plugins. The only potentially controversial change here is to move 
> PackageStructure to use the json-based plugin loading. That seems to be the 
> more modern approach, but plugin loading in libplasma is currently a mix of 
> the old and the new. As PackageStructure changed API in plasma-framework, 
> meaning any existing plugins from 4.x would need updating anyways, this seems 
> a safe enough change to make as it should impact exactly zero plugins out 
> there currently.
> 
> 
> Diffs
> -
> 
>   src/plasma/packagestructure.h fb32c22a6e7df1528b3d7a5b30b94c60a85a93e3 
>   src/plasma/pluginloader.cpp d2ba5ca2d3a96fe6f1ce26be41df3b0954b924df 
>   src/plasma/private/packages.cpp 5eb6f0021392257634dfd958c940b2945989e48b 
>   src/plasma/private/packages_p.h 0833a4ed1b5704efffccade5e52589878e8b4957 
>   src/plasmaquick/CMakeLists.txt 1ed7c67efcba0e6dbef1ff929b176090786503de 
>   src/plasmaquick/private/packages.h 7498832d0537611903c13e544db6486bab163dd3 
>   src/plasmaquick/private/packages.cpp 
> 52758482230d271712e4bb3b6d33f8fdeaa848a8 
>   src/plasmaquick/shellpluginloader.h 
> 6c56e5f7b269c3af7587a58cbe104468a2c679c4 
>   src/plasmaquick/shellpluginloader.cpp 
> 2824760e6f64a694bd14b46d2f80151304e3e4d3 
>   src/plasma/dataengine.h d87a6f8361c892a249374a5c9da7e84836195156 
>   src/plasma/package.cpp 6ad332167bb83c2f794f9f5d059e9f369ad33841 
> 
> Diff: https://git.reviewboard.kde.org/r/119988/diff/
> 
> 
> Testing
> ---
> 
> Ran a full Plasma Desktop session.
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 119988: Package structure cleanups

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119988/
---

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

This patch set does the following:

* tidy up the data engine plugin loading code
* make PackageStructure plugins use the json method as with DataEngines
* remove ShellPackage; it moves to live with plasmashell (review #119989)

The goal here is to get rid of the plasmaquick library as much as possible. It 
was unnecessary in the first place since PackageStructure supports plugins. The 
only potentially controversial change here is to move PackageStructure to use 
the json-based plugin loading. That seems to be the more modern approach, but 
plugin loading in libplasma is currently a mix of the old and the new. As 
PackageStructure changed API in plasma-framework, meaning any existing plugins 
from 4.x would need updating anyways, this seems a safe enough change to make 
as it should impact exactly zero plugins out there currently.


Diffs
-

  src/plasma/packagestructure.h fb32c22a6e7df1528b3d7a5b30b94c60a85a93e3 
  src/plasma/pluginloader.cpp d2ba5ca2d3a96fe6f1ce26be41df3b0954b924df 
  src/plasma/private/packages.cpp 5eb6f0021392257634dfd958c940b2945989e48b 
  src/plasma/private/packages_p.h 0833a4ed1b5704efffccade5e52589878e8b4957 
  src/plasmaquick/CMakeLists.txt 1ed7c67efcba0e6dbef1ff929b176090786503de 
  src/plasmaquick/private/packages.h 7498832d0537611903c13e544db6486bab163dd3 
  src/plasmaquick/private/packages.cpp 52758482230d271712e4bb3b6d33f8fdeaa848a8 
  src/plasmaquick/shellpluginloader.h 6c56e5f7b269c3af7587a58cbe104468a2c679c4 
  src/plasmaquick/shellpluginloader.cpp 
2824760e6f64a694bd14b46d2f80151304e3e4d3 
  src/plasma/dataengine.h d87a6f8361c892a249374a5c9da7e84836195156 
  src/plasma/package.cpp 6ad332167bb83c2f794f9f5d059e9f369ad33841 

Diff: https://git.reviewboard.kde.org/r/119988/diff/


Testing
---

Ran a full Plasma Desktop session.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 119942: give the applet alternatives QML file an entry

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119942/
---

(Updated Aug. 26, 2014, 7:44 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

Give the applet alternatives QML file an entry; this adds to the 
self-documenting nature of the package and prevents paths from creeping into 
usage points. This is a starting point in the package, so it should have a well 
known name.


Diffs
-

  src/plasma/private/packages.cpp 585ab1f272d48f9b998c64968beeccf0f6dd979f 

Diff: https://git.reviewboard.kde.org/r/119942/diff/


Testing
---

Loaded plasmashell and the alternatives UI.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 119942: give the applet alternatives QML file an entry

[Aaron J. Seigo]

> On Aug. 26, 2014, 5:40 p.m., Marco Martin wrote:
> > src/plasma/private/packages.cpp, line 278
> > <https://git.reviewboard.kde.org/r/119942/diff/1/?file=307679#file307679line278>
> >
> > maybe going even more explicit and giving its own subdicrectory too? 
> > (i'm fine with either)

A subdirectory for one item is probably overkill, and it does make sense there 
imho as it is with the other applet explorer bits. The name "explorer" is 
perhaps a bit unfortunate, but it is not so bad as to warrant changing it. This 
does all need documenting, however.


- Aaron J.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119942/#review65294
-------


On Aug. 26, 2014, 5:25 p.m., Aaron J. Seigo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119942/
> ---
> 
> (Updated Aug. 26, 2014, 5:25 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Give the applet alternatives QML file an entry; this adds to the 
> self-documenting nature of the package and prevents paths from creeping into 
> usage points. This is a starting point in the package, so it should have a 
> well known name.
> 
> 
> Diffs
> -
> 
>   src/plasma/private/packages.cpp 585ab1f272d48f9b998c64968beeccf0f6dd979f 
> 
> Diff: https://git.reviewboard.kde.org/r/119942/diff/
> 
> 
> Testing
> ---
> 
> Loaded plasmashell and the alternatives UI.
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 119942: give the applet alternatives QML file an entry

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119942/
---

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

Give the applet alternatives QML file an entry; this adds to the 
self-documenting nature of the package and prevents paths from creeping into 
usage points. This is a starting point in the package, so it should have a well 
known name.


Diffs
-

  src/plasma/private/packages.cpp 585ab1f272d48f9b998c64968beeccf0f6dd979f 

Diff: https://git.reviewboard.kde.org/r/119942/diff/


Testing
---

Loaded plasmashell and the alternatives UI.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KIO directory listing - CPU slows down SSD

[Aaron J. Seigo]

On Monday, June 2, 2014 20:54:11 Mark Gaiser wrote:
> On Mon, Jun 2, 2014 at 6:42 PM, Aaron J. Seigo  wrote:
> > On Thursday, May 29, 2014 16:32:28 Mark Gaiser wrote:
> >> On Thu, May 29, 2014 at 12:21 AM, Aaron J. Seigo  But don't you just move logic to the slave that way?

yes and no.

sorting and grouping are easily abstracted. sort/group currently happens 
client side and doesn't care where the data comes from. so while moving the 
logic to the slave, it should be achieved in a slave-neutral fashion.

> And lose flexibility in the apps using the slave (like dolphin?

if they need to sort / group in some way not supported by KIO they can request 
the results unsorted and then you have exactly the same situation we have now. 
there would be no loss of flexibility.

and really: how often do new sort / group methods appear in KDE file listings? 
very, very rarely. new methods could be also be moved to the ioslave side 
later on where they would be usable by all clients.

> Oh and
> complicating kio "a bit" to pass a sorting and/or grouping style.

yes, it would make the protocol more complex.

> Also, for a slave to give you the n items in a sorted way requires the
> slave to fetch _all_ items to do the sorting.

not all slaves, no. some protocols offer server-side functionality.

the file slave (among others) would indeed need to fetch all items, yes .. but 
that is *precisely* what happens currently on the client side anyways. so 
whatever mechanisms are employed client-side could be done in the slave, minus 
the IPC overhead and with whatever benefits can be gained from using lower-
level API in the slave.

> All that you will save is IPC traffic. It might

not necessarily; common use cases like "sort by name" may end up significantly 
faster if the slave can quickly gather file names and process them into order 
before stat'ing.

> not even be faster. Take a slow ftp for example. Without "slave side
> sorting" you would get your first results after 300ms, guaranteed.

slaves dealing with slow data sources could do incremental updates. done well 
the worst case should be equal to the current typical case.

i used the word "stream" and perhaps that was not the best word to use. it's 
more like synchronized models where population is informed by 

* sorting preference (inc "none")
* grouping preference (inc "none")
* offset (if any)
* hard and soft limits (if any)[1]

> With server side sorting you will have to wait however long the slave
> takes to fetch _all_ items. Then you can do a (partial) sort and send
> it to the client.

if you want perfect sorting (e.g. no re-layouts) then this is true in the 
current case too: the client has to wait for ALL results to be able to sort 
items correctly.

so let's assume that incremental sort is allowable and the streamed data can 
be updated by the slave. in that case one can view it as an incrementally 
updating window onto a dataset.

example: 1000 items to be sorted by name with a normal distribution across all 
values in the names. the client is interested in at least the first 100 items. 
the slave fetches the first 100 entries, sorts them, and they are immediately 
streamed to the client. 

the slave fetches the next 100 entries and adds them to the sorted set. 10 
items (statistically that should be about the rate) are no longer in the first 
100 items in the set; those 10 items are now streamed across with their index 
and updated indexes are sent for items at the previous indexes 89-99.

rinse / repeat until listing is complete.

now if sorting is done by name, this could result in even more speed-ups: only 
the items in the current window of results being sent would need to be stat'd. 
the rest of the items could be stat'd lazily (or when the window shifts)

obviously, for a slave listing youtube results that would be entirely 
unnecessary: simply query the youtube API for the exact result set desired.

> > KIO listing is all-or-nothing batch oriented; a stream-based approach that
> > supports seeking through listings that are pre-sorted/grouped in the slave
> > process would be moderately gorgeous. it would prevent more IPC than
> > necessary and allow the slave to use any&all service-specific features
> > for pre-sort/group of entries.
> 
> So you would save the stream in the slave side?

that would depend partially on the slave (a youtube video slave wouldn't need 
to do this; the file slave might). the client is also going to have a model in 
its process with the data that is populated, so there would always be data 
client-side.

> But how would you
> change the sorting if you just have a stream? Parse all data, sort it
> and put it in a stream again?

hard to say without some experimentation. my first thought would be to divide 
this into two cases:

Re: KIO directory listing - CPU slows down SSD

[Aaron J. Seigo]

On Tuesday, June 3, 2014 00:11:51 Mark Gaiser wrote:
> We should do this as a gsoc project. Since this would be very

unless it is a very gifted student, this is almost certainly outside the realm 
of reason for a GSoC project.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KIO directory listing - CPU slows down SSD

[Aaron J. Seigo]

On Thursday, May 29, 2014 16:32:28 Mark Gaiser wrote:
> On Thu, May 29, 2014 at 12:21 AM, Aaron J. Seigo  It's just a hobby for me to optimize it to levels that match raw speed
> (without any KIO).

meaningful speed improvements would be a reason to change things.

if wire compatibility can be maintained while reaping speed benefits (and i 
think we've seen it can be done ...), then that isn't a reason.

> I'm "trying" to improve things by making them simpler. If that
> requires a new code path: fine with me.

simplicity is a good goal, too. personally, i'd count it as a minor goal when 
looking at a long-maintained/debugged codebase, though. disturbing too much in 
the quest for simplicity often ends up introducing new bugs and re-introducing 
old ones. a lot of complexity in well-used code comes from fixing things.

> > Do you mean listEntry? listEntries is already taking a batch of UDSEntry
> > objects so not much is won there. Modifying listEntry, however, to use the
> > streaming method would be a win.
> 
> No, i mean listEntries.
> I also mean completely removing the SlaveBase::listEntry call from
> slaves. The slaves are filling a stream which would then be send to
> the client by calling (the new proposed) SlaveBase::listEntries.

as long as SlaveBase::listEntry remains so as to not break 3rd party slaves 
(and ones you don't notice lurking in git.kde.org ;) that sounds great.

> What i mean here is that the current listEntries function that takes a
> UDSEntryList can be modified to use the new path.

not sure that would win much; probably even be marginally slower due to more 
function calls. it would at least be using the exact same code path as a slave 
using the new API, which would be a consistency win ...

> > (perhaps even a back-off algo -> first time-out is 50ms; if a buffer gets
> > sent due to timeout then back-off to 100; repeat until you hit 300 ..
> > easy to implement and should hopefully get first items to the client
> > quicker)
> This is theory and it works very well. In theory!
> In practice things work a bit differently.
> 
> The gui seems to be stuttering _while_ new entries flow in. Test it in

yeah, this is an entirely different issue that while need an entirely different 
set of optimizations .. which probably won't be optimizations at all in terms 
of CPU usage but will result in more fluid UI.

> dolphin on a massive folder. In fact, those that use kio::listdir for
> listing folders only have interfaces that become usable when all
> entries are fetched. 

assuming these UIs receive maintenance over the next years, this pattern is 
likely to fade away. instant feedback and incremental listing is a current 
reality.

you know what would be very nice (but rather complex to achieve...)? for the 
client side of a KIO listing to say "i only want the first N items anyways, 
i'll let you know when i can handle more..." and for sorting to be optionally 
done on the slave side.

it is pretty ironic that a UI to show, say, videos from youtube will populate 
at least as smoothly, and on crappy enough hardware even better than, listing 
local directory contents because of this kind of incremental fetch and server-
side pre-processing.

KIO listing is all-or-nothing batch oriented; a stream-based approach that 
supports seeking through listings that are pre-sorted/grouped in the slave 
process would be moderately gorgeous. it would prevent more IPC than necessary 
and allow the slave to use any&all service-specific features for pre-sort/group 
of entries.
 
> Unless you can draft up a simple gui that work fast and fluid _while_
> entries flow in.

that's easy to do with QML. we have numerous examples of this in plasma 
active, in fact. the real trick is ensuring that your entries come to you pre-
arranged so you don't show them moving around to the user.

> > .. you'll also find some perhaps useful cleanups and optimizations in the
> > aseigo/cleanups branch in kio. feel free to cherry-pick.
> 
> You seem to like KIO :)

well, i use it a lot :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KIO directory listing - CPU slows down SSD

[Aaron J. Seigo]

On Wednesday, May 28, 2014 21:12:43 Mark Gaiser wrote:
> You've written that with the assumption of backwards compatibility.
> It's a nice idea, but why should we even try to remain backwards
> compatible?

The question should be inverted: why *should* we break compatibility?

Least change always wins, because least change means:

* least effort
* retaining knowledge (of existing code people know)
* lower change of introducing new bugs

The question is never "why shouldn't we change things" but "why should we 
change things"...

> It's all internal to SlaveBase so we can change whatever
> we want if that suits a more optimized design.

If it is more optimized, yes which is why I did a bunch of measurements on 
the seek-back-and-set method to see if it was slow. (It isn't ...)

> Consider this patch (lives for a month):
> http://p.sc2.nl/pgfto3npy
> 
> I don't see any harm in introducing a second path for slaves to send
> their entries to the client. This new path from the patch above
> completely eliminates the UDSEntry object need from a SlaveBase point
> of view. It's then up to the slaves themselves to make use of it.

Yes, this much makes sense. It allows us to preserve the wire format while 
making a significant optimization...

preserving the wire format is particularly desirable as the format is not 
formally described anywhere ("the code is the documentation") and so i would 
consider it highly fragile.

> With some work the current listEntries(const UDSEntryList &list) could
> even be adjusted to the streaming based mechanism (aka, inject a
> "startEntry" to the stream right before it does "stream << entry"
> which would make all slaves use the new path.

Do you mean listEntry? listEntries is already taking a batch of UDSEntry 
objects so not much is won there. Modifying listEntry, however, to use the 
streaming method would be a win.

> Thinking about this again, i won't even need a "endEntry" call as long
> as each new entry starts with a "startEntry".

The last entry can be closed in the listEntries(const QByteArray&) method. 
Which means endEntry() is unnecessary even with the field count.

> I can then use that to
> detect the start of a new entry.

Not a great idea, imho. It means checking EVERY field read. It also means 
having to read the next field as if it *was* "startEntry" and then if it isn't 
re-reading it as a "normal" field. That means double deserialization .. but 
even worse, if a field happens to start with the same bit sequence.. ouch. No, 
field count is a good idea.

> also removes the need to know how many fields are present in a given
> entry and conveniently also removes the need to move back in a
> datastream :)

Moving back in the datastream is stupidly cheap. That was the entire point of 
the benchmarking I did :) There's no reason to try and avoid it ...
 
> Something else that comes in mind here is the current batching. Right
> now each batch is 200 entries max. But if we know the actual byte size
> (which we do when filling a stream) then we might as well change the
> batching to not exceed X number of kb.

Yep.

> That might be more efficient
> batching. We obviously keep the timer. Send all entries currently
> collected once 300ms has passed.

Agreed ... in fact, for the *first* buffer I'd make the timer even shorter to 
give the initial impression of speed. It's a complete hack and wouldn't really 
be any faster (the contrary), but I bet it would be noticeable in the GUI. 

(perhaps even a back-off algo -> first time-out is 50ms; if a buffer gets sent 
due to timeout then back-off to 100; repeat until you hit 300 .. easy to 
implement and should hopefully get first items to the client quicker)

> > s.. i think performance wise this should be just fine, seeing as one
> > would probably be sending buffers on the order of some 100s of KB rather
> > than 100s of MB ;)
> 
> I can always increase the benchmark from 500.000 files to even more if
> needed ^_-

Heh .. but you'd never send 500k files at a time ... :) This is all *per buffer 
sent*, so if you send 400MB of data but do so in 250k chunks this extra 
seeking still won't show up in the callgraph.

On the other hand, hopefully you can make this code fast enough that you'll be 
tempted to raise it to 500k files to REALLY push it. ;)


.. you'll also find some perhaps useful cleanups and optimizations in the 
aseigo/cleanups branch in kio. feel free to cherry-pick.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KIO directory listing - CPU slows down SSD

[Aaron J. Seigo]

On Wednesday, May 28, 2014 16:23:31 Mark Gaiser wrote:
> On Wed, May 28, 2014 at 9:59 AM, David Faure  wrote:
> > On Wednesday 14 May 2014 12:50:18 Aaron J. Seigo wrote:
> >> * one entry at a time, serialized
> >> * no changing data once put there
> >> * no getting the data back out
> >>
> >> the class would then just batch up data into its QByteArray and then
> >> shunt
> >> it  across the wire when needed. no middle men, no 100s of 1000s of
> >> objects
> >>
> >> and QLists and << operators. just an API sth like:
> >> startEntry
> >> addField
> >> endEntry
> >
> > I like this idea very much. Mark: see, it's quite close to what we were
> > discussing by email recently. And I hadn't read this yet :)
> >
> > Note how this is not specific to the ioslave using QT_STATBUFF :)
>
> To be honest, i was trying to look at the most used case (which is
> kio_file) and optimize that for raw speed.
> But i kinda like the idea of this approach as well since it would work
> on all slaves :) I will try to implement that instead.
>
> What i really like about this approach is that it could eventually
> phase out UDSEntry from the slave side. Meaning that UDSEntry would be
> fully client side (where this stream would be parsed).

that would be good for performance indeed; optimizing UDSEntry would still
have value, of course, since the client side would continue to use it. so
nobody's efforts there would be wasted

> >> internally it could either batch up the fields and then stream them out
> >> when  endEntry is called, or (and this is what i would do personally) on
> >> startEntry it would set up the header but with bad data (e.g. field count
> >> of 0) and then start appending the fields as they come in and increment a
> >> field count variable. when endEntry is called, update the header with the
> >> right field count.
> >
> > Ah but batching requires a vector again. Or you meant writing into a
> > bytearray and coming back to update the count - that would rely on
> > QDataStream internals I'm afraid (how to know the offset).

no internals needed. see attached example.

> > But the alternative is to replace the field count with an "end of fields"
> > special field.
>
> Huh? "batching requires a vector again" .. i don't understand that. It

consider the current code:

void SlaveBase::listEntries(const UDSEntryList &list)
{
KIO_DATA << (quint32)list.count();

foreach (const UDSEntry &entry, list) {
stream << entry;
}

send(MSG_LIST_ENTRIES, data);
}


so David means that because you can't seek in the QDataStream that you'd have
to batch up entries in a temporary data structure so that you could write the
header first and THEN write all the entries.

thankfully, the base assumption is incorrect :)

> SomeDataStreamClass stream;
> int i = 0
> while(QT_READDIR(...) && i++) {
>   if(i = 200) { // or whatever the threshold is to send a batch of entries

... and before this happens it needs to set the # of entries it is sending.
something like a finalizeEntries() method. the class could keep state,
incrementing a variable every time startEntry() is called

also, rather than count based i'd probably make it based on the size of the
bytearray. smaller entries would mean more entries sent in a batch, larger
entries might mean less time waiting on the client side for additional items
(though that is likely to be nullified by the extra overhead of multiple
read/writes to the socket and subsequent deserialization trips). in any case,
one can then optimize the size of the transfer

>  listEntries(stream);
>  // reset stream and i.
>   }
>   startEntry()
>   addField(...)
>   addField(...)
>   ...
>   endEntry()

endEntry would need to do something similar: it would need to seek back into
the bytearray where the fields started and update the # of fields to retain
compatibility with the current code which is:

void UDSEntryPrivate::save(QDataStream &s, const UDSEntry &a)
{
const FieldHash &e = a.d->fields;

s << e.size();


> }

the point is that before listEntries is called, the # of items will need to be
placed at the head of the list so then when deserializing on the client side
the # of entries is known.

in any case, with QDataStream and a QByteArray (as currently used in the code)
you CAN seek backwards into the stream and overwrite some of the bytes and
then even seek back to the end. see the attached example to prove this for
yourself :)

of course, in this case you don't even need to seek back to the end. you just
need to seek

Re: KIO directory listing - CPU slows down SSD

[Aaron J. Seigo]

e is nearly all multicore now) and that 
the intermediate data structures have a lot of overhead (UDSEntry is already 
the definition of "intermediate data structure" and "overhead"; using readdir_r 
instead of readdir has exceptionally little impact next to that) ... he is 
essentially correct: the answer is not threading. it is decreasing the 
operations. such as by kicking UDSEntry to the curb on the ioslave side.

UDSEntry could still use optimization on the client side of ioslave 
relationship, of course ...

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 116787: Always create NETEventFilter (a QWidget subclass) in the main application thread

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116787/
---

(Updated March 19, 2014, 3:40 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, kwin and Martin Gräßlin.


Repository: kwindowsystem


Description
---

When using KWindowInfo from a thread other than the main application thread, 
the application fails on an assert triggered by creating a QWidget in another 
thread. This is due to NETEventFilter being a QWidget. This patch addresses the 
issue by using a QObject to create the NETEventFilter, which is itself first 
moved to the main application thread. 


Diffs
-

  autotests/CMakeLists.txt 073b6dd 
  autotests/kwindowsystem_threadtest.cpp PRE-CREATION 
  src/kwindowsystem.cpp 4c17baa 
  src/kwindowsystem_p_x11.h 75f3028 
  src/kwindowsystem_x11.cpp 95c396b 

Diff: https://git.reviewboard.kde.org/r/116787/diff/


Testing
---

Used in the Sprinter Windows plugin and works lovely to find desktops, windows, 
etc.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 116787: Always create NETEventFilter (a QWidget subclass) in the main application thread

[Aaron J. Seigo]

> On March 17, 2014, 8:33 p.m., Thomas Lübking wrote:
> > src/kwindowsystem_x11.cpp, line 102
> > <https://git.reviewboard.kde.org/r/116787/diff/5/?file=254570#file254570line102>
> >
> > Is this compensated anywhere?
> > Is it actually an issue (since the constructor should operate in the 
> > GUI thread)?
> 
> Aaron J. Seigo wrote:
> I commented this out to see if it resulted in any difference in function 
> with Qt5; I didn't notice any difference, but did mean to comment it back in 
> as I did not do a full examination of the code behind the scenes. I've put 
> this line back in here locally.
> 
> Martin Gräßlin wrote:
> I'm pretty sure it's not needed. We get the root window without having 
> called qApp->desktop(). Quite sure about it, because I migrated 
> QX11Info::appRootWindow() away from doing exactly what this line of code was 
> doing.

Ah, great. When I tested without that line, everything seemed to work exactly 
as expected. I just didn't have time to delve into the code further to confirm 
the anecdotal evidence ;)

So... can I push? :)


- Aaron J.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116787/#review53254
---


On March 17, 2014, 9:09 a.m., Aaron J. Seigo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116787/
> ---
> 
> (Updated March 17, 2014, 9:09 a.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> When using KWindowInfo from a thread other than the main application thread, 
> the application fails on an assert triggered by creating a QWidget in another 
> thread. This is due to NETEventFilter being a QWidget. This patch addresses 
> the issue by using a QObject to create the NETEventFilter, which is itself 
> first moved to the main application thread. 
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 073b6dd 
>   autotests/kwindowsystem_threadtest.cpp PRE-CREATION 
>   src/kwindowsystem.cpp 4c17baa 
>   src/kwindowsystem_p_x11.h 75f3028 
>   src/kwindowsystem_x11.cpp 95c396b 
> 
> Diff: https://git.reviewboard.kde.org/r/116787/diff/
> 
> 
> Testing
> ---
> 
> Used in the Sprinter Windows plugin and works lovely to find desktops, 
> windows, etc.
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 116787: Always create NETEventFilter (a QWidget subclass) in the main application thread

[Aaron J. Seigo]

> On March 17, 2014, 8:33 p.m., Thomas Lübking wrote:
> > src/kwindowsystem_x11.cpp, line 102
> > <https://git.reviewboard.kde.org/r/116787/diff/5/?file=254570#file254570line102>
> >
> > Is this compensated anywhere?
> > Is it actually an issue (since the constructor should operate in the 
> > GUI thread)?

I commented this out to see if it resulted in any difference in function with 
Qt5; I didn't notice any difference, but did mean to comment it back in as I 
did not do a full examination of the code behind the scenes. I've put this line 
back in here locally.


- Aaron J.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116787/#review53254
-------


On March 17, 2014, 9:09 a.m., Aaron J. Seigo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116787/
> ---
> 
> (Updated March 17, 2014, 9:09 a.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> When using KWindowInfo from a thread other than the main application thread, 
> the application fails on an assert triggered by creating a QWidget in another 
> thread. This is due to NETEventFilter being a QWidget. This patch addresses 
> the issue by using a QObject to create the NETEventFilter, which is itself 
> first moved to the main application thread. 
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 073b6dd 
>   autotests/kwindowsystem_threadtest.cpp PRE-CREATION 
>   src/kwindowsystem.cpp 4c17baa 
>   src/kwindowsystem_p_x11.h 75f3028 
>   src/kwindowsystem_x11.cpp 95c396b 
> 
> Diff: https://git.reviewboard.kde.org/r/116787/diff/
> 
> 
> Testing
> ---
> 
> Used in the Sprinter Windows plugin and works lovely to find desktops, 
> windows, etc.
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Releasing Deprecated modules and Tier 4 Definition

[Aaron J. Seigo]

On Saturday, March 15, 2014 16:59:54 Kevin Ottens wrote:
>  we would be releasing 5.0 with modules which would be immediately 
>  deprecated.

the modules that i am aware of include:

* kde4support
* khtml
* kjs
* kjsembed
* krunner

there are a few others that are imho borderline cases (kmediaplayer as one 
example) but the above are the clear cases.

as a side note, could we also rename kde4support to kdelibs4support? we've 
spent a lot of time and effort to stamp out the (broken) "kde4" naming.

> 2) Release the deprecated content as a separate product

this is the best option imho because:

* it keeps "Frameworks" clear in scope and focus: the set of recommended and 
actively developed APIs. this should be useful for app developers and 
packagers alike.

* it allows dropping the deprecated modules at a future time without worrying 
about what it means for "Frameworks" or how to communicate the change clearly

* it sets a clear precedent for KF6 as to how to handle frameworks that become 
deprecated during KF5's life

> 3) Roll all the deprecated modules into KDE4Support

-1, for the reasons dfaure noted.

> 4) Announce the deprecated modules will only be released three times

+1

after those releases, the core team would no longer need to:

* manage bug reports filed against those modules
* coordinate the release of those modules (which at a minimum means building 
and testing prior to release)

public communication will be clearer; instead of a vague "deprecated, 
eventually dropped" there is a clear horizon to communicate

developers will have a soft deadline for when they should want to complete 
porting away from these modules. it's only a soft deadline as the code will 
obviously still exist and someone could pick up maintenance. we all know that 
things tend to happen faster / more reliably when there is deadline.

if there is interest in maintaining the modules past that date, those 
interested can take on the effort after this point.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Releasing Deprecated modules and Tier 4 Definition

[Aaron J. Seigo]

On Sunday, March 16, 2014 11:43:12 David Faure wrote:
> There's a middle ground between "actively maintain" and "we made it
> completely impossible to even fix one bug".

at which point the person(s) doing the bug fixes can take on the project an 
handle releases. it is free software: people can fork and/or adopt. (i'll put 
more thoughts on this in my response to kevin's mail)

>  ... or to follow a change in ECM, or whatever.

if ECM changes 1.5 years (or whatever) after the initial KF5 release in a way 
that requires changing the build files, wouldn't that be a bug in ECM? at some 
point ECM needs to provide stability.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Why is KPluginInfo (const KService::Ptr service) deprecated?

[Aaron J. Seigo]

On Monday, March 17, 2014 09:55:12 Martin Gräßlin wrote:
> The documentation tells me that I should use KPluginLoader instead and use
> build the plugin with json meta data. But all the services I generate the
> plugin info for are not binary or only may be binary. Example usage from
> KWin is populating a KPluginSelector with all KWin scripts, which are
> obviously not binary plugins I could compile metadata into.

The metadata for plugins could also be delivered in the form of json; 
currently they use .desktop files, which is due to the "everything uses 
.desktop files" pattern of the past. Now plugins come with json metadata, and 
there's no reason the same couldn't be done for bundles of scripts.

I would imagine that would make the future plugin registration system (that 
dfaure is going to be working on iirc) simpler: instead of having to support 
both .desktop and json data when adding a plugin to the cache, it could simply 
accept json data.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 116787: Always create NETEventFilter (a QWidget subclass) in the main application thread

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116787/
---

(Updated March 17, 2014, 9:09 a.m.)


Review request for KDE Frameworks, kwin and Martin Gräßlin.


Changes
---

one more time ;) realized that the thread pool may not finish quickly enough 
and the next test could be run, defeating the purpose of using a QRunnable 
here. so now the autotest waits for the global thread pool to be done.


Repository: kwindowsystem


Description
---

When using KWindowInfo from a thread other than the main application thread, 
the application fails on an assert triggered by creating a QWidget in another 
thread. This is due to NETEventFilter being a QWidget. This patch addresses the 
issue by using a QObject to create the NETEventFilter, which is itself first 
moved to the main application thread. 


Diffs (updated)
-

  autotests/CMakeLists.txt 073b6dd 
  autotests/kwindowsystem_threadtest.cpp PRE-CREATION 
  src/kwindowsystem.cpp 4c17baa 
  src/kwindowsystem_p_x11.h 75f3028 
  src/kwindowsystem_x11.cpp 95c396b 

Diff: https://git.reviewboard.kde.org/r/116787/diff/


Testing
---

Used in the Sprinter Windows plugin and works lovely to find desktops, windows, 
etc.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 116787: Always create NETEventFilter (a QWidget subclass) in the main application thread

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116787/
---

(Updated March 17, 2014, 9:04 a.m.)


Review request for KDE Frameworks, kwin and Martin Gräßlin.


Changes
---

small change in method name to correct the test


Repository: kwindowsystem


Description
---

When using KWindowInfo from a thread other than the main application thread, 
the application fails on an assert triggered by creating a QWidget in another 
thread. This is due to NETEventFilter being a QWidget. This patch addresses the 
issue by using a QObject to create the NETEventFilter, which is itself first 
moved to the main application thread. 


Diffs (updated)
-

  autotests/CMakeLists.txt 073b6dd 
  autotests/kwindowsystem_threadtest.cpp PRE-CREATION 
  src/kwindowsystem.cpp 4c17baa 
  src/kwindowsystem_p_x11.h 75f3028 
  src/kwindowsystem_x11.cpp 95c396b 

Diff: https://git.reviewboard.kde.org/r/116787/diff/


Testing
---

Used in the Sprinter Windows plugin and works lovely to find desktops, windows, 
etc.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 116787: Always create NETEventFilter (a QWidget subclass) in the main application thread

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116787/
---

(Updated March 17, 2014, 9 a.m.)


Review request for KDE Frameworks, kwin and Martin Gräßlin.


Changes
---

fixed nitpicks, added a(n auto) unit test, fixed when called from main thread 
and added some documentation.


Repository: kwindowsystem


Description
---

When using KWindowInfo from a thread other than the main application thread, 
the application fails on an assert triggered by creating a QWidget in another 
thread. This is due to NETEventFilter being a QWidget. This patch addresses the 
issue by using a QObject to create the NETEventFilter, which is itself first 
moved to the main application thread. 


Diffs (updated)
-

  autotests/CMakeLists.txt 073b6dd 
  autotests/kwindowsystem_threadtest.cpp PRE-CREATION 
  src/kwindowsystem.cpp 4c17baa 
  src/kwindowsystem_p_x11.h 75f3028 
  src/kwindowsystem_x11.cpp 95c396b 

Diff: https://git.reviewboard.kde.org/r/116787/diff/


Testing
---

Used in the Sprinter Windows plugin and works lovely to find desktops, windows, 
etc.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 116787: Always create NETEventFilter (a QWidget subclass) in the main application thread

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116787/
---

(Updated March 14, 2014, 10:36 a.m.)


Review request for KDE Frameworks, kwin and Martin Gräßlin.


Changes
---

this update introduces two significant changes: NETEventFilter is no longer a 
QWidget and xcb methods are used directly instead; the static object behind 
KWindowSystem::self() is forced to the main GUI thread.


Repository: kwindowsystem


Description
---

When using KWindowInfo from a thread other than the main application thread, 
the application fails on an assert triggered by creating a QWidget in another 
thread. This is due to NETEventFilter being a QWidget. This patch addresses the 
issue by using a QObject to create the NETEventFilter, which is itself first 
moved to the main application thread. 


Diffs (updated)
-

  src/kwindowsystem.cpp 4c17baa 
  src/kwindowsystem_p_x11.h 75f3028 
  src/kwindowsystem_x11.cpp 95c396b 

Diff: https://git.reviewboard.kde.org/r/116787/diff/


Testing
---

Used in the Sprinter Windows plugin and works lovely to find desktops, windows, 
etc.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 116787: Always create NETEventFilter (a QWidget subclass) in the main application thread

[Aaron J. Seigo]

> On March 13, 2014, 10:31 p.m., Martin Gräßlin wrote:
> > did you try to make it not use QWidget anymore? There shouldn't be a reason 
> > why it is QWidget.
> 
> Aaron J. Seigo wrote:
> No; I can try it as a non QWidget ... let's see how that goes.

So, challenges a plenty :)

First, this line in NETEventFilter's ctor:

(void) qApp->desktop();

Can result in the creation of a window, which triggers an assert. Removing that 
line (besides probably breaking root window event handling in some cases) 
simply pushes the problem to kwindowinfo_x11.cpp:54:

m_info.reset(new NETWinInfo(QX11Info::connection(), _win, 
QX11Info::appRootWindow(), props, 2));

appRootWindow() does the same if the root window is not yet instantiated. When 
it is, then there's no problem and KWindowInfo works fine from non-main-app 
threads. So qApp->desktop() *must* be called from the main app thread somewhere.

Second: I'm assuming that XFixesSelectSelectionInput requires an actual window 
and not just an xcb_window_t that doesn't have a window associated with it. If 
that assumption is correct (and I don't know for certain, but some simple 
testing I did seems to support that) then we still have the same issue: Qt 
asserts when xcb_create_window is called outside the main application thread. 
So the use of XFixesSelectSelectionInput becomes a sticking point.

Thirdly, there is the complication that KWindowSystemPrivateX11 is created in 
KWindowSystemStaticContainer which is, as the name suggests, a global static 
object. Care needs to be paid that no event handling is assumed to happen in 
the thread of its creation. Reading the code, it appears that all event 
handling (and therefore requirement for an event loop) happens in the thread 
that KWindowSystem lives in .. which is fine except for KWindowSystem::self() 
which should be as easy to fix as adding this to KWindowSystemStaticContainer():

 kwm.moveToThread(QCoreApplication::instance()->thread());

ensuring that the static KWindowSystem always lives in the long-lived main 
application thread. This is exactly the sort of case I wrote about in my blog 
recently: creating internal (to the library) static objects with application 
lifespan which require an event loop in whatever thread happens to call them is 
broken. So that at least needs doing. I have patched this here and it works 
well.

So there is code in NETEventFilter that *must* be run in the man app thread 
even without the use of QWidget. .. updated patch coming. 


- Aaron J.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116787/#review52914
-------


On March 13, 2014, 6:44 p.m., Aaron J. Seigo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116787/
> ---
> 
> (Updated March 13, 2014, 6:44 p.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> When using KWindowInfo from a thread other than the main application thread, 
> the application fails on an assert triggered by creating a QWidget in another 
> thread. This is due to NETEventFilter being a QWidget. This patch addresses 
> the issue by using a QObject to create the NETEventFilter, which is itself 
> first moved to the main application thread. 
> 
> 
> Diffs
> -
> 
>   src/kwindowsystem_p_x11.h 75f3028 
>   src/kwindowsystem_x11.cpp 95c396b 
> 
> Diff: https://git.reviewboard.kde.org/r/116787/diff/
> 
> 
> Testing
> ---
> 
> Used in the Sprinter Windows plugin and works lovely to find desktops, 
> windows, etc.
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 116787: Always create NETEventFilter (a QWidget subclass) in the main application thread

[Aaron J. Seigo]

> On March 13, 2014, 10:31 p.m., Martin Gräßlin wrote:
> > did you try to make it not use QWidget anymore? There shouldn't be a reason 
> > why it is QWidget.

No; I can try it as a non QWidget ... let's see how that goes.


- Aaron J.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116787/#review52914
---


On March 13, 2014, 6:44 p.m., Aaron J. Seigo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116787/
> ---
> 
> (Updated March 13, 2014, 6:44 p.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> When using KWindowInfo from a thread other than the main application thread, 
> the application fails on an assert triggered by creating a QWidget in another 
> thread. This is due to NETEventFilter being a QWidget. This patch addresses 
> the issue by using a QObject to create the NETEventFilter, which is itself 
> first moved to the main application thread. 
> 
> 
> Diffs
> -
> 
>   src/kwindowsystem_p_x11.h 75f3028 
>   src/kwindowsystem_x11.cpp 95c396b 
> 
> Diff: https://git.reviewboard.kde.org/r/116787/diff/
> 
> 
> Testing
> ---
> 
> Used in the Sprinter Windows plugin and works lovely to find desktops, 
> windows, etc.
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 116787: Always create NETEventFilter (a QWidget subclass) in the main application thread

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116787/
---

Review request for KDE Frameworks, kwin and Martin Gräßlin.


Repository: kwindowsystem


Description
---

When using KWindowInfo from a thread other than the main application thread, 
the application fails on an assert triggered by creating a QWidget in another 
thread. This is due to NETEventFilter being a QWidget. This patch addresses the 
issue by using a QObject to create the NETEventFilter, which is itself first 
moved to the main application thread. 


Diffs
-

  src/kwindowsystem_p_x11.h 75f3028 
  src/kwindowsystem_x11.cpp 95c396b 

Diff: https://git.reviewboard.kde.org/r/116787/diff/


Testing
---

Used in the Sprinter Windows plugin and works lovely to find desktops, windows, 
etc.


Thanks,

Aaron J. Seigo

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: c++ plugins with Frameworks 5

[Aaron J. Seigo]

On Tuesday, March 4, 2014 11:52:11 David Faure wrote:
> Unless someone has a plan for it, my suggestion would be, let's model it
> after the upcoming app desktop cache, i.e. update it at install time. I'll

This sounds perfect.

In that vein, I did do some measuring of what takes time, and it really is 
nothing surprising. Extracting and parsing the json from the library files 
themselves is fast enough. My 3 year old laptop parse 1000+ plugins' info per 
second, complete with a few dozen translations in each.

What is slow is actually getting the data off disk when the OS’s disk cache is 
cold. SSDs obviously incur less penalty here and rotating disks more. The 
performance on a typical rotating laptop HDD drops to ~50-70 plugins per 
second. Not horrible, but also not awesome.

> But meanwhile this confirms that we won't need .desktop files, so Aaron, you
> can go ahead

Great :)

If we can eventually get the i18n process using the json directly, then we can 
get rid of the .desktop files altogether. Until then, however, we’ll need to 
keep them. So currently my game plan is this:

* create a small QtCore-only tool that transforms a .desktop file into a .json 
file, without translations

* adapt the tool I already have written that updates translations from 
.desktop files into the json files; it will remain QtCore only

* change the kservice_desktop_to_json cmake macro to use that tool

* adjust KPluginInfo to read the metadata correctly (inc. i18n)

once the app desktop cache is in place, then we can stop installing the 
.desktop files.

once the i18n process has been migrated to use the json files directly, then we 
can remove the .desktop files from the repositories.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: c++ plugins with Frameworks 5

[Aaron J. Seigo]

On Monday, March 3, 2014 12:35:40 Alex Merry wrote:
> Could we not extend scripty to do the same for the .json files?  We

most problably .. but I haven’t looked at scripty so do not know how much work 
it would be, and I try not to interfere with i18n workflows. but yes, this 
should be possible.

> would want a standard way of representing translatable and translated
> strings, and some library support somewhere to extract the correct
> version of the string for the current locale.

for a general purpose solution, yes. for the ability to represent plugin data, 
this can be encapsulated in e.g. KPluginInfo and made purpose-specific.

currently what i’m doing in sprinter plugin json is that there is a block of 
translated strings for each language with translations. e.g.:

“de”: {
“Name”: “..”
“Comment”: “..”
}

so when the local is set to use “de” it grabs these values. checking for 
values in the language block before returning “untranslated” strings is easy 
enough, and what kconfig does right now for INI style files.

i don’t know if we need to make this completely generic, though. it’s really 
only those two fields in the plugin info that ever get shown to the user.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

c++ plugins with Frameworks 5

[Aaron J. Seigo]

hello,

over the weekend i wrote up a small tool that does what is needed for 
libsprinter, which is a Qt5 library (no frameworks deps), to harvest 
translations from .desktop files into .json files for inclusion in its plugins.

one decision i made while writing this was that .desktop files are not of 
interest to libsprinter: they are there only to collect translations. as such, 
no .desktop files are actually installed, and all information is kept in the 
json that gets compiled into the plugin itself.

it uses a block in the json metadata that looks like this for application-
specific information:

"Sprinter": {
"GeneratesDefaultMatches": true,
"MatchSources": [
"FromDesktopShell"
],
"MatchTypes": [
"WindowType",
"DesktopType"
]
}

and the plugin info is kept in a separate PluginInfo block, which replaces 
what was in the .desktop file. translations added to the .desktop files are 
merged into the json files with a small tool that can be run by the build 
system.

pros: the data is only installed in one place; it is easy for applications to 
extend the json
cons: there is both a .json and a .desktop file in the source repository


looking at how Frameworks 5 based applications with plugins are handling this, 
a slightly different approach is being taken: the .desktop files are still 
being 
installed and ksycoca is being queried for them. at the same time, this data 
is duplicated in the plugin’s json data as well so that can be used at runtime 
to query information about a specific plugin.

pros: easy for migration
cons: the data is duplicated in two places on disk with no clear sign of which 
is the preferred means of accessing the data; application extensions to the 
json data is not well supported (in the above json snippet, all three entries 
would become strings which would need further parsing once retrieved from the 
plugin’s metaData())

really, this is a minimal-effort adjustment to accommodate the use of json in 
plugins without altering anything else. 

while this makes sense for applications in transition from 4.x to 5.x, is this 
how new applications are also expected to be written?

do we want .desktop files as well as generated .json?

if so, for applications that wish to put additional data in the json files, is 
it expected that they continue to do this by adding more “X-Foo” entries to 
the [Desktop Entry] group in their .desktop files? (as opposed to providing 
actual json)

whatever the decision is, more work will be needed: 

* desktoptjson does not support translations currently (made difficult by use 
of 
KConfig) and does not support application extensions to the json very well, 
both of which need attention. 
* in the json-only approach is adopted, KPluginInfo and possibly even ksycoca 
would need adjusting (the latter to preserve the ability to do KService-based 
queries)

i’m willing and able to do the latter if the decision is made to support json-
only. (i’m not willing to do that work if it isn’t, as i prefer not to waste 
my time :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KWindowSystem

[Aaron J. Seigo]

On Monday, March 3, 2014 10:34:40 Martin Gräßlin wrote:
> It's an incorrect usage caused by XLib not being thread safe. We just
> couldn't use KWindowInfo in the runner thread. With the switch to xcb
> that's no longer the case and it would be better to have just a QList
> and fetch the data in the thread when it's needed (and have it up to date).

that would be another way to do it in this case, yes. not as efficient, 
however, 
particularly as the terms being matched against are entirely unlikely to 
change during query.

> > it is fine to put in the apidox that a default-constructed KWindowInfo
> > object is invalid and will not provide useful information.
> 
> It's wasn't just invalid, it was crashing.

that can not be hard to fix, though? a quick glance at kwindowinfo_x11.cpp 
suggests that having an invalid WId and short-circuting the constructor in 
that case would resolve that.

> Thanks, but as said I don't think that this is a valid use-case for
> KWindowInfo due to the nature of this class. Putting it into container
> classes means that we encourage incorrect usage as KWindowInfo is only a
> snapshot of the current state and doesn't update. So any usage in
> containers means the information is already potentially outdated.

this is true even with the usage you see as being proper. in the time it takes 
to get the window info and then do whatever processing is relevant, the window 
information may also have changed. that’s just the nature of such async data.

it also means that, e.g., sorting a list of window information would now mean:

* create a collection mapping the window ID to the property to sort on (e.g. 
name)
* iterate over all windows, fetching window IDs, populating the collection
* sort
* iterate the collection in order fetching the window info again
* use

previously, the way to do this would be:

* create a collection of KWindowInfo objects
* sort
* use

given the “it crashes” fix should be trivial, why should the above be 
disallowed? API should not try and force developers to do the “right” thing, 
as that implies we know what that is in all cases.

> > and yes, the preservation of source compatibility is important, especially
> 
> > as there is no compelling reason to break it in this case. see:
> Yes it's an important case, but it's still better to break SC than have code
> which just crashes or encourages incorrect usage of the API.

crashes should be easily resolved; we disagree on the definition of ‘proper 
usage’ in this case.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KWindowSystem

[Aaron J. Seigo]

On Monday, March 3, 2014 08:35:16 you wrote:
> The reason to remove the default ctor was that if one used it, KWindowInfo
> would crash on first usage as it was completely broken. It was documented to
> be there for usages in container classes but lxr doesn't show any usage and

kde-workspace/plasma/generic/runners/windows/windowsrunner.h:68

QHash m_windows;

the copyright on the file is yours, actually :)

> IMHO there cannot be a useful usage of KWindowInfo in container classes as
> KWindowInfo doesn't update the information.

use of containers is not always about long-term storage. they can be used to 
conveniently sort sets of objects, to populate a set of objects in one part of 
the code and then process them in another, etc.

it is fine to put in the apidox that a default-constructed KWindowInfo object 
is invalid and will not provide useful information. there are many other 
classes that have similar constructors, though KWindowInfo does not provide a 
way to turn an invalid object into a valid one (which is also fine imho.

> If you have a valid use cases for container classes please bring it to
> frameworks mailing list for discussion whether we should restore SC.

done :)

and yes, the preservation of source compatibility is important, especially as 
there is no compelling reason to break it in this case. see:

https://community.kde.org/Frameworks/Epics/Splitting_kdelibs#Strive_for_Source_Compatibility


-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: desktoptojson and kservice

[Aaron J. Seigo]

On Friday, February 21, 2014 15:45:25 Sebastian Kügler wrote:
> Including the translations would mean that all translations have to be there
> at compile-time, which would mean that runtime changes to translations
> aren't picked up.

... are there such things as “runtime changes to translations”?

yes, they currently are in non-compiled files (.desktop INI-style files), but 
do 
those get updated on people’s systems without the plugin they relate to also 
being part of the update?

-- 
Aaron J. Seigo
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: desktoptojson and kservice

[Aaron J. Seigo]

On Friday, February 21, 2014 13:57:07 Kevin Krammer wrote:
> If we want to be able to parse .desktop files without KConfig, we need a
> desktop file parser.

valuable to know, indeed. thanks for sharing your experience, saves me some 
effort.

> The Razor-Qt and LxQt people might have one already.

yeah, i was wondering the same thing ... will take a look around.

a long while ago i had found such a class on the Internet. perhaps qt3 days? 
hm.. google, here i come.

-- 
Aaron J. Seigo
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: desktoptojson and kservice

[Aaron J. Seigo]

Aaron J. SeigoOn Friday, February 21, 2014 13:41:29  wrote:
> What are the plans for i18n in desktoptojson? It currently does not catch
> any translations in the .desktop file.

actually, i think it might be worse than “does not catch any translations”: if 
it does what i suspect it does, it takes the translated versions of the 
strings of whatever locale the user currently has set. so if someone using 
german as their language, whenever they run desktoptojson they’ll get german 
strings rather than the default english ones.

has anyone tested this with non-’c’ locales?

-- 
Aaron J. Seigo
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

desktoptojson and kservice

[Aaron J. Seigo]

Hi ...

So, the next fun thing I’ve run into is desktoptojson. Currently in Sprinter 
I’m just hand writing the json files for the plugins. However, in its TODO file 
is this:

* Metadata
 * localization/i18n in plugin metadata (name, description)
* license
* author information (name, email)
* icon

This is all information we currently have in .desktop files (along with the 
infrastructure and people using it to translate them ..) so it makes sense to 
stick to that and simply translate to json from them at build time. 

Which brought me to desktoptojson .. and several questions:

While there are shortcomings in QSettings for parsing random INI files, I don’t 
think any of these apply to the .desktop files we use. Would there be any 
objection / reason against dropping the use of KConfig in desktop and moving to 
QSettings, turning it into a Qt only application? 

Assuming a move to QSettings, could it then be moved out of the kservice 
repository, which is tier3, and into something more tier1? In a perfect world 
this would come with ecm, even, or something similarly build-tools related. 
Any suggestions there?

What are the plans for i18n in desktoptojson? It currently does not catch any 
translations in the .desktop file.

Do we really want json files that look like this:

{
"Comment": "List and switch between desktop activities",
"Icon": "preferences-activities",
"Name": "Activities",
"Type": "Service",
"X-KDE-Library": "krunner_activities",
"X-KDE-PluginInfo-Author": "Plasma Team",
"X-KDE-PluginInfo-Email": "plasma-de...@kde.org",
"X-KDE-PluginInfo-EnabledByDefault": true,
"X-KDE-PluginInfo-License": "LGPL",
"X-KDE-PluginInfo-Name": "org.kde.activities",
"X-KDE-PluginInfo-Version": "1.0",
"X-KDE-ServiceTypes": [
"Plasma/Runner"
],
"X-Plasma-AdvertiseSingleRunnerQueryMode": "true"
}

While easier for KPluginInfo perhaps (as it can just use the same keys when 
reading from .desktop files), this is imho really quite messy and makes it 
precarious to use for applications that may want to use KPluginInfo (or other 
tools) with json files with other data in them. Yes, we could continue to use 
“X-Foo-” prefixes .. but this is very ugly json. Litmus test: would anyone 
construct the above when told to come up with a json format to hold the above 
information if they didn’t start with those .desktop files?

What would be nicer imho is sth like:

{
"org.kde.PluginInfo": {
"Description" : {
"c" {
"Name": "Activities",
"Comment": "List and switch between desktop activities",
},
"de": {
"Name": "Aktivitäten",
"Comment": "Aktivitäten anzeigen und zwischen ihnen 
umschalten"
}
},
"Details": {
"Author": "Plasma Team",
"Email": "plasma-de...@kde.org",
"EnabledByDefault": true,
"License": "LGPL",
"Name": "org.kde.activities", 
"Version": "1.0",
"Icon": "preferences-activities",
},
"ServiceTypes": [ "Plasma/Runner" ]
},
Sprinter: {
"AdvertiseSingleRunnerQueryMode": "true"
}
}

Note that this removes a few entries such as X-KDE-Library, which isn’t used i 
KPluginInfo at all now, and "Type": “Service”, which is now redundant. It also 
introduces translations (though that means a bit more overhead as they will be 
parsed at runtime and stored in memory as part of the QPluginLoader process), 
and keeps those separate from non-translated items (Description vs Details .. 
best names I could come up with on the spot, not necessarily the best ones 
possible). It wraps it all up in a “org.kde.PluginInfo” block so that it 
doesn’t interfere with other tools.

it would be easy to augment desktoptojson to take data from other files (INI or 
json) passed in on the command line to merge them into the final product.

thoughts?

-- 
Aaron J. Seigo
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: using KDBusConnectionPool in libraries

[Aaron J. Seigo]

On Friday, February 21, 2014 08:30:02 Kevin Ottens wrote:
> Hello,
> 
> On Thursday 20 February 2014 20:03:20 Aaron J. Seigo wrote:
> > I ran into an interesting situation the other day with libkactivities: it
> > uses KDBusConnectionPool to create connections in a thread-safe manner.
> > KDBusConnectionPool creates a connection in whatever thread it happens to
> > be executed from. In libkactivities this creates a problem as a singleton
> > that is internal to the library uses KDBusConnectionPool .. so whatever
> > thread it happens to be called from first: that’s the thread it gets its
> > QDBusConnection object put into.
> 
> Well, isn't the problem that this singleton should be thread-safe or thread-
> local in the first place?

Thread local is not really an option (as explained in my original email, there 
is no guarantee that the thread the object is created in continues to exist or 
even has an event loop in it).

Thread safe and always living in the main app thread would be an option, 
however. Or living in its own thread, but I’d reserve that for functionality 
that is blocking (processing intensive, relies on synchronous calls, etc.) and 
which would be vital enough to an application’s value that it justifies another 
thread in the system.

> > Of course, if that thread has no event loop and/or exits, then things stop
> > working as expected. In the case of libkactivities, things just stop
> > working, period.
> > 
> > Either a better solution needs to be found for KDBusConnectionPool or a
> > warning should be placed prominently in the apidox about this “gotcha” 
and
> > perhaps it should be completely avoided in other frameworks where it is
> > impossible to know the threading model of the application that will be
> > using it.
> 
> Yes, adding a big fat warning sounds fair to me

Ok, I’ll work something together.

> > Or, I suppose, we could invent a policy that applications should do all
> > dbus related activities in a specific thread .. though that can be
> > difficult to know as dbus calls are often hidden within libraries.
> > 
> > .. or, does anyone have a pointer to what exactly in QDBusConnection is
> > not
> > thread safe?
> 
> It wasn't (Qt4 times)... I have no idea if that's still the case today. It
> could be that this class isn't needed anymore.

According to the docs, the only non-thread-safe call is 
QDBusConnection::sender, which is apparently kept for compatibility only since 
one should use QDBusContext in such cases instead.

Whether libdbus under it is thread safe (or used in a way that makes it thread 
safe), I don’t know. I’ll follow up with Thiago. It would be rather nice to 
see this class go away if possible.

> As for what exactly is not thread safe in QDBusConnection I don't remember.
> Since it was highlighted by Nepomuk at the time and forced upon us by its
> API (almost behind us too) Vishesh or Sebastian should know more (adding
> them in CC).

That would be good information to have .. looking forward to their response.

-- 
Aaron J. Seigo
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

using KDBusConnectionPool in libraries

[Aaron J. Seigo]

Hi..

I ran into an interesting situation the other day with libkactivities: it uses 
KDBusConnectionPool to create connections in a thread-safe manner. 
KDBusConnectionPool creates a connection in whatever thread it happens to be 
executed from. In libkactivities this creates a problem as a singleton that is 
internal to the library uses KDBusConnectionPool .. so whatever thread it 
happens to be called from first: that’s the thread it gets its QDBusConnection 
object put into.

Of course, if that thread has no event loop and/or exits, then things stop 
working as expected. In the case of libkactivities, things just stop working, 
period.

Either a better solution needs to be found for KDBusConnectionPool or a 
warning should be placed prominently in the apidox about this “gotcha” and 
perhaps it should be completely avoided in other frameworks where it is 
impossible to know the threading model of the application that will be using 
it. Or, I suppose, we could invent a policy that applications should do all 
dbus related activities in a specific thread .. though that can be difficult to 
know as dbus calls are often hidden within libraries.

.. or, does anyone have a pointer to what exactly in QDBusConnection is not 
thread safe?

-- 
Aaron J. Seigo
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 115225: Add runtime platform support to KWindowInfo

[Aaron J. Seigo]

> On Jan. 22, 2014, 3:22 p.m., Aaron J. Seigo wrote:
> > Looks quite nice, other than the missing win/mac support which you can do 
> > little about at this point.
> > 
> > Use of the explicitly shared pointer approach is a simple and nice 
> > performance booster when passing these objects around (as these tend to). 
> > +1 for that. 
> > 
> > I took a moment to ponder if there is enough duplication of window info 
> > objects for the same window ID to make it worthwhile to have a global hash 
> > of winid to dptrs for re-use between separately created instances (and not 
> > just copies of the same instance). In the desktop shell there is at least 
> > one per window for the taskbar and the pager (assuming the pager is active, 
> > anyways); the window runner usually isn't loaded in the shell directly but 
> > were it to be that'd be a third instance. So the highest duplication likely 
> > to be seen is probably 3 and so it isn't worth it.
> > 
> > It's a bit of a shame that the runtime detection requires a dptr full of 
> > virtuals; this is probably only required on Linux/UNIX where there are 
> > multiple window info protocols (xcb, wayland, ..) so sucks for the other 
> > platforms, but I also suspect that this will never actually be used in 
> > practice even on Linux as one is either going to be in a Wayland or X11 (or 
> > whatever) session and switching between them requires logging in/out. It's 
> > private API so it can be changed later if this were ever to actually show 
> > up on in runtime performance which I seriously doubt it will. (I can 
> > imagine sth horrible like a struct/union which has a pointer to an xcb and 
> > a wayland implementation, both of which have the same literal API for 
> > consistency but no base class and then a macro that calls whichever one is 
> > actually instantiated: "#define callimpl(method) if (d->xcb) { 
> > d->xcb->method(); } else { d->wayland->method(); }" win/mac/etc would have 
> > a rather simpler callimpl macro. yes, ugly as 
 #($* but it would get rid of the virtuals and allow win/mac/android/etc to be 
simple compile-time classes without unnecessary runtime detection features .. 
but probably not worth the uglification w/out good justification, which 
currently doesn't exist.
> > 
> > I haven't done anything more than add it to the compile, so I can't mark it 
> > as ShipIt with a clean conscience (as I haven't tested it at runtime), but 
> > the code looks good and the design is reasonable imho.

... or a template class instead of the #define, though that adds a layer of 
function call indirection I bet it could be 100% inlined functions and be both 
fast and non-#define-hackery.


- Aaron J.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115225/#review48044
---


On Jan. 22, 2014, 2:03 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115225/
> ---
> 
> (Updated Jan. 22, 2014, 2:03 p.m.)
> 
> 
> Review request for KDE Frameworks and kdewin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Add runtime platform support to KWindowInfo
> 
> Main idea of this change is to only pick the X11 implementation in case
> that the application is running on the X11 platform. So far it was a
> compile time switch which meant that if compiled with X11 support but
> not running on the X11 platform it would have caused runtime errors.
> 
> To make this possible a KWindowInfoPrivate class with a dummy
> implementation is provided. This is used as d-ptr for KWindowInfo.
> Thus there is one generic implementation and the implementation of
> KWindowInfo is no longer ifdefed for the supported platforms.
> 
> The platform specific code can inherit from KWindowInfoPrivate and
> overwrite the dummy method implementation. KWindowInfoPrivate provides
> a factory method where the platform specific implementation can be
> hooked into. There we can have both compile time and runtime checking.
> If there is no platfom specific implementation available the dummy
> implementation is used.
> 
> NOTE: THIS CHANGE BREAKS THE WINDOWS AND MAC BACKEND!
> 
> Windows and Mac is excluded from build. At the moment they get the
> dummy implementation. Unfortunately I don't have the possibility to
&g

Re: Review Request 115225: Add runtime platform support to KWindowInfo

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115225/#review48044
---


Looks quite nice, other than the missing win/mac support which you can do 
little about at this point.

Use of the explicitly shared pointer approach is a simple and nice performance 
booster when passing these objects around (as these tend to). +1 for that. 

I took a moment to ponder if there is enough duplication of window info objects 
for the same window ID to make it worthwhile to have a global hash of winid to 
dptrs for re-use between separately created instances (and not just copies of 
the same instance). In the desktop shell there is at least one per window for 
the taskbar and the pager (assuming the pager is active, anyways); the window 
runner usually isn't loaded in the shell directly but were it to be that'd be a 
third instance. So the highest duplication likely to be seen is probably 3 and 
so it isn't worth it.

It's a bit of a shame that the runtime detection requires a dptr full of 
virtuals; this is probably only required on Linux/UNIX where there are multiple 
window info protocols (xcb, wayland, ..) so sucks for the other platforms, but 
I also suspect that this will never actually be used in practice even on Linux 
as one is either going to be in a Wayland or X11 (or whatever) session and 
switching between them requires logging in/out. It's private API so it can be 
changed later if this were ever to actually show up on in runtime performance 
which I seriously doubt it will. (I can imagine sth horrible like a 
struct/union which has a pointer to an xcb and a wayland implementation, both 
of which have the same literal API for consistency but no base class and then a 
macro that calls whichever one is actually instantiated: "#define 
callimpl(method) if (d->xcb) { d->xcb->method(); } else { d->wayland->method(); 
}" win/mac/etc would have a rather simpler callimpl macro. yes, ugly as #($*
  but it would get rid of the virtuals and allow win/mac/android/etc to be 
simple compile-time classes without unnecessary runtime detection features .. 
but probably not worth the uglification w/out good justification, which 
currently doesn't exist.

I haven't done anything more than add it to the compile, so I can't mark it as 
ShipIt with a clean conscience (as I haven't tested it at runtime), but the 
code looks good and the design is reasonable imho.

- Aaron J. Seigo


On Jan. 22, 2014, 2:03 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115225/
> ---
> 
> (Updated Jan. 22, 2014, 2:03 p.m.)
> 
> 
> Review request for KDE Frameworks and kdewin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Add runtime platform support to KWindowInfo
> 
> Main idea of this change is to only pick the X11 implementation in case
> that the application is running on the X11 platform. So far it was a
> compile time switch which meant that if compiled with X11 support but
> not running on the X11 platform it would have caused runtime errors.
> 
> To make this possible a KWindowInfoPrivate class with a dummy
> implementation is provided. This is used as d-ptr for KWindowInfo.
> Thus there is one generic implementation and the implementation of
> KWindowInfo is no longer ifdefed for the supported platforms.
> 
> The platform specific code can inherit from KWindowInfoPrivate and
> overwrite the dummy method implementation. KWindowInfoPrivate provides
> a factory method where the platform specific implementation can be
> hooked into. There we can have both compile time and runtime checking.
> If there is no platfom specific implementation available the dummy
> implementation is used.
> 
> NOTE: THIS CHANGE BREAKS THE WINDOWS AND MAC BACKEND!
> 
> Windows and Mac is excluded from build. At the moment they get the
> dummy implementation. Unfortunately I don't have the possibility to
> compile the changes and thus don't dare to touch the code. Fixes from
> the teams are highly appreciated.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt e32a1150a2c190f23ad456ca8218b012c5d71507 
>   src/kwindowinfo.h 171f441ff329a5356ccf560341272199e20c837a 
>   src/kwindowinfo.cpp PRE-CREATION 
>   src/kwindowinfo_p.h PRE-CREATION 
>   src/kwindowinfo_p_x11.h PRE-CREATION 
>   src/kwindowinfo_x11.cpp 865d8bed085e987f97f479ea8aa0e6de8567586f 
> 
> Diff: https://git.reviewboard.kde.org/r/115225/diff/
> 
> 
> Testing
> ---
> 

Re: Framework metadata

[Aaron J. Seigo]

On Thursday, December 19, 2013 21:58:36 Martin Graesslin wrote:
> A standard which is used, is better than a not-used proprietary format. No
> matter what language it is. In the end it's about being machine readable and
> any machine readable language can be converted into another. Don't like
> XML? Write it in MAML and use a generator to convert to XML.

KDE as a community is a mostly focused on creating GUI apps, so it would seem 
natural to me to provide a simple tool to parse, display and allow one to edit 
the standard fields in these DOAP files. Then nobody ever needs to see them or 
write them by hand. This is a single day’s worth of hacking at most.

+1 DOAP, -1 for the bikeshedding .. :)

-- 
Aaron J. Seigo
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Getting ecm files from the ECM package

[Aaron J. Seigo]

On Sunday, November 3, 2013 20:51:57 Alexander Neundorf wrote:
> I wanted to release ECM as fast as possible, since this was one of the main
> points I got from the platform meeting in Randa in June 2011: people want to
> be able to use cmake stuff from KDE without depending on kdelibs.
> Stephen added files, partly without documentation, partly with strange
> interfaces, partly with temporary hacks. Together this has the effect that
> since 2 years ECM is continously not in a releasable state.
> To me this is very frustrating, because this was the goal why I created the
> ECM repository at all.

I can understand your frustration; let’s see if we can find a solution that 
works for everyone, including you and ECM’s primary and largest-by-far user: 
KDE.

> By splitting the "ECM" library into two "libraries", one geared towards KF5,
> and one containing general useful stuff, this issue would be solved.
> General useful stuff would go into ECM, and it would only be accepted if it
> is in releasable state, and this would be released e.g. once per month.
> Hacks needed to make something work in KF5 would go into the other (KF5
> tier0) package.

here is a counter-proposal:

let's use git.

:)

branch the repository:
* create a stable branch (stable) full of the things that are ready for 
release
* create a KF5 branch (frameworks) which can serve as a playground for 
necessary hacks
* if needed, create a third devel branch (stable-next, perhaps use the master 
branch for this)

create some simple workflow policies:
* anything that goes into stable must be merged into frameworks
* anything that goes into stable must be releasable at the time it goes into 
stable
* if/when the frameworks branch reaches a stable point, merge all changes into 
“stable”, or even a “stable-next”
* people working on frameworks 5 build using the frameworks branch

form a release strategy:
* release the stable branch right now as ecm 0.1 (or 1.0 .. whatever makes 
sense)
* never release frameworks branch, but instead have as release requirement 
that it is fully merged into stable-next; when that happens remove the 
frameworks branch and all future KDE Frameworks related work would enter 
stable-next

in my mind this allows:

* an immediate release of ecm
* allows KDE to back it rather than have ecm distanced from KDE
* this gives ecm a needed “reference customer”
* this gives KDE a first step into the “we’re a community you can get 
your 
app dev needs from” position
* allows frameworks needs to continue to be worked on in ecm
* avoids further disruptions, like having to think about git submodules in 
frameworks repos

would that work for everyone?

-- 
Aaron J. Seigo
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Keep the Things You Forgot

[Aaron J. Seigo]

hi :)

(i stole the subject without shame. but it seemed appropriate given the topic)

we’ve all gotten out of the habit of communicating with the outside world, 
with some notable exceptions. Martin, in particular, has become an outstanding 
blogger.

the rest of us have been overly quiet. i’m working on getting my 
bloggerificness going again rather than just hiding on irc and g+ ...

i’d like to invite all of you to blog at least once a week about the amazing 
work everyone is doing with Frameworks 5 and Plasma 2. there is SO MUCH 
ACTIVITY in the repositories and the progress is astounding.

it is important that the rest of the KDE (and beyond!) community sees this 
movement too so they know what we’re up to and that we’re still running ahead 
at great speed.

communicating is how we get more contributors, it’s how we get more users, 
it’s how we find more partners, it’s how we remind others to believe as much as 
we do in the things we’re doing.

love ‘n hugs to you all ...

-- 
Aaron J. Seigo
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 111650: Adjust RunnerManager to new ThreadWeaver API using JobPointer

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111650/#review36454
---

Ship it!


Ship It!

- Aaron J. Seigo


On July 23, 2013, 11:23 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111650/
> ---
> 
> (Updated July 23, 2013, 11:23 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Description
> ---
> 
> ThreadWeaver does not use Job* anymore which made the code compile fail and 
> some connects being wrong.
> 
> This patch adjusts RunnerManager to also use QSharedPointer 
> instead of FindMatchJob*. It
> simplifies a few things as we do no longer have to call e.g. qDeleteAll and 
> fixes the incorrect connects.
> 
> The patch also enables C++11 as it uses auto.
> 
> 
> Diffs
> -
> 
>   src/plasma/private/runnerjobs.cpp a012a6c 
>   src/plasma/private/runnerjobs_p.h cf05324 
>   src/plasma/runnermanager.h ebeb029 
>   src/plasma/runnermanager.cpp 46857f4 
> 
> Diff: http://git.reviewboard.kde.org/r/111650/diff/
> 
> 
> Testing
> ---
> 
> compiles, but couldn't run the unit tests (something is broken in my setup)
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 111171: Deprecate (in)active(Title/Text)Color in favor of KColorScheme

[Aaron J. Seigo]

> On June 22, 2013, 10:58 a.m., Thomas Lübking wrote:
> > kdeui/util/kglobalsettings.cpp, line 308
> > 
> >
> > These are the colors for the window titlbar (with ambiguous function 
> > names, though), the default for activeTitleColor used to be #30AEE8 - that 
> > is blue!
> > 
> > Now you want to return the active window background (ie. usually gray) 
> > - iow: enforce the color alignment that once caused the former KWin 
> > maintainer to for the oxygen deco into the new default ozone?
> > 
> > I mean, decorations can still keep their internal color settings (what 
> > was somehow required because the titlebar color was initially happily 
> > removed from the color kcm - instead one could define the button color in 
> > listviews...), but the implication of this change is that the titlbar can 
> > no more be individually configured and *has* to align to the window 
> > background.
> > 
> > -> The RR should clearly state that central color configuration for 
> > titlebars will ultimately be unsupported.
> > 
> > Until then, the change can imo not go in before KF5 anyway, because 
> > it's a very visible behavioral change for users of Laptop, Plastik, BII and 
> > some legacy decorations (you still /can/ compile Quartz and Keramik and 
> > what they all were called) and pot. some distro specific decorations.
> 
> Aleix Pol Gonzalez wrote:
> Are you sure these are being used? because I searched for uses of this 
> API and I didn't see such uses.
> 
> Thomas Lübking wrote:
> KWin reads the setting values by hand in 
> libkdecorations/kdecoration_p.cpp - so actually I was wrong in that aspect: 
> the change in KGlobalSettings will not have a visible impact here - no 
> guarantee for 3rd party decos beyond bespin (esp. on custom config dialogs)
> 
> Not sure whether this makes the change better, since now KWin will no 
> longer reflect what KGlobalSettings reports.
> I'm not sure how important it is to export those values via 
> KGlobalSettings (i guess they existed to use the colors on MDI windows from 
> KStyle), but even while deprecated, their behavior should not change.
> And unless the conclusion is that titlebars shall align to the window 
> content color, the advice should also not be to query that instead. It's 
> better to state "information not avialable" than to give a false information.
> 
> Martin Gräßlin wrote:
> It's worse - KWin does not just read the values by itself, it reads them 
> from kwinrc instead of kdeglobals. So whatever is set in KGlobalSettings: 
> KWin ignores it.
> 
> Thomas Lübking wrote:
> No ;-)
> Welcome to the magics of KConfig.
> I for one have no [WM] entry in kwinrc, it's only in kdeglobal - try 
> grepping it =)
> However, KConfig resolves to kdeglobal for *every* rc unless explicitly 
> told do do not, so 
> 
> kconfig kdeglobal/WM list
> and
> kconfig kwinrc/WM list
> 
> 
> will print the same value (read from kdeglobal) unless you explicitly add 
> a configuration to kwinrc (what's afaics not done anywhere - hopefully ;-)
> 
> Martin Gräßlin wrote:
> wtf?!? I hate magic functionality
> 
> Thomas Lübking wrote:
> Well, yes. The "magic" function here is the open flag:
> 
> http://api.kde.org/4.10-api/kdelibs-apidocs/kdecore/html/classKConfig.html#ad1f23964bbf8c11449e92a2596d15f7e
> 
> One can either omit cascading (system wide files) or globals (kdeglobals)
> Since the color and some WM settings were/are attractive from many 
> points, it was/is reasonable to store them globally.
> 
> Aleix Pol Gonzalez wrote:
> In any case, this seems broken to me.
> 
> If you're really serious about this, we probably should just deprecate 
> the function and advise users to just read the file directly if they want to 
> access this data and move on.
> 
> Thomas Lübking wrote:
> No, it's not broken at all - normal KConfig behavior, ever has been.
> 
> If this shall be deprecated (can't say - i don't need them and don't 
> care) your conclusion is however right:
> the current behavior shall be preserved and the clients be advised to 
> query the setting directly (as kwin does atm) for KF5.
> 
> The "serious" part is that you were about to alter functionality in even 
> frozen kdelibs and esp. imply a wider reaching change: fixed "titlebar == 
> window background" assumption, ie. no titlebar colors - which, given the 
> little war on oxygen back then, probably needs to be openly discussed before.
> 
> Personally, i'd be fine moving the setting to kwin only - as MDI is a bug 
> to begin with and the setting unlikely required by applications then.
> I simply didn't want KWin to receive bugreports "plastik: colors no more 
> supported" (what, as now  figured, would however only happen by withdrawing 
> the color kcm setting on top 

topic request for next week's meeting: build requirements for kf5

[Aaron J. Seigo]

Hi,

Post 4.11, we will be encouraging as many people working on Workspace stuff as 
possible to move over to KF5 (inc plasma-framework). It will be critical to 
have a clear, (accurately) documented and reliable build (and update) recipe.

Given the feedback in the last few weeks on building kf5, it is apparent that 
right now we lack that. So I would like to propose this as a topic for next 
week's Wednesday KF5 irc meeting.

Specifically, I'd like to see if we can find consensus on:

* cmake requirements (version of cmake, e-m-c; how to update those in future)
* qt5 (with a clear recipe that works, including for updating)
* kf5 build commitments  (dependency changes and API change scheduling)

Is that acceptable?

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: please make it easier to hack on frameworks

[Aaron J. Seigo]

On Monday, May 13, 2013 22:56:57 Oswald Buddenhagen wrote:
> however, it seems unlikely that you would get a partial build without
> any error messages because of that.

I have seen then several times myself (both on my system and other people's) 
and the answer is to blow away any old build dir and start again from scratch. 
Very annoying.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: RFC: a replacement for KPluginLoader

[Aaron J. Seigo]

On Saturday, May 4, 2013 17:06:21 David Faure wrote:
> On Monday 29 April 2013 22:19:58 Aaron J. Seigo wrote:
> > template T *create(QObject *parent = 0) const;
> > bool isVersionCompatible(quint32 minVersion, quint32 maxVersion);
> 
> That's the API for the application, the plugin itself will only know "I have
> been created with version number 3", right?

correct.
 
> About KPluginFactory: registerPlugin is always used, it's the only way to
> declare your plugin class to the macro. The reason for that strange api
...
> I wonder if this fits with the "json metadata" Qt5 idea, but maybe it does
> (the metadata could say "keywords=cookies,ssl,bookmarks")

yes, that's what i was thinking as well.

> So yeah, maybe we want to keep using KPluginFactory instead of writing yet
> another plugin framework doing pretty much the same. The version check can
> be added to KPluginFactory, to basically replace the KDE_VERSION_STRING
> based one.

that'd be good ...

> And yes, automatic catalog loading is commented out, but if we ever want to
> re-enable it, better do that in a central location than in every application

agreed

> Overall, I don't have an extremely strong opinion: it's the usual tradeoff
> between "writing something new with less features, so simpler to use" and
> "keeping the existing solution, for maximum source compatibility i.e.
> reduced porting effort".

If we keep the current solution thenwhy deprecate it? Simply keeping what is 
there would be the 
best thing from an app developer's POV.

The *only* reason I'm touching this item is because the class is marked as 
deprecated. That 
means it likely will not be in minimal builds (e.g. ones targeting small 
devices) and will one day 
disappear on us.

If it is deprecated then we need a replacement and that means something that 
uses 
QPluginLoader and that means something based on QObject.

So I don't see any room for tradeoff here at all. We either keep what we have 
now or we 
deprecate it and create a functional replacement.

> When you talked to Thiago, was there any talk about adding a macro like the
> one you mention, to Qt itself?

He does not want another class in qtbase for this. I'm not particularly sure 
how to achieve it 
without having at least a template class or a QObject interface defined 
somewhere somewhere 
given that QPluginLoader gives us back QObjects.

He was open to the idea of a macros, however.

He was not more specific as to what would be a workable solution.

> In summary:   Qt solution > KPluginFactory > Plasma-specific solution

I don't want a plasma specific solution. I also am not able to sit here with no 
solution. Every time I 
poke at either Qt or frameworks for these kinds of things, the feedback is 
vague and not 
particularly useful or encouraging. So that leaves me with the pragmatic thing: 
a solution that can 
be used now in Plasma which can demonstrate how it might be done and hopefully 
we can work 
from there to something that will be pick up by things lower in the stack.

-- 
Aaron J. Seigo


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: please make it easier to hack on frameworks

[Aaron J. Seigo]

On Tuesday, April 30, 2013 15:13:46 Frank Reininghaus wrote:
> but then using a released CMake rather than a recent git snapshot
> would not be enough to make building more convenient. We would have to
> ensure that the required CMake version is shipped by all major
> distros, because I think that downloading released CMake tarballs

releases tend to get packaged. snapshots don't.

2.10.8.2 is available in packaged form for fedora, opensuse and ubuntu. not so 
for debian, 
but then if you are using debian for cutting edge devel you are probably used 
to building 
dependencies from sources.

relying on the last release of a build tool is really not too much to ask.

-- 
Aaron J. Seigo


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: RFC: a replacement for KPluginLoader

[Aaron J. Seigo]

On Tuesday, April 30, 2013 08:10:32 Kevin Ottens wrote:
> Why not reuse KPluginFactory either as is or slightly modified? What we're

most of KPluginFactory is mostly about the macros.

i'm not sure registerPlugin makes any sense anymore given the new interface 
metadata features 
in Qt5's QPluginLoader.

It is only used by KPluginFactory::create(const char *iface, QWidget 
*parentWidget, QObject 
*parent, const QVariantList &args, const QString &keyword), which i'm not sure 
is used in many 
places (not the easiest thing to grep or search in lxr for). it incurs overhead 
for every plugin as the 
name is stored in a global hash. would be nice if that could be removed .. i 
need to look further 
into QPluginLoader and what it is possible of though.

there is also a fair amount of boilerplate in KPluginFactory around the 
QVariantList, component 
names and translation catalogs (which is currently commented out ..) so that 
could all go.

Automatically loading the correct translation catalog is something that needs 
to be re-added 
somehow as well .. and as I look at it, I'm not sure how that would get into Qt 
proper.

In any case, I guess I could start with a copy of KPluginFactory, but I don't 
think much would 
survive.

I'll look into it in any case .. and if David has any feedback before I do so, 
that'd be very much 
appreciated.

-- 
Aaron J. Seigo


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: please make it easier to hack on frameworks

[Aaron J. Seigo]

On Tuesday, April 30, 2013 11:20:47 Stephen Kelly wrote:
> I am clueless to understand why building cmake from git and installing it
> into your kf5 prefix is a showstopper. Can you tell me?

Time is limited. 

Every repository that I have to build (e.g. cmake) that is not the repository I 
am trying to work on 
(e.g. plasma-frameworks) is time lost getting my tasks done.

Moreover, it is one more thing to learn: where is cmake's git, how to build it, 
etc. For me that is not 
a big issue (I've build cmake from source many times in the past) but for 
people who might want 
to work on frameworks this kind of thing becomes a show stopper. Eventually 
they run into so 
many things they have to locate, build and keep up to date that they have no 
time / energy / 
desire to keep on.

If we want to ensure that it is as difficult as possible to contribute to 
frameworks, congratulations, 
we're doing a great job of that. If frameworks is meant to be a project for 
you, Kevin, David and 
Alexander to work on then by all means don't worry about this. If the idea is, 
however, to make it 
attractive to others to work on, then some things need to change.

The idea that it is ok to rely on unreleased software as part of the toolchain 
is one of those things.

> The fact that the required cmake version is a dated snapshot is an
> implementation detail. 

It is not an "implementation detail" if it impacts every single person who 
feels the itch to work on 
frameworks.

Given that plasma-framework depends on a relatively recent version of 
frameworks, this 
"implementation detail" is now screwing with barrier to entry to 
plasma-framework.

If the idea is to have users of frameworks, then frameworks needs to stop being 
a liability in terms 
of increasing the barrier to entry.

> http://build.kde.org/job/cmake/. I assumed anyone who cares would be
> building cmake from git because it's the most convenient and smart way to do
> it.

You assumed wrongly. I'm also not sure how "build from git" is more convenient 
than "zypper up".

> This use of the CMake RC in frameworks has found actual bugs in the cmake
> release candidates (most recently in the 2.8.11 RC 1), so we should continue
> to depend on the latest release candidate. CMake release candidates are
> actual candidates for release, that's not just a label.

So kdelibs frameworks is a test environment for cmake? I can understand that 
from the cmake 
perspective, but it is not ammenable from the perspective of kdelibs 
development. tail.wag(dog)

-- 
Aaron Seigo


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

RFC: a replacement for KPluginLoader

[Aaron J. Seigo]

hi..

currently libplasma uses the KDE plugin loading mechanisms *extensively*. they 
have also been 
marked as deprecated. the goal for frameworks, as i undestand it, is to move to 
a QPluginLoader.

QPluginLoader has a major limitation, however: it only returns a single 
instance of a class from 
the loaded library. one. ever. only. :)

obviously, applications such as Plasma Desktop need to load the same plugin 
many times, as 
needed. the solution is obvious: create a factory class that returns new 
instances of the class(es) 
inside of the library.

during our weekly plasma-workspaces meeting this topic came up and i suggested 
an approach 
(which I've also discussed with Thiago in passing) which i plan to implement at 
least for plasma-
workspace as a short-term solution to get away from the KDE plugin loading.

i will introduce a new macro (so that it remains source compatible, at least 
for libplasma) that 
creates a QObject subclass which itself provides a factory method to create new 
instances of the 
class(es) in the library.

i would also like to fold in version compatibility checks. the resulting API of 
the generated object 
will look something like:

template T *create(QObject *parent = 0) const;
bool isVersionCompatible(quint32 minVersion, quint32 maxVersion);

the version check method will help remove some boilerplate code from the 
library.

create will be used similarly to how it KPluginLoader is now: create()

the QVariantList parameter will be removed as it seems to serve little real 
world purpose with 
how we design APIs now.

thoughts, before i go write some code?

-- 
Aaron J. Seigo


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: please make it easier to hack on frameworks

[Aaron J. Seigo]

On Monday, April 29, 2013 18:22:56 Kevin Ottens wrote:
> Well it's been necessary at some point because you needed to make fast paced
> changes.

i hear that frameworks is moving slowly. (because not enough people aren't 
working on it)

now i hear that "fast paced changes" are necessary. (because we desperately 
needed something 
in a devel version of cmake, apparently.)

sorry, you can't have it both ways.

this is one of those times when by going slower (in this case: not requiring 
unreleased build 
components) will make you go faster (by making it easier for more people to 
contribute)

> I didn't expect that habit to stick though.

apparently it has.

who is doing project coordination for frameworks 5?

> When is the next
> release of cmake planned? If we could stick to that one for a while that
> would be nice.

s,nice,a requirement to ensure frameworks doesn't become an endless project 
that fails to 
complete in time,

i'm sorry to push so hard on this issue, but we are betting heavy on frameworks 
development 
being something we can participate in usefully.

4.11 will be the last feature release of kde-workspace. we will be doing bug 
fix releases for 
another 2 years after that, but all of our feature effort work will be going 
into Plasma Workspaces 
2. we are already focusing a majority of our efforts there already (just look 
at the commit log for 
the plasma-framework repo) and have reached the point we need to start moving 
code in kde-
workspace over to Qt5.

to reach our goals of an initial release in the first half of next year of 
Plasma Workspaces 2, we 
need to be able to rely on frameworks being at a bare minimum buildable and 
preferably 
hackable with as few annoyances as possible.

it would be tremendously great if we also had weekly or bi-weekly meetings 
hosted by whoever 
is the maintainer/coordinator for frameworks to touch base and see what 
progress is. we've 
been doing this for a few months now for PW2 and it has worked magic.

adding to and sorting out the entries on the kdelibs cleanups wiki page so that 
we can see what 
items are low-hanging fruit would be a bonus as this would allow more people to 
contribute in 
meaningful as well as trivial ways.

how much of the above is possible / realistic? basically, i'm looking for 
someone to coordination 
responsibility for frameworks. it's not enough to just have hackers writing 
code. we need 
someone directing traffic and ensuring bad habits like adopting devel versions 
of cmake cease.

thanks ..

-- 
Aaron J. Seigo


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

please make it easier to hack on frameworks

[Aaron J. Seigo]

hi ...

today i am unable to work on plasma-frameworks. this is because i am currently 
unable to 
compile the frameworks branch of kdelibs. in turn, this is because i do not 
have a recent enough 
version of cmake.

the cmake version required is 2.8.10.20130411. this isn't even a release 
version of cmake, but a 
dated snapshot! i am left with the option of moving to cmake from git 
(something i really would 
like to avoid) or not working on plasma-frameworks (which is not really an 
option for me).

i am sure there was a good reason to move to this new version of cmake. i am 
also sure that 
requiring devel versions of build tools to work on frameworks limits the people 
who can and will 
work on it.

would it be possible for someone who feels reponsible for frameworks 5 to 
institute some more 
developer-friendly requirements here?

thank you.

-- 
Aaron J. Seigo


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: K_EXPORT_PLUGIN

[Aaron J. Seigo]

On Tuesday, February 26, 2013 15:40:38 Aaron J. Seigo wrote:
> right now in libplasma2 we have definitions like this:

anyone? :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

workflow for adding new classes?

[Aaron J. Seigo]

Hi ...

I am hoping to move the ConfigLoader class from libplasma into the kconfig 
library. It needs some API review and a name change ... but before I go into 
details there I was wondering if there is a desired / defined workflow for 
proposing new classes for inclusion in frameworks?

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: K_EXPORT_PLUGIN

[Aaron J. Seigo]

On Monday, February 25, 2013 17:38:09 Aaron J. Seigo wrote:
> can somene point me in a useful direction? thanks.

let me try again, this time with more details on what i'm actually looking 
for[1].

right now in libplasma2 we have definitions like this:

#define K_EXPORT_PLASMA_APPLETSCRIPTENGINE(libname, classname) \
K_PLUGIN_FACTORY(factory, registerPlugin();) \
K_EXPORT_PLUGIN(factory("plasma_appletscriptengine_" #libname)) \
K_EXPORT_PLUGIN_VERSION(PLASMA_VERSION)

then in a plugin:

K_EXPORT_PLASMA_APPLETSCRIPTENGINE(declarativeappletscript, 
DeclarativeAppletScript)

and then to load it:

KService::Ptr service = .. something that gets us a service ... ;
ScriptEngine *engine  = service->createInstance(parent, 
args, &error);

that was then ... this is now. i'd like to do this:

Q_DECLARE_INTERFACE(Plasma::AppletScript, "org.kde.plasma.AppletScript")
#define PLASMA_APPLETSCRIPT_PLUGIN Q_PLUGIN_METADATA(IID 
"org.kde.plasma.AppletScript")

and then in the plugin:

 class DeclarativeAppletScript : public Plasma::AppletScript
 {
 Q_OBJECT
 PLASMA_APPLETSCRIPT_PLUGIN

so far so great, excep that service->createInstance no longer works.

if I replace this with:

KPluginLoader loader(*service);
ScriptEgnine *engine = qobject_cast(loader.instance());

it loads the plugin ... but now we have a few problems:

0) QPluginLoader::instance() returns the same instance of the plugin. i need 
*new* instances every time, which means i need a factory and not the plugin 
itself .. which means i'm back to needing something like K_PLUGIN_FACTORY

1) this is wildly not source compatible for our plugins. even if we ignore the 
changing of the macros, the constructors now change too -> no parent, no args, 
etc.

2)  we still need things like the plugin version checks which we do with 
K_EXPORT_PLUGIN_VERSION(PLASMA_VERSION). do we now need to add this to each 
plugin with a new #define (or similar) or is the idea to move to some other 
facility provided by Qt's plugin loading system? should this be rolled into 
KPluginLoader and/or KService's loading rather than be a separate call 
(KPluginLoader::pluginVersion, and comparing that to a built-in version #)?

so, yes, from a plugin WRITER's perspective, this change is all obvious and 
straightforward. from the perspective of someone creating new plugin TYPES, 
porting away from the K_PLUGIN_FACTORY thing is anything but obvious.

and that's what i'd like some input on from those involved with this set of 
changes, e.g. dfaure.


[1] yes, i too have found the plugnadpaint example in the qt5 sources! ;)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

K_EXPORT_PLUGIN

[Aaron J. Seigo]

hi ...

in kexportplugin.h there is this:

#pragma message("K_EXPORT_PLUGIN is defined as a hack here, for compat. Port to 
new plugin system (QT_PLUGIN_METADATA) instead.")

this was introduced in the first revision of this file (58606a42). 

i haven't been able to find any useful information online about this 
QT_PLUGIN_METADATA thing. currently when compiling libplasma2 we get screen 
after screen of this message, and i 'd like to get rid of it.

can somene point me in a useful direction? thanks.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: "kde5" or "kf5" ?

[Aaron J. Seigo]

On Monday, February 18, 2013 21:35:07 Kevin Ottens wrote:
> There's no will or indication that we'll release anything named KDE 5. If it
> was me the next release of our workspaces would be Plasma Workspaces 2...

It is likely we will not even release Plasma Workspaces $NEXT_VERSION_NUMBER 
at the same time as KF5.

Our goal right now is to get the code in plasma-framework working well, and 
that's what we're doing. Hopefully we can include plasma-framework with the 
first big KF5 releases, and at our current pace I can see no reason that won't 
be possible.

But the workspaces themselves (Desktop, Netbook, Active, Media Center) will 
almost certainly need another devel cycle to get them to the point where we 
can confidently release versions of them on top of KF5.

This should not hold up the release of Frameworks, which means the "SC" may 
finally end up being split. We can still have release days where we make 
available to the public multiple products at the same time (KF5, PW2, 
applications, ..) but those products may not have the same development cycles 
and practices. Several repos that make up pieces of Plasma Desktop already 
have their own development cycles (networkmanagent was probably the first, and 
now we have several others).

So, in summary: I concur that there is no KDE5. There is Frameworks, there is 
Workspaces, there are many applications ... but they are not part of a single 
product (though they do work very well, and often best, together :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KJavaAppletWidget's usage of KWindowSystem

[Aaron J. Seigo]

On Saturday, February 2, 2013 13:05:15 David Faure wrote:
> I tried removing the doNotManage() call, and...

if you replace them with:

setWindowFlags(Qt::X11BypassWindowManagerHint | windowFlags())

does it work?

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: phonon4qt5

[Aaron J. Seigo]

On Sunday, January 27, 2013 03:10:04 Harald Sitter wrote:
> as discussed previously there is now a transitional library for qt5 called
> phonon4qt5 and lives in a branch of the regular phonon git repos (pending
> release).

i just pushed a aseigo/phonon4qt5 branch that fixes compilation with current 
qt5. feel free to merge it if desired :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KDirModelV2, KDirListerV2 and UDSEntryV2 suggestions

[Aaron J. Seigo]

On Monday, January 21, 2013 21:09:59 you wrote:
> - RMB context menu
> - App wide shortcuts
> - Clipboard integration..

these are all being worked on.

> In my DirModel i took a good close look at the current DisplayRole
> stuff. It was a blocking call that could potentially slow the
> interface down. Not anymore! I added a new role called: "MimeOrThumb"
> which will display either a mime image or a thumbnail image:
> http://gitorious.org/porpoise/master/blobs/master/dirmodel.cpp#line204
> That alone won't make it non blocking.

instead of a new role, could this simply be used for DisplayRole?

> I needed the power of the
> Image{} element since that can be set to handle images asynchronous
> http://qt-project.org/doc/qt-4.8/qml-image.html#asynchronous-prop thus
> non blocking. In order to get that done, i added 2 image handlers,
> "image://mime/" and "image://thumb/ which you
> can find here:
> http://gitorious.org/porpoise/master/blobs/master/mimeimageprovider.h

creative approach; it also reminds me how ugly the API for 
QDeclarativeImageProvider is. btw, while you check the size var before using 
it the first time, you don't do so the second time. a possible crash lurks :) 
and if both size and requestedSize are passed in with valid values, size will 
probably have the wrong value set. (256x256 rather than the actual 
requestedSize that is returned) personally, i'd probably just write it like 
this:

const QSize s = requestedSize.isEmpty() ? QSize(256, 256) : requestedSize;
if (size) {
*size = s;
}
return icon.pixmap(s);

shorter, no crashes, guarantees size always gets the right value if a ptr is 
passed in. :)

> (mime) and here
> http://gitorious.org/porpoise/master/blobs/master/thumbimageprovider.h
> (thumb). On top of that i added a KImageCache (or actually modified
> where it's being called) since i want to have a single point for the
> KImageCache i made it available through my static class:
> http://gitorious.org/porpoise/master/blobs/master/util.h#line35. That
> part won't be usable in a generic KDirModel with threading.

assuming this one cache is used for multiple things by the application, and 
not just the file listing model for thumbnails, a mutex around it would 
probably be enough. it should also probably live outside the UI thread ...

btw, is there any chance that you might move this to git.kde.org in future?

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KDirModelV2, KDirListerV2 and UDSEntryV2 suggestions

[Aaron J. Seigo]

On Monday, January 21, 2013 20:18:03 you wrote:
> I'm guessing Qt5 won't be ready for full fledged desktop application -

maybe you wouldn't say that if you'd seen the QML Desktop Components demo 
applications :)

jump to around minute 57 of http://www.youtube.com/watch?v=Y1pqL5bXe0A

native menus are currently being worked on, and this is targetted for 
inclusion in Qt 5.1

> Aaron, do you have any news regarding that DirModel of yours and Marco
> that is threaded?

oh, we don't have such a thing right now. we need to write one. what we 
threaded was the nepomuk model, which is used in Active for most (though not 
all) file listings.

it's one of the reasons i'm watching this thread with such interest as an 
improved KDirModel that doesn't disturb the gui thread even while loading 
would be very nice.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KDirModelV2, KDirListerV2 and UDSEntryV2 suggestions

[Aaron J. Seigo]

On Monday, January 21, 2013 17:53:48 Frank Reininghaus wrote:
> http://ppenz.blogspot.de/2011/08/introducing-dolphin-20.html

Ah, that's right .. it uses ItemViewsNG. of which the last commits seem to be 
in 2010, and QGraphicsView is not receiving much attention now either :/

i suppose as long as ItemViewsNG+QGraphicsView doesn't bitrot too much and 
performance remains acceptable, then there's no "forced" changes required 
towards QML. maybe that will hold for a number of years yet, even.

when Qt Desktop Components become available for use and testing (supposed to 
come with 5.1), that might be another option as it comes with a few things a 
dolphin-type application will need/want: visual integration with the host 
desktop (e.g. QStyle bridging) and various item views that QML currently lacks 
... but that will still require a QAIM based model for directory listings that 
is performant.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Plans and status Frameworks5, Plasma2, Qt5 and Wayland

[Aaron J. Seigo]

On Saturday, January 19, 2013 17:54:12 Valentin Rusu wrote:
> There is a sentence on page 3 that reads
> "1. KWin on Qt5 (work in progress, planned for 4.11)"
> 
> Knowing that KWin version's number is tightly coupled to KDE's version, a
> question stumbled upon my mind. Are there any plans to release KDE 4.xx
> version on top of Qt5? Or that figure only refers to KWin's version?

there is an effort to port kwin from xlib to xcb. that code will be in the 
(Qt4-based) KDE SC 4.11 release.

moving to xcb is a requirement for kwin to build with Qt5, and was motivated 
in large part by this requirement.

so, no, this is not about a KDE SC 4.x release on Qt5 :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KDirModelV2, KDirListerV2 and UDSEntryV2 suggestions

[Aaron J. Seigo]

On Tuesday, January 15, 2013 10:40:58 Frank Reininghaus wrote:
> Right, but considering that Dolphin doesn't even use KDirModel any
> more, going the QAbstractItemView way would feel like throwing away
> most of what happened in Dolphin in the last 2 years.

dolphin still uses QAbstractItemModels, no? that's all that matters; whether 
it uses KDirModel or KFileItemModle or SomeOtherModel is a detail.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KDirModelV2, KDirListerV2 and UDSEntryV2 suggestions

[Aaron J. Seigo]

On Friday, January 11, 2013 13:39:47 you wrote:
> Just wondering a "little" thing about making KDirModel threaded.. How
> can you even do that?

KDirModel would remain a QAIM subclass. inside of KDirModel, it would manage 
one (or more) threads to do the actual work.

QAIM is itself fast enough (as long as one avoids calling dataChanged 100k 
times for one row at a time or other such crazyness :), so that's fine.

the data handling (fetching, parsing, allocating) is what usually needs to be 
put into a thread.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KDirModelV2, KDirListerV2 and UDSEntryV2 suggestions

[Aaron J. Seigo]

On Thursday, January 10, 2013 19:37:39 you wrote:
> >> Is there another
> >> dirmodel fork somewhere that does that? Link please :)
> > 
> > well, what we ought to do is do a threaded version and then put it
> > somewhere we can all share it properly :) i'll discuss with Marco what he
> > thinks and come back with a proposal.
> 
> you make it sound like it's closed source..? no link to some exotic
> highly experimental git branch?

hehe.. no, none yet ... reason i want to talk about with Marco is this will 
impact our devel efforts elsewhere in the plasma-mobile code, so we would need 
to factor any changes in there.

and this really seems like the sort of thing that ought to be available in the 
Plasma Components in kde-runtime so that everyone can use it (and hack on it) 
without making copies :)

> > so +1 from me for making KDirModel do its business in an encapsulated
> > thread.
> I guess that is the way forward then. But i do wonder, if the
> KDirModel API stays the same but the internals change completely, is
> that KF5 material or can it go in 4.xx as well?

imho, probably KF5 because:

a) we want KF5 sooner rather than later, so efforts should focus there.[1]
b) KDirModel is such a key component with a high complexity and few people who 
know it very well; so it needs lots of testing, and 5.0 would be a better 
target for that kind of development imho
c) it will give us something to market about 5.0 -> SO MUCH FASTER / MORE 
FLUID DIRECTORY LISTINGS (that's mostly, but only mostly, tongue-in-cheek)

[1] as an aside: we've re-focused effort on libplasma2 in KF5. the new QML 
based containment stuff being worked on has been moved to libplasma2-only and i 
expect other such development in Plasma to continue that way. we're also 
expecting / planning Plasma Active 5 to be focused on moving to Qt5 + KF5.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KDirModelV2, KDirListerV2 and UDSEntryV2 suggestions

[Aaron J. Seigo]

On Thursday, January 10, 2013 18:41:04 Frank Reininghaus wrote:
> before considering such a thing, one would need to check if that is
> really a significant bottleneck. 

if it means having to calculate by iterating over every single item in a 
listing, then it will eventually become a bottleneck. the goal should be to 
only list and process items that are actually being viewed. temporary 
innacuracies in the scroll bar are far better than a slow view.

> Quite a bit of code would need to be
> changed, and I can imagine that it's not completely trivial to get
> this right.

it probably doesn't need to be perfect. it only needs to be "good enough". if 
the scroll bar updates a few pixels here and there while dragging, i'll bet 
that's better in terms of real world usage experience than any lag in the 
view.
 
> > if that is the case currently, i would suggest dropping this goal
> > and only change the spacing dimension when new icons are shown that
> > require
> > it. (again, easy enough with QML.)
> 
> Well, the hard part is probably to port Dolphin's views to QML in the
> first place ;-) I must admit that my QML knowledge is rather limited,
> but AFAIK, it does not provide any kind of tree view out of the box.

no tree views, no. at least not until desktop components come in 5.1.

icon and list views are trivial. tree views can be 'faked', but yes, that will 
take a bit of effort. far less effort than doing it in c++ though :)

> Moreover, it seems to me that QML views work best with
> QAbstractItemModel subclasses. 

yes.

> However, one of the main goals Peter
> had when rewriting the view engine was to get rid of QModelIndex,
> SortFilterProxyModels and things like these. And I really have the
> feeling that using plain ints to index items and not needing things
> like mapToSource() in every other function really does make things
> easier.

with QML, you can get rid of C++ SortFilterProxyModels and do this in QML 
itself. it uses proper models in the background (so it's fast) and it becomes 
insanely easy to get sorting and filtering.

with QML, QModelIndex will only appear in the actual model itself (KDirModel, 
e.g.; or in the case of Mark's "porpoise" DirModel which adds thumbnailing on 
top of KDirModel) so you only care about this in the one model.

the UI, and even the sorting and filtering bits, no longer see any of that 
complexity. this is perhaps one of the best bits of QML when it comes to data 
models.

the downside to QML and models is that it doesn't understand models that are 
trees out of the box. it basically wants big flat lists. you can work around 
this with fairly minimal effort, though.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KDirModelV2, KDirListerV2 and UDSEntryV2 suggestions

[Aaron J. Seigo]

On Thursday, January 10, 2013 14:12:30 you wrote:
> part very rapidly. No issue thus far. But when i start scrolling
> _while_ the data is dripping in it's ... not very responsive.

after a quick look: 

the DirModel is doing all the data retrieval in the main thread as well as 
instantly on demand (no batching, e.g.). this means:

* every allocation of a FileItem and all the work to populate it takes cycles 
away from the UI thread at the same time as the list is trying to scroll
* KFileItems are created *and* populated regardless of whether they will be 
used or not, which means during scrolling things will get slower the more you 
scroll. they are also fully populated on creation.

neither of these things is good. (and some of these issues have already been 
touched on in this thread)

this probably requires changes to KDirModel such as:

* put data processing into a thread; whether that means putting the whole of 
KDirModel into a worker thread or just having a thread pool for processing new 
KDirModelNodes / KFileItems .. others more familiar with the code would be 
able to answer that far better than i can right now :) but there should be 
near-zero time spent in the GUI thread for actually getting the file 
information

* KFileItem needs to support lazy loading to avoid unecessary work when the 
information is not actually requested. there is even a TODO about this in 
KFileItemPrivate::init() in the 4.10 branch :)

* methods like KDirModelPrivate::_k_slotNewItems need be faster and/or have 
their work moved into a thread. the number of times 
beginInsertRows/endInsertRows is called should also be minimized as that will 
innevitably create pauses in the GUI thread while it reacts to this

perhaps some of this is already done or being done in the frameworks branch 
(haven't looked yet; apologies in advance for that)

as to the DirModel class:

* the image cache is done in the main thread. that means updating a database 
and writing to disk in the UI thread .. when the goal is 60fps or better, this 
is likely going to be too slow.

* KIO::PreviewJobs are also in the main GUI thread. could be another cause for 
GUI latency.

* DirModel::rebuildUrlToIndex is O(n) and called whenever there are new items

* DirModel::updatePreview calls setData for each preview that it gets back. 
that causes dataChanged to be emitted. again, for each index. this needs to be 
batched up and/or DirModel should override data() itself to return 
Qt::DecorationRole when it has a preview.

btw, DirModel::itemForIndex looks like it is leaking memory.

so it seems DirModel is need of a scalibility refactor/rewrite, and KDirModel 
needs to be made read for use with threads in some fashion as well have its 
base performance improved with more lazy loading and batching of requests.

(this feels earily similar to the model rewrites Marco ended up doing for 
Plasma Active ... threads and batching, threads and batching. the UI thread 
must not be disturbed! :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KDirModelV2, KDirListerV2 and UDSEntryV2 suggestions

[Aaron J. Seigo]

On Thursday, January 10, 2013 11:30:35 Mark wrote:
> very rapidly, the actual view is still barely usable as long as files
> are dripping in. This is the same in QML (ListView) as in QWidgets
> The issue here is that the view - even though it's not
> showing the items - is doing all the calculations for every item
> added

if so, then the QML is being done "wrong" in some way. the default views in 
QML (along with some we've written ourselves) only creates delegates for items 
that are shown along with +/-N pages for caching (so when you scroll you don't 
see things updating; the updates are happening N pages further down the 
scrolling)

under no circumstances should the entire set of icons be laid out at any time. 
memory usage and CPU usage simply prohibits this, as you noted :)

QML makes it really simple to just lay out what is needed (shown +/- some 
scroll-ahead pages); this in turn makes it easier to write the model so it is 
lazy loading: only load the essentials (even progressively; QML views handle 
this just fine) and then when requested load details.

because QML binds data to properties, the async nature of this does not cause 
further complexity in your code: it all happens in the QML runtime. a new 
delegate is made for a given index (automatically based on scrol position and 
view size), it binds to the data for that index, the model then knows 
something has requested that data and goes to fetch it (whenever/however it 
feels :) and when it updates that data then the delegate also automatically is 
updated .. all without dealing with paint events and other annoyances of views 
with delegates written in the imperative style.

this allows huge (even "infinite"!) size models to be written that feed views 
that are performant even on mobile devices ... with very little effort.

in the case of dolphin, i can imagine a goal that could lead to over-
calculating: wanting even spacing between all icons, which in turn means 
knowing how much space each and every icon will require (thumbnail? how much 
text? etc.) if that is the case currently, i would suggest dropping this goal 
and only change the spacing dimension when new icons are shown that require 
it. (again, easy enough with QML.)

also, the model itself can process some statistics (e.g. longest name) as it 
loads. this can be put into a Q_PROPERTY that the QML can then access. with 
C++/QWidget, the annoyance with this approach is then you need to reference 
the C++ object with the exact type and you end up welding models and views 
together and .. with QML all the properties are dynamic. you can test for 
their existence and use them if they are there ... and this is also rather 
efficiently implemented.

all of this is a very long-winded way of saying:

if you do QML correctly and write the model accordingly,
you should be able to have a stupidly fast view.
(at least from the user's perspective :)

i know because i've seen QML views on models with millions of entries 
scrolling insanely smoothly with complex and beautiful delegates. i've also 
helped write a couple. including doing it "wrong" a few times while learning 
:)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KDirModelV2, KDirListerV2 and UDSEntryV2 suggestions

[Aaron J. Seigo]

On Wednesday, January 9, 2013 16:42:25 Mark wrote:
> Show data as early as possible even if it's incomplete. Defer the

imho, one place that dolphin currently fails in going too far in this 
direction is that folders do not get a folder icon until thumbnails start 
dribbling in. using the generic mimetype icon for a folder for even a split 
second not only looks bad, it impacts the usability of the listing as the 
folder/file division is significant in these contexts.

so while i agree with your theory, there is a lower limit to the amount of 
data needed to be minimally useful.

0.02 :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: kmenubar

[Aaron J. Seigo]

On Friday, February 24, 2012 16:27:09 Stephen Kelly wrote:
> Would it make any sense to keep it somewhere in kdelibs which is not
> kde4support?

i thought about that, but i can't really figure a place where it would make 
sense.

it is an x11 specific concept and really only useful for "behind the scenes" 
support of workspace related features (clipboard, the old global menu 
approach, active x11 window compositor ..), which is why it's realy only used 
in kde-workspace + classes in kdelibs that had to do with custom workspace 
integration.

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: kmenubar

[Aaron J. Seigo]

On Tuesday, February 21, 2012 14:14:29 Aaron J. Seigo wrote:
> feedback?

i will translate the silence as an affirmative and move forward with the 
proposed changes unless someone says otherwise soon :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KF5 Volunteer day 1: Done

[Aaron J. Seigo]

On Friday, February 24, 2012 15:13:15 Stephen Kelly wrote:
> I guess KDE wants everyone to be able to throw the paint on the bikeshed
> without ever having to learn anything about the tools.

no, not at all.

as long as we present achievable learning curves, everyone will benefit. and 
it's very cool that you are taking the time to do things like look through the 
commit logs of branches for merging. it definitely adds a lot.

git can present a very steep learning curve, however, and we've found that we 
lose contribution due to percieve complexity when it's thrust too heavily in 
front of people. that's all i was trying to get at, not that people should be 
encouraged to underachieve :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KF5 Volunteer day 1: Done

[Aaron J. Seigo]

On Friday, February 24, 2012 10:57:31 Stephen Kelly wrote:
> Clean readable history is important for now and for the future.

as long as we don't become too obsessive and start expecting git mastery from 
everyone who comes along. we need to keep the bar for participation low enough 
to be reached in practice (note: *not* "reachable in theory").

i'm not advocating a messy history or screwy git repositories ... but wish to 
keep focused on being a realistic participation target. that is at least 
partially a matter of perception as well; if it sounds too difficult, people 
will assume it is dificult.

and pragmatically .. if people spend 1 hour coding and then even 30 minutes 
fiddling with commit granularity and atomicity ... we will have fewer people 
contributing. not only because of the time spent on git fu, but because they 
come to contribute code not play with git.

at the same time, your offer of guidance and help is really appreciated. :)

cheers ...

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

kmenubar

[Aaron J. Seigo]

hi :)

KMenuBar uses KSelectionWatcher in libkdeui. 

the entire purpose of KMenuBar is to export a top-level menu (aka "macOS 
style"). it's only relevant on Linux/Unix and these days we have the DBusMenu 
specification which is supported by Qt itself. admittedly the DBusMenu spec is 
more limited in what it can host (e.g. no custom widgets in menus) but that is 
probably a good thing anyways (limits misuse, even if well-intentioned, of 
menus) and it is being used successfully by various apps/plasmoids.

KMenuBar is used directly by a handful of significant apps (korganizer, amarok, 
kdevelop, umbrello, ..) as well as in KXMLGuiBuilder and KMainWindow in 
libkdeui. the kdeui classes can be ported to use QMenuBar quite easily. we'll 
need to keep KMenuBar around for the apps that use it, however.

proposal:

* move KMenuBar and kmanagerselection.h/cpp to staging/kde4support/kdeui/

* port xmlgui related classes from KMenuBar to QMenuBar

the latter is a source incompatible change, though a trivial one. use of 
KMenuBar in apps seems to be limited to the inheritted QMenuBar functionality 
so the impact should be tiny when they port away from kde4support libs.

this will leave kde-workspace with a small but solvable problem when 
Frameworks is adopted. we don't want to link against kde4support libraries if 
at all possible, so the most sensible thing might be to, at that point, simply 
copy kmanagerselection.h/cpp into kde-workspace/libs/kworkspace/ for use by 
workspace libs. (it used for startup tracking and some things in kwin.)

feedback?

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: libkwindowsystem in staging

[Aaron J. Seigo]

On Monday, February 20, 2012 18:14:59 Stephen Kelly wrote:
> Do you intend to port it to Qt5 or know what that involves?

I don't know what that involves yet, but this is something that Martin will be 
working on shortly.
 
> With Qt going window system agnostic, should KDE follow?

Yes; however we need these helpers since we actually do interact with window 
management. We just need to hide the details 

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

libkwindowsystem in staging

[Aaron J. Seigo]

hi all...

there is a new library in staging: libkwindowsystem

it pulls in kdeui/windowmanagement/ and Plasma::WindowEffects as well as a few 
x11 helper classes from kdeui.

these helper classes are public API in Platform 4.x but are only used by the 
classes in libkwindowsystem. as such i've made them private.


i did duplicate one class from kdeui as a short term measure by making a copy 
of the header and source file: KXErrorHandler. this is also used in 
kdeui/util/kmanagerselection.cpp.

KSelectionWatcher and KSelectionOwner are rather x11 specific concepts on the 
one hand .. and on the other they are only used by kde-workspace (so could be 
moved into there easily enough) and KMenuBar in kdeui.

they are used in KMenuBar for global menu integration. however, as global menu 
support is now in Qt 4.8 (and therefore will be in 5.0 as well) i'd like to 
propose the following:

* KMenuBar loses the KDE-specific implementation; this also cuts the dependency 
on KSelectionOwner/Watcher completely in kdelibs

* move kmanagerselection.h/cpp into libkworkspace in kde-workspace for the 4.9 
and use that in kde-workspace (pre-porting for Frameworks 5 :)

* remove kmanagerselection.h/cpp and kxerrorhandler.h/cpp from kdeui in 
frameworks branch

thoughts / input / objections?



as for the "definition of done" progress for libkwindowsystem the following are 
done: 

No dependency on kdeui or kdecore
Modularized CMakeLists.txt (afaik, anyways :)
Does not install any Find*.cmake modules itself
All Find*.cmake modules needed by the framework and used also by other 
frameworks have been upstreamed to extra-cmake-modules (via the kde-
buildsystem mailing list) or directly to cmake (via the cmake list)
Follows the policy on directory organization
Has an appointed maintainer
Maintains source compatibility over kdelibs (modulo the above unused 
classes becoming private)

not done are:
Installs a Config.cmake file for itself (due to not yet being clear on how 
this should look in the frameworks dir; i've done this for libkactivities, but 
i need to find out what, if anything, has changed there)
Unit tested (pending upcoming work by mgraeslin)
Does not use any Q_WS_* defines. (pending upcoming work by mgraeslin)

-- 
Aaron Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: From kdelibs4 to KDE frameworks... how to make KDE more cross platform...

[Aaron J. Seigo]

On Monday, February 13, 2012 17:51:23 Shaun Reich wrote:
> hate to chime in as well, but i think replacing the Windows shell
> should definitely be something that's looked at. imho it makes a lot
> of sense. face it, the Windows shell sucks.

how many windows users realistically change the shell, even though it's 
already possible now and such shells exist?

this is even smaller than the # of people who really dislike the windows 7 
shell THAT much.

compare with: how much effort is required to make this REALLY work well?

i'm not going to stand in people's way of working on this (because .. well .. 
i haven't so far ;) but if you do the math it's pretty evident that the 
work/reward ratio is microscopic.

if we just say, "nah. not a priority. don't plan around that eventuality" then 
it becomes far more evident that it is the applications that need to be 
focussed on.

and when we realize that the applications should be the _focus_, then we 
arrive at the conclusions of the original post in this thread :)

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: From kdelibs4 to KDE frameworks... how to make KDE more cross platform...

[Aaron J. Seigo]

On Saturday, February 11, 2012 20:00:01 Alexander Neundorf wrote:
> It is cool to be able to replace the Windows shell... but does it make sense

fwiw, it's always been my opinion that it does not make sense to do this.

it doesn't solve a real problem on the windows platform, and as such does not 
really gain us more users or deliver value to existing users...

so +1 for emphasizing on apps on win/mac

-- 
Aaron J. Seigo

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: kactivities

[Aaron J. Seigo]

On Thursday, December 8, 2011 21:37:22 Alexander Neundorf wrote:
> On Monday 05 December 2011, Aaron J. Seigo wrote:
> > hi..
> > 
> > the Plasma team is working on the kactivities repository with the idea in
> > mind that it is a "frameworks 5" style project right from the start.
> > 
> > it currently contains a library (libkactivities), a runtime componen
> > (kactivitymanagerd) and some runtime data (ontologies).
> > 
> > what it currently lacks is full unit test coverage and upstreaming of
> > Find*.cmake files (it carries with it its own FindKActivities.cmake; i
> > will
> > email buildsystem about this one ..)
> 
> Two comments: a library should not install a find-module for itself, this
> doesn't make sense (like putting the remote control on the TV, or the
> treasure map inside the treasure chest), because once you found the
> find-module, the hard work is already done, the location where the package
> is has been found.

yes, i know .. the module isn't actually installed with libkactivities. it's 
just there to have something to copy to other modules that need to find it. 

> So, instead it should install a Config.cmake file, which instead of

it has one of those as well, but it looked a bit different to the one in the 
example. i've updated it so that it reflects the example. thanks :)

> Then, once it installs such a Config.cmake file, a find-module is strictly
> not necessary anymore, since cmake knows a set of directories (including
> using CMAKE_PREFIX_PATH) where to search for them.

ah! that's a nice feature. great, then that part is done for kactivities.

> Personally I still like to have a minimal find-module in that case two,
> which basically does two things:

make sense..

again, thanks :)

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: examples

[Aaron J. Seigo]

On Thursday, December 8, 2011 21:39:53 Alexander Neundorf wrote:
> So if it is a separate library, having the examples with the library is

this makes exactly the point: we want people using as many of our libraries as 
possible, so we are motivated to make that as easy as possible.

there are two use cases where having it all in one place really makes a 
difference:

* someone who is already using multiple KDE frameworks -> they have just one 
place to look to get all the examples. looking at your build service example 
was quick and easy for me this morning because i already had kdeexamples 
checked out, though certainly i did not grab it originally because of that 
particular example. so it saved me time.

* someone who is using one framework, or a subset of them -> by having all 
examples on hand, it allows them to see what else is possible with KDE 
Frameworks

so on the one hand, it's a way to decrease the effort needed to keep up with 
all that is great in KDE land and on the other hand it's a great way to expose 
more people to the breadth of our offering. 

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: kactivities

[Aaron J. Seigo]

On Monday, December 5, 2011 21:53:25 Dario Freddi wrote:
> 2011/12/5 Aaron J. Seigo :
> > hi..
> > 
> > the Plasma team is working on the kactivities repository with the idea in
> > mind that it is a "frameworks 5" style project right from the start.
> > 
> > it currently contains a library (libkactivities), a runtime componen
> > (kactivitymanagerd) and some runtime data (ontologies).
> 
> Slightly OT: is this what is currently to be found in kde-workspace?
> Is that going to replace the workspace lib in KF5?

it already has replaced it. :)

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: i18n in Frameworks, the lightweight version

[Aaron J. Seigo]

On Tuesday, December 6, 2011 14:14:44 Chusslove Illich wrote:
> > [: Aaron J. Seigo :]
> > when there is already one implementation that comes bundled with Qt and
> > does everything needed/wanted, it is simply impossible to justify that
> > kind of dependency on a small device. [...] in this case, another option
> > is to not remove translation scripting on such systems (and therefore the
> > dependency), but that would be a shame imho.
> 
> Frankly, I'm inclined to completely ignore such a limited device in any
> decision concerning a general piece of software.

you are free to ignore it. hopefully i am free to not ignore it, however. :)

with plasma active, our mid-term goal is to get disk and memory usage down to 
the point where we can confidently put it on devices with 256MB of RAM and less 
than 1GB of storage and have something pleasant and usable. we have a ways to 
go yet (obviously :), but if we get blocked from making these kinds of changes 
in frameworks code then we're unlikely to achieve success.

so if the optimization can be made without side effects and someone is willing 
to do the work, how can we justify saying "no, please don't improve the code"?

as a team collaborating on frameworks, we need to be sensitive to the needs 
and goals of each other and accomodate those goals when they don't conflict 
with our own. and when they do conflict, we should work on finding compromise 
if 
possible.

hopefully that makes sense to everyone :)

> I don't think it would be a shame at all to kill translation scripting on
> such a device. It could even result in non-negligible performance gain in
> some contexts (especially in latency, when stuff is loaded). The kill switch
> already exists in the code, and can be thrown in kdeglobals. This
> functionality should remain, KJS or QtScript.

interesting; i can't locate the config key to use by looking at the code. where 
might i find this, as it could be a nice improvement for Plasma Active 2.

> (I would additionaly provide a
> build system switch, to not even compile the scripting module.)

assuming this doesn't greatly deteriorate the quality of translations and that 
it remains compatible with existing translations, then this could go into the 
mobile build profile.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: i18n in Frameworks, the lightweight version

[Aaron J. Seigo]

On Tuesday, December 6, 2011 12:49:51 Chusslove Illich wrote:
> Right, that is the reason. I said that performance and maintainance are not
> an issue at present, and I find it hard to take dependency as an issue as
> long as KJS is part of Frameworks. It is beyond great that you offer to port
> it yourself, but I don't see a solid reason yet.

we do not want to load a second JS implementation into memory on Plasma 
Active. in fact, we'd prefer not to ship KJS/KHTML at all with Plasma Active 
as that gives us another free ~10MB on disk with zero functionality loss.

when there is already one implementation that comes bundled with Qt and does 
everything needed/wanted, it is simply impossible to justify that kind of 
dependency on a small device. yes, that means for every such duplication we 
have we need to justify to ourselves why we have it. if there is no good 
justification for it, it needs to be resolved.

in this case, another option is to not remove translation scripting on such 
systems (and therefore the dependency), but that would be a shame imho.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: examples

[Aaron J. Seigo]

On Tuesday, December 6, 2011 10:32:34 Mario Fux wrote:
> So why not leaving the examples in examples directories in the frameworks
> and building some cmake or what magic to create a tarball or something with
> all examples collected. Probably together with the (new) KDE demo app done
> this summer of code?

if we can automate this, that'd be an acceptable solution imho. but we need 
someone to do that work, and i'm not volunteering for that particular task :)

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: i18n in Frameworks, the lightweight version

[Aaron J. Seigo]

On Monday, December 5, 2011 12:23:30 Chusslove Illich wrote:
> Regarding yesterday's chat, to shortly (really!) sum up my stance on i18n.
> 
> 1) I see no point at striving for single translation system for apps based
> on Qt and Qt plus KDE Frameworks. Qt should keep the tr()/Linguist system,
> and Frameworks should continue to offer the i18n()/PO system.
> 
> 2) The application developer would choose which translation system to use
> based on whatever criteria, like choosing between any other two libraries
> covering the same need. We could put up an article describing the relative
> advantages of the two systems.

that is precisely the problem. nobody wants to choose between two translation 
systems. nobody wants to have to figure out what libraryFoo uses for 
translation (that one uses tr, this one use i18n, this other one also uses 
i18n, that one uses tr ..) and try to align their app with that.

on the device side, i certainly do not want the overhead of two translation 
systems.

there is simply no way to justify the current state of affairs, other than 
"it's how it's always been done, it's already implemented".

> 5) KDE i18n currently uses KJS. I saw that KJS will be a tier 1 component,
> and if I recall tier 1 components must not depend on other tier 1
> components -- that would make i18n tier 2, right? If so, is that actually
> bad? Is i18n needed in other tier 1 components? The alternative is of course
> to port to QtScript, but I wouldn't want to do it right now without an
> organization-related reason (i.e. considering performance/extension-API
> irrelevant for the moment).

the reason to move to QtScript is pretty straight forward: the code would be a 
lot cleaner, likely more performant and we would lose a dependency that is 
redundant with what is provided in Qt.

we also have more people who are deeply familiar with using QtScript.

from a dependencies perspective, it makes sense.
from a performance perspective, it makes sense.
from a maintenance perspective, it makes sense.

the only reason i can see to not move to QtScript is that the code using KJS 
is already written. i offered in the past to port this to QtScript, and that 
offer remains.

looking at:

http://techbase.kde.org/Localization/Concepts/Transcript#The_Transcript_Interface

there are ~30 function calls and the one Ts object in the javascript 
namespace. that is not a large amount of work.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel