Differential D11411

MPRIS control: fix instance indicator with multiple app instances
ClosedPublic
Actions

Authored by kossebau on Mar 17 2018, 1:39 PM.

Details

Reviewers
mtijink
Group Reviewers
KDE Connect
Commits
R224:8d6236bb7ecb: MPRIS control: fix instance indicator with multiple app instances
Summary

Not sure what operator+ overload has been used exactly for the int,
in any case it does not work as intended and needs e.g. an explicit
QString::number() invocation.

Also start with number 2 for duplicated instances.

Test Plan

Start multiple instances of an MPRIS player (e.g. Gwenview). Before the
second instance would get labelled with "Name []", with this patch it is
labelled with "Name [2]".

Diff Detail

Repository
R224 KDE Connect
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau requested review of this revision.Mar 17 2018, 1:39 PM
kossebau created this revision.
kossebau added a comment.Mar 17 2018, 1:41 PM

While looking as this: perhaps it make sense to also switch to use "<#>" instead of "[#]", like known e.g. from KWin? Is there any standard for such instance label numbering?

nicolasfella added a subscriber: nicolasfella.Mar 17 2018, 1:46 PM

I guess <> makes sense. I dont't know if there is a standard for this (maybe ask VDG ), but feel free to go ahead with <>

mtijink accepted this revision.Mar 17 2018, 7:36 PM
mtijink added a subscriber: mtijink.

Patch looks good. Another option is (2), as used for files.

This revision is now accepted and ready to land.Mar 17 2018, 7:36 PM
kossebau added a comment.Mar 17 2018, 10:36 PM

Too many choices. Keeping [] for now :) Thanks for reviews!

Closed by commit R224:8d6236bb7ecb: MPRIS control: fix instance indicator with multiple app instances (authored by kossebau). · Explain WhyMar 17 2018, 10:37 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 29800

View Options

plugins/mpriscontrol/mpriscontrolplugin.cpp

Loading...