Re: Review Request 123535: Visible error messagebox on fatal loading errors

2015-04-28 Thread David Edmundson

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

(Updated April 28, 2015, 7:56 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit 9e5f357a89273b39c9897fa6bcbb211a746f4aca by David 
Edmundson to branch master.


Repository: plasma-workspace


Description
---

BUG: 346792


Diffs
-

  shell/shellmanager.cpp c6137ad63346c306adeab8d15fb6914ef2533aa8 

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


Testing
---


Thanks,

David Edmundson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 123540: Don't trigger updates when no actual updates happened + never modify the KNotification object from the popup plugin

2015-04-28 Thread Martin Klapetek

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

Review request for KDE Frameworks and Plasma.


Bugs: 345973
https://bugs.kde.org/show_bug.cgi?id=345973


Repository: knotifications


Description
---

This patch checks if the properties being set are actually changed before 
emitting the update signal, which would cause a re-emit of the notification.

Furthermore, NotifyByPopup now no longer changes the KNotification object when 
the server does not support certain capabilities but only does local checking 
and modifications.

This fixes KNotification use with Ubuntu's NotifyOSD.


Diffs
-

  src/knotification.cpp afac82f 
  src/notifybypopup.cpp 316ff2b 

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


Testing
---

Before this patch there was a continous loop of setting empty actions because 
the server does not support them -> triggers update -> triggers setting empty 
actions -> triggers update -> and so on. This made the notification popup look 
stuck and would block all other notifications from appearing (NotifyOSD 
supports only one popup at a time).

Now everything behaves correctly.


Thanks,

Martin Klapetek

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123540: Don't trigger updates when no actual updates happened + never modify the KNotification object from the popup plugin

2015-04-28 Thread David Edmundson

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

Ship it!


Ship It!

- David Edmundson


On April 28, 2015, 8:33 a.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123540/
> ---
> 
> (Updated April 28, 2015, 8:33 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Bugs: 345973
> https://bugs.kde.org/show_bug.cgi?id=345973
> 
> 
> Repository: knotifications
> 
> 
> Description
> ---
> 
> This patch checks if the properties being set are actually changed before 
> emitting the update signal, which would cause a re-emit of the notification.
> 
> Furthermore, NotifyByPopup now no longer changes the KNotification object 
> when the server does not support certain capabilities but only does local 
> checking and modifications.
> 
> This fixes KNotification use with Ubuntu's NotifyOSD.
> 
> 
> Diffs
> -
> 
>   src/knotification.cpp afac82f 
>   src/notifybypopup.cpp 316ff2b 
> 
> Diff: https://git.reviewboard.kde.org/r/123540/diff/
> 
> 
> Testing
> ---
> 
> Before this patch there was a continous loop of setting empty actions because 
> the server does not support them -> triggers update -> triggers setting empty 
> actions -> triggers update -> and so on. This made the notification popup 
> look stuck and would block all other notifications from appearing (NotifyOSD 
> supports only one popup at a time).
> 
> Now everything behaves correctly.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123540: Don't trigger updates when no actual updates happened + never modify the KNotification object from the popup plugin

2015-04-28 Thread Martin Klapetek

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

(Updated April 28, 2015, 9:48 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 2a8002d00713a7768eae3fde215efeadfe1aaa8d by Martin 
Klapetek to branch master.


Bugs: 345973
https://bugs.kde.org/show_bug.cgi?id=345973


Repository: knotifications


Description
---

This patch checks if the properties being set are actually changed before 
emitting the update signal, which would cause a re-emit of the notification.

Furthermore, NotifyByPopup now no longer changes the KNotification object when 
the server does not support certain capabilities but only does local checking 
and modifications.

This fixes KNotification use with Ubuntu's NotifyOSD.


Diffs
-

  src/knotification.cpp afac82f 
  src/notifybypopup.cpp 316ff2b 

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


Testing
---

Before this patch there was a continous loop of setting empty actions because 
the server does not support them -> triggers update -> triggers setting empty 
actions -> triggers update -> and so on. This made the notification popup look 
stuck and would block all other notifications from appearing (NotifyOSD 
supports only one popup at a time).

Now everything behaves correctly.


Thanks,

Martin Klapetek

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123473: Port mouse theme kcm to QML

2015-04-28 Thread Christoph Feck

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


We get many crashes since porting KCM modules to QML, see bug 346634, bug 
345882, bug 344651, bug 345675 and duplicates.

- Christoph Feck


On April 27, 2015, 4:57 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123473/
> ---
> 
> (Updated April 27, 2015, 4:57 p.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This is more an experiment on how much modules can be closely ported (and in 
> how much time).
> the mouse theme kcm should be pretty much feature complete.
> the main problem is the size combobox missing the cursor image due to the 
> QtQuickControls ComboBox being very limited and without a customizable 
> delegate.
> all the other functions such as add/remove/ghns seems to work well
> 
> 
> Diffs
> -
> 
>   applets/icontasks/metadata.desktop f0b237c 
>   containments/folder/metadata.desktop a6d08a7 
>   kcms/access/kcmaccess.desktop 825b6d7 
>   kcms/baloo/kcm_baloofile.desktop 2eee6fc 
>   kcms/cursortheme/CMakeLists.txt 83f3ba2 
>   kcms/cursortheme/Messages.sh 79450c7 
>   kcms/cursortheme/cursortheme.desktop f443208 
>   kcms/cursortheme/kcm_cursortheme.desktop PRE-CREATION 
>   kcms/cursortheme/kcmcursortheme.h d9e32b2 
>   kcms/cursortheme/kcmcursortheme.cpp 44576ff 
>   kcms/cursortheme/package/contents/ui/Delegate.qml PRE-CREATION 
>   kcms/cursortheme/package/contents/ui/main.qml PRE-CREATION 
>   kcms/cursortheme/package/metadata.desktop PRE-CREATION 
>   kcms/cursortheme/xcursor/itemdelegate.h 9acb0e9 
>   kcms/cursortheme/xcursor/itemdelegate.cpp e737005 
>   kcms/cursortheme/xcursor/previewwidget.h 4a11e2d 
>   kcms/cursortheme/xcursor/previewwidget.cpp 79d1305 
>   kcms/cursortheme/xcursor/sortproxymodel.h 95c9646 
>   kcms/cursortheme/xcursor/sortproxymodel.cpp b9d6309 
>   kcms/cursortheme/xcursor/thememodel.h bcf046a 
>   kcms/cursortheme/xcursor/thememodel.cpp 4e4647f 
>   kcms/cursortheme/xcursor/themepage.h 98c69fd 
>   kcms/cursortheme/xcursor/themepage.cpp 687bd65 
>   kcms/cursortheme/xcursor/themepage.ui 6efe60b 
>   kcms/desktoppaths/desktoppath.desktop eb2fad5 
>   kcms/lookandfeel/autotests/lookandfeel/metadata.desktop 3360a85 
>   kcms/lookandfeel/kcm_lookandfeel.desktop 8550e5c 
>   kcms/lookandfeel/package/metadata.desktop 6595d6e 
>   kcms/touchpad/src/applet/qml/metadata.desktop e9a0bc1 
>   kcms/touchpad/src/kcm/kcm_touchpad.desktop c537e5f 
>   kcms/touchpad/src/kded/kcm_touchpad.notifyrc 9e51e0e 
>   kcms/touchpad/src/kded/kded_touchpad.desktop ec076a9 
>   kcms/useraccount/kcm_useraccount.desktop 46ef110 
>   layout-templates/org.kde.plasma.desktop.defaultPanel/metadata.desktop 
> 89d7fc3 
> 
> Diff: https://git.reviewboard.kde.org/r/123473/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> cursorskcm.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/04/23/72f14417-e14c-4385-9e8e-959dd1f2d8e4__cursorskcm.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Plasma 5.3 is out

2015-04-28 Thread Jonathan Riddell
https://www.kde.org/announcements/plasma-5.3.0.php

party time
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123530: Improve interactivity for the user icon and search field in kickoff

2015-04-28 Thread Sebastian Kügler


> On April 27, 2015, 2:56 p.m., David Edmundson wrote:
> > Personally I don't think anyone needs to open the accounts KCM so often we 
> > need to link to it. I suspect it'll get more accidental opens than legit 
> > ones.
> > 
> > The other changes seems good though.
> 
> Lukáš Tinkl wrote:
> I for one would expect it to be interactive, and it closely matches what 
> Windows (and Gnome Shell too afaik) does.
> 
> Thomas Pfeiffer wrote:
> By "User Account kcm" you mean the "Password and User Account" kcm, 
> right? I think that's okay, as it lets you edit the information that is shown 
> there. If we get lots of bug reports form people accidentally clicking it, we 
> might reconsider.
> 
> Lukáš Tinkl wrote:
> Yup exactly, it lets you edit what is being shown there - the user name 
> and your face/icon.

I must say that I quite like this idea, too. I've seen too many reviews where 
the user didn't bother to change the face icon in Kickoff, while it looks 
really cool and is a nice personalization point.


- Sebastian


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


On April 27, 2015, 5:04 p.m., Lukáš Tinkl wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123530/
> ---
> 
> (Updated April 27, 2015, 5:04 p.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This patch makes the user icon and search field clickable. Upon clicking the 
> former, the User Account kcm is opened, in the latter case the search field 
> gets visible and activated.
> 
> 
> Diffs
> -
> 
>   applets/kickoff/package/contents/ui/FullRepresentation.qml 15dde96 
>   applets/kickoff/package/contents/ui/Header.qml dfabbe5 
> 
> Diff: https://git.reviewboard.kde.org/r/123530/diff/
> 
> 
> Testing
> ---
> 
> Everything works as expected, pressing Esc either cancels the search and/or 
> closes the menu popup.
> 
> 
> Thanks,
> 
> Lukáš Tinkl
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123473: Port mouse theme kcm to QML

2015-04-28 Thread Marco Martin

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

(Updated April 28, 2015, 1:16 p.m.)


Review request for Plasma and KDE Usability.


Changes
---

this latest version should be ok enough, tested cursor change, 
download/install/uninstall, all seems to work well


Repository: plasma-desktop


Description
---

This is more an experiment on how much modules can be closely ported (and in 
how much time).
the mouse theme kcm should be pretty much feature complete.
the main problem is the size combobox missing the cursor image due to the 
QtQuickControls ComboBox being very limited and without a customizable delegate.
all the other functions such as add/remove/ghns seems to work well


Diffs (updated)
-

  applets/icontasks/metadata.desktop f0b237c 
  containments/folder/metadata.desktop a6d08a7 
  kcms/access/kcmaccess.desktop 825b6d7 
  kcms/baloo/kcm_baloofile.desktop 2eee6fc 
  kcms/cursortheme/CMakeLists.txt 83f3ba2 
  kcms/cursortheme/Messages.sh 79450c7 
  kcms/cursortheme/cursortheme.desktop f443208 
  kcms/cursortheme/kcm_cursortheme.desktop PRE-CREATION 
  kcms/cursortheme/kcmcursortheme.h d9e32b2 
  kcms/cursortheme/kcmcursortheme.cpp 44576ff 
  kcms/cursortheme/package/contents/ui/Delegate.qml PRE-CREATION 
  kcms/cursortheme/package/contents/ui/main.qml PRE-CREATION 
  kcms/cursortheme/package/metadata.desktop PRE-CREATION 
  kcms/cursortheme/xcursor/itemdelegate.h 9acb0e9 
  kcms/cursortheme/xcursor/itemdelegate.cpp e737005 
  kcms/cursortheme/xcursor/previewwidget.h 4a11e2d 
  kcms/cursortheme/xcursor/previewwidget.cpp 79d1305 
  kcms/cursortheme/xcursor/sortproxymodel.h 95c9646 
  kcms/cursortheme/xcursor/sortproxymodel.cpp b9d6309 
  kcms/cursortheme/xcursor/thememodel.h bcf046a 
  kcms/cursortheme/xcursor/thememodel.cpp 4e4647f 
  kcms/cursortheme/xcursor/themepage.h 98c69fd 
  kcms/cursortheme/xcursor/themepage.cpp 687bd65 
  kcms/cursortheme/xcursor/themepage.ui 6efe60b 
  kcms/desktoppaths/desktoppath.desktop eb2fad5 
  kcms/lookandfeel/autotests/lookandfeel/metadata.desktop 3360a85 
  kcms/lookandfeel/kcm_lookandfeel.desktop 8550e5c 
  kcms/lookandfeel/package/metadata.desktop 6595d6e 
  kcms/touchpad/src/applet/qml/metadata.desktop e9a0bc1 
  kcms/touchpad/src/kcm/kcm_touchpad.desktop c537e5f 
  kcms/touchpad/src/kded/kcm_touchpad.notifyrc 9e51e0e 
  kcms/touchpad/src/kded/kded_touchpad.desktop ec076a9 
  kcms/useraccount/kcm_useraccount.desktop 46ef110 
  layout-templates/org.kde.plasma.desktop.defaultPanel/metadata.desktop 89d7fc3 

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


Testing
---


File Attachments


cursorskcm.png
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/04/23/72f14417-e14c-4385-9e8e-959dd1f2d8e4__cursorskcm.png


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123530: Improve interactivity for the user icon and search field in kickoff

2015-04-28 Thread David Edmundson

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

Ship it!


Ship It!

- David Edmundson


On April 27, 2015, 5:04 p.m., Lukáš Tinkl wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123530/
> ---
> 
> (Updated April 27, 2015, 5:04 p.m.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This patch makes the user icon and search field clickable. Upon clicking the 
> former, the User Account kcm is opened, in the latter case the search field 
> gets visible and activated.
> 
> 
> Diffs
> -
> 
>   applets/kickoff/package/contents/ui/FullRepresentation.qml 15dde96 
>   applets/kickoff/package/contents/ui/Header.qml dfabbe5 
> 
> Diff: https://git.reviewboard.kde.org/r/123530/diff/
> 
> 
> Testing
> ---
> 
> Everything works as expected, pressing Esc either cancels the search and/or 
> closes the menu popup.
> 
> 
> Thanks,
> 
> Lukáš Tinkl
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123530: Improve interactivity for the user icon and search field in kickoff

2015-04-28 Thread Lukáš Tinkl

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

(Updated April 28, 2015, 1:36 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and KDE Usability.


Changes
---

Submitted with commit bae60a355524ca6b88d87aed28d7ecb002369777 by Lukáš Tinkl 
to branch master.


Repository: plasma-desktop


Description
---

This patch makes the user icon and search field clickable. Upon clicking the 
former, the User Account kcm is opened, in the latter case the search field 
gets visible and activated.


Diffs
-

  applets/kickoff/package/contents/ui/FullRepresentation.qml 15dde96 
  applets/kickoff/package/contents/ui/Header.qml dfabbe5 

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


Testing
---

Everything works as expected, pressing Esc either cancels the search and/or 
closes the menu popup.


Thanks,

Lukáš Tinkl

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123529: update kmenuedit docbook and screenshots to plasma 5.3

2015-04-28 Thread Sebastian Kügler

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



doc/index.docbook (line 182)


~/.local is the XDG default, ~/.local5 was what we used during KF5 
development. Ultimately, it dependso on what the user set.

Same for the following lines.


Env vars seem to refer to a custom set XDG path, otherwise it looks good to me.

- Sebastian Kügler


On April 27, 2015, 1:14 p.m., Burkhard Lück wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123529/
> ---
> 
> (Updated April 27, 2015, 1:14 p.m.)
> 
> 
> Review request for Documentation and Plasma.
> 
> 
> Repository: kmenuedit
> 
> 
> Description
> ---
> 
> see summary
> 
> 
> Diffs
> -
> 
>   doc/done.png 202b4e0 
>   doc/index.docbook ebc1e9d 
>   doc/selectinternet.png deeb67c 
> 
> Diff: https://git.reviewboard.kde.org/r/123529/diff/
> 
> 
> Testing
> ---
> 
> builds
> 
> 
> Thanks,
> 
> Burkhard Lück
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123539: Improve visibility of running widget checkmark

2015-04-28 Thread Marco Martin

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

Ship it!


Ship It!

- Marco Martin


On April 27, 2015, 9:35 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123539/
> ---
> 
> (Updated April 27, 2015, 9:35 p.m.)
> 
> 
> Review request for Plasma, KDE Usability and Andrew Lake.
> 
> 
> Bugs: 342112
> https://bugs.kde.org/show_bug.cgi?id=342112
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> As suggested by Andrew show a badge kind of thing with the number of running 
> applets inside.
> 
> 
> Diffs
> -
> 
>   desktoppackage/contents/explorer/AppletDelegate.qml 2e53c1e 
>   desktoppackage/contents/explorer/WidgetExplorer.qml f164c18 
> 
> Diff: https://git.reviewboard.kde.org/r/123539/diff/
> 
> 
> Testing
> ---
> 
> After fixing a nasty bug in the model it shows the proper number now. It also 
> doesn't break if you happen to have 10 or more widgets of the same kind. I 
> did not know what kind of font color to use.
> 
> 
> File Attachments
> 
> 
> Badge in action
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/04/27/1b8483bb-fe3b-4701-92d3-02b437e63843__widgetexplorernumber2.jpg
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123539: Improve visibility of running widget checkmark

2015-04-28 Thread Andrew Lake

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

Ship it!


Ship It!

- Andrew Lake


On April 27, 2015, 9:35 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123539/
> ---
> 
> (Updated April 27, 2015, 9:35 p.m.)
> 
> 
> Review request for Plasma, KDE Usability and Andrew Lake.
> 
> 
> Bugs: 342112
> https://bugs.kde.org/show_bug.cgi?id=342112
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> As suggested by Andrew show a badge kind of thing with the number of running 
> applets inside.
> 
> 
> Diffs
> -
> 
>   desktoppackage/contents/explorer/AppletDelegate.qml 2e53c1e 
>   desktoppackage/contents/explorer/WidgetExplorer.qml f164c18 
> 
> Diff: https://git.reviewboard.kde.org/r/123539/diff/
> 
> 
> Testing
> ---
> 
> After fixing a nasty bug in the model it shows the proper number now. It also 
> doesn't break if you happen to have 10 or more widgets of the same kind. I 
> did not know what kind of font color to use.
> 
> 
> File Attachments
> 
> 
> Badge in action
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/04/27/1b8483bb-fe3b-4701-92d3-02b437e63843__widgetexplorernumber2.jpg
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123224: KIO::suggestName suggests wrong name for some filenames

2015-04-28 Thread David Faure

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



src/core/global.cpp (line 396)


startsWith('.')  (using the QChar overload)

Do we even need this special case, with the way the code is now? It seems 
to me that removing this first if() branch would work just the same.



src/core/global.cpp (line 398)


oldName.mid(1), faster (and more readable) than section with an empty 
separator.



src/core/global.cpp (line 399)


isEmpty() rather than isNull(), not point in being specific about the 
difference between the two.



src/core/global.cpp (line 402)


Faster: nameSuffix.prepend('.')  (using the QChar overload).



src/core/global.cpp (line 403)


oldName.left(...) or .mid(0, ...)
... I'm not even sure what section(empty string, ...) really does :-)



src/core/global.cpp (line 414)


does not exists -> does not exist



src/core/global.cpp (line 415)


basename +=



src/core/global.cpp (line 417)


QString suggestedName = 

(it's not used before this line, so it should be declared here; could even 
be "const QString suggestedName = ..." since it's not modified later)


- David Faure


On April 26, 2015, 12:19 p.m., Ashish Bansal wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123224/
> ---
> 
> (Updated April 26, 2015, 12:19 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Aleix Pol Gonzalez.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> For filenames like filename-1.6.tar.gz, KIO::suggestName suggests wrong 
> name(something like filename-1 2.6.tar.gz).
> Expected name: filename-1.6 (1).tar.gz
> 
> 
> Diffs
> -
> 
>   autotests/globaltest.cpp ff8725d 
>   src/core/global.cpp f18ac10 
> 
> Diff: https://git.reviewboard.kde.org/r/123224/diff/
> 
> 
> Testing
> ---
> 
> Works fine!
> 
> 
> Thanks,
> 
> Ashish Bansal
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123224: KIO::suggestName suggests wrong name for some filenames

2015-04-28 Thread Ashish Bansal

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

(Updated April 28, 2015, 2:38 p.m.)


Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Arjun AK.


Changes
---

(David) (but Arjun's patch was then reverted) -- also add bug number.


Bugs: 341773
https://bugs.kde.org/show_bug.cgi?id=341773


Repository: kio


Description
---

For filenames like filename-1.6.tar.gz, KIO::suggestName suggests wrong 
name(something like filename-1 2.6.tar.gz).
Expected name: filename-1.6 (1).tar.gz


Diffs
-

  autotests/globaltest.cpp ff8725d 
  src/core/global.cpp f18ac10 

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


Testing
---

Works fine!


Thanks,

Ashish Bansal

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123224: KIO::suggestName suggests wrong name for some filenames

2015-04-28 Thread Aleix Pol Gonzalez

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



src/core/global.cpp (line 396)


you can change that for:
if (oldName.lastIndexOf('.') == 0)



src/core/global.cpp (line 412)


Also char overload.


- Aleix Pol Gonzalez


On April 28, 2015, 4:38 p.m., Ashish Bansal wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123224/
> ---
> 
> (Updated April 28, 2015, 4:38 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Arjun AK.
> 
> 
> Bugs: 341773
> https://bugs.kde.org/show_bug.cgi?id=341773
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> For filenames like filename-1.6.tar.gz, KIO::suggestName suggests wrong 
> name(something like filename-1 2.6.tar.gz).
> Expected name: filename-1.6 (1).tar.gz
> 
> 
> Diffs
> -
> 
>   autotests/globaltest.cpp ff8725d 
>   src/core/global.cpp f18ac10 
> 
> Diff: https://git.reviewboard.kde.org/r/123224/diff/
> 
> 
> Testing
> ---
> 
> Works fine!
> 
> 
> Thanks,
> 
> Ashish Bansal
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123539: Improve visibility of running widget checkmark

2015-04-28 Thread Heiko Tietze


> On April 27, 2015, 10:02 nachm., andreas kainz wrote:
> > colors maybe green and white as used in the mount symbol

Graphics should be theme aware. Please make sure badges are readable with 
Breeze Dark or Wonton Soup.


- Heiko


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


On April 27, 2015, 9:35 nachm., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123539/
> ---
> 
> (Updated April 27, 2015, 9:35 nachm.)
> 
> 
> Review request for Plasma, KDE Usability and Andrew Lake.
> 
> 
> Bugs: 342112
> https://bugs.kde.org/show_bug.cgi?id=342112
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> As suggested by Andrew show a badge kind of thing with the number of running 
> applets inside.
> 
> 
> Diffs
> -
> 
>   desktoppackage/contents/explorer/AppletDelegate.qml 2e53c1e 
>   desktoppackage/contents/explorer/WidgetExplorer.qml f164c18 
> 
> Diff: https://git.reviewboard.kde.org/r/123539/diff/
> 
> 
> Testing
> ---
> 
> After fixing a nasty bug in the model it shows the proper number now. It also 
> doesn't break if you happen to have 10 or more widgets of the same kind. I 
> did not know what kind of font color to use.
> 
> 
> File Attachments
> 
> 
> Badge in action
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/04/27/1b8483bb-fe3b-4701-92d3-02b437e63843__widgetexplorernumber2.jpg
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 123544: kioclient ls: properly construct QUrl.

2015-04-28 Thread Mark Gaiser

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

Review request for KDE Frameworks and Plasma.


Repository: kde-cli-tools


Description
---

The ls command was using direct user input as QUrl. That in turn caused the 
QUrl to be constructed without a scheme. Constructing it via makeURL(...) (like 
all the others do) makes it a valid QUrl with scheme. This allows for example a 
command like "kioclient ls ~" to work again. Again because it worked in the 
kdelibs version thus this was a regression.


Diffs
-

  kioclient/kioclient.cpp 74c9985 

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


Testing
---

kioclient ls  works. No need to prefix the path with "file://" anymore.


Thanks,

Mark Gaiser

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123539: Improve visibility of running widget checkmark

2015-04-28 Thread Kai Uwe Broulik

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

(Updated April 28, 2015, 6:34 nachm.)


Review request for Plasma, KDE Usability and Andrew Lake.


Changes
---

Eike came up with this idea:
- No inset gradient, just a solid Rectangle
- Inverted text color
- A section "cut off" from the icon

The seemingly random 1.5 is a compromise between too much of a glow and too big 
of a moat


Bugs: 342112
https://bugs.kde.org/show_bug.cgi?id=342112


Repository: plasma-desktop


Description
---

As suggested by Andrew show a badge kind of thing with the number of running 
applets inside.


Diffs (updated)
-

  desktoppackage/contents/explorer/AppletDelegate.qml 2e53c1e 

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


Testing
---

After fixing a nasty bug in the model it shows the proper number now. It also 
doesn't break if you happen to have 10 or more widgets of the same kind. I did 
not know what kind of font color to use.


File Attachments (updated)


Badge in action
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/04/27/1b8483bb-fe3b-4701-92d3-02b437e63843__widgetexplorernumber2.jpg
Fancy effect in action
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/04/28/289d6677-bb68-4b69-a43a-3becf226ad3b__badgeshader2.png


Thanks,

Kai Uwe Broulik

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123539: Improve visibility of running widget checkmark

2015-04-28 Thread David Edmundson

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


code looks good.


desktoppackage/contents/explorer/AppletDelegate.qml (line 101)


it's a bit odd that we have background colour on top of the text highlight 
colour.

Though AFAIk that combo is always garunteed to be visible.. so meh.



desktoppackage/contents/explorer/AppletDelegate.qml (line 119)


you can also add 

supportsAtlasTextures: true

as we're not tiling any of the sources.


- David Edmundson


On April 28, 2015, 6:34 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123539/
> ---
> 
> (Updated April 28, 2015, 6:34 p.m.)
> 
> 
> Review request for Plasma, KDE Usability and Andrew Lake.
> 
> 
> Bugs: 342112
> https://bugs.kde.org/show_bug.cgi?id=342112
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> As suggested by Andrew show a badge kind of thing with the number of running 
> applets inside.
> 
> 
> Diffs
> -
> 
>   desktoppackage/contents/explorer/AppletDelegate.qml 2e53c1e 
> 
> Diff: https://git.reviewboard.kde.org/r/123539/diff/
> 
> 
> Testing
> ---
> 
> After fixing a nasty bug in the model it shows the proper number now. It also 
> doesn't break if you happen to have 10 or more widgets of the same kind. I 
> did not know what kind of font color to use.
> 
> 
> File Attachments
> 
> 
> Badge in action
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/04/27/1b8483bb-fe3b-4701-92d3-02b437e63843__widgetexplorernumber2.jpg
> Fancy effect in action
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/04/28/289d6677-bb68-4b69-a43a-3becf226ad3b__badgeshader2.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123544: kioclient ls: properly construct QUrl.

2015-04-28 Thread Aleix Pol Gonzalez

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

Ship it!


Can you add add_definitions(-DQT_NO_URL_CAST_FROM_STRING)?

That will help spotting these issues.

- Aleix Pol Gonzalez


On April 28, 2015, 8:18 p.m., Mark Gaiser wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123544/
> ---
> 
> (Updated April 28, 2015, 8:18 p.m.)
> 
> 
> Review request for Plasma and David Faure.
> 
> 
> Repository: kde-cli-tools
> 
> 
> Description
> ---
> 
> The ls command was using direct user input as QUrl. That in turn caused the 
> QUrl to be constructed without a scheme. Constructing it via makeURL(...) 
> (like all the others do) makes it a valid QUrl with scheme. This allows for 
> example a command like "kioclient ls ~" to work again. Again because it 
> worked in the kdelibs version thus this was a regression.
> 
> 
> Diffs
> -
> 
>   kioclient/kioclient.cpp 74c9985 
> 
> Diff: https://git.reviewboard.kde.org/r/123544/diff/
> 
> 
> Testing
> ---
> 
> kioclient ls  works. No need to prefix the path with "file://" anymore.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123544: kioclient ls: properly construct QUrl.

2015-04-28 Thread Mark Gaiser


> On apr 28, 2015, 7:11 p.m., Aleix Pol Gonzalez wrote:
> > Can you add add_definitions(-DQT_NO_URL_CAST_FROM_STRING)?
> > 
> > That will help spotting these issues.

Ok, will add that and commit later tonight or tomorrow.


- Mark


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


On apr 28, 2015, 6:18 p.m., Mark Gaiser wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123544/
> ---
> 
> (Updated apr 28, 2015, 6:18 p.m.)
> 
> 
> Review request for Plasma and David Faure.
> 
> 
> Repository: kde-cli-tools
> 
> 
> Description
> ---
> 
> The ls command was using direct user input as QUrl. That in turn caused the 
> QUrl to be constructed without a scheme. Constructing it via makeURL(...) 
> (like all the others do) makes it a valid QUrl with scheme. This allows for 
> example a command like "kioclient ls ~" to work again. Again because it 
> worked in the kdelibs version thus this was a regression.
> 
> 
> Diffs
> -
> 
>   kioclient/kioclient.cpp 74c9985 
> 
> Diff: https://git.reviewboard.kde.org/r/123544/diff/
> 
> 
> Testing
> ---
> 
> kioclient ls  works. No need to prefix the path with "file://" anymore.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123529: update kmenuedit docbook and screenshots to plasma 5.3

2015-04-28 Thread Burkhard Lück

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

(Updated April 28, 2015, 7:26 nachm.)


Review request for Documentation and Plasma.


Changes
---

fix wrong xdg path


Repository: kmenuedit


Description
---

see summary


Diffs (updated)
-

  doc/done.png 202b4e0 
  doc/index.docbook ebc1e9d 
  doc/selectinternet.png deeb67c 

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


Testing
---

builds


Thanks,

Burkhard Lück

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 123547: Task manager : New instances can now be opened by mid click.

2015-04-28 Thread Yoann Laissus

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

Review request for Plasma.


Repository: plasma-desktop


Description
---

Currently, only the shortcut Shift + Left click is available.
In KDE4, we had both.


Diffs
-


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


Testing
---

Tested with classic task manager and the icon only one.
Tested with all mouse button combinaisons and keyboard modifiers.


Thanks,

Yoann Laissus

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123547: Task manager : New instances can now be opened by mid click.

2015-04-28 Thread Yoann Laissus

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

(Updated avr. 28, 2015, 8:45 après-midi)


Review request for Plasma.


Repository: plasma-desktop


Description
---

Currently, only the shortcut Shift + Left click is available.
In KDE4, we had both.


Diffs (updated)
-

  applets/taskmanager/package/contents/ui/Task.qml f5fc723 

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


Testing
---

Tested with classic task manager and the icon only one.
Tested with all mouse button combinaisons and keyboard modifiers.


Thanks,

Yoann Laissus

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123547: Task manager : New instances can now be opened by mid click.

2015-04-28 Thread David Edmundson

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



applets/taskmanager/package/contents/ui/Task.qml (line 108)


now shift + right click will also start a new instance, that's probably 
unintended?


- David Edmundson


On April 28, 2015, 8:45 p.m., Yoann Laissus wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123547/
> ---
> 
> (Updated April 28, 2015, 8:45 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Currently, only the shortcut Shift + Left click is available.
> In KDE4, we had both.
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/Task.qml f5fc723 
> 
> Diff: https://git.reviewboard.kde.org/r/123547/diff/
> 
> 
> Testing
> ---
> 
> Tested with classic task manager and the icon only one.
> Tested with all mouse button combinaisons and keyboard modifiers.
> 
> 
> Thanks,
> 
> Yoann Laissus
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123547: Task manager : New instances can now be opened by mid click.

2015-04-28 Thread Eike Hein

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


People  frequently request either "new instance" or "close window" for 
middle-click, so unfortunately I think this will have to be a config option and 
I won't accept a patch that only implements the one behavior.

- Eike Hein


On April 28, 2015, 8:45 p.m., Yoann Laissus wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123547/
> ---
> 
> (Updated April 28, 2015, 8:45 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Currently, only the shortcut Shift + Left click is available.
> In KDE4, we had both.
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/Task.qml f5fc723 
> 
> Diff: https://git.reviewboard.kde.org/r/123547/diff/
> 
> 
> Testing
> ---
> 
> Tested with classic task manager and the icon only one.
> Tested with all mouse button combinaisons and keyboard modifiers.
> 
> 
> Thanks,
> 
> Yoann Laissus
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123547: Task manager : New instances can now be opened by mid click.

2015-04-28 Thread Martin Gräßlin


> On April 29, 2015, 1:48 a.m., Eike Hein wrote:
> > People  frequently request either "new instance" or "close window" for 
> > middle-click, so unfortunately I think this will have to be a config option 
> > and I won't accept a patch that only implements the one behavior.

Even more I would expect that it copies the current copy buffer into a running 
instance. Middle click is paste after all.


- Martin


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


On April 28, 2015, 10:45 p.m., Yoann Laissus wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123547/
> ---
> 
> (Updated April 28, 2015, 10:45 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Currently, only the shortcut Shift + Left click is available.
> In KDE4, we had both.
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/Task.qml f5fc723 
> 
> Diff: https://git.reviewboard.kde.org/r/123547/diff/
> 
> 
> Testing
> ---
> 
> Tested with classic task manager and the icon only one.
> Tested with all mouse button combinaisons and keyboard modifiers.
> 
> 
> Thanks,
> 
> Yoann Laissus
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel