Re: KDE in prefix install

[Sebastian Kügler]

Very useful, thanks Allan! :)

On woensdag 28 maart 2018 17:37:18 CEST Allan Sandfeld Jensen wrote:
> On Mittwoch, 28. März 2018 17:14:36 CEST Sebastian Kügler wrote:
> > What I usually do is copy the dbus files into the system paths, see
> > attached script. That should also make it work (after dbus restart).
> 
> You don't need that if you just set the dirs:
> 
> /etc/dbus-1/session.d/plasma-opt.conf:
>  1.0// EN"
>  "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd;>
> 
>   /opt/kde5/share/dbus-1/services/
> 
> 
> /etc/dbus-1/system.d/plasma-opt.conf:
>  1.0// EN"
>  "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd;>
> 
> /opt/kde5/share/dbus-1/system-services
> /opt/kde5/etc/dbus-1/system.d
> 
> 
> But that still leaves the problem that he polkit-1 policy files for actions
> are neither generated nor installed. Any idea why that is the case?
> 
> 'Allan


-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: KDE in prefix install

[Sebastian Kügler]

What I usually do is copy the dbus files into the system paths, see attached 
script. That should also make it work (after dbus restart).

-- sebas

On woensdag 28 maart 2018 12:52:16 CEST Allan Sandfeld Jensen wrote:
> So after leaving neon to use a newer ubuntu (needed it for my hardware), I
> have started building KDE myself again and tried setting my tradition
> install in /opt and building with kdesrc-build.
> 
> I ran into a number of issues, many minor things I have already fixed by
> patches. But there were two bigger issues with dbus-1 and polkit.
> 
> First kscreen doesn't work because it can't find its backends. While not
> easily found, I did eventually find suggestions online to fix this by adding
> a file in /etc/dbus-1/session.d with the prefix service dir. I talked to
> the dbus people and they seemed positive about automatically using XDG env
> variables to do that automatically in the future, but currently you still
> need to add that file. Perhaps kdesrc-build could document that and maybe
> have a sudo install mode that installs that?
> 
> The second issue was that powerdevil brighness controls didn't work. This
> was because of something similar with the actions and services not
> installed for polkit-1, this is a bit more complicated as polkit-1 does not
> allow you to add extra prefix directories. So you need to add the actions
> in /usr/share/ polkit-1/actions. And those files you need to install are
> not even generated. I have tracked it down abit and found that kauth
> actually has a tool to generate the action files and have a cmake command
> kauth_install_actions that is even used by powerdevil, but the .policy
> files are still not generated nor installed in prefix or /usr/share by
> kdesrc-build. Anyone know why this is?
> 
> 'Allan


-- 
sebas

http://www.kde.org | http://vizZzion.org

sync-dbus-services.sh
Description: application/shellscript

Re: OneTimePass-Plasma

[Sebastian Kügler]

Thanks, both! Also good luck with this project, it sounds useful!

On zaterdag 3 maart 2018 10:33:45 CET Luca Beltrame wrote:
> Il giorno Fri, 02 Mar 2018 21:47:01 +0100
> 
> Sebastian Kügler <se...@kde.org> ha scritto:
> > For those not familiar wit hGoogle Authenticator, what does your
> > application do, what problem does it solve for the user? From your
> 
> It provides time-based one-time passwords (TOTP) for two-factor
> authentication: despite the name, it is not just used by Google or by
> Google products.
> 
> Other non-Google implementations exist, for example the Yubico
> Authenticator (which exists as a Qt application and a mobile
> application) which uses the YubiKey hardware token to generate the
> one-time password.
> 
> Examples of software using this particular approach in the wild are
> Github and Gitlab if you enable their 2FA support.


-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: OneTimePass-Plasma

[Sebastian Kügler]

Hi James,

For those not familiar wit hGoogle Authenticator, what does your application 
do, what problem does it solve for the user? From your description and the 
github page, I have no idea...

Cheers,

-- sebas

On vrijdag 2 maart 2018 11:24:27 CET James Augustus Zuccon wrote:
> Hi all,
> 
> I've been working on a Google Authenticator-like Application for Plasma
> (QML/C++). It also comes with a KRunner that can be triggered with "otp
> searchterm".
> 
> It uses KWallet to store passwords and liboath to generate the token (so a
> message is displayed if the KWallet Subsystem is disabled).
> 
> So far, I think the build is relatively stable, but would someone be
> willing to view the code and suggest improvements where I've failed to
> follow KDE conventions?
> 
> There are quite a few additions I would like to make to this (QR Scanner)
> and quite a few other applications I'd like to work on for KDE in
> anticipation of Plasma Mobile.
> 
> https://github.com/jimtendo/onetimepass-plasma
> 
> Thank you.


-- 
sebas

http://www.kde.org | http://vizZzion.org

D8652: Add supported transformations to OutputDevice

[Sebastian Kügler]

sebas added a comment.
Restricted Application edited projects, added Plasma on Wayland; removed Plasma.


  Dingdong?

REPOSITORY
  R127 KWayland

BRANCH
  supported-transformations

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

To: graesslin, #frameworks, #kwin, #plasma, sebas, davidedmundson
Cc: sebas, davidedmundson, plasma-devel, schernikov, leezu, ZrenBot, ngraham, 
alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, 
apol, mart, hein

D9335: invalidate the runtime cache on install

[Sebastian Kügler]

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

REPOSITORY
  R290 KPackage

BRANCH
  packagefix

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

To: mart, #plasma, sebas
Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

Re: DragonPlayer

[Sebastian Kügler]

On vrijdag 8 december 2017 15:25:06 CET Nate Graham wrote:
> Sad to say, DragonPlayer seems dead-ish. It hasn't gotten any code
> changes since January of this year. Kubuntu has stopped shipping it by
> default, replacing it with VLC. Bugzilla tickets continue to pile up.
> 
> What's the way forward here? Should we look for a new maintainer or
> admit defeat and just recommend VLC or MPV or something else?

Looking for a new maintainer sounds like the way to go.

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org

D9108: Remove implicit string casting

[Sebastian Kügler]

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

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

To: apol, #plasma, #frameworks, sebas
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

Re: liquidshell in kdereview

[Sebastian Kügler]

On Sat, 02 Dec 2017 10:27:16 +0100
Albert Astals Cid <aa...@kde.org> wrote:

> El divendres, 1 de desembre de 2017, a les 14:05:19 CET, Sebastian
> Kügler va escriure:
> > On Thu, 30 Nov 2017 18:41:28 +0100
> > 
> > Martin Koller <kol...@aon.at> wrote:
> > > On Donnerstag, 30. November 2017 10:04:51 CET Sebastian Kügler
> > > wrote:
> > > > On woensdag 29 november 2017 21:23:15 CET Martin Koller wrote:
> > > > > On Freitag, 3. November 2017 21:30:19 CET Martin Koller wrote:
> > > > > > Hi all,
> > > > > > 
> > > > > > I'd like to announce an application I've implemented over
> > > > > > the last few weeks - liquidshell
> > > > > 
> > > > > since more than 3 weeks have passed, I hopefully have
> > > > > addressed all issues and I got no further comments, are there
> > > > > any blocking points you see or can I proceed requesting the
> > > > > move to extragear ?
> > > > > 
> > > > > A note regarding the comment to not use the start-here-kde
> > > > > logo: I got in contact with the visual design group and got
> > > > > the information to stay with this logo.
> > > > 
> > > > I strongly disagree. Could you point me to the discussion?
> > > 
> > > it was mail exchange in private (german) mails with the
> > > breeze/oxygen-icons maintainer "kainz.a" <kain...@gmail.com>
> > 
> > In any case, this is not just a visual design question, it's a
> > branding and marketing question just as much, and a general
> > communication and positioning question as well.
> 
> Yes, the logo has to be changed.
> 
> > 
> > I'm vetoing any move to released software at the very least until
> > the logo has changed. I also don't think that a technical review of
> > the code is enough to warrant us to ship liquidshell as a finished
> > product, for the reasons that Martin Flöser pointed out. It would
> > harm KDE as a whole and Plasma specifically (ironically, the
> > product that you use most parts from).
> 
> This is an interesting position, are we supposed to subordinate
> technical decisions to marketing now? 

We are doing a review not just based on the code, but based on "is this
suitable and a good idea for KDE to release in this way". It's not
purely marketing, but general communication.

If I wrote a very simple application that just poppped up a Window that
said "Krita is crap", the code could technically be totally fine, but
it may still not be a good idea to do it. If Martin wants to release
this under the KDE umbrella, we should make sure we're not actively
harming other people working inside KDE.

> Also are you even sure we get better marketing by not allowing
> liquidshell to be a KDE thing?
> 
> I can very well see headlines like "Plasma developers force other KDE 
> developers outside of KDE".
> 
> I would like to empathize that we are at our core Innovation.
> 
> And yes, I agree that doing a desktop shell in QWidgets doesn't seem
> like Innovation to me, but Innovation is the process of having new
> ideas and products.

Absolutely, but it should also be advertised in a healthy way, and
liquidshell in its current form isn't.

> Maybe this one has great "external" success and against your and my
> prediction gives KDE 1000 new developers that besides contributing to
> it also end up contributing to other parts of our software range.
> 
> Let's try to think positive :)

That's the point Martin Flöser already made: liquidshell should be
advertised based on its merits, not based on "plasmashell is shit,
here's something better (I think)"

Cheers,

-- sebas

Re: liquidshell in kdereview

[Sebastian Kügler]

On Thu, 30 Nov 2017 18:41:28 +0100
Martin Koller <kol...@aon.at> wrote:

> On Donnerstag, 30. November 2017 10:04:51 CET Sebastian Kügler wrote:
> > On woensdag 29 november 2017 21:23:15 CET Martin Koller wrote:  
> > > On Freitag, 3. November 2017 21:30:19 CET Martin Koller wrote:
> > > 
> > >   
> > > > Hi all,
> > > > 
> > > > I'd like to announce an application I've implemented over the
> > > > last few weeks - liquidshell  
> > > since more than 3 weeks have passed, I hopefully have addressed
> > > all issues and I got no further comments, are there any blocking
> > > points you see or can I proceed requesting the move to extragear ?
> > > 
> > > A note regarding the comment to not use the start-here-kde logo:
> > > I got in contact with the visual design group and got the
> > > information to stay with this logo.  
> > 
> > I strongly disagree. Could you point me to the discussion?  
> 
> it was mail exchange in private (german) mails with the
> breeze/oxygen-icons maintainer "kainz.a" <kain...@gmail.com>

In any case, this is not just a visual design question, it's a branding
and marketing question just as much, and a general communication and
positioning question as well.

I'm vetoing any move to released software at the very least until the
logo has changed. I also don't think that a technical review of the
code is enough to warrant us to ship liquidshell as a finished product,
for the reasons that Martin Flöser pointed out. It would harm KDE as a
whole and Plasma specifically (ironically, the product that you use
most parts from).

Cheers,

-- sebas

Re: liquidshell in kdereview

[Sebastian Kügler]

On woensdag 29 november 2017 21:23:15 CET Martin Koller wrote:
> On Freitag, 3. November 2017 21:30:19 CET Martin Koller wrote:
> 
> 
> > Hi all,
> > 
> > I'd like to announce an application I've implemented over the last few
> > weeks - liquidshell
> since more than 3 weeks have passed, I hopefully have addressed all issues
> and I got no further comments, are there any blocking points you see or can
> I proceed requesting the move to extragear ?
> 
> A note regarding the comment to not use the start-here-kde logo:
> I got in contact with the visual design group and got the information to
> stay with this logo.

I strongly disagree. Could you point me to the discussion?
-- 
sebas

http://www.kde.org | http://vizZzion.org

D8652: Add supported transformations to OutputDevice

[Sebastian Kügler]

sebas accepted this revision.
sebas added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> outputdevice.h:244
> + * @returns The transformations supported on this OutputDevice
> + * @since 5.XX
> + * @see supportedTransformationAdded

5.40 I guess :)

> outputdevice_interface.h:133
> + * Due to that there is no remove operation for supported 
> transformations.
> + * @since 5.XX
> + **/

5.40

REPOSITORY
  R127 KWayland

BRANCH
  supported-transformations

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

To: graesslin, #frameworks, #kwin, #plasma, sebas
Cc: sebas, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, apol, mart

D7898: Only send OutputConfig sendApplied / sendFailed to the right resource

[Sebastian Kügler]

sebas added a comment.


  FWIW, I tested it, it fixes kwin crashing when adding a setApplied() call on 
the OutputConfiguration, and it also means the applied signal arrives.

REPOSITORY
  R127 KWayland

BRANCH
  master

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

To: davidedmundson, #plasma, sebas
Cc: sebas, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, apol, mart, hein

D7898: Only send OutputConfig sendApplied / sendFailed to the right resource

[Sebastian Kügler]

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


  Yes, the initial idea was to send applied() to all connected resources. That 
is not s_allResources, so your patch is correct. The resources that are 
actually bound are not currently tracked.

REPOSITORY
  R127 KWayland

BRANCH
  master

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

To: davidedmundson, #plasma, sebas
Cc: sebas, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, apol, mart, hein

Re: new here, and bug question

[Sebastian Kügler]

Hi Fred,

On woensdag 20 september 2017 16:21:44 CEST fredj...@fredjame.cnc.net
wrote:
> Please pardon the top-posting ... I am including the message to/from
> kde-community below, as background ... please note the link to a bug
> listed on Mageia, as it pretty much clarifies what I am referring to.
> https://bugs.mageia.org/show_bug.cgi?id=21624
> I am looking for any advice/guidance to squash this bug, even if it
> is "no longer supported"  ... I don't think something should be
> dropped in a broken state ... to me, it just doesn't speak well for
> the future. Please also forgive the cross posting to a Mageia
> list ... just trying to keep them in the loop, as that list is where
> this quest started.

I'm sorry, KDE can't help you with this:

- As you said, KDM is unsupported and its maintainer isn't active in
  KDE anymore
- The problem you're experiencing isn't even a problem KDE has anything
  to do with, it stems from a change that apparently Mageia developers
  made to the KDM login manager.

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: kaboutlicense api extension ::spdxId()?

[Sebastian Kügler]

On dinsdag 11 juli 2017 13:41:17 CEST Harald Sitter wrote:
> I was wondering if anyone had an opinion on extending kaboutlicense
> with a ::spdxId() instance method returning the license's spdx id [1].
> 
> Use case at hand is kpackagetool, which maps X-KDE-PluginInfo-License
> of (e.g.) plasma applets to appstream metadata. Appstream however uses
> the standardized spdx identifiers so we need a conversion table
> somewhere. And here I am thinking why not kaboutlicense itself.
> 
> QString KAboutLicense::spdxId() const
> {
> switch (d->_licenseKey) {
> case KAboutLicense::File:
> return QStringLiteral("");
> case KAboutLicense::GPL_V2:
> return QStringLiteral("GPL-2.0");
>...
> }
> 
> [1] https://spdx.org/licenses/

This would make sense and allow 3rd party tools using SPDX keys to make sense 
of X-KDE-PluginInfo-License keys, indeed. I think it's useful.

> (on a side node: it may be useful to switch our plugininfos to spdx
> ids by default at some point in the future, our current format is not
> only deficient in the lack of standardization but also doesn't cover
> 'and later' qualification at all)

It does parse "and later", it's indicated by the + sign, but it's not 
reflected in the enum, GPLv2+ would be mapped to GPLv2, so you're right, our 
current system is lacking in that regard (but could be extended, although I 
don't know what would break then.

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: KTitleWidget and the native Mac style

[Sebastian Kügler]

On dinsdag 4 juli 2017 17:55:01 CEST René J.V. Bertin wrote:
> Me neither for myself, I'm happy with how my QtCurve theme looks but I'm
> also trying to improve the way KDE software looks using the native style.

The frame in my understanding is old weight, and can go (do check with the VDG 
though!), but the larger font isn't. It's semantically a sane choice (titles 
often have larger fonts), but to me, more importantly, reducing its size will 
lead to regressions in Plasma. 

I have no problems with moving its rendering to the style so it can depend on 
the platform used, or another technically sound solution, but a change should 
not introduce regressions in Plasma.

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: Kirigami in Frameworks

[Sebastian Kügler]

On zaterdag 1 juli 2017 11:27:21 CEST Albert Astals Cid wrote:
> https://websvn.kde.org/trunk/l10n-kf5/templates/messages/frameworks/libkirig
> ami2plugin_qt.pot?revision=1492189=markup
> 
> Disagrees with you

Ow, okay. I see, I only grepped for i18n, not for qsTr(), sorry for the noise!

> On ds., jul. 1, 2017 at 9:02, Sebastian Kügler
> <se...@kde.org> wrote:
> 
> On vrijdag 30 juni 2017 23:16:49 CEST Albert Astals Cid wrote:
> > What happened to the "no new strings after the first two weeks" rule?
> 
> Kirigami doesn't have any translatable strings.
-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: Kirigami in Frameworks

[Sebastian Kügler]

On vrijdag 30 juni 2017 23:16:49 CEST Albert Astals Cid wrote:
> What happened to the "no new strings after the first two weeks" rule?

Kirigami doesn't have any translatable strings.
-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: unwanted breeze('ish) icons in KFileWidget on Mac

[Sebastian Kügler]

On dinsdag 20 juni 2017 16:10:59 CEST René J.V. Bertin wrote:
> So where do those icons come from, and why are some applications immune?
> Pointers highly appreciated!

A screenshot says more than a thousand words... :)
-- 
sebas

http://www.kde.org | http://vizZzion.org

D6215: Introduce aboutToShow() signal

[Sebastian Kügler]

sebas added a comment.


  I like this, and I'll likely need it in my kscreen OSD code aswell 
(positioning there is unreliable, and my best guess is that this is the exact 
same problem @mart is trying to solve here).
  
  +1

REPOSITORY
  R242 Plasma Framework (Library)

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

To: mart, #plasma, davidedmundson
Cc: sebas, hein, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart, lukas

D5747: add pid to plasma window management protocol

[Sebastian Kügler]

sebas closed this revision.
sebas added a comment.


  Issue has been fixed elsewhere, otherwise code is submitted.

REPOSITORY
  R127 KWayland

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

To: sebas, #plasma, hein, graesslin
Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
mart, hein, lukas

D5872: pidChanged also signals dataChanged in WindowModel

[Sebastian Kügler]

sebas abandoned this revision.
sebas added a comment.


  Okay, thanks David.

REPOSITORY
  R127 KWayland

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

To: sebas, #plasma, hein, davidedmundson
Cc: graesslin, bshah, davidedmundson, plasma-devel, #frameworks, ZrenBot, 
spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, 
sebas, apol, hein, lukas

D5872: pidChanged also signals dataChanged in WindowModel

[Sebastian Kügler]

sebas added a comment.


  Possibly, in this case, I disagree. The pid is initially unknown set to in 
the client, and can change afterwards. In reality, the pid of the window 
doesn't change, but it can still be set after being initially 0. In fact, it 
has to be.
  
  In the end, I don't care much, but having it connected to dataChanged makes 
this whole thing way more consistent, and it makes sense semantically. IMO, not 
putting it behind dataChanged is semantic wanking making the interface harder 
to understand, not easier, and not better.

REPOSITORY
  R127 KWayland

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

To: sebas, #plasma, hein, davidedmundson
Cc: bshah, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
apol, hein, lukas

D5872: pidChanged also signals dataChanged in WindowModel

[Sebastian Kügler]

sebas requested review of this revision.

REPOSITORY
  R127 KWayland

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

To: sebas, #plasma, hein, davidedmundson
Cc: bshah, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
apol, hein, lukas

D5872: pidChanged also signals dataChanged in WindowModel

[Sebastian Kügler]

sebas added a comment.


  What's d581?

REPOSITORY
  R127 KWayland

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

To: sebas, #plasma, hein, davidedmundson
Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein, 
lukas

D5872: pidChanged also signals dataChanged in WindowModel

[Sebastian Kügler]

sebas created this revision.
Restricted Application added projects: Plasma on Wayland, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  Eike removed this, but I'm not sure why. All other roles trigger
  datachanged, so I don't see why pid shouldn't.
  
  This patch fixes the tests, which I should not have broken in the first
  place.

REPOSITORY
  R127 KWayland

BRANCH
  master

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

AFFECTED FILES
  src/client/plasmawindowmodel.cpp

To: sebas, #plasma, hein
Cc: plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein, lukas

D5747: add pid to plasma window management protocol

[Sebastian Kügler]

This revision was automatically updated to reflect the committed changes.
Closed by commit R127:0b4d8a7fc1df: add pid to plasma window management 
protocol (authored by sebas).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5747?vs=14442=14463

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

AFFECTED FILES
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/plasma-window-management.xml
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: sebas, #plasma, hein, graesslin
Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
hein, lukas

D5747: add pid to plasma window management protocol

[Sebastian Kügler]

sebas updated this revision to Diff 14442.
sebas added a comment.
Restricted Application edited projects, added Plasma on Wayland; removed Plasma.


  - Update docs: the pid is just set, but doesn't logically change
  - ws--

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5747?vs=14418=14442

BRANCH
  sebas/processid

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

AFFECTED FILES
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/plasma-window-management.xml
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: sebas, #plasma, hein, graesslin
Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
hein, lukas

D5747: add pid to plasma window management protocol

[Sebastian Kügler]

sebas updated this revision to Diff 14418.
sebas added a comment.
Restricted Application edited projects, added Plasma; removed Plasma on Wayland.


  - Drop pid change handling in the model code
  - Extend model test
  
  --Eike on sebas' box

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5747?vs=14410=14418

BRANCH
  sebas/processid

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

AFFECTED FILES
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/plasma-window-management.xml
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: sebas, #plasma, hein, graesslin
Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D5747: add pid to plasma window management protocol

[Sebastian Kügler]

sebas updated this revision to Diff 14410.
sebas added a comment.
Restricted Application edited projects, added Plasma on Wayland; removed Plasma.


  - also test default

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5747?vs=14407=14410

BRANCH
  sebas/processid

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

AFFECTED FILES
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/plasma-window-management.xml
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: sebas, #plasma, hein, graesslin
Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein, 
lukas

D5747: add pid to plasma window management protocol

[Sebastian Kügler]

sebas updated this revision to Diff 14407.
sebas marked 3 inline comments as done.
sebas added a comment.
Restricted Application edited projects, added Plasma; removed Plasma on Wayland.


  Address review comments
  
  - Minor corrections in docs
  - Use uint for the pid
  - uint32 for pids instead of int32
  - Fix @since in docs
  - send initial value when it's not 0
  - Add autotest in TestWindowManagement as well

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5747?vs=14249=14407

BRANCH
  sebas/processid

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

AFFECTED FILES
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/plasma-window-management.xml
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: sebas, #plasma, hein, graesslin
Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D5812: Add missing block to make sure initial pid is sent

[Sebastian Kügler]

sebas added a comment.


  Adding it to https://phabricator.kde.org/D5747 for merge simplicity. @hein 
please abandon.

REPOSITORY
  R127 KWayland

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

To: hein, #plasma, sebas, graesslin
Cc: plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein, lukas

D5747: add pid to plasma window management protocol

[Sebastian Kügler]

sebas created this revision.
Restricted Application added projects: Plasma on Wayland, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  This patch adds a pid event to the plasma window management protocol. It
  allows the compositor to tell allow a mapping between windows and processes.
  
  Bumps the version number of the interface to 8 to indicate this.

TEST PLAN
  autotest added, passed

REPOSITORY
  R127 KWayland

BRANCH
  sebas/processid

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

AFFECTED FILES
  autotests/client/test_plasma_window_model.cpp
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/plasma-window-management.xml
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: sebas, #plasma, hein, graesslin
Cc: plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, lukas

D5721: reload icon when usesPlasmaTheme changes

[Sebastian Kügler]

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

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  phab/useplasmatheme

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

To: mart, #plasma, sebas
Cc: plasma-devel, #frameworks, spstarr, progwolff, Zren, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

[Differential] [Accepted] D4689: IconItem: Add roundToIconSize property

[Sebastian Kügler]

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


  Almost good, you can add the signalspy and then ship it from my side.

INLINE COMMENTS

> iconitemtest.cpp:526
> +
> +item->setProperty("roundToIconSize", false);
> +

Might as well check for the roundToIconSizeChanged signal here as well. We 
should test what we reasonably can, and that's an easy one.

> drosca wrote in iconitem.h:147
> The property is documented, I think there's no point in documenting the 
> getters/setters as you can't use them from QML anyway.

Right. :)

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  arcpatch-D4689

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: drosca, sebas, #plasma
Cc: sebas, mart, davidedmundson, plasma-devel, #frameworks, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, apol

[Differential] [Commented On] D4831: Add new component for the greyed out labels in Item Delegates.

[Sebastian Kügler]

sebas added a comment.


  I like it. If kbroulik is happy, consider that a +1 from me, too.

REPOSITORY
  R242 Plasma Framework (Library)

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: davidedmundson, #plasma
Cc: sebas, broulik, plasma-devel, #frameworks, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, apol

[Differential] [Requested Changes] D4689: IconItem: Add roundToIconSize property

[Sebastian Kügler]

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


  I think a problem with using roundToIconSize as isolated property is that it 
really isn't. It has intended effects on the sizing of the item, but the 
current version of the patch doesn't reflect that. I think it needs to trigger 
a series of size change signals. See my comments inline.

INLINE COMMENTS

> iconitemtest.cpp:524
> +
> +item->setProperty("roundToIconSize", false);
> +

Just checking the values here is not enough, the property change results in 
changes in paintedWidth, but we're currently not signalling that these props 
have changed. See also my comment below.

> iconitem.cpp:313
> +}
> +void IconItem::setRoundToIconSize(bool roundToIconSize)
> +{

Changing roundToIconSize may change the size of the icon, so should we trigger 
size changes (paintedWidth / paintedHeight / boundingRect likely? Perhaps 
others.) and re-rendering here as well? Right now, judging from the code, 
changing this property mid-flight is broken. We may even have to go as far as 
loading a new pixmap (in loadPixmap() from a quick glance).

Please also add unit tests for the effects on the iconitem's size properties.

> iconitem.h:147
>  
> +bool isRoundingToIconSize() const;
> +void setRoundToIconSize(bool roundToIconSize);

bool roundToIconSize() const;

we generally don't use "is" in new code where we can avoid it, and the ing 
makes this even more inconsistent. I'd much prefer the above suggestion.

Please also add api docs, at least for new code.

REPOSITORY
  R242 Plasma Framework (Library)

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: drosca, #plasma, sebas
Cc: sebas, mart, davidedmundson, plasma-devel, #frameworks, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, apol

[Differential] [Accepted] D4572: Migrate AppearAnimation and DisappearAnimation to Animators

[Sebastian Kügler]

sebas accepted this revision.
sebas added a reviewer: sebas.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: broulik, #plasma, sebas
Cc: plasma-devel, #frameworks, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

Re: Trusting .desktop files

[Sebastian Kügler]

On Saturday, February 11, 2017 7:24:11 AM UTC Martin Gräßlin wrote:
> What I don't like in general is that this is all happening as $user.
> Thus any malicious program running as $user can also just change the
> list of trusted Exec= values.
> 
> So my suggestion is: let's use polkit.
> 
> The list of trusted .desktop files must be root owned and per user.
> Everytime a user asks for executing a not known (or changed) desktop
> file, it goes through polkit. To detect changes of the desktop file I
> would suggest to store the shasum of the desktop file in addition. This
> would prevent malicious programs to just change the desktop file.
> 
> What do you think? Sensible? Too much?

I like the approach, though it does sound a bit like overkill. But then, going 
the extra mile to improve security is right within our mission, so I think the 
approach is feasible, as it provides a lot of value for what we regard as our 
core competence.

I can imagine this mechanism to be useful for other things as well, such as 
scripts, binaries and such that are user-writable.
-- 
sebas

http://www.kde.org • http://vizZzion.org

Re: Review Request 122732: Add std::function overloads for KServiceTypeTrader

[Sebastian Kügler]

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



Is this still relevant? If not, please discard it...

- Sebastian Kügler


On Feb. 26, 2015, 7:44 p.m., Alex Richardson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122732/
> ---
> 
> (Updated Feb. 26, 2015, 7:44 p.m.)
> 
> 
> Review request for KDE Frameworks, Marco Martin and Sebastian Kügler.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> These are a lot more flexible and less error-prone and will eventually
> allow us to remove the trader query language in KF6 once all users in
> KService are gone.
> 
> REVIEW: 122732
> 
> 
> Diffs
> -
> 
>   autotests/kservicetest.cpp d46f868185c3bf45138d80d04f4eb0d2840de9ca 
>   autotests/ksycocathreadtest.cpp fbd889b28a32397fbf9245827ff8b54405b82e3d 
>   src/services/kservicetypetrader.h 8e46812c2eeddca225e978a4dd55aa4cc5e902d0 
>   src/services/kservicetypetrader.cpp 
> 290e44e9161c8db47278543714426fdd3b5a87af 
> 
> Diff: https://git.reviewboard.kde.org/r/122732/diff/
> 
> 
> Testing
> ---
> 
> Unit tests pass
> 
> There are still some more classes that use the string based checks, I'll add 
> a std::function overload to them as well once this has been approved.
> 
> 
> Thanks,
> 
> Alex Richardson
> 
>

Re: Review Request 129917: Add a cache monitor to the System Load Viewer applet

[Sebastian Kügler]

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




applets/systemloadviewer/package/contents/config/main.xml (line 52)
<https://git.reviewboard.kde.org/r/129917/#comment68316>

I think it's overkill to enable this by default, given it's only 
interesting to a limited group of users, and others probably won't understand 
it, or its utility.


- Sebastian Kügler


On Feb. 6, 2017, 8:05 p.m., Pascal VITOUX wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129917/
> ---
> 
> (Updated Feb. 6, 2017, 8:05 p.m.)
> 
> 
> Review request for kde-workspace and Plasma.
> 
> 
> Repository: kdeplasma-addons
> 
> 
> Description
> ---
> 
> Add a cache monitor to the System Load Viewer applet using 'cache/dirty' and 
> 'cache/writeback' infos from ksysguard 5.9.0
> 
> The monitor displays the dirty + writeback amount proportionaly scaled with 
> the maximal amount reached, until the remaining dirty amount goes below a 
> minimal threshold of 10MB.
> 
> An interesting usecase is to show the real progress of a datas transfer to a 
> slow storage device like a USB drive.
> 
> 
> Diffs
> -
> 
>   applets/systemloadviewer/package/contents/config/main.xml 6bf16d5aa 
>   applets/systemloadviewer/package/contents/ui/ColorSettings.qml b9247aa15 
>   applets/systemloadviewer/package/contents/ui/GeneralSettings.qml f1ab40a1b 
>   applets/systemloadviewer/package/contents/ui/SystemLoadViewer.qml 5a0bc0649 
> 
> Diff: https://git.reviewboard.kde.org/r/129917/diff/
> 
> 
> Testing
> ---
> 
> I use it since several months without issue.
> 
> 
> File Attachments
> 
> 
> panel applets : circular, bar and compact bar monitors
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2017/02/06/9067d5be-c1b4-4168-be20-76965b4a40ac__panel.png
> desktop applet, with tooltip
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2017/02/06/4eb7a092-7368-448f-8385-7c18ab298389__desktop-tooltip.png
> desktop applet, circular monitors
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2017/02/06/26f769fb-85e9-45f7-90e6-55035770be6e__desktop-circular.png
> 
> 
> Thanks,
> 
> Pascal VITOUX
> 
>

Re: Review Request 129917: Add a cache monitor to the System Load Viewer applet

[Sebastian Kügler]

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



Could you please add screenshots of the new feature? This is important for us 
to review the visual impact your patch has. Thanks already for your work on 
this! I think it's a useful addition.

- Sebastian Kügler


On Feb. 5, 2017, 6:11 p.m., Pascal VITOUX wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129917/
> ---
> 
> (Updated Feb. 5, 2017, 6:11 p.m.)
> 
> 
> Review request for kde-workspace and Plasma.
> 
> 
> Repository: kdeplasma-addons
> 
> 
> Description
> ---
> 
> Add a cache monitor to the System Load Viewer applet using 'cache/dirty' and 
> 'cache/writeback' infos from ksysguard 5.9.0
> 
> The monitor displays the dirty + writeback amount proportionaly scaled with 
> the maximal amount reached, until the remaining dirty amount goes below a 
> minimal threshold of 10MB.
> 
> An interesting usecase is to show the real progress of a datas transfer to a 
> slow storage device like a USB drive.
> 
> 
> Diffs
> -
> 
>   applets/systemloadviewer/package/contents/config/main.xml 6bf16d5aa 
>   applets/systemloadviewer/package/contents/ui/ColorSettings.qml b9247aa15 
>   applets/systemloadviewer/package/contents/ui/GeneralSettings.qml f1ab40a1b 
>   applets/systemloadviewer/package/contents/ui/SystemLoadViewer.qml 5a0bc0649 
> 
> Diff: https://git.reviewboard.kde.org/r/129917/diff/
> 
> 
> Testing
> ---
> 
> I use it since several months without issue.
> 
> 
> Thanks,
> 
> Pascal VITOUX
> 
>

Re: Review Request 127387: Refactor the backend loading code

[Sebastian Kügler]

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

(Updated Feb. 4, 2017, 11:18 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma, Solid, Daniel Vrátil, and Martin Gräßlin.


Repository: libkscreen


Description
---

Refactor the backend loading code

Untangle the large plugin loading logic in the BackendManager static
into  separate bits. This makes the code clearer and easier to auto-test.

- listBackends() compiles a list of backends from the plugin paths
- preferredBackend() picks the backend, in this priority:
- if KSCREEN_BACKEND is set in the environment, this is the only
  used method to find the backend plugin
- if platform is X11, the XRandR backend is picked
- if platform is wayland, KWayland backend is picked
- if neither is the case, QScreen backend is picked

It does introduce a slight behavioral change: The mechanism was based on
falling through, so it would consider another backend if the logically
picked on fails to load. This is undesired behavior, however, since the
backendloader may be able to load the plugin, but that doesn't mean that
the plugin actually work.

Parsing of the KSCREEN_BACKEND variable is kept case-insensitive.


autotests for new backend loading logic

- makes sure we find plugins installed
- pick plugins from env var

We can't sensible test all the runtime cases yet, but this at least
covers the code paths around those few lines that do runtime detection
of x11 and wayland.


Diffs
-

  autotests/CMakeLists.txt 26c7952 
  autotests/testbackendloader.cpp PRE-CREATION 
  src/backendmanager.cpp 382ae71 
  src/backendmanager_p.h 150883b 

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


Testing
---

manual runtime tests and autotests pass


Thanks,

Sebastian Kügler

Re: Review Request 114437: Add right click contextmenu for Plasma Components TextField

[Sebastian Kügler]

> On Jan. 22, 2017, 10:20 p.m., Albert Astals Cid wrote:
> > Sebas should I commited this? After 3 years i'm not sure kde-runtime is the 
> > best thing to update in it's almost frozen state.
> 
> Sebastian Kügler wrote:
> I'd just discard it. I don't think anybody is releasing kde-runtime 
> anyway, and the proper way to fix this is to do it in QtQuick component's 
> Label. Thanks for cleaning up, though!
> 
> Albert Astals Cid wrote:
> I do release kde-runtime
> 
> 
> http://download.kde.org/stable/applications/16.12.1/src/kde-runtime-16.12.1.tar.xz.mirrorlist

Ah, okay. Up to you then. I think this patch is rather safe, but I wouldn't 
want to get support calls for a regression...


- Sebastian


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


On Dec. 13, 2013, 3:37 p.m., Leszek Lesner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114437/
> ---
> 
> (Updated Dec. 13, 2013, 3:37 p.m.)
> 
> 
> Review request for KDE Runtime.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> This adds a right click context menu for PlasmaComponents.TextField which 
> allows to cut, copy and paste text. 
> 
> 
> Diffs
> -
> 
>   plasma/declarativeimports/plasmacomponents/qml/TextField.qml 10a3d1f 
> 
> Diff: https://git.reviewboard.kde.org/r/114437/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Leszek Lesner
> 
>

Re: Review Request 114437: Add right click contextmenu for Plasma Components TextField

[Sebastian Kügler]

> On Jan. 22, 2017, 10:20 p.m., Albert Astals Cid wrote:
> > Sebas should I commited this? After 3 years i'm not sure kde-runtime is the 
> > best thing to update in it's almost frozen state.

I'd just discard it. I don't think anybody is releasing kde-runtime anyway, and 
the proper way to fix this is to do it in QtQuick component's Label. Thanks for 
cleaning up, though!


- Sebastian


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


On Dec. 13, 2013, 3:37 p.m., Leszek Lesner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114437/
> ---
> 
> (Updated Dec. 13, 2013, 3:37 p.m.)
> 
> 
> Review request for KDE Runtime.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> This adds a right click context menu for PlasmaComponents.TextField which 
> allows to cut, copy and paste text. 
> 
> 
> Diffs
> -
> 
>   plasma/declarativeimports/plasmacomponents/qml/TextField.qml 10a3d1f 
> 
> Diff: https://git.reviewboard.kde.org/r/114437/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Leszek Lesner
> 
>

Re: All Frameworks modules are now available as repositories on Phabricator

[Sebastian Kügler]

On Tuesday, December 27, 2016 8:56:00 PM UTC Luigi Toscano wrote:
> Elvis Angelaccio ha scritto:
> > On Mon, Dec 26, 2016 at 1:19 AM, Luigi Toscano <luigi.tosc...@tiscali.it> 
wrote:
> >> ... so that it's easier to use Differential for reviews.

Much appreciated, thanks!

> > Thanks!
> > What about tasks? Should we create a Workboard for the Frameworks
> > project? Or is it planned that each framework be a Project with its
> > own workboard?
> 
> This is entirely up to each maintainer. I personally think that 80% of the
> modules in Frameworks don't need a separate project for each of them, but
> again, if someone needs a separate project (and workboard), sure, it can be
> created.

Note that tasks can be on more than one workboard at the same time (they even 
carry status per workboard). So we can have a frameworks board with all tasks 
and project-specific board with just the relevant tasks, for example.
-- 
sebas

Sebastian Kügler•http://vizZzion.org•http://www.kde.org

Re: Review Request 129607: Display Application version in About dialog header

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On Dec. 7, 2016, 11:31 a.m., Jean-Baptiste Mardelle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129607/
> ---
> 
> (Updated Dec. 7, 2016, 11:31 a.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Sebastian Kügler.
> 
> 
> Repository: kxmlgui
> 
> 
> Description
> ---
> 
> Last year, a change in kaboutapplicationdialog (review 124133) moved the 
> application version in a separate tab. However for applications, it is really 
> annoying (several Kdenlive users complained and I fully agree) to not have 
> this information clearly displayed as soon as the About dialog is displayed. 
> I understand the interest of having a separate version tab displaying the 
> various underlying libraries used, but I propose to display again the 
> application version in the About dialog header (see screenshot).
> 
> 
> Diffs
> -
> 
>   src/kaboutapplicationdialog.cpp 156410e 
> 
> Diff: https://git.reviewboard.kde.org/r/129607/diff/
> 
> 
> Testing
> ---
> 
> Reverts a recent change, tested and works.
> 
> 
> File Attachments
> 
> 
> About dialog with version
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/12/03/c02f4767-2a74-4c25-8599-de410678867e__about.png
> Updated patch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/12/03/ee13cb47-2162-4be7-8255-3dce7f470c2b__about2.png
> renamed to libraries
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/12/07/f01ed0c3-1391-48cf-a153-a177d9a7b54a__about3.png
> 
> 
> Thanks,
> 
> Jean-Baptiste Mardelle
> 
>

Re: Review Request 129588: http slave: add newlines to long error message

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On Nov. 29, 2016, 6:23 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129588/
> ---
> 
> (Updated Nov. 29, 2016, 6:23 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Otherwise Dolphin shows an error dialog with too much width.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/http/http.cpp 62eb09d2882097c3dea99ea73fd1933cea4376a7 
> 
> Diff: https://git.reviewboard.kde.org/r/129588/diff/
> 
> 
> Testing
> ---
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Before
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/11/29/bb368736-5987-4800-996f-2e05c4971d7f__Spectacle.a29637.png
> After
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/11/29/456f0afe-d192-4e09-8c9a-1fa0b27f5bb4__6wJEi10.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

Re: Review Request 129607: Display Application version in About dialog header

[Sebastian Kügler]

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



I understand the problem, but I don't like the duplication you're introducing 
with this patch: The version number is now on the screen two times when you 
open the versions tab, and also, why have a versions tab, when the version is 
already displayed.

- Sebastian Kügler


On Dec. 3, 2016, 12:51 p.m., Jean-Baptiste Mardelle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129607/
> ---
> 
> (Updated Dec. 3, 2016, 12:51 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Sebastian Kügler.
> 
> 
> Repository: kxmlgui
> 
> 
> Description
> ---
> 
> Last year, a change in kaboutapplicationdialog (review 124133) moved the 
> application version in a separate tab. However for applications, it is really 
> annoying (several Kdenlive users complained and I fully agree) to not have 
> this information clearly displayed as soon as the About dialog is displayed. 
> I understand the interest of having a separate version tab displaying the 
> various underlying libraries used, but I propose to display again the 
> application version in the About dialog header (see screenshot).
> 
> 
> Diffs
> -
> 
>   src/kaboutapplicationdialog.cpp 156410e 
> 
> Diff: https://git.reviewboard.kde.org/r/129607/diff/
> 
> 
> Testing
> ---
> 
> Reverts a recent change, tested and works.
> 
> 
> File Attachments
> 
> 
> About dialog with version
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/12/03/c02f4767-2a74-4c25-8599-de410678867e__about.png
> 
> 
> Thanks,
> 
> Jean-Baptiste Mardelle
> 
>

Re: Review Request 126226: kdetemplate_add_app_templates installs previews

[Sebastian Kügler]

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

(Updated Dec. 2, 2016, 12:13 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks and Marco Martin.


Repository: extra-cmake-modules


Description
---

kapptemplate can't deal with templats without previews, so make sure we install 
the preview image.

Without this patch, the installed templates show up broken in kapptemplate. 
With it, they work as expected.

I'm assuming here that the preview has the baseName + .png, otherwise we'd have 
to read Icon field from the .kdevtemplate file, but that seems way too much 
hassle. The error message resulting from a wrong file name will show the 
expected filename, so it doesn't exactly hide the error.


Diffs
-

  kde-modules/KDETemplateMacro.cmake 796c3f1 

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


Testing
---

Installed templates/ from plasma-framework, this patch makes them work in 
kapptemplate.


Thanks,

Sebastian Kügler

Re: Review Request 129598: fix build, needs QtDBus

[Sebastian Kügler]

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

(Updated Dec. 2, 2016, 11:09 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 04389b36653b2d0df8a3ac2fc62bafa1b86e by Sebastian 
Kügler to branch master.


Repository: frameworkintegration


Description
---

The new appstream kpackage handler needs to link Qt DBus, but the find package 
call is missing.


Diffs
-

  CMakeLists.txt e97e698 

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


Testing
---

Builds and works on my machine


Thanks,

Sebastian Kügler

Review Request 129598: fix build, needs QtDBus

[Sebastian Kügler]

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

Review request for KDE Frameworks.


Repository: frameworkintegration


Description
---

The new appstream kpackage handler needs to link Qt DBus, but the find package 
call is missing.


Diffs
-

  CMakeLists.txt e97e698 

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


Testing
---

Builds and works on my machine


Thanks,

Sebastian Kügler

[Differential] [Accepted] D3314: Properly test environment variable

[Sebastian Kügler]

sebas accepted this revision.
sebas added a reviewer: sebas.
This revision is now accepted and ready to land.

BRANCH
  master

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: apol, #frameworks, sebas

Re: Review Request 129118: Remove discovery associated to a key when removing a definition

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On Oct. 7, 2016, 4:10 p.m., Antonio Larrosa Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129118/
> ---
> 
> (Updated Oct. 7, 2016, 4:10 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> Plasmashell was calling Package::removeDefinition but a later call to
> Package::filePath still returned a value because there was an entry in
> d->discoveries but no way to remove it, which made plasma use old
> values.
> 
> 
> Diffs
> -
> 
>   src/kpackage/package.cpp 363a67ed5b7c592aea4845b84ca05e63da05991a 
> 
> Diff: https://git.reviewboard.kde.org/r/129118/diff/
> 
> 
> Testing
> ---
> 
> I gdb'd a patched plasmashell (1) to fix a wallpaper aspect ratio problem
> while changing the screen resolution between 1024x768 and 1280x720
> and tracing Image::findPreferedImageInPackage . 
> 
> package.filePath("preferred") is not empty at the beginning of the method
> even if package.removeDefinition("preferred") was just called, so it seems
> the key should be removed from d->discoveries too.
> 
> (1) Plasma patch is comming to phabricator in the next minutes.
> 
> 
> Thanks,
> 
> Antonio Larrosa Jimenez
> 
>

Re: Review Request 128749: Backport karchive fix for out of directory files

[Sebastian Kügler]

> On Sept. 7, 2016, 9:49 a.m., Sebastian Kügler wrote:
> > This patch seems to have caused a regression when installing some wallpaper 
> > packages, Marco fixed it in the knewstuff frameworks with 
> > 39b33ddd1e21c017b. We may have a problem now -- we'll have to fix Plasma 4? 
> > :/
> 
> Albert Astals Cid wrote:
> a quick look on that commit, how is related to this one? they seem to be 
> touching different parts of the code
> 
> Sebastian Kügler wrote:
> Wallpaper packages were unzipped into the parent directory instead of 
> creating a subdirectory for the wallpaper. This commit caused this 
> regression, Marco's commit fixed it in knewstuff. (It should be relatively 
> easy to test, try installing my "Borneo Pink" wallpaper from the wallpaper 
> dialog in Plasma and look into ~/.local/share/wallpapers/ directory (with and 
> without patches).
> 
> Albert Astals Cid wrote:
> I'm going to be away from a computer for almost 3 weeks, make sure you 
> cotnact Andreas/David off this site (it's easy to ignore comments on already 
> submitted reviews) if they don't answer to try to get this fixed.
> 
> Albert Astals Cid wrote:
> Was there any progress on this front?

Not from my side, it slipped off my radar. :/


- Sebastian


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


On Aug. 25, 2016, 10:34 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128749/
> ---
> 
> (Updated Aug. 25, 2016, 10:34 p.m.)
> 
> 
> Review request for kdelibs, Andreas Cord-Landwehr and David Faure.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> What the summary says
> 
> 
> Diffs
> -
> 
>   kdecore/io/karchive.cpp eb0bf2e 
>   kdecore/tests/karchivetest.h e64e0ed 
>   kdecore/tests/karchivetest.cpp 7786b98 
> 
> Diff: https://git.reviewboard.kde.org/r/128749/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

Re: Review Request 128740: uncompress archives in subfolders

[Sebastian Kügler]

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


Ship it!




Tested and confirmed working. Thanks, Marco!

- Sebastian Kügler


On Aug. 23, 2016, 2:43 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128740/
> ---
> 
> (Updated Aug. 23, 2016, 2:43 p.m.)
> 
> 
> Review request for KDE Frameworks, Sebastian Kügler and Jeremy Whiting.
> 
> 
> Repository: knewstuff
> 
> 
> Description
> ---
> 
> if the archive downloaded by ghns has more than a file inside (either single 
> file or a folder at the root), uncompress it in a folder called the same as 
> the file name (without path or extension)
> 
> this fixes downloading and installing plasma wallpaper packages (those that 
> provide multiple resolutions).
> I don't know if this behavior would in turn break other applications, but i 
> don't think so as it seems the behavior changed between kde4 and kf5.
> if needed i can make it do this only in the case of wallpapers (if 
> (standardResourceDirectory == QLatin1String("wallpaper"))) but i think this 
> should be the correct behavior
> 
> 
> Diffs
> -
> 
>   src/core/installation.cpp a027418 
> 
> Diff: https://git.reviewboard.kde.org/r/128740/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

Re: Review Request 128641: Fix argument type in QMetaObject invoke *NEEDS BACKPORTING TO 5.25*

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On Aug. 9, 2016, 11:04 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128641/
> ---
> 
> (Updated Aug. 9, 2016, 11:04 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and David Faure.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Commit bb70febdbe397b617e5c41aff5494fdbc185fa88 ported an argument from
> QPoint to QRectF. However, not all local invocations were updated correctly 
> with one passing a QRect, which fails.
> 
> *NEEDS BACKPORTING TO 5.25*
> 
> 
> Diffs
> -
> 
>   src/scriptengines/qml/plasmoid/containmentinterface.cpp 
> 4efc1e109bbab8ef43b686be25574ed5bd66d9ce 
> 
> Diff: https://git.reviewboard.kde.org/r/128641/diff/
> 
> 
> Testing
> ---
> 
> Before: Couldn't drag applet from Widget Explorer to Desktop
> After: Can
> 
> Checked the other invocations are correct.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

Re: Review Request 128640: Keep compatiable slot createApplet with Frameworks 5.24 *THIS NEEDS BACKPORTING TO 5.25*

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On Aug. 9, 2016, 11:03 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128640/
> ---
> 
> (Updated Aug. 9, 2016, 11:03 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and David Faure.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Commit bb70febdbe397b617e5c41aff5494fdbc185fa88 changed the slot for
> adding createApplet, turning the final argument from QPoint to QRect.
> 
> If the rectangle size is nothing, it acts like the original code.
> 
> Despite this being private API (ish) there is a hacks in plasma-
> workspace that call methods on the view via QMetaObject invoke. This
> obviously fails. As we need compatibility for Plasma/5.7 and earlier a
> compatibility slot needs to stay.
> 
> 
> *THIS NEEDS BACKPORTING TO 5.25*
> 
> 
> Diffs
> -
> 
>   src/scriptengines/qml/plasmoid/containmentinterface.h 
> c058f8358b4aa123749959a3de5b0667d7a1fecc 
>   src/scriptengines/qml/plasmoid/containmentinterface.cpp 
> 4efc1e109bbab8ef43b686be25574ed5bd66d9ce 
> 
> Diff: https://git.reviewboard.kde.org/r/128640/diff/
> 
> 
> Testing
> ---
> 
> Before: alternatives context menu on the desktop just deleted the applet
> Now: works
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

Re: Review Request 128594: Add a kapptemplate for Plasma Wallpaper

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On Aug. 3, 2016, 5:05 p.m., Friedrich W. H. Kossebau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128594/
> ---
> 
> (Updated Aug. 3, 2016, 5:05 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> This template should allow people to get a basic working wallpaper plugin 
> within minutes.
> 
> 
> Patch (in separate commits) also
> - updates the READMEs of the plasmoid templates to point to correct QML2 wiki 
> pages.
> - mobves the plasmoid and wallpaper templates into own toplevel category 
> "Plasma/", out of "KDE/", given this is a platform target of its own
> 
> 
> Diffs
> -
> 
>   templates/CMakeLists.txt b51777d 
>   templates/cpp-plasmoid/README d3582db 
>   templates/cpp-plasmoid/cpp-plasmoid.kdevtemplate 42924b0 
>   templates/plasma-wallpaper/CMakeLists.txt PRE-CREATION 
>   templates/plasma-wallpaper/Messages.sh PRE-CREATION 
>   templates/plasma-wallpaper/README PRE-CREATION 
>   templates/plasma-wallpaper/package/contents/config/main.xml PRE-CREATION 
>   templates/plasma-wallpaper/package/contents/ui/config.qml PRE-CREATION 
>   templates/plasma-wallpaper/package/contents/ui/main.qml PRE-CREATION 
>   templates/plasma-wallpaper/package/metadata.desktop PRE-CREATION 
>   templates/plasma-wallpaper/plasma-wallpaper.kdevtemplate PRE-CREATION 
>   templates/qml-plasmoid/README 6814263 
>   templates/qml-plasmoid/qml-plasmoid.kdevtemplate 2675e71 
> 
> Diff: https://git.reviewboard.kde.org/r/128594/diff/
> 
> 
> Testing
> ---
> 
> Installed the template, then created a new wallpaper plugin using 
> kapptemplate and following the README, then selected in wallpaper config, 
> incl. editing the plugin config by entering a new text.
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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

Re: [Kde-hardware-devel] multiscreen logging

[Sebastian Kügler]

On woensdag 27 juli 2016 17:14:21 CEST Bhushan Shah wrote:
> I wonder if code is failsafe in case of such clash while opening
> logfile? Because for sure, we don't want another crashes..

I don't see how it would crash.
-- 
sebas

http://www.kde.org | http://vizZzion.org
___
Kde-hardware-devel mailing list
Kde-hardware-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-hardware-devel

Re: [Kde-hardware-devel] multiscreen logging

[Sebastian Kügler]

On dinsdag 26 juli 2016 14:03:50 CEST Sebastian Kügler wrote:
> What do you think? If you like the idea, I'll polish up my branch and will 
> post it for review, so we can discuss the actual implementation.

https://phabricator.kde.org/D2295 works well for my purposes.
-- 
sebas

http://www.kde.org | http://vizZzion.org
___
Kde-hardware-devel mailing list
Kde-hardware-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-hardware-devel

Re: Snappy sprint reporty musing

[Sebastian Kügler]

On Tuesday, July 26, 2016 2:27:34 PM CEST Aleix Pol wrote:
> > Could you expand on the distribution mechanism?
> 
> I don't understand the question.

That's what you already did with your email, so thanks. :-)
-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: Snappy sprint reporty musing

[Sebastian Kügler]

On Tuesday, July 26, 2016 1:08:28 PM CEST Harald Sitter wrote:
> - a store REST API (of which the reference version is the ubuntu store)

So something like this exists for flatpaks as well, and it's open source? For 
snappy, we'd either have to use the ubuntu store (non-free, right?) or write 
our own from scratch?

Could you expand on the distribution mechanism?

Thanks for the very interesting report!
-- 
sebas

http://www.kde.org | http://vizZzion.org

[Kde-hardware-devel] multiscreen logging

[Sebastian Kügler]

Hey,

[Please keep both lists addressed.]

Debugging multiscreen issues is a nightmare:

- there are at least 4 different processes involved (kded, kcmshell / 
  systemsettings, kscreen_backend_launcher and plasmashell)
- some are critical during log in
- they IPC with each other
- especially the backend launcher's debug is really hard to get at

This means that:
- it's hard (almost impossible) for users to get us good and useful logs
- it's hard for ourselves to debug and find out what's exactly going on, 
especially when multiple components need to play in tune

Yesternight, after debugging the so-many-th issue, it occurred to me that we 
need to make this way easier to debug. Q(C)Debug falls short in that we get 
logs of individual components, if we're lucky. If we're really lucky, we get 
timestamps, so we can get a rough idea of what is going down when.

All of these problems can be solved with a relatively simple, shared log file.

So I'd like to switch most of (lib)kscreen's debug output to logging to a 
file. The files has then logs from multiple processes and will be much easier 
to go through.

API-wise, my main thing is having log messages and a bit of context, so it'd 
look like this:

Log::instance()->setContext("handling resume event");
// ...
Log::log("Enabled output" + output->name());

In the log, we'd then get

[TIMESTAMP] (kded: handling resume event) Enabled output eDP-1


The key here is that we get a mostly correct sequence of things, that all the 
info is there right away, and that it's easier for the user to annotate the 
log ("Now plugging in my external display as HDMI-2").

The file is simple enough that even plasmashell could log to the file (at 
least until we've fixed the interaction between screen handling and 
plasmashell), so we get the full picture.

libkscreen[sebas/log] has a basic implementation. It's incomplete in the sense 
that it needs a bit of autotesting (just haven't gotten to it yet), review and 
then switching over the code-base. (For now it's on by default, but can be 
disabled using a env var.)

I hope that this makes it much more straight-forward for us (and even users) 
to figure out where their multiscreen headaches are coming from, and that it 
gives us a one-stop shop (pretty much) to tell users what info we need to fix 
their bugs. ("Just send me the logfile in ~/.local/share/kscreen.log").

What do you think? If you like the idea, I'll polish up my branch and will 
post it for review, so we can discuss the actual implementation.

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org
___
Kde-hardware-devel mailing list
Kde-hardware-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-hardware-devel

Re: Review Request 128466: Rename Checksums tab to Integrity

[Sebastian Kügler]

> On July 18, 2016, 12:05 p.m., Sebastian Kügler wrote:
> > Please don't ship it, yet.
> > 
> > 
> > I find the UI illogical. There's a groupbox grouping the checksum buttons, 
> > but then you can input the checksum above, so essentially, the groupbox is 
> > unnecessary and confusing.
> > 
> > Perhaps the whole thing could be simplified by naming the tab "Checksums" 
> > and removing the groupbox altogether, as it's not providing any semantic 
> > value.
> > 
> > A usability reviewer should have a look.
> 
> Kai Uwe Broulik wrote:
> This dialog has been created in Review 128283 and Usability has been 
> involved from the beginning...
> 
> Sebastian Kügler wrote:
> It has changed in a significant way, though, making it illogical.
> 
> (Not that I understand the "Share" title in the original review, but 
> that's another matter.)
> 
> This needs more work.
> 
> Elvis Angelaccio wrote:
> > Perhaps the whole thing could be simplified by naming the tab 
> "Checksums" and removing the groupbox altogether, as it's not providing any 
> semantic value.
> 
> Preview here: https://share.kde.org/index.php/s/RUs9gAhIQqpFIqF
> 
> Sebastian Kügler wrote:
> This looks logical to me, and it's simpler: Very good!
> 
> (Take that as "sebas withdraws his objection" :))
> 
> Thomas Pfeiffer wrote:
> Clear -1 to removing the group box.
> 
> Here is tha rationale:
> For most "regular" users, only the lineedit at the top is relevant. The 
> calculate stuff is just distraction and - worse - potential confusion. If we 
> remove any visual distinction between the two, we just make the situation 
> worse for them.
> 
> I agree that calling the tab "Checksums" is still better, though, because 
> "Integrity" is too vague.
> 
> For the "average user" I just re-tested this with, what would actually 
> help her is creating a second box around the verification part, calling the 
> top one "Verify checksum" and the bottom "Calculate checksums".
> That way if she was told e.g. by a website to verify a checksum, she'd 
> knew she could simply ignore the whole calculation part.
> 
> Overall simplicity should not be the top priority here: The priority 
> should be to make it clear to users who just want to check whether a download 
> went okay which part they should care about and which they can ignore, while 
> still providing the calculation part for advanced users who want to do that.
> 
> Of course another option would be to split it in two tabs, but that might 
> make the tab bar quite long especially in languages like German.
> 
> Sebastian Kügler wrote:
> The latter part seems redundant then. How can you verify a checksum 
> without calculating it?
> 
> Thomas Pfeiffer wrote:
> See _that_ is why the bottom box was originally called "Share". Of course 
> the verification part also does calculation, but it does so without you 
> telling it to. You paste in the checksum, and it tells you whether it matches 
> or not.
> The manual calculation at the bottom is for people who share a file with 
> others and want to accompany it with a checksum for _them_ to verify it.
> 
> I think we might get away with "Calculate" anyway because those who don#t 
> know much about checksums don't need to know that the verify part calculates 
> as well, and those who know it should still be able to use the thing 
> correctly.
> 
> Sebastian Kügler wrote:
> That 'Share' title completely puzzled me, and I think I'm the kind of 
> user this dialog should work for very well. (I need to verify checksums all 
> the time.)
> 
> To be quite honest, from getting it explained, I get the strong 
> impression that you're overthinking it.
> 
> To me, the most logical would be:
> 
> * Calculate checksums at the top
> * Under that, the input field so I can c/p or type my checksum in there 
> to have it compared automatically. 
> 
> That's both, the order of the workflow as well as the logical order of 
> operation. 'Calculate' underneath would raise exactly the same question as I 
> put above: "...but but but ... it could not verify it without calculating it, 
> yet I have to hit a button to calculate ... maybe I should try again and 
> maybe I should just use the commandline to be sure". 
> 
> Point in case: for this kind of stuff, simplicity trumps since it makes 
> it easier to TRUST the dialog. I can't trust anything I don't fully 

Re: Review Request 128466: Rename Checksums tab to Integrity

[Sebastian Kügler]

> On July 18, 2016, 12:05 p.m., Sebastian Kügler wrote:
> > Please don't ship it, yet.
> > 
> > 
> > I find the UI illogical. There's a groupbox grouping the checksum buttons, 
> > but then you can input the checksum above, so essentially, the groupbox is 
> > unnecessary and confusing.
> > 
> > Perhaps the whole thing could be simplified by naming the tab "Checksums" 
> > and removing the groupbox altogether, as it's not providing any semantic 
> > value.
> > 
> > A usability reviewer should have a look.
> 
> Kai Uwe Broulik wrote:
> This dialog has been created in Review 128283 and Usability has been 
> involved from the beginning...
> 
> Sebastian Kügler wrote:
> It has changed in a significant way, though, making it illogical.
> 
> (Not that I understand the "Share" title in the original review, but 
> that's another matter.)
> 
> This needs more work.
> 
> Elvis Angelaccio wrote:
> > Perhaps the whole thing could be simplified by naming the tab 
> "Checksums" and removing the groupbox altogether, as it's not providing any 
> semantic value.
> 
> Preview here: https://share.kde.org/index.php/s/RUs9gAhIQqpFIqF
> 
> Sebastian Kügler wrote:
> This looks logical to me, and it's simpler: Very good!
> 
> (Take that as "sebas withdraws his objection" :))
> 
> Thomas Pfeiffer wrote:
> Clear -1 to removing the group box.
> 
> Here is tha rationale:
> For most "regular" users, only the lineedit at the top is relevant. The 
> calculate stuff is just distraction and - worse - potential confusion. If we 
> remove any visual distinction between the two, we just make the situation 
> worse for them.
> 
> I agree that calling the tab "Checksums" is still better, though, because 
> "Integrity" is too vague.
> 
> For the "average user" I just re-tested this with, what would actually 
> help her is creating a second box around the verification part, calling the 
> top one "Verify checksum" and the bottom "Calculate checksums".
> That way if she was told e.g. by a website to verify a checksum, she'd 
> knew she could simply ignore the whole calculation part.
> 
> Overall simplicity should not be the top priority here: The priority 
> should be to make it clear to users who just want to check whether a download 
> went okay which part they should care about and which they can ignore, while 
> still providing the calculation part for advanced users who want to do that.
> 
> Of course another option would be to split it in two tabs, but that might 
> make the tab bar quite long especially in languages like German.
> 
> Sebastian Kügler wrote:
> The latter part seems redundant then. How can you verify a checksum 
> without calculating it?
> 
> Thomas Pfeiffer wrote:
> See _that_ is why the bottom box was originally called "Share". Of course 
> the verification part also does calculation, but it does so without you 
> telling it to. You paste in the checksum, and it tells you whether it matches 
> or not.
> The manual calculation at the bottom is for people who share a file with 
> others and want to accompany it with a checksum for _them_ to verify it.
> 
> I think we might get away with "Calculate" anyway because those who don#t 
> know much about checksums don't need to know that the verify part calculates 
> as well, and those who know it should still be able to use the thing 
> correctly.
> 
> Sebastian Kügler wrote:
> That 'Share' title completely puzzled me, and I think I'm the kind of 
> user this dialog should work for very well. (I need to verify checksums all 
> the time.)
> 
> To be quite honest, from getting it explained, I get the strong 
> impression that you're overthinking it.
> 
> To me, the most logical would be:
> 
> * Calculate checksums at the top
> * Under that, the input field so I can c/p or type my checksum in there 
> to have it compared automatically. 
> 
> That's both, the order of the workflow as well as the logical order of 
> operation. 'Calculate' underneath would raise exactly the same question as I 
> put above: "...but but but ... it could not verify it without calculating it, 
> yet I have to hit a button to calculate ... maybe I should try again and 
> maybe I should just use the commandline to be sure". 
> 
> Point in case: for this kind of stuff, simplicity trumps since it makes 
> it easier to TRUST the dialog. I can't trust anything I don't fully 

Re: Review Request 128466: Rename Checksums tab to Integrity

[Sebastian Kügler]

> On July 18, 2016, 12:05 p.m., Sebastian Kügler wrote:
> > Please don't ship it, yet.
> > 
> > 
> > I find the UI illogical. There's a groupbox grouping the checksum buttons, 
> > but then you can input the checksum above, so essentially, the groupbox is 
> > unnecessary and confusing.
> > 
> > Perhaps the whole thing could be simplified by naming the tab "Checksums" 
> > and removing the groupbox altogether, as it's not providing any semantic 
> > value.
> > 
> > A usability reviewer should have a look.
> 
> Kai Uwe Broulik wrote:
> This dialog has been created in Review 128283 and Usability has been 
> involved from the beginning...
> 
> Sebastian Kügler wrote:
> It has changed in a significant way, though, making it illogical.
> 
> (Not that I understand the "Share" title in the original review, but 
> that's another matter.)
> 
> This needs more work.
> 
> Elvis Angelaccio wrote:
> > Perhaps the whole thing could be simplified by naming the tab 
> "Checksums" and removing the groupbox altogether, as it's not providing any 
> semantic value.
> 
> Preview here: https://share.kde.org/index.php/s/RUs9gAhIQqpFIqF
> 
> Sebastian Kügler wrote:
> This looks logical to me, and it's simpler: Very good!
> 
> (Take that as "sebas withdraws his objection" :))
> 
> Thomas Pfeiffer wrote:
> Clear -1 to removing the group box.
> 
> Here is tha rationale:
> For most "regular" users, only the lineedit at the top is relevant. The 
> calculate stuff is just distraction and - worse - potential confusion. If we 
> remove any visual distinction between the two, we just make the situation 
> worse for them.
> 
> I agree that calling the tab "Checksums" is still better, though, because 
> "Integrity" is too vague.
> 
> For the "average user" I just re-tested this with, what would actually 
> help her is creating a second box around the verification part, calling the 
> top one "Verify checksum" and the bottom "Calculate checksums".
> That way if she was told e.g. by a website to verify a checksum, she'd 
> knew she could simply ignore the whole calculation part.
> 
> Overall simplicity should not be the top priority here: The priority 
> should be to make it clear to users who just want to check whether a download 
> went okay which part they should care about and which they can ignore, while 
> still providing the calculation part for advanced users who want to do that.
> 
> Of course another option would be to split it in two tabs, but that might 
> make the tab bar quite long especially in languages like German.
> 
> Sebastian Kügler wrote:
> The latter part seems redundant then. How can you verify a checksum 
> without calculating it?
> 
> Thomas Pfeiffer wrote:
> See _that_ is why the bottom box was originally called "Share". Of course 
> the verification part also does calculation, but it does so without you 
> telling it to. You paste in the checksum, and it tells you whether it matches 
> or not.
> The manual calculation at the bottom is for people who share a file with 
> others and want to accompany it with a checksum for _them_ to verify it.
> 
> I think we might get away with "Calculate" anyway because those who don#t 
> know much about checksums don't need to know that the verify part calculates 
> as well, and those who know it should still be able to use the thing 
> correctly.
> 
> Sebastian Kügler wrote:
> That 'Share' title completely puzzled me, and I think I'm the kind of 
> user this dialog should work for very well. (I need to verify checksums all 
> the time.)
> 
> To be quite honest, from getting it explained, I get the strong 
> impression that you're overthinking it.
> 
> To me, the most logical would be:
> 
> * Calculate checksums at the top
> * Under that, the input field so I can c/p or type my checksum in there 
> to have it compared automatically. 
> 
> That's both, the order of the workflow as well as the logical order of 
> operation. 'Calculate' underneath would raise exactly the same question as I 
> put above: "...but but but ... it could not verify it without calculating it, 
> yet I have to hit a button to calculate ... maybe I should try again and 
> maybe I should just use the commandline to be sure". 
> 
> Point in case: for this kind of stuff, simplicity trumps since it makes 
> it easier to TRUST the dialog. I can't trust anything I don't fully 

Re: Review Request 128466: Rename Checksums tab to Integrity

[Sebastian Kügler]

> On July 18, 2016, 12:05 p.m., Sebastian Kügler wrote:
> > Please don't ship it, yet.
> > 
> > 
> > I find the UI illogical. There's a groupbox grouping the checksum buttons, 
> > but then you can input the checksum above, so essentially, the groupbox is 
> > unnecessary and confusing.
> > 
> > Perhaps the whole thing could be simplified by naming the tab "Checksums" 
> > and removing the groupbox altogether, as it's not providing any semantic 
> > value.
> > 
> > A usability reviewer should have a look.
> 
> Kai Uwe Broulik wrote:
> This dialog has been created in Review 128283 and Usability has been 
> involved from the beginning...
> 
> Sebastian Kügler wrote:
> It has changed in a significant way, though, making it illogical.
> 
> (Not that I understand the "Share" title in the original review, but 
> that's another matter.)
> 
> This needs more work.
> 
> Elvis Angelaccio wrote:
> > Perhaps the whole thing could be simplified by naming the tab 
> "Checksums" and removing the groupbox altogether, as it's not providing any 
> semantic value.
> 
> Preview here: https://share.kde.org/index.php/s/RUs9gAhIQqpFIqF
> 
> Sebastian Kügler wrote:
> This looks logical to me, and it's simpler: Very good!
> 
> (Take that as "sebas withdraws his objection" :))
> 
> Thomas Pfeiffer wrote:
> Clear -1 to removing the group box.
> 
> Here is tha rationale:
> For most "regular" users, only the lineedit at the top is relevant. The 
> calculate stuff is just distraction and - worse - potential confusion. If we 
> remove any visual distinction between the two, we just make the situation 
> worse for them.
> 
> I agree that calling the tab "Checksums" is still better, though, because 
> "Integrity" is too vague.
> 
> For the "average user" I just re-tested this with, what would actually 
> help her is creating a second box around the verification part, calling the 
> top one "Verify checksum" and the bottom "Calculate checksums".
> That way if she was told e.g. by a website to verify a checksum, she'd 
> knew she could simply ignore the whole calculation part.
> 
> Overall simplicity should not be the top priority here: The priority 
> should be to make it clear to users who just want to check whether a download 
> went okay which part they should care about and which they can ignore, while 
> still providing the calculation part for advanced users who want to do that.
> 
> Of course another option would be to split it in two tabs, but that might 
> make the tab bar quite long especially in languages like German.
> 
> Sebastian Kügler wrote:
> The latter part seems redundant then. How can you verify a checksum 
> without calculating it?
> 
> Thomas Pfeiffer wrote:
> See _that_ is why the bottom box was originally called "Share". Of course 
> the verification part also does calculation, but it does so without you 
> telling it to. You paste in the checksum, and it tells you whether it matches 
> or not.
> The manual calculation at the bottom is for people who share a file with 
> others and want to accompany it with a checksum for _them_ to verify it.
> 
> I think we might get away with "Calculate" anyway because those who don#t 
> know much about checksums don't need to know that the verify part calculates 
> as well, and those who know it should still be able to use the thing 
> correctly.

That 'Share' title completely puzzled me, and I think I'm the kind of user this 
dialog should work for very well. (I need to verify checksums all the time.)

To be quite honest, from getting it explained, I get the strong impression that 
you're overthinking it.

To me, the most logical would be:

* Calculate checksums at the top
* Under that, the input field so I can c/p or type my checksum in there to have 
it compared automatically. 

That's both, the order of the workflow as well as the logical order of 
operation. 'Calculate' underneath would raise exactly the same question as I 
put above: "...but but but ... it could not verify it without calculating it, 
yet I have to hit a button to calculate ... maybe I should try again and maybe 
I should just use the commandline to be sure". 

Point in case: for this kind of stuff, simplicity trumps since it makes it 
easier to TRUST the dialog. I can't trust anything I don't fully understand or 
have doubts about, and that's what the groupbox design and the share title 
cause: doubts.


- Sebastian


--

Re: Review Request 128466: Rename Checksums tab to Integrity

[Sebastian Kügler]

> On July 18, 2016, 12:05 p.m., Sebastian Kügler wrote:
> > Please don't ship it, yet.
> > 
> > 
> > I find the UI illogical. There's a groupbox grouping the checksum buttons, 
> > but then you can input the checksum above, so essentially, the groupbox is 
> > unnecessary and confusing.
> > 
> > Perhaps the whole thing could be simplified by naming the tab "Checksums" 
> > and removing the groupbox altogether, as it's not providing any semantic 
> > value.
> > 
> > A usability reviewer should have a look.
> 
> Kai Uwe Broulik wrote:
> This dialog has been created in Review 128283 and Usability has been 
> involved from the beginning...
> 
> Sebastian Kügler wrote:
> It has changed in a significant way, though, making it illogical.
> 
> (Not that I understand the "Share" title in the original review, but 
> that's another matter.)
> 
> This needs more work.
> 
> Elvis Angelaccio wrote:
> > Perhaps the whole thing could be simplified by naming the tab 
> "Checksums" and removing the groupbox altogether, as it's not providing any 
> semantic value.
> 
> Preview here: https://share.kde.org/index.php/s/RUs9gAhIQqpFIqF
> 
> Sebastian Kügler wrote:
> This looks logical to me, and it's simpler: Very good!
> 
> (Take that as "sebas withdraws his objection" :))
> 
> Thomas Pfeiffer wrote:
> Clear -1 to removing the group box.
> 
> Here is tha rationale:
> For most "regular" users, only the lineedit at the top is relevant. The 
> calculate stuff is just distraction and - worse - potential confusion. If we 
> remove any visual distinction between the two, we just make the situation 
> worse for them.
> 
> I agree that calling the tab "Checksums" is still better, though, because 
> "Integrity" is too vague.
> 
> For the "average user" I just re-tested this with, what would actually 
> help her is creating a second box around the verification part, calling the 
> top one "Verify checksum" and the bottom "Calculate checksums".
> That way if she was told e.g. by a website to verify a checksum, she'd 
> knew she could simply ignore the whole calculation part.
> 
> Overall simplicity should not be the top priority here: The priority 
> should be to make it clear to users who just want to check whether a download 
> went okay which part they should care about and which they can ignore, while 
> still providing the calculation part for advanced users who want to do that.
> 
> Of course another option would be to split it in two tabs, but that might 
> make the tab bar quite long especially in languages like German.

The latter part seems redundant then. How can you verify a checksum without 
calculating it?


- Sebastian


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


On July 16, 2016, 12:35 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128466/
> ---
> 
> (Updated July 16, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Dominik Haumann.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Dominik suggested to rename the `Checksums` tab to `Integrity`, so that we 
> can "free" the Checksums string and use it as the title of the groupbox below 
> (in place of the current `Share` string, which can be confusing).
> 
> Preview in the attached screenshot.
> 
> 
> Diffs
> -
> 
>   src/widgets/checksumswidget.ui 03c64db 
>   src/widgets/kpropertiesdialog.cpp 808765c 
> 
> Diff: https://git.reviewboard.kde.org/r/128466/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Before
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/6771ed06-c803-4d18-abe3-91e4f97c8c76__checksums-tab.png
> After
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/b2cd12c8-6bbf-4123-9e8e-59cb0c29cbdb__Spectacle.TJ7614.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

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

Re: Review Request 128466: Rename Checksums tab to Integrity

[Sebastian Kügler]

> On July 18, 2016, 12:05 p.m., Sebastian Kügler wrote:
> > Please don't ship it, yet.
> > 
> > 
> > I find the UI illogical. There's a groupbox grouping the checksum buttons, 
> > but then you can input the checksum above, so essentially, the groupbox is 
> > unnecessary and confusing.
> > 
> > Perhaps the whole thing could be simplified by naming the tab "Checksums" 
> > and removing the groupbox altogether, as it's not providing any semantic 
> > value.
> > 
> > A usability reviewer should have a look.
> 
> Kai Uwe Broulik wrote:
> This dialog has been created in Review 128283 and Usability has been 
> involved from the beginning...
> 
> Sebastian Kügler wrote:
> It has changed in a significant way, though, making it illogical.
> 
> (Not that I understand the "Share" title in the original review, but 
> that's another matter.)
> 
> This needs more work.
> 
> Elvis Angelaccio wrote:
> > Perhaps the whole thing could be simplified by naming the tab 
> "Checksums" and removing the groupbox altogether, as it's not providing any 
> semantic value.
> 
> Preview here: https://share.kde.org/index.php/s/RUs9gAhIQqpFIqF

This looks logical to me, and it's simpler: Very good!

(Take that as "sebas withdraws his objection" :))


- Sebastian


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


On July 16, 2016, 12:35 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128466/
> ---
> 
> (Updated July 16, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Dominik Haumann.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Dominik suggested to rename the `Checksums` tab to `Integrity`, so that we 
> can "free" the Checksums string and use it as the title of the groupbox below 
> (in place of the current `Share` string, which can be confusing).
> 
> Preview in the attached screenshot.
> 
> 
> Diffs
> -
> 
>   src/widgets/checksumswidget.ui 03c64db 
>   src/widgets/kpropertiesdialog.cpp 808765c 
> 
> Diff: https://git.reviewboard.kde.org/r/128466/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Before
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/6771ed06-c803-4d18-abe3-91e4f97c8c76__checksums-tab.png
> After
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/b2cd12c8-6bbf-4123-9e8e-59cb0c29cbdb__Spectacle.TJ7614.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

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

Re: Review Request 128466: Rename Checksums tab to Integrity

[Sebastian Kügler]

> On July 18, 2016, 12:05 p.m., Sebastian Kügler wrote:
> > Please don't ship it, yet.
> > 
> > 
> > I find the UI illogical. There's a groupbox grouping the checksum buttons, 
> > but then you can input the checksum above, so essentially, the groupbox is 
> > unnecessary and confusing.
> > 
> > Perhaps the whole thing could be simplified by naming the tab "Checksums" 
> > and removing the groupbox altogether, as it's not providing any semantic 
> > value.
> > 
> > A usability reviewer should have a look.
> 
> Kai Uwe Broulik wrote:
> This dialog has been created in Review 128283 and Usability has been 
> involved from the beginning...

It has changed in a significant way, though, making it illogical.

(Not that I understand the "Share" title in the original review, but that's 
another matter.)

This needs more work.


- Sebastian


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


On July 16, 2016, 12:35 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128466/
> ---
> 
> (Updated July 16, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Dominik Haumann.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Dominik suggested to rename the `Checksums` tab to `Integrity`, so that we 
> can "free" the Checksums string and use it as the title of the groupbox below 
> (in place of the current `Share` string, which can be confusing).
> 
> Preview in the attached screenshot.
> 
> 
> Diffs
> -
> 
>   src/widgets/checksumswidget.ui 03c64db 
>   src/widgets/kpropertiesdialog.cpp 808765c 
> 
> Diff: https://git.reviewboard.kde.org/r/128466/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Before
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/6771ed06-c803-4d18-abe3-91e4f97c8c76__checksums-tab.png
> After
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/b2cd12c8-6bbf-4123-9e8e-59cb0c29cbdb__Spectacle.TJ7614.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

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

Re: Review Request 128466: Rename Checksums tab to Integrity

[Sebastian Kügler]

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



Please don't ship it, yet.


I find the UI illogical. There's a groupbox grouping the checksum buttons, but 
then you can input the checksum above, so essentially, the groupbox is 
unnecessary and confusing.

Perhaps the whole thing could be simplified by naming the tab "Checksums" and 
removing the groupbox altogether, as it's not providing any semantic value.

A usability reviewer should have a look.

- Sebastian Kügler


On July 16, 2016, 12:35 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128466/
> ---
> 
> (Updated July 16, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Dominik Haumann.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Dominik suggested to rename the `Checksums` tab to `Integrity`, so that we 
> can "free" the Checksums string and use it as the title of the groupbox below 
> (in place of the current `Share` string, which can be confusing).
> 
> Preview in the attached screenshot.
> 
> 
> Diffs
> -
> 
>   src/widgets/checksumswidget.ui 03c64db 
>   src/widgets/kpropertiesdialog.cpp 808765c 
> 
> Diff: https://git.reviewboard.kde.org/r/128466/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Before
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/6771ed06-c803-4d18-abe3-91e4f97c8c76__checksums-tab.png
> After
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/b2cd12c8-6bbf-4123-9e8e-59cb0c29cbdb__Spectacle.TJ7614.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

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

Re: web server for appstream metadata screenshots

[Sebastian Kügler]

On dinsdag 14 juni 2016 01:03:39 CEST Albert Astals Cid wrote:
> El dimarts, 14 de juny de 2016, a les 0:54:06 CEST, Matthias Klumpp va 
> 
> escriure:
> > 2016-06-14 0:34 GMT+02:00 Albert Astals Cid :
> > > [...]
> > >
> > > You can not have all of these three:
> > >  * Free commit to screenshots used by appstream
> >
> > Create a Git/$whatever repository just for screenshots somewhere, and
> > make its contents available on the web, e.g. via static.kde.org /
> > screenshots.kde.org, etc.
> >
> > >  * Shared screenshots between www and appstream
> >
> > Have www embed screenshots from the xyz.kde.org server, instead of
> > keeping them with the other www-related files in SVN. This also allows
> > www to migrate to Git more easily, unless there are other large files
> > stored in there.
> 
> You realize this breaks the "protected contents in www" if everyone can
> upload  files to xyz.kde.org and they magically show up in www, right?

Different screenshots for website and appstream.

That said, i really don't mind developers being able to update screenshots. 
They shouldn't be able to put new text on the website, or screw with the 
scripts, but simply putting screenshots somewhere to be picked up is not a 
problem in my books. (And yes, I realize that I can write "Free Tibet!" on a 
screenshot and upload that.)

We're responsible people. The problem is not irresponsibility, it's that our 
websites don't get updated often and that our app metadata isn't very 
complete.
-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: web server for appstream metadata screenshots

[Sebastian Kügler]

Thanks all for the input!

On woensdag 8 juni 2016 13:46:17 CEST Burkhard Lück wrote:
> > I've been adding appstream metadata to one of the apps I maintain, among
> > that are also screenshots, in the form of a URL. That means that I have to
> > put the screenshots on a webserver.
> >
> > Do we already have a canonical location for these screenshots? 
> 
> Scripty tracks 93 appdata files in trunk kf5
> 
> 65 of these appdata files have a png on http://kde.org/images/screenshots/
> 
> These pngs are also used for https://www.kde.org/applications/
> 
> Means you have only one source for the app screenshot and don't need to
> sync  two locations

I've done that now for Cuttlefish.

The obvious problem with that is that it needs interaction from someone with 
commit rights to the website, so we created a gatekeeper here in the space 
that should be under the control of the app developer.

In practice, that means, that less app devs will provide screenshots with 
their appstream metadata, and appstream adoption is not quite excellent to 
begin with.

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org

web server for appstream metadata screenshots

[Sebastian Kügler]

Hey,

I've been adding appstream metadata to one of the apps I maintain, among that 
are also screenshots, in the form of a URL. That means that I have to put the 
screenshots on a webserver.

Do we already have a canonical location for these screenshots? If not, let's 
create one before we get people hosting them on imgur, their private webserver 
or their router-behind-a-dsl-line. :)

Cheers,
-- 
sebas

Sebastian Kügler•http://vizZzion.org•http://www.kde.org

Re: Review Request 128085: Fix check that CPU is a valid CPU

[Sebastian Kügler]

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


Ship it!




- Sebastian Kügler


On June 3, 2016, 12:15 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128085/
> ---
> 
> (Updated June 3, 2016, 12:15 p.m.)
> 
> 
> Review request for KDE Frameworks and Solid.
> 
> 
> Repository: solid
> 
> 
> Description
> ---
> 
> We have a check that an devices/system/cpu/cpuN entry contains a
> processor and not just an empty socket, however it relies on somewhat
> outdated /sys files.
> 
> cpuN/cpufreq isn't guaranteed to exist as for some systems (my AMD
> processor at least).
> 
> The docs in the linux kernel imply topology/core_id should always exist,
> and should still work as a valid check that we have a populated CPU
> socket.
> 
> 
> Diffs
> -
> 
>   src/solid/devices/backends/udev/udevmanager.cpp 
> 3f3a671798e84e6d577df7c3b9b80150ac4d01fc 
> 
> Diff: https://git.reviewboard.kde.org/r/128085/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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

Re: [Kde-hardware-devel] Review Request 128085: Fix check that CPU is a valid CPU

[Sebastian Kügler]

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


Ship it!




- Sebastian Kügler


On June 3, 2016, 12:15 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128085/
> ---
> 
> (Updated June 3, 2016, 12:15 p.m.)
> 
> 
> Review request for KDE Frameworks and Solid.
> 
> 
> Repository: solid
> 
> 
> Description
> ---
> 
> We have a check that an devices/system/cpu/cpuN entry contains a
> processor and not just an empty socket, however it relies on somewhat
> outdated /sys files.
> 
> cpuN/cpufreq isn't guaranteed to exist as for some systems (my AMD
> processor at least).
> 
> The docs in the linux kernel imply topology/core_id should always exist,
> and should still work as a valid check that we have a populated CPU
> socket.
> 
> 
> Diffs
> -
> 
>   src/solid/devices/backends/udev/udevmanager.cpp 
> 3f3a671798e84e6d577df7c3b9b80150ac4d01fc 
> 
> Diff: https://git.reviewboard.kde.org/r/128085/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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

Re: Review Request 127870: plasma-framework: AppletInterface::downloadPath()

[Sebastian Kügler]

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


Ship it!




ping :)

- Sebastian Kügler


On May 8, 2016, 4:15 p.m., Allen Winter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127870/
> ---
> 
> (Updated May 8, 2016, 4:15 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Deprecate downloadPath(const QString ) in favor of downloadPath() since 
> the file argument is not used.
> 
> 
> Diffs
> -
> 
>   src/scriptengines/qml/plasmoid/appletinterface.h 6cf71aa 
>   src/scriptengines/qml/plasmoid/appletinterface.cpp 876af2c 
> 
> Diff: https://git.reviewboard.kde.org/r/127870/diff/
> 
> 
> Testing
> ---
> 
> compiles. make test still passes.
> 
> 
> Thanks,
> 
> Allen Winter
> 
>

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

Re: Review Request 127842: take highlight and highlightedText from proper color group

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On May 5, 2016, 9:54 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127842/
> ---
> 
> (Updated May 5, 2016, 9:54 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> lately the highlight color of plasma became a very washed-out blue, (since it 
> switched to use the global system colors) that was because it actually taken 
> the highlight color (and highlightedText) from the wrong source.
> there is a correct color group in kcolorscheme for that, that is Selection, 
> take those two colors from that (also means highlight and highlightedtext 
> always be the same in all plasma::theme::colorgroups but that's not a problem)
> 
> update test to reflect that
> 
> 
> Diffs
> -
> 
>   autotests/themetest.cpp ce512a4 
>   src/declarativeimports/core/quicktheme.h ac5e121 
>   src/plasma/private/theme_p.h d2246de 
>   src/plasma/private/theme_p.cpp e6d55c3 
> 
> Diff: https://git.reviewboard.kde.org/r/127842/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 127779: use system colors for monochrome icons

[Sebastian Kügler]

> On May 2, 2016, 1:17 p.m., Sebastian Kügler wrote:
> > src/kiconloader.cpp, line 869
> > <https://git.reviewboard.kde.org/r/127779/diff/9/?file=461529#file461529line869>
> >
> > Where does this one get deleted?
> 
> Marco Martin wrote:
> it's a QScopedPointer, so it gets deleted when the function returns

Ah right ... sorry for being blind. :)


- Sebastian


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


On May 4, 2016, 9:56 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127779/
> ---
> 
> (Updated May 4, 2016, 9:56 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Breeze uses stylesheet information to color its icons with some "named" 
> colors that change depending from the current system color scheme, this is 
> already used since some time in the icons used by the Plasma shell.
> But QWidget applications have the same problem, when the user changes the 
> color scheme from breeze to breeze dark (or any color scheme), most of the 
> icons will be black on black.
> This patch clones (a bit simplified and taking only the most "important" 
> colors) the logic used by Plasma::Svg to color icons with the stylesheet.
> 
> even tough it's doing more things at icon generation, an application that 
> uses a lot of icons like Dolphin doesn't seem to have noticeable startup time 
> difference, even when the image cache is not present yet, so i hope is not an 
> unacceptable performance tradeoff (successive loads are unchanged as are from 
> the image cache).
> 
> still some questions and things that can be optimized, like
> 
> * an optional key in the theme metadata file to explicitly enable this, to 
> not have it running in themes that don't care?
> 
> * can i use karchive in this framework?, so it would work with svgz icons as 
> well
> 
> * right now to refresh icons at runtime it depends from a patch in the colors 
> kcm to emit iconchanges as well, alternatively kiconloader could watch for 
> kcolorscheme changes as well, but i don't think is strictly necessary
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 2e838e8 
>   autotests/coloredsvgicon.svg PRE-CREATION 
>   autotests/kiconloader_unittest.cpp 0e47cc8 
>   autotests/resources.qrc a19c963 
>   src/CMakeLists.txt 0e30a35 
>   src/kiconloader.cpp 75ab482 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp 0996054 
> 
> Diff: https://git.reviewboard.kde.org/r/127779/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> dadel1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/04/28/0fc42425-947c-479e-9759-09c7a703a456__dadel1.png
> Gwenview in fullscreen flips colors
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/04/1c0cab70-a54f-4533-b5e9-0b3af07ae087__dadel2.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 127819: [autotests] Use -displayfd as argument to start Xvfb

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On May 3, 2016, 6:32 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127819/
> ---
> 
> (Updated May 3, 2016, 6:32 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Makes the test more reliable, our side blocks till the server is
> fully started.
> 
> The approach is used by KWin's autotest.
> 
> 
> Diffs
> -
> 
>   autotests/netrootinfotestwm.cpp 3816ce2c0ff4ca7a6c5b1c37532e10f0fafec63c 
>   autotests/netwininfotestclient.cpp 222b5b1e959fa44bb2b242757ff32abf31b63be7 
>   autotests/netwininfotestwm.cpp 9670a14dcb8faebf2ac6af8d56ead3681fa11715 
> 
> Diff: https://git.reviewboard.kde.org/r/127819/diff/
> 
> 
> Testing
> ---
> 
> 
> 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 127779: use system colors for monochrome icons

[Sebastian Kügler]

> On May 2, 2016, 1:09 p.m., Sebastian Kügler wrote:
> > Okay, let's try this in master and see if it's going to cause problems.
> 
> Kai Uwe Broulik wrote:
> Beware this is frameworks and the next frameworks release will be 
> upcoming Saturday
> 
> Marco Martin wrote:
> I was aiming at first for this one. Do you think has potential issues 
> that would make it better having it in 5.23?

I don't see any, but then, it's pretty intrusive. I'd trust your judgment, 
Marco.


- Sebastian


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


On April 29, 2016, 9:25 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127779/
> ---
> 
> (Updated April 29, 2016, 9:25 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Breeze uses stylesheet information to color its icons with some "named" 
> colors that change depending from the current system color scheme, this is 
> already used since some time in the icons used by the Plasma shell.
> But QWidget applications have the same problem, when the user changes the 
> color scheme from breeze to breeze dark (or any color scheme), most of the 
> icons will be black on black.
> This patch clones (a bit simplified and taking only the most "important" 
> colors) the logic used by Plasma::Svg to color icons with the stylesheet.
> 
> even tough it's doing more things at icon generation, an application that 
> uses a lot of icons like Dolphin doesn't seem to have noticeable startup time 
> difference, even when the image cache is not present yet, so i hope is not an 
> unacceptable performance tradeoff (successive loads are unchanged as are from 
> the image cache).
> 
> still some questions and things that can be optimized, like
> 
> * an optional key in the theme metadata file to explicitly enable this, to 
> not have it running in themes that don't care?
> 
> * can i use karchive in this framework?, so it would work with svgz icons as 
> well
> 
> * right now to refresh icons at runtime it depends from a patch in the colors 
> kcm to emit iconchanges as well, alternatively kiconloader could watch for 
> kcolorscheme changes as well, but i don't think is strictly necessary
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 2e838e8 
>   autotests/coloredsvgicon.svg PRE-CREATION 
>   autotests/kiconloader_unittest.cpp 0e47cc8 
>   autotests/resources.qrc a19c963 
>   src/CMakeLists.txt 0e30a35 
>   src/kiconloader.cpp 75ab482 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp 0996054 
> 
> Diff: https://git.reviewboard.kde.org/r/127779/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> dadel1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/04/28/0fc42425-947c-479e-9759-09c7a703a456__dadel1.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 127810: Fix some gcc5.3 compile warnings in kwayland

[Sebastian Kügler]

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


Ship it!




- Sebastian Kügler


On May 1, 2016, 8:32 p.m., Allen Winter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127810/
> ---
> 
> (Updated May 1, 2016, 8:32 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Martin Gräßlin.
> 
> 
> Repository: kwayland
> 
> 
> Description
> ---
> 
> eliminate -Wmissing-include-dirs, -Wunused-parameter and -Wsign-compare 
> warnings from gcc5.3.1
> 
> 
> Diffs
> -
> 
>   autotests/client/CMakeLists.txt ed0970b 
>   src/client/outputdevice.cpp 6fc3d3c 
>   src/client/outputmanagement.cpp 9aaf331 
>   src/client/plasmawindowmanagement.cpp 81c6a3b 
> 
> Diff: https://git.reviewboard.kde.org/r/127810/diff/
> 
> 
> Testing
> ---
> 
> still compiles.
> a warning remains that I don't know how to fix yet: 
> /data/kde5/src/frameworks/kwayland/src/client/buffer_p.h:32:15: warning: 
> ‘KWayland::Client::Buffer::Private’ has a field 
> ‘KWayland::Client::Buffer::Private::nativeBuffer’ whose type uses the 
> anonymous namespace
> 
> but otherwise most other compiler warnings are fixed with this patch
> 
> 
> Thanks,
> 
> Allen Winter
> 
>

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

Re: Review Request 127779: use system colors for monochrome icons

[Sebastian Kügler]

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




autotests/kiconloader_unittest.cpp (line 441)
<https://git.reviewboard.kde.org/r/127779/#comment64522>

Has the image been recolored to red ...

(word order makes it easier to read)



src/kiconloader.cpp (line 290)
<https://git.reviewboard.kde.org/r/127779/#comment64524>

If the icon is an SVG file, ...



src/kiconloader.cpp (line 292)
<https://git.reviewboard.kde.org/r/127779/#comment64523>

text (lower case), perhaps @see the colors that are available?



src/kiconloader.cpp (line 869)
<https://git.reviewboard.kde.org/r/127779/#comment64525>

Where does this one get deleted?


- Sebastian Kügler


On April 29, 2016, 9:25 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127779/
> ---
> 
> (Updated April 29, 2016, 9:25 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Breeze uses stylesheet information to color its icons with some "named" 
> colors that change depending from the current system color scheme, this is 
> already used since some time in the icons used by the Plasma shell.
> But QWidget applications have the same problem, when the user changes the 
> color scheme from breeze to breeze dark (or any color scheme), most of the 
> icons will be black on black.
> This patch clones (a bit simplified and taking only the most "important" 
> colors) the logic used by Plasma::Svg to color icons with the stylesheet.
> 
> even tough it's doing more things at icon generation, an application that 
> uses a lot of icons like Dolphin doesn't seem to have noticeable startup time 
> difference, even when the image cache is not present yet, so i hope is not an 
> unacceptable performance tradeoff (successive loads are unchanged as are from 
> the image cache).
> 
> still some questions and things that can be optimized, like
> 
> * an optional key in the theme metadata file to explicitly enable this, to 
> not have it running in themes that don't care?
> 
> * can i use karchive in this framework?, so it would work with svgz icons as 
> well
> 
> * right now to refresh icons at runtime it depends from a patch in the colors 
> kcm to emit iconchanges as well, alternatively kiconloader could watch for 
> kcolorscheme changes as well, but i don't think is strictly necessary
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 2e838e8 
>   autotests/coloredsvgicon.svg PRE-CREATION 
>   autotests/kiconloader_unittest.cpp 0e47cc8 
>   autotests/resources.qrc a19c963 
>   src/CMakeLists.txt 0e30a35 
>   src/kiconloader.cpp 75ab482 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp 0996054 
> 
> Diff: https://git.reviewboard.kde.org/r/127779/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> dadel1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/04/28/0fc42425-947c-479e-9759-09c7a703a456__dadel1.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 127779: use system colors for monochrome icons

[Sebastian Kügler]

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


Ship it!




Okay, let's try this in master and see if it's going to cause problems.

- Sebastian Kügler


On April 29, 2016, 9:25 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127779/
> ---
> 
> (Updated April 29, 2016, 9:25 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Breeze uses stylesheet information to color its icons with some "named" 
> colors that change depending from the current system color scheme, this is 
> already used since some time in the icons used by the Plasma shell.
> But QWidget applications have the same problem, when the user changes the 
> color scheme from breeze to breeze dark (or any color scheme), most of the 
> icons will be black on black.
> This patch clones (a bit simplified and taking only the most "important" 
> colors) the logic used by Plasma::Svg to color icons with the stylesheet.
> 
> even tough it's doing more things at icon generation, an application that 
> uses a lot of icons like Dolphin doesn't seem to have noticeable startup time 
> difference, even when the image cache is not present yet, so i hope is not an 
> unacceptable performance tradeoff (successive loads are unchanged as are from 
> the image cache).
> 
> still some questions and things that can be optimized, like
> 
> * an optional key in the theme metadata file to explicitly enable this, to 
> not have it running in themes that don't care?
> 
> * can i use karchive in this framework?, so it would work with svgz icons as 
> well
> 
> * right now to refresh icons at runtime it depends from a patch in the colors 
> kcm to emit iconchanges as well, alternatively kiconloader could watch for 
> kcolorscheme changes as well, but i don't think is strictly necessary
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 2e838e8 
>   autotests/coloredsvgicon.svg PRE-CREATION 
>   autotests/kiconloader_unittest.cpp 0e47cc8 
>   autotests/resources.qrc a19c963 
>   src/CMakeLists.txt 0e30a35 
>   src/kiconloader.cpp 75ab482 
>   src/kicontheme.h 3190665 
>   src/kicontheme.cpp 0996054 
> 
> Diff: https://git.reviewboard.kde.org/r/127779/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> dadel1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/04/28/0fc42425-947c-479e-9759-09c7a703a456__dadel1.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

[Kde-hardware-devel] hail to the new plasma-pa maintainer

[Sebastian Kügler]

Hi all,

David has been working tirelessly on plasma-pa, our pulseaudio volume 
controller applet. Since by now, he understands the code much better than me 
(the current maintainer), it's only fair to transfer maintainership to him.

So, please applaud the new maintainer of plasma-pa: David Rosca.

Thanks David for taking over!

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org
___
Kde-hardware-devel mailing list
Kde-hardware-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-hardware-devel

Re: Review Request 127602: Add a test case for KWindowEffects::isEffectAvailable

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On April 7, 2016, 2:12 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127602/
> ---
> 
> (Updated April 7, 2016, 2:12 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Just verifies that reading the property on the root window works as
> expected.
> 
> 
> Diffs
> -
> 
>   autotests/kwindoweffectstest.cpp 06a52a1e6e3d0fb4c967ff9c5295bb3a0381ee51 
> 
> Diff: https://git.reviewboard.kde.org/r/127602/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

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

Re: remove khelpcenter from next Plasma release?

[Sebastian Kügler]

Hi Yuri,

On Wednesday, March 09, 2016 06:37:09 PM Yuri Chornoivan wrote:
> Yes. That's what I meant.
> 
> But generally, I cannot say that I like the way the things are going. The  
> UserBase project does not have significant updates on applications pages  
> (although it was claimed that users are willing to write docs online),  
> there is a trend to sweep docs under the carpet, move them online, cut off  
> docs from applications, compile and package without docs, release without
> docs at all. That's not pretty and fair.
> 
> Sorry for being too rough. Please forgive me.

Being rough is fine, as long as you're also specific and constructive. You're 
not, so your being rough is just a useless rant, not something anybody can 
really work with.

How about you point out concrete problems and solutions for them? Most of us 
are not here for listening to rants, but want to actually improve something. 
Don't poison the atmosphere for those people.

The effect of your email (having written docs quite extensively in the past 
year) is that you're generalizing way too much, to the point I feel offended 
(and stupid for doing the unrecognized work in the first place).

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: [Kde-hardware-devel] Review Request 127387: Refactor the backend loading code

[Sebastian Kügler]

> On March 15, 2016, 11:42 p.m., David Edmundson wrote:
> > src/backendmanager.cpp, line 216
> > <https://git.reviewboard.kde.org/r/127387/diff/1/?file=453202#file453202line216>
> >
> > is the name argument still used?
> > 
> > It looks like that used to be the name of the backend to load, which 
> > you now determine yourself inside the function, rendering it useless.

Yes, I want to kill "name", but it's used from a couple of places in the API 
(but doesn't get used in the end, it's not needed since we share all the logic 
now with in- and out-of-process loading. I'll remove the remainders of name in 
a separate patch.


- Sebastian


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


On March 15, 2016, 5:57 p.m., Sebastian Kügler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127387/
> ---
> 
> (Updated March 15, 2016, 5:57 p.m.)
> 
> 
> Review request for Plasma, Solid, Daniel Vrátil, and Martin Gräßlin.
> 
> 
> Repository: libkscreen
> 
> 
> Description
> ---
> 
> Refactor the backend loading code
> 
> Untangle the large plugin loading logic in the BackendManager static
> into  separate bits. This makes the code clearer and easier to auto-test.
> 
> - listBackends() compiles a list of backends from the plugin paths
> - preferredBackend() picks the backend, in this priority:
> - if KSCREEN_BACKEND is set in the environment, this is the only
>   used method to find the backend plugin
> - if platform is X11, the XRandR backend is picked
> - if platform is wayland, KWayland backend is picked
> - if neither is the case, QScreen backend is picked
> 
> It does introduce a slight behavioral change: The mechanism was based on
> falling through, so it would consider another backend if the logically
> picked on fails to load. This is undesired behavior, however, since the
> backendloader may be able to load the plugin, but that doesn't mean that
> the plugin actually work.
> 
> Parsing of the KSCREEN_BACKEND variable is kept case-insensitive.
> 
> 
> autotests for new backend loading logic
> 
> - makes sure we find plugins installed
> - pick plugins from env var
> 
> We can't sensible test all the runtime cases yet, but this at least
> covers the code paths around those few lines that do runtime detection
> of x11 and wayland.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 26c7952 
>   autotests/testbackendloader.cpp PRE-CREATION 
>   src/backendmanager.cpp 382ae71 
>   src/backendmanager_p.h 150883b 
> 
> Diff: https://git.reviewboard.kde.org/r/127387/diff/
> 
> 
> Testing
> ---
> 
> manual runtime tests and autotests pass
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

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

Re: [Kde-hardware-devel] Review Request 127387: Refactor the backend loading code

[Sebastian Kügler]

> On March 16, 2016, 11:45 a.m., Martin Gräßlin wrote:
> > src/backendmanager.cpp, line 165
> > <https://git.reviewboard.kde.org/r/127387/diff/1/?file=453202#file453202line165>
> >
> > startsWith always goes with QLatin1String

QString() is needed for .arg() (which QLatin1String doesn't have).


- Sebastian


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


On March 15, 2016, 5:57 p.m., Sebastian Kügler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127387/
> ---
> 
> (Updated March 15, 2016, 5:57 p.m.)
> 
> 
> Review request for Plasma, Solid, Daniel Vrátil, and Martin Gräßlin.
> 
> 
> Repository: libkscreen
> 
> 
> Description
> ---
> 
> Refactor the backend loading code
> 
> Untangle the large plugin loading logic in the BackendManager static
> into  separate bits. This makes the code clearer and easier to auto-test.
> 
> - listBackends() compiles a list of backends from the plugin paths
> - preferredBackend() picks the backend, in this priority:
> - if KSCREEN_BACKEND is set in the environment, this is the only
>   used method to find the backend plugin
> - if platform is X11, the XRandR backend is picked
> - if platform is wayland, KWayland backend is picked
> - if neither is the case, QScreen backend is picked
> 
> It does introduce a slight behavioral change: The mechanism was based on
> falling through, so it would consider another backend if the logically
> picked on fails to load. This is undesired behavior, however, since the
> backendloader may be able to load the plugin, but that doesn't mean that
> the plugin actually work.
> 
> Parsing of the KSCREEN_BACKEND variable is kept case-insensitive.
> 
> 
> autotests for new backend loading logic
> 
> - makes sure we find plugins installed
> - pick plugins from env var
> 
> We can't sensible test all the runtime cases yet, but this at least
> covers the code paths around those few lines that do runtime detection
> of x11 and wayland.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 26c7952 
>   autotests/testbackendloader.cpp PRE-CREATION 
>   src/backendmanager.cpp 382ae71 
>   src/backendmanager_p.h 150883b 
> 
> Diff: https://git.reviewboard.kde.org/r/127387/diff/
> 
> 
> Testing
> ---
> 
> manual runtime tests and autotests pass
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

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

Re: Review Request 127405: Make sure PlasmaQuick export file is properly found

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On March 17, 2016, 12:37 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127405/
> ---
> 
> (Updated March 17, 2016, 12:37 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Uses `"quotes"` rather than `` to include a neigbour header file.
> 
> 
> Diffs
> -
> 
>   src/plasmaquick/appletquickitem.h 99fcf1b 
>   src/plasmaquick/configmodel.h ed2df2e 
>   src/plasmaquick/configview.h 5ac9c26 
>   src/plasmaquick/dialog.h 63efa04 
>   src/plasmaquick/packageurlinterceptor.h 343e311 
>   src/plasmaquick/shellpluginloader.h bdc13be 
>   src/plasmaquick/view.h b59271d 
> 
> Diff: https://git.reviewboard.kde.org/r/127405/diff/
> 
> 
> Testing
> ---
> 
> Aparently it was a problem on some systems. It was eventually fixed for me, 
> so I thought I was crazy. This should be the fix nevertheless (assuming 
> include directories are being exported properly).
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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

Re: [Kde-hardware-devel] Review Request 127387: Refactor the backend loading code

[Sebastian Kügler]

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

(Updated March 16, 2016, 1:54 p.m.)


Review request for Plasma, Solid, Daniel Vrátil, and Martin Gräßlin.


Changes
---

Also address Kai's change requests. :)


Repository: libkscreen


Description
---

Refactor the backend loading code

Untangle the large plugin loading logic in the BackendManager static
into  separate bits. This makes the code clearer and easier to auto-test.

- listBackends() compiles a list of backends from the plugin paths
- preferredBackend() picks the backend, in this priority:
- if KSCREEN_BACKEND is set in the environment, this is the only
  used method to find the backend plugin
- if platform is X11, the XRandR backend is picked
- if platform is wayland, KWayland backend is picked
- if neither is the case, QScreen backend is picked

It does introduce a slight behavioral change: The mechanism was based on
falling through, so it would consider another backend if the logically
picked on fails to load. This is undesired behavior, however, since the
backendloader may be able to load the plugin, but that doesn't mean that
the plugin actually work.

Parsing of the KSCREEN_BACKEND variable is kept case-insensitive.


autotests for new backend loading logic

- makes sure we find plugins installed
- pick plugins from env var

We can't sensible test all the runtime cases yet, but this at least
covers the code paths around those few lines that do runtime detection
of x11 and wayland.


Diffs (updated)
-

  autotests/CMakeLists.txt 26c7952 
  autotests/testbackendloader.cpp PRE-CREATION 
  src/backendmanager.cpp 382ae71 
  src/backendmanager_p.h 150883b 

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


Testing
---

manual runtime tests and autotests pass


Thanks,

Sebastian Kügler

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

Re: remove khelpcenter from next Plasma release?

[Sebastian Kügler]

On Tuesday, March 15, 2016 05:52:55 PM Jeremy Whiting wrote:
> As an application developer I agree it makes sense to have khelpcenter
> released with KDE Applications. I also agree with Albert's point that
> having online documentation isn't the best since it could be newer
> than what's actually running. People using LTS distributions or
> "stable" variants of less often released distributions will have very
> old (to those of us that run from git) versions of stuff. Having
> online documentation for plasma 5.7 to look at while you're running
> plasma 4 would just confuse.

This can be solved, of course, by having versioned documentation online and 
point to that.

> Also, thanks Luigi for stepping up to maintain it.

+1
-- 
sebas

http://www.kde.org | http://vizZzion.org

Re: [Kde-hardware-devel] Review Request 127421: use baseName rather than fileName.startsWith

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On March 18, 2016, 4:50 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127421/
> ---
> 
> (Updated March 18, 2016, 4:50 p.m.)
> 
> 
> Review request for Solid and Sebastian Kügler.
> 
> 
> Repository: libkscreen
> 
> 
> Description
> ---
> 
> xrandr1.1 and xrandr start with the same filepath, so the backend loaded
> was picking up the wrong plugin when trying to find "xrandr".
> 
> 
> Diffs
> -
> 
>   src/backendmanager.cpp e52571e3c572a9db58c5e79a84d61a00af42eb9d 
> 
> Diff: https://git.reviewboard.kde.org/r/127421/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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

Re: Review Request 127418: Set text on QCheckbox widget rather than using a separate label

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On March 18, 2016, 1:51 a.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127418/
> ---
> 
> (Updated March 18, 2016, 1:51 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 245580
> https://bugs.kde.org/show_bug.cgi?id=245580
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This fixes (amongst other focus bugs) the label having the right
> enabled effect if the permissions are not editable.
> 
> CCBUG: 245580
> 
> 
> Diffs
> -
> 
>   src/widgets/kpropertiesdialog.cpp 59e189fb79e8033d8f8983b19aa914767dac52f6 
> 
> Diff: https://git.reviewboard.kde.org/r/127418/diff/
> 
> 
> Testing
> ---
> 
> Opened kpropertiesdialog on a folder and a file
> 
> Faked being a file system without ACL (by adding && false) into line 2128 so 
> that we see the full grid of options
> 
> Opened kpropertiesdialog on a folder and a file
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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

Re: [Kde-hardware-devel] Review Request 127387: Refactor the backend loading code

[Sebastian Kügler]

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

(Updated March 16, 2016, 2:04 p.m.)


Review request for Plasma, Solid, Daniel Vrátil, and Martin Gräßlin.


Changes
---

Addressed Martin's comments.


Repository: libkscreen


Description
---

Refactor the backend loading code

Untangle the large plugin loading logic in the BackendManager static
into  separate bits. This makes the code clearer and easier to auto-test.

- listBackends() compiles a list of backends from the plugin paths
- preferredBackend() picks the backend, in this priority:
- if KSCREEN_BACKEND is set in the environment, this is the only
  used method to find the backend plugin
- if platform is X11, the XRandR backend is picked
- if platform is wayland, KWayland backend is picked
- if neither is the case, QScreen backend is picked

It does introduce a slight behavioral change: The mechanism was based on
falling through, so it would consider another backend if the logically
picked on fails to load. This is undesired behavior, however, since the
backendloader may be able to load the plugin, but that doesn't mean that
the plugin actually work.

Parsing of the KSCREEN_BACKEND variable is kept case-insensitive.


autotests for new backend loading logic

- makes sure we find plugins installed
- pick plugins from env var

We can't sensible test all the runtime cases yet, but this at least
covers the code paths around those few lines that do runtime detection
of x11 and wayland.


Diffs (updated)
-

  autotests/CMakeLists.txt 26c7952 
  autotests/testbackendloader.cpp PRE-CREATION 
  src/backendmanager.cpp 382ae71 
  src/backendmanager_p.h 150883b 

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


Testing
---

manual runtime tests and autotests pass


Thanks,

Sebastian Kügler

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

Re: [Kde-hardware-devel] Review Request 127387: Refactor the backend loading code

[Sebastian Kügler]

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

(Updated March 16, 2016, 1:50 p.m.)


Review request for Plasma, Solid, Daniel Vrátil, and Martin Gräßlin.


Changes
---

Addressed feedback, thanks David and Martin!


Repository: libkscreen


Description
---

Refactor the backend loading code

Untangle the large plugin loading logic in the BackendManager static
into  separate bits. This makes the code clearer and easier to auto-test.

- listBackends() compiles a list of backends from the plugin paths
- preferredBackend() picks the backend, in this priority:
- if KSCREEN_BACKEND is set in the environment, this is the only
  used method to find the backend plugin
- if platform is X11, the XRandR backend is picked
- if platform is wayland, KWayland backend is picked
- if neither is the case, QScreen backend is picked

It does introduce a slight behavioral change: The mechanism was based on
falling through, so it would consider another backend if the logically
picked on fails to load. This is undesired behavior, however, since the
backendloader may be able to load the plugin, but that doesn't mean that
the plugin actually work.

Parsing of the KSCREEN_BACKEND variable is kept case-insensitive.


autotests for new backend loading logic

- makes sure we find plugins installed
- pick plugins from env var

We can't sensible test all the runtime cases yet, but this at least
covers the code paths around those few lines that do runtime detection
of x11 and wayland.


Diffs (updated)
-

  autotests/CMakeLists.txt 26c7952 
  autotests/testbackendloader.cpp PRE-CREATION 
  src/backendmanager.cpp 382ae71 
  src/backendmanager_p.h 150883b 

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


Testing
---

manual runtime tests and autotests pass


Thanks,

Sebastian Kügler

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

[Kde-hardware-devel] Review Request 127387: Refactor the backend loading code

[Sebastian Kügler]

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

Review request for Plasma, Solid, Daniel Vrátil, and Martin Gräßlin.


Repository: libkscreen


Description
---

Refactor the backend loading code

Untangle the large plugin loading logic in the BackendManager static
into  separate bits. This makes the code clearer and easier to auto-test.

- listBackends() compiles a list of backends from the plugin paths
- preferredBackend() picks the backend, in this priority:
- if KSCREEN_BACKEND is set in the environment, this is the only
  used method to find the backend plugin
- if platform is X11, the XRandR backend is picked
- if platform is wayland, KWayland backend is picked
- if neither is the case, QScreen backend is picked

It does introduce a slight behavioral change: The mechanism was based on
falling through, so it would consider another backend if the logically
picked on fails to load. This is undesired behavior, however, since the
backendloader may be able to load the plugin, but that doesn't mean that
the plugin actually work.

Parsing of the KSCREEN_BACKEND variable is kept case-insensitive.


autotests for new backend loading logic

- makes sure we find plugins installed
- pick plugins from env var

We can't sensible test all the runtime cases yet, but this at least
covers the code paths around those few lines that do runtime detection
of x11 and wayland.


Diffs
-

  autotests/CMakeLists.txt 26c7952 
  autotests/testbackendloader.cpp PRE-CREATION 
  src/backendmanager.cpp 382ae71 
  src/backendmanager_p.h 150883b 

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


Testing
---

manual runtime tests and autotests pass


Thanks,

Sebastian Kügler

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

Re: Review Request 127345: Make it possible for an applet to offer a test object

[Sebastian Kügler]

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


Ship it!




Ship It!

- Sebastian Kügler


On March 12, 2016, 1:13 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127345/
> ---
> 
> (Updated March 12, 2016, 1:13 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> In an attempt to make it possible for plasmoid to test themselves in the 
> different shells, provide API to exatract the object that will test the 
> plasmoid instance.
> 
> 
> Diffs
> -
> 
>   src/plasma/corona.cpp 5d17550 
>   src/plasma/private/packages.cpp a5ba81a 
>   src/plasmaquick/appletquickitem.h 4f25f5d 
>   src/plasmaquick/appletquickitem.cpp 9242e9e 
>   src/plasmaquick/private/appletquickitem_p.h 9c24734 
> 
> Diff: https://git.reviewboard.kde.org/r/127345/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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