Re: RFC: Theming and styles of KDE applications

[Albert Vaca Cintora]

+1

Every time I use a KDE app on Gnome I get those unusable black-on-black
icons or some other visual issues.

Even if this is because Gnome doesn't follow some XDG spec we use, if we
can provide a workaround for Windows we should be able to provide a
workaround in Gnome.

Re: Should we stop distributing source tarballs?

[Albert Vaca Cintora]

It seems a lot of people feel conservative in favor of tarballs, so
maybe I aimed too far. At least I think the discussion brought some
interesting points that we can explore further. Some I identified:

- The tarballs should contain no changes with respect to git, or
minimal changes obviously justifiable in a diff.
- Tarballs should only be generated in a reproducible manner using
scripts. Ideally by the CI only.
- We should start to sign tarballs in the CI.
- We should start to sign commits and tags. Git recently made this
super easy by allowing signing with the ssh keys that we all are
already using to push things, so no excuses for not enabling this.
Sample config below:

[user]
signingkey = 
[commit]
gpgsign = true
[gpg]
format = ssh
[tag]
forceSignAnnotated = true

Should we stop distributing source tarballs?

[Albert Vaca Cintora]

Hi KDE folks,

The recent xz backdoor scandal made me realize how bad and obsolete
distributing tarballs is. The source of truth for our code are the
repositories, and releases can simply be tags on those repos.

As a big free software community, I think we should lead by example
and get rid of tarballs altogether (as I hope to see in other projects
as well) after the recent events.

Packagers can git pull.

If we ever replace git with something else, that something else will
have tags as well.

What's the advantage of providing tarballs?

Albert

Re: [announcement] Telegram bridging to be retired Wed. 20 Sept. | 5 to-dos

[Albert Vaca Cintora]

KDE Connect has a Telegram group with 660 members (vs 100 on matrix). I
don't know if we are the exception with such a big group on Telegram, but
this is going to be a hit to our community :(

On Tue, 22 Aug 2023, 09:02 Joseph P. De Veaugh-Geiss, 
wrote:

> Hello KDE community,
>
> apologies for cross-posting!
>
> The time has finally come: both Telegram <-> Matrix bridges will be shut
> down in 4 weeks on *Wednesday 20 September*. Let's start the
> co-ordination process now so everything goes as smoothly as possible.
>
> For all KDE contributors: please read at least the "Five To-Dos" below
> to be informed about what will happen and what needs to be done.
>
> Below that there is some additional information about the bridging
> situation at KDE. Consult these notes if you want more background
> information about why the Telegram bridge is being retired.
>
> Cheers,
> Joseph
>
> _Five To-Dos_
>
>1. *General*: On Wednesday 20 September the Telegram bridging to KDE
> Matrix rooms will be shut down. To make the transition go smoothly,
> teams should start co-ordinating for the shutdown now. The Matrix room
> for co-ordination is "Telegram shutdown co-ordination" at
> https://go.kde.org/matrix/#/#telegram-shutdown:kde.org.
>
>2. *Co-ordination*: This includes: (i) migrating all contributors to
> Matrix, and (ii) deleting the Telegram rooms before the bridge is
> shutdown or -- at most -- one day after the shutdown. Keeping Telegram
> rooms open when they are no longer being used will cause unnecessary
> confusion. Importantly, do not later add a non-KDE Telegram bridge to
> KDE's Matrix rooms as that will not solve the problems from doubled user
> accounts and lack of control over Telegram; see below for operational
> issues with Telegram bridging.
>
>3. *Action needed by Telegram room admins*: Due to the unexpected
> shutdown of the public Libera.Chat bridge, we will have to move rooms
> over to the matterbridge (ircsomebot) bridge as we work through moving
> channels over to our own Libera.Chat IRC bridge. This will require
> someone with admin in the Telegram room to ensure @ircsomebot is in the
> Telegram room with admin. This needs to be done after we unbridge the
> Matrix bridge from the Matrix side, so the room can continue to be
> bridged until the Telegram shutdown on 20 September.
>
>4. *Are there exceptions?": There /may/ be some rooms that focus on
> interacting with people external to the KDE Community who would benefit
> by having a Telegram bridge. We are thinking teams like those involved
> in the KDE Network program: https://community.kde.org/The_KDE_Network.
> Not all will need an exception, and it may turn out most don't. We would
> like to start putting together a list of these rooms so we can review
> for potential exceptions and estimate the scale of how much support is
> needed. However, this should be kept to a minimum; see below for
> operational issues with Telegram bridging. We understand that there are
> large internal KDE communities which rely heavily on Telegram, and we
> understand that shutting down the Telegram bridge is less than ideal for
> these rooms, but the issues we have with the Telegram bridge mean we
> need to keep exceptions to a minimum and only for those teeams whose
> work has a primarily external focus.
>
>5. *Getting a Matrix account*: We can offer KDE Contributors (usually
> those in the developers group, but will consider other requests) a
> Matrix account on our KDE Matrix Homeserver. To request an account
> please file a sysadmin ticket https://go.kde.org/systickets. However, as
> Matrix is federated you do not need to have an account on our homeserver
> to access KDE's Matrix rooms -- you can use any homeserver! A list of
> some alternative Matrix servers is available on
> https://joinmatrix.org/servers/.
>
> To co-ordinate with other teams to make this transition go as smoothly
> as possible, please reach out to the "Telegram shutdown co-ordination"
> Matrix room: https://go.kde.org/matrix/#/#telegram-shutdown:kde.org.
>
> _Additional Information Re Bridges_
>
>* Four years ago it was decided to add Matrix to IRC as KDE's
> official IM platforms (bridged together). IRC is not planned to be shut
> down.
>
>* Telegram is not Free Software and has never been an official
> platform for KDE communications. However, it has been used unofficially
> in a number of areas.
>
>* EMS hosts KDE's Matrix instance and the current Telegram bridge,
> and the majority of issues our community have with Matrix are related to
> bridges. Due to the huge extra load and poor performance Telegram
> bridging has in the Matrix rooms, it was agreed with EMS that the bridge
> would be only run for about a year until people had time to migrate to
> Matrix.
>
>* However, instead of people migrating away from Telegram, we have
> seen an increase in contributors using /both/ Matrix and Telegram, which
> has doubled the number of users we have to cope 

Re: sentry evaluation

[Albert Vaca Cintora]

I didn't know we had Sentry! How can we get crash reports from KDE
Connect through it? That would be super useful. I've used Sentry in
the past for other projects and I see a big value in it.

Actually, in the case of the KDE Connect Android app, we already get
crash reports aggregated by popularity by default via the Play Store.
Same with the Windows version published in the Windows Store. It's
very useful to fix the most common crashes, and it would be great to
have something like that for Linux as well.

About the alternative of using bugzilla for crashes: We can't know how
common a crash is with the crashes that seldomly get uploaded to
bugzilla. So +1 for keeping Sentry.

Re: Gitlab Turn-off Issues

[Albert Vaca Cintora]

On Sun, Jun 28, 2020, 20:29 Ben Cooksley  wrote:

On Mon, Jun 29, 2020 at 4:28 AM Allen Winter  wrote:
>
> Can we remove the Issues feature on all invent projects?
> We're starting to see more and more people adding Issues.
>
> For KOrganizer, bshah replaced Issues with Bugzilla.
>
> I very much doubt we want or are ready to replace Bugzilla.

As has been discussed *far too many times* already, we have NO
INTENTION to replace Bugzilla at this stage.
This matter has been discussed on many threads, including on this list
in the past.

Issues on Gitlab is intended to be the replacement for Phabricator Tasks.
Disabling them would leave projects with no developer task management
functionality, which would be highly undesirable as they are heavily
used by a number of projects.


What prevents a user from seeing a link labeled "issues" and thinking "this
is where I report my issues with this product"?

Currently, nothing, so I've decided to accept that bugs now come from two
places: Bugzilla and Gitlab issues.


>
> Your request to disable them for all KDE projects is therefore declined.
>
> Regards,
> Ben Cooksley
> KDE Sysadmin
>

D26888: work around to fully support the windows backend

[Albert Vaca Cintora]

albertvaka added a comment.


  @vonreth @broulik Can you folks give this a final review? We have been 
building KDE Connect for Windows off a custom branch with this patch, but I 
would like to use official builds of KNotifications so it would be great if we 
can merge it.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, vonreth, broulik, #kde_connect
Cc: albertvaka, meven, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

Re: Update on Status of Gitlab Migration

[Albert Vaca Cintora]

On Sat, Apr 11, 2020 at 11:36 AM Ben Cooksley  wrote:
>
> Good morning Community,
>
> I'm pleased to report that this week we reached a major milestone,
> with all the necessary technical components now being in place on our
> side for our migration to Gitlab to take place.

Regarding this: is the subdomain going to stay invent.kde.org once we
have officially moved? I find it's a bit confusing to use that instead
of gitlab.kde.org

Albert

Re: Update on Status of Gitlab Migration

[Albert Vaca Cintora]

On Sat, Apr 11, 2020 at 11:36 AM Ben Cooksley  wrote:
>
> Good morning Community,
>
> I'm pleased to report that this week we reached a major milestone,
> with all the necessary technical components now being in place on our
> side for our migration to Gitlab to take place.

Regarding this: is the subdomain going to stay invent.kde.org once we
have officially moved? I find it's a bit confusing to use that instead
of gitlab.kde.org

Albert

Re: Update on Status of Gitlab Migration

[Albert Vaca Cintora]

On Sat, Apr 11, 2020 at 11:36 AM Ben Cooksley  wrote:
>
> Good morning Community,
>
> I'm pleased to report that this week we reached a major milestone,
> with all the necessary technical components now being in place on our
> side for our migration to Gitlab to take place.

Regarding this: is the subdomain going to stay invent.kde.org once we
have officially moved? I find it's a bit confusing to use that instead
of gitlab.kde.org

Albert

D27210: add KDEconnect Icons

[Albert Vaca Cintora]

albertvaka added a comment.


  Hey, thanks for working on this!
  
  A "brand" re-design is something that we could really use. One of the things 
that I would like to change from the current icon/logo, though (and that this 
new one doesn't change), is the fact that there is a mobile phone in it.
  
  You can use KDE Connect between two computers, or between two phones 
(although it's not the main development focus at the moment). So I think a more 
generic icon that embodies "syncing" without any specific device would suit us 
better, IMO. Plus I find a bit weird to have a phone app whose icon is... a 
phone.

REPOSITORY
  R266 Breeze Icons

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

To: mbruchert, #vdg, #kde_connect
Cc: albertvaka, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D26888: work around to fully support the windows backend

[Albert Vaca Cintora]

albertvaka added inline comments.

INLINE COMMENTS

> notifybysnore.cpp:135-139
> +if (id == -1) {
> +emit actionInvoked(0, actionNum);
> +} else {
> +emit actionInvoked(id, actionNum);
> +}

This could become:

  if (id == -1) {
  id = 0
  }
  emit actionInvoked(id, actionNum);

To not duplicate the emit line. Still, it feels hackish.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, vonreth, broulik, #kde_connect
Cc: albertvaka, meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

Re: Sysadmin Load Reduction: Subversion Infrastructure

[Albert Vaca Cintora]

On Sat, Nov 9, 2019 at 6:53 PM Ben Cooksley  wrote:
>
> Unfortunately Rosetta does not have the necessary free disk space, as
> the Subversion repository is approximately 80GB these days in size,
> and operating WebSVN requires a full copy of the Subversion repository
> locally in order to operate.

Is this also the case if you run WebSVN next to the SVN server itself
(ie: on the same machine)? If a single repo copy can be shared by
WebSVN and the actual SVN server, that would reduce the storage needs.

Albert

How to prevent users from opening GitLab issues?

[Albert Vaca Cintora]

As we transition to Gitlab, our users find two different ways of
reporting issues: in Bugzilla and in Gitlab. Since we don't have plans
to deprecate bugzilla, is there any way we can disable issues in Gitlab?

Albert

Re: HiDPI issues with KDE applications

[Albert Vaca Cintora]

If this has to be done for all apps, why isn't it done at Qt level?

On Sat, Sep 28, 2019 at 5:05 PM Christoph Cullmann
 wrote:
>
> Hi,
>
> I just checked again the HIDPI support of Kate/KWrite on Windows and it
> "sucked".
>
> It seems we never properly did setup the stuff to auto-scale, e.g. the
>
>   QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling, true);
>
> call was missing, we only had the
>
>   QCoreApplication::setAttribute(Qt::AA_UseHighDpiPixmaps, true);
>
> part that we sprinkled in most of KDE's things in the past.
>
> I now rectified that for Kate/KWrite, shall we do this for all our
> applications?
>
> Greetings
> Christoph
>
> --
> Ignorance is bliss...
> https://cullmann.io | https://kate-editor.org

D23694: Add support for sshfs to the fstab backend

[Albert Vaca Cintora]

albertvaka added a comment.


  If it's a problem for kdeconnect mounts to appear there, how can we hide it? 
It's an sshfs mountpoint like any other, only that it is mounted 
programmatically.

REPOSITORY
  R245 Solid

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

To: lbeltrame, bruns, broulik, fvogt, #kde_connect
Cc: albertvaka, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

Re: Possible to move some KF5 frameworks to invent?

[Albert Vaca Cintora]

On Mon, Aug 12, 2019, 18:46 Ben Cooksley  wrote:

> On Mon, Aug 12, 2019 at 10:37 PM Albert Vaca Cintora
>  wrote:
> >
> > Could we use sysadmin/repo-metadata to know which branch is stable and
> therefore should be protected and trigger the hooks for closing bugs, etc?
>
> Unfortunately that only tells us what the current stable branch is -
> it doesn't let us know what the other (either historical or up and
> coming) stable branches are called.
>

Maybe that is enough? IMO, it makes sense to not consider a bug is closed
until the commit that fixes it has been merged to either master or the
current stable branch.

Re: Possible to move some KF5 frameworks to invent?

[Albert Vaca Cintora]

Could we use sysadmin/repo-metadata to know which branch is stable and
therefore should be protected and trigger the hooks for closing bugs, etc?

On Mon, Aug 12, 2019, 17:39 Ben Cooksley  wrote:

> On Mon, Aug 12, 2019 at 6:24 PM Kevin Ottens  wrote:
> >
> > Hello,
>
> Hi Kevin,
>
> >
> > On Sunday, 11 August 2019 22:14:19 CEST Albert Astals Cid wrote:
> > > With phabricator you can do a "force push" to your review[1], with
> gitlab
> > > you can not[2].
> > > [...]
> > > [2] without having your own fork of a repository, that is annoying for
> > > various reasons
> >
> > I'm genuinely surprised about that claim. Are we sure that it's not a
> setting
> > on our instance forcing this? I'm currently working on a project where
> you can
> > force push in non-protected branches without your own fork.
>
> Our hooks prevent force pushes from taking place within our mainline
> repositories.
>
> This is done for a couple of reasons.
>
> The first, that in order for us to reliably operate things like
> closing of bugs on Bugzilla and sending emails and IRC Notifications,
> it is necessary for the hooks to be able to detect when a commit is
> "new" and therefore needs to be processed for this sort of action.
> Unfortunately due to how Git works, there is no difference between a
> genuinely new commit, and one that has simply been rebased - they're
> both "new" as far as the hooks are concerned. This means we cannot
> allow rebasing within any branch which is subject to notification or
> other hook processing.
>
> The second, is that recovering your work when someone has
> rebased/force pushed the branch underneath you, is something which is
> non-trivial unless you are familiar with the process. Even for those
> who are experienced, it is possible to make mistakes in the process of
> doing so, and either lose commits or mangle the metadata associated
> with the commit (such as the authorship information - an incident
> which happened earlier this year). At the time KDE migrated to Git it
> was decided on a community wide basis that familiarity with this isn't
> something we should expect casual contributors to have.
>
> Those familiar to Git will be aware that it is very much possible to
> limit the scope of hooks like our Notification hooks, while still
> allowing them to be caught when entering branches that are subject to
> Notification. At this time the only proposals i've seen for this have
> been for "everything but master and stable branches" which
> unfortunately is not a scalable solution we can support due to the
> significant variance in schemes used for naming stable branches across
> different projects (not without pushing the maintenance role for
> handling all these different naming schemes on to Sysadmin)
>
> >
> > Regards.
> > --
> > Kevin Ottens, http://ervin.ipsquad.net
> >
> > enioka Haute Couture - proud patron of KDE, https://hc.enioka.com/en
>
> Regards,
> Ben
>

D22580: Notify users when not using KDE_INSTALL_USE_QT_SYS_PATHS about prefix.sh

[Albert Vaca Cintora]

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

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: apol, #frameworks, albertvaka
Cc: albertvaka, kde-frameworks-devel, kde-buildsystem, LeGast00n, sbergeron, 
bencreasy, michaelh, ngraham, bruns

D22580: Notify users when not using KDE_INSTALL_USE_QT_SYS_PATHS about prefix.sh

[Albert Vaca Cintora]

albertvaka added inline comments.

INLINE COMMENTS

> KDEInstallDirs.cmake:721
> +if(NOT KDE_INSTALL_USE_QT_SYS_PATHS)
> +message(NOTICE "Installing in ${CMAKE_INSTALL_PREFIX}, remember set the 
> environment. See ${CMAKE_CURRENT_BINARY_DIR}/prefix.sh")
> +endif()

Installing in `${CMAKE_INSTALL_PREFIX}`. You will need to run `./prefix.sh` to 
set the environment for this app.

?

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks
Cc: albertvaka, kde-frameworks-devel, kde-buildsystem, LeGast00n, sbergeron, 
bencreasy, michaelh, ngraham, bruns

D22105: WIP : Fix SFTP Plugin of KIO for Windows

[Albert Vaca Cintora]

albertvaka added inline comments.

INLINE COMMENTS

> brute4s99 wrote in kio_sftp.cpp:2257
> yes! thanks Hannah! 

You can keep QT_STAT_LNK on every platform and remove the ifdef. That's the 
point of the abstraction Qt provides.

For consistency, I would also change the other (S_IFDIR, etc.) to their Qt 
counterparts.

REPOSITORY
  R320 KIO Extras

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

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: andriusr, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, 
alexde, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov

D22105: WIP : Fix SFTP Plugin of KIO for Windows

[Albert Vaca Cintora]

albertvaka added inline comments.

INLINE COMMENTS

> andriusr wrote in kio_sftp.cpp:2257
> I'm not sure what is intended to achieve here, the remote file _can_ be a 
> symlink, and IIRC when we had sftp on dolphin in KDE4 those symlinks were 
> shown as such.

I think what they mean is that if you remove the ifdef it should just work.

REPOSITORY
  R320 KIO Extras

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

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: andriusr, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, 
alexde, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov

D21369: [WIP] Add AbstractContact properties for KContact::PhoneNumber objects

[Albert Vaca Cintora]

albertvaka added a comment.


  If I understand correctly, the only reason to use KContacts here is so we can 
use its PhoneNumber type. We could change it to a QString?

REPOSITORY
  R307 KPeople

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

To: sredman, apol
Cc: albertvaka, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21661: add snoretoast backend for KNotifications on Windows

[Albert Vaca Cintora]

albertvaka added inline comments.

INLINE COMMENTS

> notifybysnore.h:45
> +QMap> m_notifications;
> +QString program = QStringLiteral("SnoreToast.exe");
> +QLocalServer *server;

Isn't LibSnore abstracting this? Why do we need to call the exe instead of 
using some method in LibSnore?

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

Re: konqueror.org

[Albert Vaca Cintora]

Killing some satellite websites will ease the maintenance, though, so I'm
with Jonathan. The screenshots being so old seem a prof to me that its
being hard for us as a community to keep up with it.

Albert

On Sun, Jun 2, 2019, 12:34 Albert Astals Cid  wrote:

> El dissabte, 1 de juny de 2019, a les 13:07:41 CEST, Jonathan Riddell va
> escriure:
> > Can you call konqueror.org website unmaintained?  The screenshots are
> all
> > from KDE 4 times.  We can just make it forward to the new
> > kde.org/applications page instead
>
> Or just update the screenshots and all its contents are still valid and
> much more complete than whatever is there in kde.org/applications.
>
> Cheers,
>   Albert
>
> >
> > Jonathan
> >
>
>
>
>
>

D16951: Add mouse button icons

[Albert Vaca Cintora]

albertvaka added a reviewer: KDE Connect.

REPOSITORY
  R266 Breeze Icons

BRANCH
  mouse-buttons (branched from master)

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

To: ndavis, #vdg, ngraham, nicolasfella, #kde_connect
Cc: trickyricky26, abetts, ngraham, rizzitello, nicolasfella, 
kde-frameworks-devel, michaelh, bruns

D16692: A QApplication object needs to be instantiated for kio-kdeconnect to work on KDE Neon

[Albert Vaca Cintora]

albertvaka added a comment.


  In D16692#355068 , @fvogt wrote:
  
  > In D16692#355067 , @albertvaka 
wrote:
  >
  > > In D16692#354990 , @fvogt 
wrote:
  > >
  > > > I'm wondering why this specifically mentions "KDE Neon" both in the 
title and in the commit message.
  > > >
  > > > Is this workaround/fix only needed on neon or is the message wrong?
  > >
  > >
  > > I think only Neon ships frameworks 5.51
  >
  >
  > Why would you think that? 5.51 got released almost a month ago.
  
  
  Of curse I don't know every distro in the planet, but we got like a gazillion 
bug reports and I would say all of them are from Neon users, so this fix was 
quite directed to them.
  
  See: https://bugs.kde.org/show_bug.cgi?id=400178
  
  You can rename it, although it has been merged already.

REPOSITORY
  R224 KDE Connect

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

To: eduisters, #kde_connect, #frameworks, albertvaka
Cc: fvogt, sredman, apol, jriddell, nicolasfella, albertvaka, kdeconnect, 
shivanshukantprasad, skymoore, wistak, dvalencia, rmenezes, julioc, Leptopoda, 
timothyc, jdvr, yannux, Danial0_0, johnq, Pitel, adeen-s, SemperPeritus, 
daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, tctara

D16692: A QApplication object needs to be instantiated for kio-kdeconnect to work on KDE Neon

[Albert Vaca Cintora]

albertvaka added a comment.


  In D16692#354990 , @fvogt wrote:
  
  > I'm wondering why this specifically mentions "KDE Neon" both in the title 
and in the commit message.
  >
  > Is this workaround/fix only needed on neon or is the message wrong?
  
  
  I think only Neon ships frameworks 5.51

REPOSITORY
  R224 KDE Connect

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

To: eduisters, #kde_connect, #frameworks, albertvaka
Cc: fvogt, sredman, apol, jriddell, nicolasfella, albertvaka, kdeconnect, 
shivanshukantprasad, skymoore, wistak, dvalencia, rmenezes, julioc, Leptopoda, 
timothyc, jdvr, yannux, Danial0_0, johnq, Pitel, adeen-s, SemperPeritus, 
daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, tctara

D16692: A QApplication object needs to be instantiated for kio-kdeconnect to work on KDE Neon

[Albert Vaca Cintora]

This revision was automatically updated to reflect the committed changes.
Closed by commit R224:560e8638e8dd: A QApplication object needs to be 
instantiated for kio-kdeconnect to work on… (authored by eduisters, committed 
by albertvaka).

REPOSITORY
  R224 KDE Connect

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16692?vs=44954=44964

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

AFFECTED FILES
  kio/CMakeLists.txt
  kio/kdeconnect.json
  kio/kdeconnect.protocol
  kio/kiokdeconnect.cpp

To: eduisters, #kde_connect, #frameworks, albertvaka
Cc: sredman, apol, jriddell, nicolasfella, albertvaka, kdeconnect, 
shivanshukantprasad, skymoore, wistak, dvalencia, rmenezes, julioc, Leptopoda, 
timothyc, jdvr, yannux, Danial0_0, johnq, Pitel, adeen-s, SemperPeritus, 
daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, tctara

D16189: kio_help: Fix crash in QCoreApplication when accessing help://

[Albert Vaca Cintora]

albertvaka added a comment.


  Old apps that didn't have a QApplication are broken by this version of KIO. 
Definitely a regression to me :/

REPOSITORY
  R241 KIO

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

To: mpyne, #frameworks, sitter, dfaure, broulik
Cc: sredman, albertvaka, broulik, dfaure, kde-frameworks-devel, michaelh, 
ngraham, bruns

D16692: A QApplication object needs to be instantiated for kio-kdeconnect to work on KDE Neon

[Albert Vaca Cintora]

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


  Let's ship this, but I still think we should see if something can be done on 
the KIO side to not break old apps.

REPOSITORY
  R224 KDE Connect

BRANCH
  kde-neon

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

To: eduisters, #kde_connect, #frameworks, albertvaka
Cc: sredman, apol, jriddell, nicolasfella, albertvaka, kdeconnect, 
shivanshukantprasad, skymoore, wistak, dvalencia, rmenezes, julioc, Leptopoda, 
timothyc, jdvr, yannux, Danial0_0, johnq, Pitel, adeen-s, SemperPeritus, 
daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, tctara

D16189: kio_help: Fix crash in QCoreApplication when accessing help://

[Albert Vaca Cintora]

albertvaka added a comment.


  As I commented in this similar patch (https://phabricator.kde.org/D16692 ) I 
think this is a regression that should be fixed in KIO.

REPOSITORY
  R241 KIO

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

To: mpyne, #frameworks, sitter, dfaure, broulik
Cc: albertvaka, broulik, dfaure, kde-frameworks-devel, michaelh, ngraham, bruns

D16692: A QApplication object needs to be instantiated for kio-kdeconnect to work on KDE Neon

[Albert Vaca Cintora]

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


  In my opinion this is a regression in KIO and it would be nice to check if it 
can somehow be fixed there: Upgrading KIO should not break existing apps.
  
  Even if we make a release with this fix, it doesn't guarantee all distros 
will pick it up.

INLINE COMMENTS

> kiokdeconnect.cpp:155
>  KIO::UDSEntry entry;
> -entry.insert(KIO::UDSEntry::UDS_NAME, QStringLiteral("files"));
> -entry.insert(KIO::UDSEntry::UDS_DISPLAY_NAME, name);

Also don't change insert to fastInsert, as it requires KIO 5.48 and we want to 
support older than that.

REPOSITORY
  R224 KDE Connect

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

To: eduisters, #kde_connect, #frameworks, albertvaka
Cc: albertvaka, kdeconnect, shivanshukantprasad, skymoore, wistak, dvalencia, 
rmenezes, julioc, Leptopoda, timothyc, jdvr, yannux, Danial0_0, johnq, Pitel, 
adeen-s, SemperPeritus, daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, 
menasshock, tctara, apol

D11683: Make it possible to request a plugin configuration module programatically

[Albert Vaca Cintora]

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

INLINE COMMENTS

> kpluginselector.h:214
> + * Shows the configuration dialog for the plugin @p pluginId if it's 
> available
> + */
> +void showConfiguration(const QString );

Remember to add the `@since 5.X` annotation before pushing the change.

REPOSITORY
  R295 KCMUtils

BRANCH
  master

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

To: apol, #frameworks, #kde_connect, albertvaka
Cc: albertvaka, michaelh, ngraham

D6906: Create a "." entry even on empty dirs

[Albert Vaca Cintora]

albertvaka added a comment.


  Mmmm it could be because kio_desktop is a ForwardingSlave, so the entry is 
added first by its forwardee (kio_file I think) and then again by kio_desktop 
itself?
  
  I'm not sure if what I'm saying even makes sense because I don't really know 
how KIO works. Will investigate a bit...

REPOSITORY
  R241 KIO

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

To: albertvaka, #frameworks, elvisangelaccio
Cc: aacid, dfaure

D6906: Create a "." entry even on empty dirs

[Albert Vaca Cintora]

albertvaka added a comment.


  Reverted the patch. Couldn't figure out what is wrong with it though... I 
don't think that "." is being emitted multiple times because of this patch. It 
looks more like if kio_desktop was causing listEntries to be called twice.

REPOSITORY
  R241 KIO

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

To: albertvaka, #frameworks, elvisangelaccio
Cc: aacid, dfaure

D6906: Create a "." entry even on empty dirs

[Albert Vaca Cintora]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:0ac78aa3fba4: Emit a "." UDSEntry when not present, even 
on empty directories. (authored by albertvaka).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6906?vs=17194=17207

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

AFFECTED FILES
  src/core/slavebase.cpp

To: albertvaka, #frameworks, elvisangelaccio

D6906: Create a "." entry even on empty dirs

[Albert Vaca Cintora]

albertvaka added a reviewer: elvisangelaccio.

REPOSITORY
  R241 KIO

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

To: albertvaka, #frameworks, elvisangelaccio

D6906: Create a "." entry even on empty dirs

[Albert Vaca Cintora]

albertvaka edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: albertvaka, #frameworks

D6906: Create a "." entry even on empty dirs

[Albert Vaca Cintora]

albertvaka created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  Fixes bug 382046

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/core/slavebase.cpp

To: albertvaka, #frameworks

[Differential] [Abandoned] D4663: Allow setting the timeout value.

[Albert Vaca Cintora]

albertvaka abandoned this revision.
albertvaka added a comment.


  I'm discarding this patch and will find an alternative way to solve the 
problem in KDE Connect. I still think, though, that notifications are a way 
better solution to require interaction from the user than SNIs, so I don't 
think I will switch to SNIs.
  
  In https://phabricator.kde.org/D4663#88444, @colomar wrote:
  
  > As long as we can't get there, I agree with Martin that an SNI is a better 
solution than a permanently shown popup notification, as it attracts attention 
without covering anything on the screen.
  
  
  Even without this patch, we already support persistent notifications. Of 
course, they are not implemented as a permanently shown popup: we have a 
notifications menu that groups them, in a similar way to what Android, iOS, 
Gnome, Mac and Windows do. I think it's way easier to use this functionality 
already present in notifications than to implement all the changes you propose 
to SNIs (and that without taking into account that our SNIs would behave 
completely different than in any other desktop).
  
  In https://phabricator.kde.org/D4663#88456, @mck182 wrote:
  
  > Ever wondered why the spec you're referring to
  >  is full of only Gnome's extensions and is hosted on Gnome's
  >  servers? It's because Gnome considers the Galago project,
  >  the actual cross-desktop notifications project, dead.
  
  
  The Galago spec hasn't been updated since 2006. IMHO it's actually a good 
thing that Gnome took the lead here and continued to maintain it: otherwise we 
would have nothing (and no, don't say we would have SNIs... even Windows and 
Mac have deprecated them in favor of persistent notifications!). KNotifications 
has for a long time implemented most of the fetures that were added by Gnome, 
so it's not a Gnome-only thing anymore even if it hasn't moved upstream to 
Freedesktop yet.
  
  I don't see why you guys stick to the idea of using SNIs, when that would 
require a ton of changes to them, when we already have most of what we need in 
the notifications system. Not only it's already implemented, but also it is 
what every major desktop and phone OS out there already use!

REPOSITORY
  R289 KNotifications

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

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

To: albertvaka, #frameworks, apol
Cc: colomar, mck182, #frameworks

[Differential] [Commented On] D4663: Allow setting the timeout value.

[Albert Vaca Cintora]

albertvaka added a comment.


  I agree that just rising the timeout is suboptimal but I still like it better 
than a modal dialog... Actually, notifications were invented as a way to avoid 
harassing the user with modal dialogs when apps need attention.
  
  Apart from that, and even if we end up implementing this in a diferent way 
for KDE Connect, I don't think that KNotification as a framework should hide 
this feature from the developer. Regardless of the notification server 
implementation, the underlying protocol that KNotification exposes does allow 
defining a timeout.
  
  And, about SNIs... It's true that they have a title, text and actions like 
notifications. But they also happen to be represented in every major desktop 
(eg: Windows, MacOS, Gnome, Unity and Plasma) as an icon in the system tray 
that you have to right click to access a context menu with the actions (unlike 
notifications). Are you sure that's what Android does?
  
  Gnome implements something much more similar to the notification systems 
nowadays present in Android and iOS (and even Windows and MacOS, which are 
trying to deprecate their system tray icons), and they do that using the 
notifications cross-desktop standard. So I wouldn't criticize Gnome for doing 
their own thing here.

REPOSITORY
  R289 KNotifications

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

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

To: albertvaka, #frameworks, apol
Cc: mck182, #frameworks

[Differential] [Commented On] D4663: Allow setting the timeout value.

[Albert Vaca Cintora]

albertvaka added a comment.


  I don't think indicators are the future of notifications :/
  
  If you have a look at modern systems like Android (which had the luxury of 
designing a notification system without having to think about legacy), they 
have nothing like app indicators: they use persistent notifications instead. 
Gnome has also taken that way [1], so if we make appindicarors a core part of 
kdeconnect, we will never look native on Gnome... I'm trying to find a 
different solution.
  
  [1] https://wiki.gnome.org/Design/OS/MessageTray/Compatibility

REPOSITORY
  R289 KNotifications

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

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

To: albertvaka, #frameworks, apol
Cc: mck182, #frameworks

[Differential] [Commented On] D4663: Allow setting the timeout value.

[Albert Vaca Cintora]

albertvaka added a comment.


  I understand the potential of it being misused, even though I don't think we 
should treat developers as if they didn't know what they are doing. Also, if we 
want to have a maximum timeout, this is something that should be enforced on 
the server (ie: Plasma) instead of the client. There will be apps using 
libnotify or any other lib that can set their timeouts.
  
  The reason I added this is because in KDE Connect we show notifications when 
there is an incoming "pair request", and I wanted to make the notification stay 
there for as long as the request is valid. Since the notification is the only 
way to accept the pair request, I think it results in a bad user experience if 
the notification disappears before the user can act on it.
  
  There is an alternative solution to my use case: making the notification 
persistent and manually dismissing it when the request is no longer valid, but 
I've decided to not implement it this way because some notification servers 
don't support persistent notifications (eg: Ubuntu Unity).
  
  What do you think?

REPOSITORY
  R289 KNotifications

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

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

To: albertvaka, #frameworks, apol
Cc: mck182, #frameworks

[Differential] [Updated] D4663: Allow setting the timeout value.

[Albert Vaca Cintora]

albertvaka added a reviewer: apol.

REPOSITORY
  R289 KNotifications

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

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

To: albertvaka, #frameworks, apol
Cc: #frameworks

[Differential] [Request, 43 lines] D4663: Allow setting the timeout value.

[Albert Vaca Cintora]

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

REPOSITORY
  R289 KNotifications

BRANCH
  master

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

AFFECTED FILES
  src/knotification.cpp
  src/knotification.h
  src/notifybypopup.cpp

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

To: albertvaka
Cc: #frameworks

[Differential] [Updated] D4663: Allow setting the timeout value.

[Albert Vaca Cintora]

albertvaka added a reviewer: Frameworks.

REPOSITORY
  R289 KNotifications

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

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

To: albertvaka, #frameworks
Cc: #frameworks

[Differential] [Closed] D4213: Mark non-persistent notifications as transient.

[Albert Vaca Cintora]

This revision was automatically updated to reflect the committed changes.
Closed by commit R289:579b75aac8e2: Mark non-persistent notifications as 
transient. (authored by albertvaka).

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4213?vs=10371=10423

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

AFFECTED FILES
  src/notifybypopup.cpp

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

To: albertvaka, #frameworks, #plasma, apol
Cc: #frameworks

[Differential] [Updated] D4213: Mark non-persistent notifications as transient.

[Albert Vaca Cintora]

albertvaka added a reviewer: Frameworks.

REPOSITORY
  R289 KNotifications

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

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

To: albertvaka, #frameworks
Cc: #frameworks

[Differential] [Request, 4 lines] D4213: Mark non-persistent notifications as transient.

[Albert Vaca Cintora]

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

REVISION SUMMARY
  On Gnome and other desktops using their extension of the notifications
  spec, notifications are persistent unless they set the transient hint.
  
  This tries to make notifications more consistent across desktops.

TEST PLAN
  Manual testing on Gnome.

REPOSITORY
  R289 KNotifications

BRANCH
  master

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

AFFECTED FILES
  src/notifybypopup.cpp

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

To: albertvaka
Cc: #frameworks

[Differential] [Closed] D4142: Support "default actions", as specified in [1].

[Albert Vaca Cintora]

This revision was automatically updated to reflect the committed changes.
Closed by commit R289:8b161b971fae: Support "default actions", as specified in 
[1]. (authored by albertvaka).

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4142?vs=10186=10258

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

AFFECTED FILES
  src/knotification.cpp
  src/knotification.h
  src/notifybypopup.cpp

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

To: albertvaka, #frameworks, #vdg, mart
Cc: colomar, broulik, mart, #frameworks

[Differential] [Commented On] D4142: Support "default actions", as specified in [1].

[Albert Vaca Cintora]

albertvaka added a comment.


  In https://phabricator.kde.org/D4142#77834, @broulik wrote:
  
  > Care needs to be taken that existing notifications don't suddenly behave 
differently but that moving from actions() to defaultAction() is a conscious 
and explicit choice in the application that emits the notification.
  
  
  I've already made sure that existing apps behave as they currently do. Even 
more: apps that decide to move from setActions to setDefaultAction will still 
display exactly the same way in Plasma, with an action button (until/unless we 
change the Plasmoid to honor the default action, which we can discuss when I 
submit the patch for the Plasmoid).
  
  In https://phabricator.kde.org/D4142#77832, @mart wrote:
  
  > code wise the change makes sense..
  >  that said, this assumes we do an important behavior change in our ui, 
which i don't think is a good thing, as is disregards completely how our users 
are used to, in the name of "other platforms do things differently".
  
  
  No, there is no change in our UI at all. In Plasma and KDE apps we can decide 
to never use this feature if we think that the UX is better that way, but IMO 
this is a generic (not Plasma-specific) framework and it should provide access 
to the functionality anyway. This is a really common use case for notifications 
on most platforms, so if we want that some day this framework can be useful on, 
for example, Android or Gnome, this is something we have to provide. I think 
that it would be good to adopt it on Plasma and KDE apps, but this is a 
completely different discussion that we can have on the mailing list.
  
  And no, I don't think that our users will freak out if we start behaving like 
every other notification system.
  
  In https://phabricator.kde.org/D4142#77861, @colomar wrote:
  
  > Hm, that's not easy to decide. Whether one likes the "click to make go 
away" behavior or not is highly subjective. I, for one, find it very annoying 
on e.g. OS  when a notification covers something I want to see on the screen 
and there is no way to make it go away other than waiting. Others find it 
annoying to have to explicitly click a button to execute the default action.
  
  
  Same thing: the framework giving access to this feature doesn't mean your app 
has to use it. Even more: it doesn't even change the behaviour in Plasma! In 
Plasma the default action will still appear as a button unless we change the 
plasmoid. This will, however, make it possible to use this feature on desktops 
that do support default actions.

REPOSITORY
  R289 KNotifications

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

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

To: albertvaka, #frameworks, #vdg
Cc: colomar, broulik, mart, #frameworks

[Differential] [Request, 57 lines] D4142: Support "default actions", as specified in [1].

[Albert Vaca Cintora]

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

REVISION SUMMARY
  Default actions are the actions triggered when the notification is simply
  clicked, without adding any "action buttons". Until now, with
  KNotifications we could only make notifications that go away when clicked,
  which might not be what the user expects (see notifications on smartphones
  or other desktop environments).
  
  Since KNotifications uses an int->name mapping (instead of string->name),
  we need to manually convert between "default" and "0". Interestingly
  enough, there was already code to handle the action "0" and emit
  activated(), which is documented as "Emitted only when the default
  activation has occurred" but in practice was never actually used.
  
  [1] 
https://people.gnome.org/~mccann/docs/notification-spec/notification-spec-latest.html

TEST PLAN
  Tests pass.

REPOSITORY
  R289 KNotifications

BRANCH
  master

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

AFFECTED FILES
  src/knotification.cpp
  src/knotification.h
  src/knotificationmanager.cpp
  src/notifybypopup.cpp

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

To: albertvaka
Cc: #frameworks

[Differential] [Commented On] D2545: Cleanup KSharedUiServerProxy before qApp exists

[albertvaka (Albert Vaca Cintora)]

albertvaka added a comment.


  Since there is no fix on Qt, should we merge this?

BRANCH
  master

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

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

To: kfunk, vonreth, dfaure
Cc: albertvaka, mutlaqja, arrowdodger, #frameworks

[Differential] [Commented On] D2546: Cleanup DBus-related resources before qApp exits

[albertvaka (Albert Vaca Cintora)]

albertvaka added a comment.


  Since there is no fix on Qt, should we merge this?

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

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

To: kfunk, vonreth, dfaure
Cc: albertvaka, #frameworks

Re: Review Request 127169: By default, make KDE_INSTALL_USE_QT_SYS_PATHS share the same directory scheme as Qt if they share the prefix

[Albert Vaca Cintora]

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


Ship it!




I don't see a reason not to change it.

It makes a lot of sense to install things in the same places as Qt, especially 
when Qt has to find the stuff we install.

- Albert Vaca Cintora


On feb. 24, 2016, 9:09 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127169/
> ---
> 
> (Updated feb. 24, 2016, 9:09 a.m.)
> 
> 
> Review request for Extra Cmake Modules, KDE Frameworks and Albert Vaca 
> Cintora.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> Make Qt and ECM-based projects use the same directory sctructure (i.e. where 
> plugins are, libs, etc.) by default. Otherwise it creates a tiny mess that 
> might be controlled but usually won't.
> 
> In the end, otherwise, people need to keep adapting their systems with 
> environment variables anyway. All distros end up setting always this setting 
> as ON, as well as brave developers who don't have separate prefixes for Qt 
> and KDE.
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake ebd48fa 
> 
> Diff: https://git.reviewboard.kde.org/r/127169/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

Re: [Kde-hardware-devel] Review Request 123263: WIP: KDE Connect backend for Solid

[Albert Vaca Cintora]

 On July 31, 2015, 12:38 p.m., Aleix Pol Gonzalez wrote:
  Is it done yet? What's the status?
 
 Kai Uwe Broulik wrote:
 I don't know, someone marked it as submitted but it cearly isn't.

What's left before we can merge it?


- Albert


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


On July 31, 2015, 12:15 p.m., Kai Uwe Broulik wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123263/
 ---
 
 (Updated July 31, 2015, 12:15 p.m.)
 
 
 Review request for kdeconnect, Solid, Albert Vaca Cintora, and Emmanuel 
 Pescosta.
 
 
 Repository: solid
 
 
 Description
 ---
 
 This adds a KDE Connect backend to Solid enabling us to show phone battery 
 status in the device notifier, and have the phone appear in the device 
 notifier like an external storage.
 
 It is pretty synchronous at the moment. Any ideas how I could improve that? I 
 guess when someone wants to get devices, it creates a new device interface 
 and calls allDevices() or devicesFromQuery() immediately afterwards, so I 
 cannot just do that stuff async and defer population? Or call deviceAdded for 
 each of the initial ones once I have them?
 
 It requires adjustment in the kdeconnect KIO slave so it can handle addresses 
 like kdeconnect://org/kde/kdeconnect/123456 or 
 kdeconnect:udi=/org/kde/kdeconnect/123456 (like mtp kio does) because all 
 Dolphin or the device notifier have is the UDI. It also needs a new device 
 action that matches Portable Media Player with the kdeconnect protocol, 
 similar to mtp.
 
 By adjusting the predicate in Dolphin's Places sidebar (and probably the file 
 dialog) to query for the kdeconnect protocol, we can have its places 
 automatically updated without having KDE Connect manually mess with 
 KFilePlaces.
 
 
 Diffs
 -
 
   src/solid/devices/CMakeLists.txt 9271ae1 
   src/solid/devices/backends/kdeconnect/CMakeLists.txt PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectbattery.h PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectbattery.cpp PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdevice.h PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdevice.cpp PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdeviceinterface.h 
 PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdeviceinterface.cpp 
 PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectmanager.h PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectmanager.cpp PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectportablemediaplayer.h 
 PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectportablemediaplayer.cpp 
 PRE-CREATION 
   src/solid/devices/managerbase.cpp eee4de5 
 
 Diff: https://git.reviewboard.kde.org/r/123263/diff/
 
 
 Testing
 ---
 
 With some crash fixes already pushed to plasma-workspace it works pretty well.
 
 
 Thanks,
 
 Kai Uwe Broulik
 


___
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 123263: WIP: KDE Connect backend for Solid

[Albert Vaca Cintora]

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


I don't understand the portable media player part. Does it do something? 
Besides that, I think we need to fix the issues opened by Lamarque and Alex, 
but looks good to me :)

- Albert Vaca Cintora


On abr. 5, 2015, 1:56 p.m., Kai Uwe Broulik wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123263/
 ---
 
 (Updated abr. 5, 2015, 1:56 p.m.)
 
 
 Review request for kdeconnect, Solid, Albert Vaca Cintora, and Emmanuel 
 Pescosta.
 
 
 Repository: solid
 
 
 Description
 ---
 
 This adds a KDE Connect backend to Solid enabling us to show phone battery 
 status in the device notifier, and have the phone appear in the device 
 notifier like an external storage.
 
 It is pretty synchronous at the moment. Any ideas how I could improve that? I 
 guess when someone wants to get devices, it creates a new device interface 
 and calls allDevices() or devicesFromQuery() immediately afterwards, so I 
 cannot just do that stuff async and defer population? Or call deviceAdded for 
 each of the initial ones once I have them?
 
 It requires adjustment in the kdeconnect KIO slave so it can handle addresses 
 like kdeconnect://org/kde/kdeconnect/123456 or 
 kdeconnect:udi=/org/kde/kdeconnect/123456 (like mtp kio does) because all 
 Dolphin or the device notifier have is the UDI. It also needs a new device 
 action that matches Portable Media Player with the kdeconnect protocol, 
 similar to mtp.
 
 By adjusting the predicate in Dolphin's Places sidebar (and probably the file 
 dialog) to query for the kdeconnect protocol, we can have its places 
 automatically updated without having KDE Connect manually mess with 
 KFilePlaces.
 
 
 Diffs
 -
 
   src/solid/devices/CMakeLists.txt 9271ae1 
   src/solid/devices/backends/kdeconnect/CMakeLists.txt PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectbattery.h PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectbattery.cpp PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdevice.h PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdevice.cpp PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdeviceinterface.h 
 PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdeviceinterface.cpp 
 PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectmanager.h PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectmanager.cpp PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectportablemediaplayer.h 
 PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectportablemediaplayer.cpp 
 PRE-CREATION 
   src/solid/devices/managerbase.cpp eee4de5 
 
 Diff: https://git.reviewboard.kde.org/r/123263/diff/
 
 
 Testing
 ---
 
 With some crash fixes already pushed to plasma-workspace it works pretty well.
 
 
 Thanks,
 
 Kai Uwe Broulik
 


___
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 123263: WIP: KDE Connect backend for Solid

[Albert Vaca Cintora]

 On abr. 5, 2015, 10:54 a.m., Lamarque Souza wrote:
  src/solid/devices/backends/kdeconnect/kdeconnectbattery.cpp, line 79
  https://git.reviewboard.kde.org/r/123263/diff/1/?file=360131#file360131line79
 
  Is it possible to retrieve a correct value for this and the other 
  values below (timeToEmpty, timeToFull, etc)?
 
 Kai Uwe Broulik wrote:
 I suppose, but that needs implementing on the KDE Connect side

From Android we could expose any of these: 
http://developer.android.com/reference/android/os/BatteryManager.html

Which ones would be useful in solid?


- Albert


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


On abr. 5, 2015, 1:56 p.m., Kai Uwe Broulik wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123263/
 ---
 
 (Updated abr. 5, 2015, 1:56 p.m.)
 
 
 Review request for kdeconnect, Solid, Albert Vaca Cintora, and Emmanuel 
 Pescosta.
 
 
 Repository: solid
 
 
 Description
 ---
 
 This adds a KDE Connect backend to Solid enabling us to show phone battery 
 status in the device notifier, and have the phone appear in the device 
 notifier like an external storage.
 
 It is pretty synchronous at the moment. Any ideas how I could improve that? I 
 guess when someone wants to get devices, it creates a new device interface 
 and calls allDevices() or devicesFromQuery() immediately afterwards, so I 
 cannot just do that stuff async and defer population? Or call deviceAdded for 
 each of the initial ones once I have them?
 
 It requires adjustment in the kdeconnect KIO slave so it can handle addresses 
 like kdeconnect://org/kde/kdeconnect/123456 or 
 kdeconnect:udi=/org/kde/kdeconnect/123456 (like mtp kio does) because all 
 Dolphin or the device notifier have is the UDI. It also needs a new device 
 action that matches Portable Media Player with the kdeconnect protocol, 
 similar to mtp.
 
 By adjusting the predicate in Dolphin's Places sidebar (and probably the file 
 dialog) to query for the kdeconnect protocol, we can have its places 
 automatically updated without having KDE Connect manually mess with 
 KFilePlaces.
 
 
 Diffs
 -
 
   src/solid/devices/CMakeLists.txt 9271ae1 
   src/solid/devices/backends/kdeconnect/CMakeLists.txt PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectbattery.h PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectbattery.cpp PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdevice.h PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdevice.cpp PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdeviceinterface.h 
 PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectdeviceinterface.cpp 
 PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectmanager.h PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectmanager.cpp PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectportablemediaplayer.h 
 PRE-CREATION 
   src/solid/devices/backends/kdeconnect/kdeconnectportablemediaplayer.cpp 
 PRE-CREATION 
   src/solid/devices/managerbase.cpp eee4de5 
 
 Diff: https://git.reviewboard.kde.org/r/123263/diff/
 
 
 Testing
 ---
 
 With some crash fixes already pushed to plasma-workspace it works pretty well.
 
 
 Thanks,
 
 Kai Uwe Broulik
 


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

Re: Review Request 122206: [kio] Make tests optional

[Albert Vaca Cintora]

 On March 16, 2015, 8:37 p.m., Albert Vaca Cintora wrote:
  I know this is merged already but this patch is being applied to every KDE 
  package and I want to keep the discussion in a single place.
  
  We already have a toggle option in CMake that is BUILD_TESTING. If Gentoo 
  wants to not build the tests (I'm not judging if they should, let them be 
  free to do it), they can just set BUILD_TESTING to OFF. I understand that 
  CMake will still try to find Qt5Test and fail, but here is where I think we 
  got it wrong:
  
  This patch does the following:
  if (Qt5Test is not found) 
  BUILD_TESTING = OFF
  
  What I think this patch should be doing is this:
  if (BUILD_TESTING == OFF) 
  Don't look for Qt5Test
  
  Did I miss something or this seems more reasonable to you guys as well?
 
 Michael Palimaka wrote:
 One of the original versions of these test patches looked something like:
 
 if (BUILD_TESTING)
 add_subdirectory(autotests)
 endif ()
 
 with `find_package(Qt5Test)` inside the autotests directory. While this 
 is used a bit in frameworks, it was rejected from a lot of plasma packages 
 because it relies on a magic variable (although it is defined by ECM).
 
 As a result there's a whole range of approaches across 
 frameworks/plasma/apps all doing the same thing. It would be nice if we could 
 agree on something and be consistent.

Is that the reason why we are doing it this way? :/ I don't think this is a 
magic variable at all, and if you want it to be even less magic you can set it 
in advance in a line before the if.


- Albert


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


On Feb. 6, 2015, 4:14 p.m., Andreas Sturmlechner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122206/
 ---
 
 (Updated Feb. 6, 2015, 4:14 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 [kio] Make tests optional
 This is a small patch to CMakeLists.txt to only depend on Qt5Test if 
 BUILD_TESTING.
 
 
 Diffs
 -
 
   CMakeLists.txt c1ed03f6cac648517828aec60e896baf9fbcfd9d 
 
 Diff: https://git.reviewboard.kde.org/r/122206/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Andreas Sturmlechner
 


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

Re: Review Request 122206: [kio] Make tests optional

[Albert Vaca Cintora]

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


I know this is merged already but this patch is being applied to every KDE 
package and I want to keep the discussion in a single place.

We already have a toggle option in CMake that is BUILD_TESTING. If Gentoo 
wants to not build the tests (I'm not judging if they should, let them be free 
to do it), they can just set BUILD_TESTING to OFF. I understand that CMake will 
still try to find Qt5Test and fail, but here is where I think we got it wrong:

This patch does the following:
if (Qt5Test is not found) 
BUILD_TESTING = OFF

What I think this patch should be doing is this:
if (BUILD_TESTING == OFF) 
Don't look for Qt5Test

Did I miss something or this seems more reasonable to you guys as well?

- Albert Vaca Cintora


On Feb. 6, 2015, 4:14 p.m., Andreas Sturmlechner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122206/
 ---
 
 (Updated Feb. 6, 2015, 4:14 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 [kio] Make tests optional
 This is a small patch to CMakeLists.txt to only depend on Qt5Test if 
 BUILD_TESTING.
 
 
 Diffs
 -
 
   CMakeLists.txt c1ed03f6cac648517828aec60e896baf9fbcfd9d 
 
 Diff: https://git.reviewboard.kde.org/r/122206/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Andreas Sturmlechner
 


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

Re: Review Request 122913: Added an event() version that takes no icon and will use a default one

[Albert Vaca Cintora]

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

(Updated March 14, 2015, 4:58 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Martin Klapetek.


Changes
---

Submitted with commit b368fb958df0ce4cc4128029f5649640f46d1559 by Albert Vaca 
to branch master.


Repository: knotifications


Description
---

Added an event() version that takes no icon and will use a default one 
depending on the notification type


Diffs
-

  src/knotification.h f2dcd74e26a4feefe53dc0e536b0178d5ce287e1 
  src/knotification.cpp 293de09bae8d16b77df81ee2fe447c3246a476b5 

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


Testing
---


Thanks,

Albert Vaca Cintora

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

Re: Review Request 122913: Added an event() version that takes no icon and will use a default one

[Albert Vaca Cintora]

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

(Updated mar. 14, 2015, 5:57 a.m.)


Review request for KDE Frameworks and Martin Klapetek.


Repository: knotifications


Description
---

Added an event() version that takes no icon and will use a default one 
depending on the notification type


Diffs (updated)
-

  src/knotification.h f2dcd74e26a4feefe53dc0e536b0178d5ce287e1 
  src/knotification.cpp 293de09bae8d16b77df81ee2fe447c3246a476b5 

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


Testing
---


Thanks,

Albert Vaca Cintora

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

Re: Review Request 122864: Added event() version that takes StandardEvent eventId and QString iconName

[Albert Vaca Cintora]

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

(Updated March 14, 2015, 4:58 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Aleix Pol Gonzalez.


Changes
---

Submitted with commit dedf55c937f95259121c4985f07345e06dd5b47b by Albert Vaca 
to branch master.


Repository: knotifications


Description
---

This allows to use icon names instead of QPixmaps when using StandardEvents
instead of app-defined events.


Diffs
-

  src/knotification.cpp 293de09bae8d16b77df81ee2fe447c3246a476b5 
  src/knotification.h f2dcd74e26a4feefe53dc0e536b0178d5ce287e1 

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


Testing
---


Thanks,

Albert Vaca Cintora

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

Re: Review Request 122864: Added event() version that takes StandardEvent eventId and QString iconName

[Albert Vaca Cintora]

 On mar. 9, 2015, 11:20 a.m., Martin Klapetek wrote:
  Ship It!
 
 Martin Klapetek wrote:
 Actually wait, StandardEvents should have standard icons, why would you 
 need to change them?
 
 Albert Vaca Cintora wrote:
 They don't have a standard, default icon: you have to specify it and as 
 of now the only way you have for it is with a QPixmap.
 
 Martin Klapetek wrote:
 Indeed, you are correct. Ship this then, it makes sense to have an 
 alternative for the QPixmap only.
 
 Another overload should be added that actually uses the default icons 
 automatically though. Would you be up for that?

Sure: https://git.reviewboard.kde.org/r/122913/


- Albert


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


On mar. 9, 2015, 6:02 a.m., Albert Vaca Cintora wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122864/
 ---
 
 (Updated mar. 9, 2015, 6:02 a.m.)
 
 
 Review request for KDE Frameworks and Aleix Pol Gonzalez.
 
 
 Repository: knotifications
 
 
 Description
 ---
 
 This allows to use icon names instead of QPixmaps when using StandardEvents
 instead of app-defined events.
 
 
 Diffs
 -
 
   src/knotification.cpp 293de09bae8d16b77df81ee2fe447c3246a476b5 
   src/knotification.h f2dcd74e26a4feefe53dc0e536b0178d5ce287e1 
 
 Diff: https://git.reviewboard.kde.org/r/122864/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Albert Vaca Cintora
 


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

Re: Review Request 122900: KPluginSelector to provide initialization arguments for the configuration modules

[Albert Vaca Cintora]

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

Ship it!


It even works! :D


src/kpluginselector.h
https://git.reviewboard.kde.org/r/122900/#comment53110

@since 5.9



src/kpluginselector.h
https://git.reviewboard.kde.org/r/122900/#comment53111

@since 5.9


- Albert Vaca Cintora


On mar. 11, 2015, 1:53 a.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122900/
 ---
 
 (Updated mar. 11, 2015, 1:53 a.m.)
 
 
 Review request for kdeconnect and KDE Frameworks.
 
 
 Repository: kcmutils
 
 
 Description
 ---
 
 KCMs make it possible to receive a list of arguments we're not passing yet. 
 This patch makes it possible to specify these arguments by specifying a 
 static list of arguments for all the plugins in the selector.
 
 
 Diffs
 -
 
   src/kpluginselector.h 0125991 
   src/kpluginselector.cpp 98ab59e 
   src/kpluginselector_p.h d80cd2e 
 
 Diff: https://git.reviewboard.kde.org/r/122900/diff/
 
 
 Testing
 ---
 
 Tested over at kdeconnect, where we need this.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


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

Review Request 122913: Added an event() version that takes no icon and will use a default one

[Albert Vaca Cintora]

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

Review request for KDE Frameworks and Martin Klapetek.


Repository: knotifications


Description
---

Added an event() version that takes no icon and will use a default one 
depending on the notification type


Diffs
-

  src/knotification.h f2dcd74e26a4feefe53dc0e536b0178d5ce287e1 
  src/knotification.cpp 293de09bae8d16b77df81ee2fe447c3246a476b5 

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


Testing
---


Thanks,

Albert Vaca Cintora

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

Re: Review Request 122913: Added an event() version that takes no icon and will use a default one

[Albert Vaca Cintora]

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

(Updated mar. 12, 2015, 4:37 a.m.)


Review request for KDE Frameworks and Martin Klapetek.


Changes
---

Had to remove a default empty QPixmap parameter from a different event()
header, that would make a call to one of them ambiguous. This means that
when recompiling an app that used event() with StandardEvent and didn't
specify an icon, it will now pick the version that uses standard icons
instead of the one that shows no icon at all. (Which seems ok to me).


Repository: knotifications


Description
---

Added an event() version that takes no icon and will use a default one 
depending on the notification type


Diffs (updated)
-

  src/knotification.h f2dcd74e26a4feefe53dc0e536b0178d5ce287e1 
  src/knotification.cpp 293de09bae8d16b77df81ee2fe447c3246a476b5 

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


Testing
---


Thanks,

Albert Vaca Cintora

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

Re: Review Request 122864: Added event() version that takes StandardEvent eventId and QString iconName

[Albert Vaca Cintora]

 On mar. 9, 2015, 11:20 a.m., Martin Klapetek wrote:
  Ship It!
 
 Martin Klapetek wrote:
 Actually wait, StandardEvents should have standard icons, why would you 
 need to change them?

They don't have a standard, default icon: you have to specify it and as of now 
the only way you have for it is with a QPixmap.


- Albert


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


On mar. 9, 2015, 6:02 a.m., Albert Vaca Cintora wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122864/
 ---
 
 (Updated mar. 9, 2015, 6:02 a.m.)
 
 
 Review request for KDE Frameworks and Aleix Pol Gonzalez.
 
 
 Repository: knotifications
 
 
 Description
 ---
 
 This allows to use icon names instead of QPixmaps when using StandardEvents
 instead of app-defined events.
 
 
 Diffs
 -
 
   src/knotification.cpp 293de09bae8d16b77df81ee2fe447c3246a476b5 
   src/knotification.h f2dcd74e26a4feefe53dc0e536b0178d5ce287e1 
 
 Diff: https://git.reviewboard.kde.org/r/122864/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Albert Vaca Cintora
 


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

Review Request 122864: Added event() version that takes StandardEvent eventId and QString iconName

[Albert Vaca Cintora]

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

Review request for KDE Frameworks and Aleix Pol Gonzalez.


Repository: knotifications


Description
---

This allows to use icon names instead of QPixmaps when using StandardEvents
instead of app-defined events.


Diffs
-

  src/knotification.cpp 293de09bae8d16b77df81ee2fe447c3246a476b5 
  src/knotification.h f2dcd74e26a4feefe53dc0e536b0178d5ce287e1 

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


Testing
---


Thanks,

Albert Vaca Cintora

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

Re: Review Request 112177: Split URL drop functionality from KLineEdit

[Albert Vaca Cintora]

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

(Updated Aug. 20, 2013, 8:46 p.m.)


Review request for KDE Frameworks.


Description
---

The replace content on URL drop functionality that was present in KLineEdit has 
been moved to a separate class, that can be installed as an event filter in any 
QLineEdit (KLineEdit, QComboBox, KUrlRequester...). 

KLineEdit now uses this new class when the property urlDropsEnabled is 
enabled, but this property has been marked as deprecated in favour of the new 
alternative.


Diffs
-

  tier1/kwidgetsaddons/src/lineediturldropeventfilter.cpp PRE-CREATION 
  tier1/kwidgetsaddons/src/lineediturldropeventfilter.h PRE-CREATION 
  tier1/kwidgetsaddons/src/CMakeLists.txt c17c648 
  staging/kcompletion/src/klineedit.cpp 213b196 
  staging/kcompletion/src/klineedit.h e9f3332 
  staging/kcompletion/src/kcombobox.h 0d4e912 

Diff: http://git.reviewboard.kde.org/r/112177/diff/


Testing
---

Manual testing + tests passed


Thanks,

Albert Vaca Cintora

___
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 111699: Fixed no such signal message in UPowerManager at startup

[Albert Vaca Cintora]

 On July 26, 2013, 12:05 p.m., Lukáš Tinkl wrote:
  I'm curious why this is needed, after all, your change just uses a 
  different mechanism to connect the signals
 
 Albert Vaca Cintora wrote:
 I'm not sure if the current code actually works, but it gives that error 
 in runtime so I think it is better to change it anyway.
 
 Lukáš Tinkl wrote:
 Hmm, it works here but... can you test (both cases) whether you get the 
 signals delivered, e.g. by pulling your battery out of the notebook?

I was wrong, It works both ways. I'll close this reviewboard now.


- Albert


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


On July 25, 2013, 10:36 p.m., Albert Vaca Cintora wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/111699/
 ---
 
 (Updated July 25, 2013, 10:36 p.m.)
 
 
 Review request for Solid and Àlex Fiestas.
 
 
 Description
 ---
 
 The following message was shown when UPowerManager was instantiated (e.g.: 
 when kded4 starts):
 
 Object::connect: No such signal QDBusAbstractInterface::DeviceAdded(QString) 
 in 
 /home/vaka/kde4/src/kdelibs/solid/solid/backends/upower/upowermanager.cpp:67
 Object::connect: No such signal 
 QDBusAbstractInterface::DeviceRemoved(QString) in 
 /home/vaka/kde4/src/kdelibs/solid/solid/backends/upower/upowermanager.cpp:69
 
 This patch fixes to connection to the mentioned DBus signal (the normal 
 connect construction that was used doesn't work for dbus signals) so this 
 message doesn't appear anymore, and probably fixes the underlying 
 functionality that required this signal to be connected.
 
 
 Diffs
 -
 
   solid/solid/backends/upower/upowermanager.cpp bae234b 
 
 Diff: http://git.reviewboard.kde.org/r/111699/diff/
 
 
 Testing
 ---
 
 Manual testing.
 
 
 Thanks,
 
 Albert Vaca Cintora
 


___
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 111699: Fixed no such signal message in UPowerManager at startup

[Albert Vaca Cintora]

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

(Updated July 28, 2013, 5:07 p.m.)


Status
--

This change has been discarded.


Review request for Solid and Àlex Fiestas.


Description
---

The following message was shown when UPowerManager was instantiated (e.g.: when 
kded4 starts):

Object::connect: No such signal QDBusAbstractInterface::DeviceAdded(QString) in 
/home/vaka/kde4/src/kdelibs/solid/solid/backends/upower/upowermanager.cpp:67
Object::connect: No such signal QDBusAbstractInterface::DeviceRemoved(QString) 
in /home/vaka/kde4/src/kdelibs/solid/solid/backends/upower/upowermanager.cpp:69

This patch fixes to connection to the mentioned DBus signal (the normal connect 
construction that was used doesn't work for dbus signals) so this message 
doesn't appear anymore, and probably fixes the underlying functionality that 
required this signal to be connected.


Diffs
-

  solid/solid/backends/upower/upowermanager.cpp bae234b 

Diff: http://git.reviewboard.kde.org/r/111699/diff/


Testing
---

Manual testing.


Thanks,

Albert Vaca Cintora

___
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 111699: Fixed no such signal message in UPowerManager at startup

[Albert Vaca Cintora]

 On July 26, 2013, 12:05 p.m., Lukáš Tinkl wrote:
  I'm curious why this is needed, after all, your change just uses a 
  different mechanism to connect the signals

I'm not sure if the current code actually works, but it gives that error in 
runtime so I think it is better to change it anyway.


- Albert


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


On July 25, 2013, 10:36 p.m., Albert Vaca Cintora wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/111699/
 ---
 
 (Updated July 25, 2013, 10:36 p.m.)
 
 
 Review request for Solid and Àlex Fiestas.
 
 
 Description
 ---
 
 The following message was shown when UPowerManager was instantiated (e.g.: 
 when kded4 starts):
 
 Object::connect: No such signal QDBusAbstractInterface::DeviceAdded(QString) 
 in 
 /home/vaka/kde4/src/kdelibs/solid/solid/backends/upower/upowermanager.cpp:67
 Object::connect: No such signal 
 QDBusAbstractInterface::DeviceRemoved(QString) in 
 /home/vaka/kde4/src/kdelibs/solid/solid/backends/upower/upowermanager.cpp:69
 
 This patch fixes to connection to the mentioned DBus signal (the normal 
 connect construction that was used doesn't work for dbus signals) so this 
 message doesn't appear anymore, and probably fixes the underlying 
 functionality that required this signal to be connected.
 
 
 Diffs
 -
 
   solid/solid/backends/upower/upowermanager.cpp bae234b 
 
 Diff: http://git.reviewboard.kde.org/r/111699/diff/
 
 
 Testing
 ---
 
 Manual testing.
 
 
 Thanks,
 
 Albert Vaca Cintora
 


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

Re: Review Request 111184: Url drop functionality in KUrlRequester does not depend on KLineEdit (since we have to port it to QLineEdit)

[Albert Vaca Cintora]

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

(Updated July 16, 2013, 10:03 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


Description
---

KLineEdit and KComboBox had the property enableUrlDrops that was added to use 
it in KUrlRequester. Since KUrlRequester is part of KIO, it can not use 
Kde4Support and should be ported to use QLineEdit and QComboBox. As I said on 
my email to the frameworks list, it was easier to implement this directly in 
KUrlRequester than patching Qt, even though it was originally planned as a Qt 
task. It was my intention to also remove that property from KLineEdit and 
KComboBox, but finally I didn't include it in the patch (see Ervin's comment in 
the mailing list).

In the patch I have also added a TODO because I think it would be good to 
detect any url without needing the mimetype to be set, but I have not done it 
yet because it should be discussed first.

(Reviewed by aacid and apol)


Diffs
-

  KDE5PORTING.html ba67bdc 
  kdeui/widgets/kcombobox.h ccb019d 
  kdeui/widgets/klineedit.h 7ac22f6 
  kio/kfile/kurlrequester.cpp fa6b234 

Diff: http://git.reviewboard.kde.org/r/84/diff/


Testing
---


Thanks,

Albert Vaca Cintora

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

Re: Review Request 111184: Url drop functionality in KUrlRequester does not depend on KLineEdit (since we have to port it to QLineEdit)

[Albert Vaca Cintora]

 On June 25, 2013, 6:46 a.m., Kevin Ottens wrote:
  kio/kfile/kurlrequester.cpp, line 483
  http://git.reviewboard.kde.org/r/84/diff/2/?file=165624#file165624line483
 
  Why don't you use KUrlMimeData like the original code in KLineEdit? 
  It'll be more robust AFAICT.
 
 Albert Vaca Cintora wrote:
 I'm using hasUrls() instead because KUrlMimeData returns a list of urls, 
 which is more expensive than what Qt does (a string comparison). 
 
 Also, if our way of checking URLs is better than the way Qt uses, we 
 should contribute it to Qt instead of avoid using their methods (we are going 
 to rely a lot more on Qt since KF5, so we have to trust what Qt does).
 
 Kevin Ottens wrote:
 Well, KUrlMimeData has some specific handling for KIO, which is a big 
 no-no for Qt. That's more what I had in mind... we don't need the metadata 
 part in that context though, so it's likely a moot point.
 
 That said we look for the application/x-kde4-urilist mimetype too (again 
 something we can't really push in Qt). David, any reason why we have this 
 extra mimetype? Doesn't really makes sense to me.
 
 David Faure wrote:
 KUrlMimeData has some specific handling for KIO, and not just metadata, 
 but also shipping two lists of URLs: KIO urls and most-local urls. For 
 instance desktop:/foo and file://home/dfaure/Desktop/foo, the first one being 
 aimed at KIO-enabled apps and the second one at non-KIO-enabled apps.
 
 The code here simply checks for there are urls, and lets QLineEdit do 
 the extraction.
 This means that the most-local urls are going to be pasted, not the KIO 
 ones.
 But that's not really specific to kurlrequester nor the 
 clear-before-insert feature... (e.g. konqueror's location bar would have the 
 same issue)
 
 WAIT . who determined that only KUrlRequester used the url drop 
 feature of KLineEdit?
 That feature is ON by default, so looking for calls to 
 setUrlDropsEnabled(true) is definitely wrong.
 Konqueror's location bar definitely uses that feature too. I suspect the 
 location bar in dolphin and the one in kfiledialog do as well, at least.
 
 It seems to me that putting the code into KUrlRequester is just wrong. It 
 should be an event filter that can be plugged onto any QLineEdit. And then it 
 can take care of both: clear before insert, and using KUrlMimeData to do the 
 URL extraction.
 
 Kevin Ottens wrote:
 Hmmm... You're right, as I mentionned earlier that was my only concern 
 that url drops were enabled by default... But somehow I assumed that most of 
 our URL bars were KUrlRequester. Dunno what happened in my brain that day as 
 obviously it's not the case. They're either KComboBox or KLineEdit.
 
 OK... so if we want to make the url drops feature more reusable we should 
 make that feature an event filter in its own class as I initially proposed.
 
 But (and that's a strong but), I now realize that we never pushed toward 
 KUrlRequester to use QLineEdit or QComboBox. It can keep using KLineEdit or 
 KComboBox just fine. They're not getting deprecated, they'll be in 
 KCompletion (and the future KioWidgets will be able to use KCompletion just 
 fine IMO).
 
 It makes me think this patch should in fact be dropped.

I see your point and agree with you. However I don't think that every other 
class using this feature can keep using KLineEdit as before (some will need to 
be ported to QLineEdit even if I was wrong and this is not the case of 
KUrlRequester) and I don't think this feature is generic enough to send a patch 
to Qt. I could then move it to a separate class (usable from wherever we need) 
as Ervin suggested initially, but... do you feel it necessary or can we just 
drop this feature from the apps we migrate to QLineEdit? 

Since you have a more comprehensive perspective of the project as I have, if 
you think we should keep it I will write the patch to move it to an independent 
class.


- Albert


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


On June 25, 2013, 10:46 a.m., Albert Vaca Cintora wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/84/
 ---
 
 (Updated June 25, 2013, 10:46 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Description
 ---
 
 KLineEdit and KComboBox had the property enableUrlDrops that was added to use 
 it in KUrlRequester. Since KUrlRequester is part of KIO, it can not use 
 Kde4Support and should be ported to use QLineEdit and QComboBox. As I said on 
 my email to the frameworks list, it was easier to implement this directly in 
 KUrlRequester than patching Qt, even though

Re: Review Request 111184: Url drop functionality in KUrlRequester does not depend on KLineEdit (since we have to port it to QLineEdit)

[Albert Vaca Cintora]

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

(Updated June 25, 2013, 10:46 a.m.)


Review request for KDE Frameworks.


Description
---

KLineEdit and KComboBox had the property enableUrlDrops that was added to use 
it in KUrlRequester. Since KUrlRequester is part of KIO, it can not use 
Kde4Support and should be ported to use QLineEdit and QComboBox. As I said on 
my email to the frameworks list, it was easier to implement this directly in 
KUrlRequester than patching Qt, even though it was originally planned as a Qt 
task. It was my intention to also remove that property from KLineEdit and 
KComboBox, but finally I didn't include it in the patch (see Ervin's comment in 
the mailing list).

In the patch I have also added a TODO because I think it would be good to 
detect any url without needing the mimetype to be set, but I have not done it 
yet because it should be discussed first.

(Reviewed by aacid and apol)


Diffs (updated)
-

  KDE5PORTING.html ba67bdc 
  kdeui/widgets/kcombobox.h ccb019d 
  kdeui/widgets/klineedit.h 7ac22f6 
  kio/kfile/kurlrequester.cpp fa6b234 

Diff: http://git.reviewboard.kde.org/r/84/diff/


Testing
---


Thanks,

Albert Vaca Cintora

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

Review Request 111184: Moved url drop functionality from KLineEdit to KUrlChooser

[Albert Vaca Cintora]

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

Review request for KDE Frameworks.


Description
---

KLineEdit and KComboBox had the property enableUrlDrops that was added to use 
it in KUrlRequester. Since KUrlRequester is part of KIO, it can not use 
Kde4Support and should be ported to use QLineEdit and QComboBox. As I said on 
my email to the frameworks list, it was easier to implement this directly in 
KUrlRequester than patching Qt, even though it was originally planned as a Qt 
task. It was my intention to also remove that property from KLineEdit and 
KComboBox, but finally I didn't include it in the patch (see Ervin's comment in 
the mailing list).

In the patch I have also added a TODO because I think it would be good to 
detect any url without needing the mimetype to be set, but I have not done it 
yet because it should be discussed first.

(Reviewed by aacid and apol)


Diffs
-

  kio/kfile/kurlrequester.cpp fa6b234 

Diff: http://git.reviewboard.kde.org/r/84/diff/


Testing
---


Thanks,

Albert Vaca Cintora

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

Re: Review Request 111184: Url drop functionality in KUrlRequester does not depend on KLineEdit (since we have to port it to QLineEdit)

[Albert Vaca Cintora]

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

(Updated June 24, 2013, 10:15 p.m.)


Review request for KDE Frameworks.


Summary (updated)
-

Url drop functionality in KUrlRequester does not depend on KLineEdit (since we 
have to port it to QLineEdit)


Description
---

KLineEdit and KComboBox had the property enableUrlDrops that was added to use 
it in KUrlRequester. Since KUrlRequester is part of KIO, it can not use 
Kde4Support and should be ported to use QLineEdit and QComboBox. As I said on 
my email to the frameworks list, it was easier to implement this directly in 
KUrlRequester than patching Qt, even though it was originally planned as a Qt 
task. It was my intention to also remove that property from KLineEdit and 
KComboBox, but finally I didn't include it in the patch (see Ervin's comment in 
the mailing list).

In the patch I have also added a TODO because I think it would be good to 
detect any url without needing the mimetype to be set, but I have not done it 
yet because it should be discussed first.

(Reviewed by aacid and apol)


Diffs
-

  kio/kfile/kurlrequester.cpp fa6b234 

Diff: http://git.reviewboard.kde.org/r/84/diff/


Testing
---


Thanks,

Albert Vaca Cintora

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

Re: Review Request 110430: [Taskbar applet] Added actions to be perfomed on middle click

[Albert Vaca Cintora]

 On May 15, 2013, 6:06 a.m., Aaron J. Seigo wrote:
  I am not in favour of this patch for a couple of reasons. First, there is a 
  port underway to QML which does not need to be delayed further by adding 
  more features. More importantly, however, middle click is a special case 
  of not the first two mouse buttons (should we support N button mice?) and 
  it adds yet more configuration to an already complex and full configuration 
  dialog. It also conflicts with the meaning of middle click to expand / 
  collapse groups (a fairly hidden feature I also was not particularly happy 
  with tbh).

Hello Aaron and thank for your reply.

Let me defend the inclusion of this patch from the problems you mention:

- Difficulty to port to QML: This feature is implemented in a ~10 lines switch 
(not counting the GUI-related code), so porting it should not be a problem (I 
could do it, if needed).

- Support for N button mice: The desktop should adapt to the current hardware, 
and nowadays most mice have 3 buttons (not N, but neither only 2). Moreover, 
lots of apps have adopted the behaviour of closing tabs with a middle click, so 
adding this feature for applications in the taskbar is consistent with them.

- Complexity of the configuration dialog: I agree that we should try to keep 
all the configuration dialogs simple, but not adding new features because of 
that reminds me of Gnome3 ;) I think this is not solution for the 
long-discussed problem of the user-friendliness.

Finally, and more important, it has 2 open bugs (+ 4 duplicates) so there are 
real users requesting it. If it's not going to be added, those bugs should be 
marked as wontfix.


- Albert


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


On May 14, 2013, 10:35 p.m., Albert Vaca Cintora wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/110430/
 ---
 
 (Updated May 14, 2013, 10:35 p.m.)
 
 
 Review request for kde-workspace and Plasma.
 
 
 Description
 ---
 
 This patch adds a feature present in KDE3 and requested by some users (see 
 open bugs), that is performing an action when a taskbar entry is 
 middle-clicked. I have added three possible actions: Close the application, 
 hide it or move it to the current desktop, where the first two were user 
 requests.
 
 
 This addresses bugs 182689 and 190951.
 http://bugs.kde.org/show_bug.cgi?id=182689
 http://bugs.kde.org/show_bug.cgi?id=190951
 
 
 Diffs
 -
 
   plasma/desktop/applets/tasks/tasks.h fe79a12 
   plasma/desktop/applets/tasks/tasks.cpp 0a86dcf 
   plasma/desktop/applets/tasks/tasksConfig.ui 6f3ff18 
   plasma/desktop/applets/tasks/windowtaskitem.cpp f840076 
 
 Diff: http://git.reviewboard.kde.org/r/110430/diff/
 
 
 Testing
 ---
 
 Manual testing.
 
 
 Thanks,
 
 Albert Vaca Cintora
 

Re: Review Request 110430: [Taskbar applet] Added actions to be perfomed on middle click

[Albert Vaca Cintora]

 On May 15, 2013, 6:06 a.m., Aaron J. Seigo wrote:
  I am not in favour of this patch for a couple of reasons. First, there is a 
  port underway to QML which does not need to be delayed further by adding 
  more features. More importantly, however, middle click is a special case 
  of not the first two mouse buttons (should we support N button mice?) and 
  it adds yet more configuration to an already complex and full configuration 
  dialog. It also conflicts with the meaning of middle click to expand / 
  collapse groups (a fairly hidden feature I also was not particularly happy 
  with tbh).
 
 Albert Vaca Cintora wrote:
 Hello Aaron and thank for your reply.
 
 Let me defend the inclusion of this patch from the problems you mention:
 
 - Difficulty to port to QML: This feature is implemented in a ~10 lines 
 switch (not counting the GUI-related code), so porting it should not be a 
 problem (I could do it, if needed).
 
 - Support for N button mice: The desktop should adapt to the current 
 hardware, and nowadays most mice have 3 buttons (not N, but neither only 2). 
 Moreover, lots of apps have adopted the behaviour of closing tabs with a 
 middle click, so adding this feature for applications in the taskbar is 
 consistent with them.
 
 - Complexity of the configuration dialog: I agree that we should try to 
 keep all the configuration dialogs simple, but not adding new features 
 because of that reminds me of Gnome3 ;) I think this is not solution for the 
 long-discussed problem of the user-friendliness.
 
 Finally, and more important, it has 2 open bugs (+ 4 duplicates) so there 
 are real users requesting it. If it's not going to be added, those bugs 
 should be marked as wontfix.
 
 Aaron J. Seigo wrote:
 porting it should not be a problem (I could do it, if needed).
 
 that is definitely a point in your favor. assuming this feature addition 
 is accepted: there's little point to putting it in the c++ version, however, 
 only to put it later in the qml version (which is supposed to be in 4.11). so 
 putting it directly in the QML version would make the most sense.
 
 Moreover, lots of apps have adopted the behaviour of closing tabs with a 
 middle click
 
 to point out the obvious: windows are not tabs. this also implies that 
 middle click to close is actually a *good* feature. i think it is for power 
 users .. but i have seen too many instances where these kinds of magic click 
 that button and magically something happens features lead to confusion, and 
 confusion leads to distrust of the system as a whole.
 
 yes, the default is to do nothing in your patch (+1 for that), but if 
 sitting down to someone else's system results in wildly unpredictable 
 behaviour  ... keep in mind this is not a random component, but a default 
 part of every plasma desktop installation, so it has to work better than most.
 
 how much faster is middle click versus right-click-close? fast enough to 
 warrant the risk of surprising behaviour for people who aren't expecting it?
 
 Complexity of the configuration dialog: I agree that we should try to 
 keep all the configuration dialogs simple
 
 currently that page has 11 settings. ad-hoc testing i've done in the past 
 hints that many of these settings are already not understandable to many 
 users. i really don't want to make this worse. several of the plasmoids have 
 room for more options. the taskbar, folderview, clock and system tray are 
 four that really don't. adding feature over feature is leading to an 
 increasing mess that nobody cares to clean up.
 
 if this patch introduced a re-think of the configuration presentation so 
 that it is easier to understand and more approachable, i would consider that 
 a fair trade for accepting a feature i'm not particularly in favour of :)
 
 but not adding new features because of that reminds me of Gnome3
 
 for future reference: when i see this kind of statement made, i usually 
 add -1 to my overall weighting. i do not agree with many design decisions in 
 other projects, but design can not be done well if it is primarily directed 
 by not being that other group. common sense and reasoning should be applied 
 to each case without the justification of not like them, otherwise we just 
 create the opposite kind of error.
 
 it has 2 open bugs (+ 4 duplicates) so there are real users requesting 
 it.
 
 for any product with a large enough user base, given enough time and an 
 open feature request tracker, any possible feature will be requested (usually 
 multiple times). this means that any feature, regardless of intrinsic value, 
 can be justified using this argument.
 
 (the least useful case of this i've seen is when people submit the 
 feature request, then later a patch and then use the feature request as 
 evidence it is wanted ;)
 

 
 Martin

Review Request 110430: [Taskbar applet] Added actions to be perfomed on middle click

[Albert Vaca Cintora]

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

Review request for kde-workspace.


Description
---

This patch adds a feature present in KDE3 and requested by some users (see open 
bugs), that is performing an action when a taskbar entry is middle-clicked. I 
have added three possible actions: Close the application, hide it or move it to 
the current desktop, where the first two were user requests.


This addresses bugs 182689 and 190951.
http://bugs.kde.org/show_bug.cgi?id=182689
http://bugs.kde.org/show_bug.cgi?id=190951


Diffs
-

  plasma/desktop/applets/tasks/tasks.h fe79a12 
  plasma/desktop/applets/tasks/tasks.cpp 0a86dcf 
  plasma/desktop/applets/tasks/tasksConfig.ui 6f3ff18 
  plasma/desktop/applets/tasks/windowtaskitem.cpp f840076 

Diff: http://git.reviewboard.kde.org/r/110430/diff/


Testing
---

Manual testing.


Thanks,

Albert Vaca Cintora