[Differential] [Commented On] D3650: lower mouse acceleration limit to 0.0

2016-12-13 Thread sitter (Harald Sitter) 
sitter added a comment.


  Ok. Kindly take note of potential adjustment needs then.

REPOSITORY
  R119 Plasma Desktop

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

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

To: sitter, plasma-devel
Cc: graesslin, broulik, jensreuterberg, lesliezhai, ali-mohamed, abetts, sebas

[Differential] [Commented On] D3649: Make kirigami.qrc ressource path relative to CMAKE_CURRENT_SOURCE_DIR

2016-12-13 Thread franckarrecot (Franck Arrecot) 
franckarrecot added a comment.


  thanks, I can't push though, do I need special access or special urls to do 
so ?

REPOSITORY
  R169 Kirigami

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

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

To: franckarrecot, mart, apol
Cc: apol, plasma-devel

[Differential] [Commented On] D3649: Make kirigami.qrc ressource path relative to CMAKE_CURRENT_SOURCE_DIR

2016-12-13 Thread franckarrecot (Franck Arrecot) 
franckarrecot added a comment.


  my current urls are :
  
  origin  git://anongit.kde.org/kirigami.git (fetch)
  origin  git://anongit.kde.org/kirigami.git (push)

REPOSITORY
  R169 Kirigami

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

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

To: franckarrecot, mart, apol
Cc: apol, plasma-devel

[Differential] [Updated] D2687: WIP: [Icon Widget] Bring back properties dialog

2016-12-13 Thread broulik (Kai Uwe Broulik) 
broulik marked 9 inline comments as done.

REPOSITORY
  R120 Plasma Workspace

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

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

To: broulik, #plasma, dfaure
Cc: mart, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

[Differential] [Commented On] D3660: Plasma 5.9 "Canopee" Wallpaper

2016-12-13 Thread mart (Marco Martin) 
mart added a comment.


  like it a lot!

REPOSITORY
  R119 Plasma Desktop

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

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

To: kvermette, #plasma
Cc: mart, broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas

[Differential] [Commented On] D3658: Set Automatic login flag to adjust SDDM config directly

2016-12-13 Thread mart (Marco Martin) 
mart added a comment.


  did the helper already supported the action or is a different rr?

REPOSITORY
  R128 User Manager

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

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

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

[Differential] [Commented On] D3655: Kicker backend changes for Simple Menu.

2016-12-13 Thread mart (Marco Martin) 
mart added inline comments.

INLINE COMMENTS

> appentry.h:79
>  AppGroupEntry(AppsModel *parentModel, KServiceGroup::Ptr group,
> -bool flat, bool sorted, bool separators, int appNameFormat);
> +bool paginate, int pageSize, bool flat, bool sorted, bool 
> separators, int appNameFormat);
>  

a constructor with a list of bool parameters doesn't look particularly good, 
but as i see was already like that before adding the paginate option, so is 
probably a bit late.. I would probably have seen bettea couple of different 
subclasses for different entries style? (same for the model)

> appsmodel.h:46
>  public:
> -explicit AppsModel(const QString &entryPath = QString(), bool flat = 
> false, bool sorted = true, bool separators = true, QObject *parent = 0);
> +explicit AppsModel(const QString &entryPath = QString(), bool 
> paginate = false, int pageSize = 24,
> +bool flat = false, bool sorted = true, bool separators = true, 
> QObject *parent = 0);

are all those parameters really needed right in the ctor?

REPOSITORY
  R119 Plasma Desktop

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

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

To: hein, #plasma, mart
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Accepted] D3654: [TextField] Allow disabling reveal password button through KIOSK restriction

2016-12-13 Thread mart (Marco Martin) 
mart accepted this revision.
mart added a reviewer: mart.
mart added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> TextField.qml:49
> +// this takes into account kiosk restriction
> +readonly property bool __effectiveRevealPasswordButtonShown: 
> revealPasswordButtonShown
> +  && 
> KAuthorized.authorize("lineedit_reveal_password")

instead of having a __ name, could also be a property of an object that's 
inside instead of the root object, so would be pretty much inaccessible from 
the outside

REPOSITORY
  R242 Plasma Frameworks

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

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

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

[Differential] [Accepted] D3651: Add utility to send broadcast notifications

2016-12-13 Thread mart (Marco Martin) 
mart accepted this revision.
mart added a reviewer: mart.
This revision is now accepted and ready to land.

REPOSITORY
  R126 KDE CLI Utilities

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

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

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

[Differential] [Changed Subscribers] D3651: Add utility to send broadcast notifications

2016-12-13 Thread bshah (Bhushan Shah) 
bshah added inline comments.

INLINE COMMENTS

> main.cpp:79
> +if (!uids.isEmpty()) {
> +properties.insert(QStringLiteral("uids"), uids);
> +}

I wonder if it can take usernames instead of uids? It is slightly easier to 
remember usernames then uids

REPOSITORY
  R126 KDE CLI Utilities

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

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

To: broulik, #plasma, mart
Cc: bshah, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

[Differential] [Updated, 594 lines] D2687: [Icon Widget] Bring back properties dialog

2016-12-13 Thread broulik (Kai Uwe Broulik) 
broulik retitled this revision from "WIP: [Icon Widget] Bring back properties 
dialog" to "[Icon Widget] Bring back properties dialog".
broulik updated the test plan for this revision.
broulik updated this revision to Diff 8967.
broulik added a comment.


  - Ensure Link desktop file is actually .desktop so the properties dialog 
offers to change the URL
  - Emit jump list actions changed when they do (fixes jump list actions not 
working when applet is created for the first time)
  - Call sync() on the KDesktopFile we created for the file Links so the applet 
works when it is created for the first time for a file
  - KIO::suggestName always appends suffix, so check whether the file exists 
before
  - use new KRun to run the apps and QProcess::startDetached as suggested by 
David F
  - Cleanup a bit, at least try to ;)

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D2687?vs=6501&id=8967

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

AFFECTED FILES
  applets/icon/CMakeLists.txt
  applets/icon/iconapplet.cpp
  applets/icon/iconapplet.h
  applets/icon/package/contents/config/main.xml
  applets/icon/package/contents/ui/main.qml

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

To: broulik, #plasma, dfaure
Cc: mart, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

[Differential] [Commented On] D3651: Add utility to send broadcast notifications

2016-12-13 Thread broulik (Kai Uwe Broulik) 
broulik added a comment.


  > I wonder if it can take usernames instead of uids?
  
  Perhaps not "instead" but in addition. The tool is mostly meant to be used by 
background services and scripts, so remembering should not be an issue but we 
can offer both. Will make a patch for the notifications dataengine.

REPOSITORY
  R126 KDE CLI Utilities

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

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

To: broulik, #plasma, mart
Cc: bshah, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

[Differential] [Commented On] D2687: [Icon Widget] Bring back properties dialog

2016-12-13 Thread mart (Marco Martin) 
mart added a comment.


  for me +1

REPOSITORY
  R120 Plasma Workspace

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

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

To: broulik, #plasma, dfaure
Cc: mart, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

[Differential] [Commented On] D3654: [TextField] Allow disabling reveal password button through KIOSK restriction

2016-12-13 Thread broulik (Kai Uwe Broulik) 
broulik added a comment.


  > could also be a property of an object that's inside instead of the root 
object
  
  I also need to access it from the style and didn't want to randomly do 
control.someRandomObject.foo

REPOSITORY
  R242 Plasma Frameworks

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

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

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

[Differential] [Updated, 594 lines] D2687: [Icon Widget] Bring back properties dialog

2016-12-13 Thread broulik (Kai Uwe Broulik) 
broulik updated this revision to Diff 8968.
broulik added a comment.


  - Create context menu actions on demand, saves some cycles on startup and 
more importantly allows us to follow SystemImmutable

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D2687?vs=8967&id=8968

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

AFFECTED FILES
  applets/icon/CMakeLists.txt
  applets/icon/iconapplet.cpp
  applets/icon/iconapplet.h
  applets/icon/package/contents/config/main.xml
  applets/icon/package/contents/ui/main.qml

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

To: broulik, #plasma, dfaure
Cc: mart, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

[Differential] [Commented On] D3658: Set Automatic login flag to adjust SDDM config directly

2016-12-13 Thread davidedmundson (David Edmundson) 
davidedmundson added a comment.


  Already exists in the sddm kcm.

REPOSITORY
  R128 User Manager

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

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

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

[Differential] [Request, 1 line] D3663: [Task Manager] Don't emit urls droppde on drag enter

2016-12-13 Thread broulik (Kai Uwe Broulik) 
broulik created this revision.
broulik added reviewers: Plasma, hein, davidedmundson.
broulik set the repository for this revision to R119 Plasma Desktop.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  Otherwise we'll unexpectedly create a launcher right away.

TEST PLAN
  Dragged an app from kickoff to task manager, no longer got a launcher created 
immediately.
  
  Regression introduced by 
https://phabricator.kde.org/R119:c50948648012e66b71b757e211432190d592c90d

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  applets/taskmanager/package/contents/ui/MouseHandler.qml

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

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

[Differential] [Updated] D3663: [Task Manager] Don't emit urls dropped on drag enter

2016-12-13 Thread broulik (Kai Uwe Broulik) 
broulik retitled this revision from "[Task Manager] Don't emit urls droppde on 
drag enter" to "[Task Manager] Don't emit urls dropped on drag enter".

REPOSITORY
  R119 Plasma Desktop

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

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

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

[Differential] [Accepted] D3663: [Task Manager] Don't emit urls dropped on drag enter

2016-12-13 Thread davidedmundson (David Edmundson) 
davidedmundson accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R119 Plasma Desktop

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

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

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

[Differential] [Closed] D3663: [Task Manager] Don't emit urls dropped on drag enter

2016-12-13 Thread broulik (Kai Uwe Broulik) 
This revision was automatically updated to reflect the committed changes.
Closed by commit R119:643e424d7f1e: [Task Manager] Don't emit urls dropped on 
drag enter (authored by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3663?vs=8969&id=8970

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

AFFECTED FILES
  applets/taskmanager/package/contents/ui/MouseHandler.qml

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

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

Re: Review Request 129528: Fix "Unable to assign [undefined] to int" log

2016-12-13 Thread Maximiliano Curia 


> On Nov. 22, 2016, 11:05 a.m., David Edmundson wrote:
> > lookandfeel/contents/components/Battery.qml, line 48
> > 
> >
> > Given we don't actually care about the value used as long as it's an 
> > int as this will be hidden anyway, this can be just:
> > 
> > percent: pmSource.data["Battery"]["Percent"] || 0
> > 
> > without a function.
> 
> Maximiliano Curia wrote:
> I've changed this and the "Has Battery" (which I'm not sure if it can 
> ever be undefined, but it surely isn't always true). But, I won't be able to 
> test this right now.

Tested, the second version works fine.


- Maximiliano


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


On Nov. 22, 2016, 1:04 p.m., Maximiliano Curia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129528/
> ---
> 
> (Updated Nov. 22, 2016, 1:04 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> The sddm breeze theme battery icon is trying to access the battery
> percent. In a batteryless system it produces the syslog message:
> Nov 14 22:28:23 samd sddm-greeter[3210]: 
> file:///usr/share/sddm/themes/breeze/components/Battery.qml:39:18: Unable to 
> assign [undefined] to int
> 
> Based on https://gist.github.com/Zren/4e5709d842965227088f6e1d3fd42016
> 
> Debian-Bug: https://bugs.debian.org/844194
> 
> 
> Diffs
> -
> 
>   lookandfeel/contents/components/Battery.qml 
> c4a94ebb6e3ca2f7b46e768f2c9bde22c9d6bbdf 
> 
> Diff: https://git.reviewboard.kde.org/r/129528/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maximiliano Curia
> 
>

[Differential] [Commented On] D3660: Plasma 5.9 "Canopee" Wallpaper

2016-12-13 Thread kvermette (Ken Vermette) 
kvermette added a comment.


  In https://phabricator.kde.org/D3660#68450, @broulik wrote:
  
  > Overall I quite like it, especially the stylized 9 referencing Plasma 5.9 
(not sure if that was fully intentional, though? ;)
  >
  > The blurry jaggy, coronal mass ejection like edges of the yellow band, look 
weird.
  
  
  Kai!!! You were always the champion of my weird artistic fetishes! What 
happened to the days of the intel-glitch-bar? :P
  
  In all seriousness, you're the second person to mention it in a negative way, 
with Bhushan. Out of a sample of, like, 8 people that's too high a frequency to 
think it won't be called out en-masse. So, unless everyone runs in screaming to 
bring it back, the crackle is gone. Attached is the modified version. :)
  
  F708879: nocrackle-2560-1600.png 

REPOSITORY
  R119 Plasma Desktop

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

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

To: kvermette, #plasma
Cc: mart, broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas

[Differential] [Commented On] D3655: Kicker backend changes for Simple Menu.

2016-12-13 Thread hein (Eike Hein) 
hein added a comment.


  Marco: Yeah I agree it's fugly. The constructor was originally added for 
atomicity and then it grew ove time. Let's refactor separately from this maybe 
though?

INLINE COMMENTS

> broulik wrote in wheelinterceptor.cpp:52
> Is this for another patch?
> 
> Also, you lose pixel-precise scrolling here

Forgot to mention this one in the description. It's used by the Simple Menu UI 
code to do page flips (hence pixel-precision is also not needed).

REPOSITORY
  R119 Plasma Desktop

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

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

To: hein, #plasma, mart
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

Re: Minutes Monday Plasma Meeting

2016-12-13 Thread Marco Martin 
On Mon, Dec 12, 2016 at 12:40 PM, Sebastian Kügler  wrote:
> Plasma Meeting minutes 12-12-2016
>
> Present: Sho, kbroulik, romangg, bshah, notmart, d_ed, sebas
>
> Logo discussion:
> - We want to finalize this topic (finally), notmart to check with mgraesslin


He is ok, and yes, we should go ahead with this asap

--
Marco Martin

[Differential] [Accepted] D3655: Kicker backend changes for Simple Menu.

2016-12-13 Thread mart (Marco Martin) 
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

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

To: hein, #plasma, mart
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Closed] D3655: Kicker backend changes for Simple Menu.

2016-12-13 Thread hein (Eike Hein) 
This revision was automatically updated to reflect the committed changes.
Closed by commit R119:3e88ac63f25d: Kicker backend changes for Simple Menu. 
(authored by hein).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3655?vs=8955&id=8972

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

AFFECTED FILES
  applets/kicker/package/contents/ui/main.qml
  applets/kicker/plugin/appentry.cpp
  applets/kicker/plugin/appentry.h
  applets/kicker/plugin/appsmodel.cpp
  applets/kicker/plugin/appsmodel.h
  applets/kicker/plugin/rootmodel.cpp
  applets/kicker/plugin/rootmodel.h
  applets/kicker/plugin/wheelinterceptor.cpp
  applets/kicker/plugin/wheelinterceptor.h

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

To: hein, #plasma, mart
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Request, 2 lines] D3666: Test the variable type without warning

2016-12-13 Thread apol (Aleix Pol Gonzalez) 
apol created this revision.
apol added reviewers: Kirigami, mart.
Restricted Application added a project: Kirigami.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  type===undefined also raises a warning which we don't desire, use typeof()
  instead, which doesn't.

REPOSITORY
  R169 Kirigami

BRANCH
  master

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

AFFECTED FILES
  src/controls/private/DefaultListItemBackground.qml

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

To: apol, #kirigami, mart
Cc: plasma-devel, apol

[Differential] [Accepted] D3666: Test the variable type without warning

2016-12-13 Thread mart (Marco Martin) 
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R169 Kirigami

BRANCH
  master

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

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

To: apol, #kirigami, mart
Cc: plasma-devel, apol

[Differential] [Request, 20 lines] D3667: [MPRIS Dataengine] Let clients distinguish media players by process id more easily

2016-12-13 Thread subdiff (Roman Gilg) 
subdiff created this revision.
subdiff added a reviewer: Plasma.
subdiff added a subscriber: plasma-devel.
subdiff set the repository for this revision to R120 Plasma Workspace.
subdiff added a project: Plasma.

REVISION SUMMARY
  The MPRIS specification recommends to media players to distinguish instances 
of them by appending its process id. For example this is supported by the 
Dragon Player. The patch makes it possible for connected clients of MPRIS 
Dataengine to query this directly and by that distinguish multiple instances of 
a player.
  
  While clients were able to do this earlier already by checking the source 
name and doing the same string operations as here, it's more convenient with 
this patch, since they then don't have to do it on their own anymore but just 
need to query the data.

TEST PLAN
  Tested it with my prototype of the redesigned taskbar.

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  dataengines/mpris2/playercontainer.cpp

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

To: subdiff, #plasma
Cc: plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

[Differential] [Commented On] D3667: [MPRIS Dataengine] Let clients distinguish media players by process id more easily

2016-12-13 Thread broulik (Kai Uwe Broulik) 
broulik added a comment.


  LGTM overall
  
  "Prototype of redesigned taskbar" made me curious ;)

INLINE COMMENTS

> playercontainer.cpp:123
> +int secondaryInstancePid = -1;
> +int lastPoint = m_dbusAddress.lastIndexOf(".");
> +QString end = m_dbusAddress.right(m_dbusAddress.length() - (lastPoint + 
> 1));

QLatin1Char?

> playercontainer.cpp:124
> +int lastPoint = m_dbusAddress.lastIndexOf(".");
> +QString end = m_dbusAddress.right(m_dbusAddress.length() - (lastPoint + 
> 1));
> +const QString pre = "instance";

Can you use rightRef and the like, if possible?

> playercontainer.cpp:128
> +end.remove(0, pre.length());
> +bool convertSuccess;
> +int i = end.toInt(&convertSuccess);

bool ok;

is sufficient in that context imho

REPOSITORY
  R120 Plasma Workspace

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

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

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

[Differential] [Changed Subscribers] D3667: [MPRIS Dataengine] Let clients distinguish media players by process id more easily

2016-12-13 Thread davidedmundson (David Edmundson) 
davidedmundson added inline comments.

INLINE COMMENTS

> playercontainer.cpp:119
> +// at the end of its dbus address. This code bit sets 
> "SecondaryInstancePid" to
> +//  in this case, otherwise it defaults to -1 (for the first one or 
> players
> +// which don't implement the standard correctly (looking at you VLC 
> [v2.2.2]!)

There is a way to do it which will work for all players much more reliably:

  auto message = QDBusMessage::createMethodCall("org.freedesktop.DBus", "/", 
"org.freedesktop.DBus", "GetConnectionUnixProcessID");
  message.setArguments({m_dbusAddress});
  
  QDBusReply reply = QDBusConnection::sessionBus().call(message); 

//safe to be blocking as we're calling a method on DBus server not a client, so 
no different from QDBusConnection::connect()

   if (!reply.isError) {
 secondaryInstancePid = reply.value();
  }

REPOSITORY
  R120 Plasma Workspace

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

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

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

[Differential] [Commented On] D3667: [MPRIS Dataengine] Let clients distinguish media players by process id more easily

2016-12-13 Thread davidedmundson (David Edmundson) 
davidedmundson added inline comments.

INLINE COMMENTS

> davidedmundson wrote in playercontainer.cpp:119
> There is a way to do it which will work for all players much more reliably:
> 
>   auto message = QDBusMessage::createMethodCall("org.freedesktop.DBus", "/", 
> "org.freedesktop.DBus", "GetConnectionUnixProcessID");
>   message.setArguments({m_dbusAddress});
>   
>   QDBusReply reply = QDBusConnection::sessionBus().call(message); 
> 
> //safe to be blocking as we're calling a method on DBus server not a client, 
> so no different from QDBusConnection::connect()
> 
>if (!reply.isError) {
>  secondaryInstancePid = reply.value();
>   }

Ninja edit.

If you do go with that approach don't use the code above.

Use 
secondaryInstancePid = 
QDBusConnectionInterface::servicePid(m_dbusAddress).value();

REPOSITORY
  R120 Plasma Workspace

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

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

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

Re: Review Request 128038: [libtaskmanager] Stop highlighted window effect in group item

2016-12-13 Thread Anthony Fieroni 


> On Дек. 7, 2016, 3:59 след обяд, Anthony Fieroni wrote:
> > You not against i plan to ship this patch next week.

Ping, Eike, it works fine.


- Anthony


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


On Дек. 4, 2016, 1:07 след обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128038/
> ---
> 
> (Updated Дек. 4, 2016, 1:07 след обяд)
> 
> 
> Review request for Plasma, Kai Uwe Broulik, David Edmundson, and Eike Hein.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> 1. Enable highlighted window, tooltips and grouping
> 2. Open group item tasl by clicking left mouse button
> 3. Move mouse cursor over item in group
> 
> Before:
> 1. Items are samll and tough readable
> 2. Items aren't visible cause a highligthWindow effect
> 3. Clicking on tooltip close button case highlightWindow effect to not stop
> 
> After:
> 1. Items are proof readable
> 2. Items are visible
> 3. HighlightWdindow effect is stopped
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/GroupDialog.qml 829fcf0 
>   applets/taskmanager/package/contents/ui/Task.qml 0a59d53 
>   applets/taskmanager/package/contents/ui/ToolTipWindowMouseArea.qml e3551df 
>   applets/taskmanager/package/contents/ui/main.qml e53b194 
>   applets/taskmanager/plugin/backend.h a3788b0 
>   applets/taskmanager/plugin/backend.cpp 4ef5b88 
> 
> Diff: https://git.reviewboard.kde.org/r/128038/diff/
> 
> 
> Testing
> ---
> 
> Screenshot with expected behavior by clicking left button on group item.
> 
> 
> File Attachments
> 
> 
> Vertical panel
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/12/03/35dd438c-fae2-4daa-ba30-09f70ea3b920__Screenshot_20161203_210914.png
> Horizontal panel
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/12/03/fe7c9cab-bb2a-4d17-85de-1934c81a33dc__Screenshot_20161203_211120.png
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Review Request 129652: update network, smb, thumbnail ioslave docbooks

2016-12-13 Thread Burkhard Lück 

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

Review request for Documentation and Plasma.


Repository: kio-extras


Description
---

bump date + releaseinfo (do we need releaseinfo here?)
network: use double slash (//)
smb: fix systemsettings modul, remove para about really old samba version
thumbnail: add dolphin, fix link to sources


Diffs
-

  doc/kioslave/network/index.docbook 697d366 
  doc/kioslave/smb/index.docbook aef3adb 
  doc/kioslave/thumbnail/index.docbook 334bf6d 

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


Testing
---

checkXML5 index.docbook


Thanks,

Burkhard Lück