Re: Review Request 125984: kcm_formats: Do not write out an unusable value for the "C" locale

[Luca Beltrame]

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


As someone who touched that part ages ago, +1.

- Luca Beltrame


On Nov. 7, 2015, 2:14 p.m., Jonathan Marten wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125984/
> ---
> 
> (Updated Nov. 7, 2015, 2:14 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 354984
> https://bugs.kde.org/show_bug.cgi?id=354984
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> See the referenced bug.  The special locale value "C" needs to be written out 
> as simply that and not with an encoding suffix added, otherwise the resulting 
> value is not accepted by the locale system.
> 
> 
> Diffs
> -
> 
>   kcms/formats/kcmformats.cpp ea9dece 
> 
> Diff: https://git.reviewboard.kde.org/r/125984/diff/
> 
> 
> Testing
> ---
> 
> Built plasma-desktop with these changes, reset 'kcmshell5 formats' settings, 
> checked correct operation of desktop and of CLI and GUI applications.
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

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

[Powerdevil] [Bug 354623] Full charge not trigger notification

[Anthony via KDE Bugzilla]

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

--- Comment #5 from Anthony  ---
I update to 5.4.3 pre-release notification still not fire.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 125976: add an update() method

[Sebastian Kügler]

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


In general, I like the idea. We have to be a bit careful to keep the code 
maintainable though.

Please also remove the extraneous debug output.


src/kpackage/package.h (line 329)


Capital If



src/kpackage/package.cpp (line 786)


QLatin1Char('/') ?



src/kpackage/packagestructure.h (line 80)


and an older version (what?)...?



src/kpackage/packagestructure.h (line 81)


If



src/kpackage/private/packagejobthread.cpp (line 321)


spaces around <<; why a qWarning without any explanation?



src/kpackage/private/packagejobthread.cpp (line 326)


Would probably be good to give the installed path here as well. Also, 
"Impossible *to* remove"...



src/kpackage/private/packagejobthread_p.h (line 58)


As you note yourself, perhaps just make it an enum? It's not private API, 
OK, but the code here is getting really convoluted that I think another boolean 
trap won't help...



src/kpackage/private/versionparser.cpp (line 25)


I think we can remove most of these includes?


- Sebastian Kügler


On Nov. 6, 2015, 5:57 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125976/
> ---
> 
> (Updated Nov. 6, 2015, 5:57 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Kai Uwe Broulik.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> new job based update() function that compared to install()
> if a package with the same pluginId is already installed,
> removes the old one before installing the new one, if
> and only if the version of the new one is more recent
> 
> 
> Diffs
> -
> 
>   autotests/plasmoidpackagetest.h f730dce 
>   autotests/plasmoidpackagetest.cpp 567 
>   src/kpackage/CMakeLists.txt 3696f37 
>   src/kpackage/package.h 4ada8da 
>   src/kpackage/package.cpp 539b21a 
>   src/kpackage/packagestructure.h 9427b42 
>   src/kpackage/packagestructure.cpp 0070514 
>   src/kpackage/private/packagejob.cpp 0d2241b 
>   src/kpackage/private/packagejob_p.h 267429f 
>   src/kpackage/private/packagejobthread.cpp ca523b3 
>   src/kpackage/private/packagejobthread_p.h bf8a266 
>   src/kpackage/private/versionparser.cpp PRE-CREATION 
>   src/kpackagetool/CMakeLists.txt 78e0fb0 
> 
> Diff: https://git.reviewboard.kde.org/r/125976/diff/
> 
> 
> Testing
> ---
> 
> covered by autotests
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 125984: kcm_formats: Do not write out an unusable value for the "C" locale

[Jonathan Marten]

> On Nov. 7, 2015, 1:48 p.m., Sebastian Kügler wrote:
> > kcms/formats/kcmformats.cpp, line 124
> > 
> >
> > spaces around the comparison operator, please.

OK, will correct.


- Jonathan


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


On Nov. 7, 2015, 1:14 p.m., Jonathan Marten wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125984/
> ---
> 
> (Updated Nov. 7, 2015, 1:14 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 354984
> https://bugs.kde.org/show_bug.cgi?id=354984
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> See the referenced bug.  The special locale value "C" needs to be written out 
> as simply that and not with an encoding suffix added, otherwise the resulting 
> value is not accepted by the locale system.
> 
> 
> Diffs
> -
> 
>   kcms/formats/kcmformats.cpp ea9dece 
> 
> Diff: https://git.reviewboard.kde.org/r/125984/diff/
> 
> 
> Testing
> ---
> 
> Built plasma-desktop with these changes, reset 'kcmshell5 formats' settings, 
> checked correct operation of desktop and of CLI and GUI applications.
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

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

Re: Review Request 125978: [TabBarLayout] Layout sooner

[Kai Uwe Broulik]

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

(Updated Nov. 7, 2015, 12:17 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit 606c7f651d668349da79313806d5ecefbc6ed221 by Kai Uwe 
Broulik to branch master.


Repository: plasma-framework


Description
---

Layout after 10ms rather than 150ms. Still long enough so we don't layout 
multiple times (it layouts twice in a row but did that before too)


Diffs
-

  src/declarativeimports/plasmacomponents/qml/private/TabBarLayout.qml 6bc6fc9 

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


Testing
---

Fixes the few frames after plasmashell startup where the Kickoff buttons are 
cramped together. I didn't notice a performance penalty while resizing or 
navigating.

(For some reason the layout's height changes when Kickoff expands/collapses, 
needlessly causing a re-layout but I couldn't figure out why that is)


Thanks,

Kai Uwe Broulik

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

Re: Review Request 125939: Workaround self-destructing menu

[Kai Uwe Broulik]

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

(Updated Nov. 7, 2015, 12:17 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit af32d8b42fd9e24682aebf5ad26aa376821f84b0 by Kai Uwe 
Broulik to branch master.


Bugs: 353779
http://bugs.kde.org/show_bug.cgi?id=353779


Repository: plasma-framework


Description
---

I have really no idea what's happening and this patch is plain wrong but I 
cannot figure out why this happens. Basically, if I popup() the "determining 
minetype" thing afterwards if the menu becomes empty for whatever reason 
(clear(), remove the one and only action in it etc) it just self-destructs in 
exec() and returns a null QAction.
Any idea what's going on there?


Diffs
-

  src/scriptengines/qml/plasmoid/containmentinterface.cpp 772031f 

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


Testing
---

Now I can drop stuff that offers multiple actions (image for instance will 
yield an icon and "use as wallpaper") and actually choose from them.


Thanks,

Kai Uwe Broulik

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

Re: Review Request 125984: kcm_formats: Do not write out an unusable value for the "C" locale

[Sebastian Kügler]

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


Looks good, a coding style comment inline, though.


kcms/formats/kcmformats.cpp (line 124)


spaces around the comparison operator, please.


- Sebastian Kügler


On Nov. 7, 2015, 1:14 p.m., Jonathan Marten wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125984/
> ---
> 
> (Updated Nov. 7, 2015, 1:14 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 354984
> https://bugs.kde.org/show_bug.cgi?id=354984
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> See the referenced bug.  The special locale value "C" needs to be written out 
> as simply that and not with an encoding suffix added, otherwise the resulting 
> value is not accepted by the locale system.
> 
> 
> Diffs
> -
> 
>   kcms/formats/kcmformats.cpp ea9dece 
> 
> Diff: https://git.reviewboard.kde.org/r/125984/diff/
> 
> 
> Testing
> ---
> 
> Built plasma-desktop with these changes, reset 'kcmshell5 formats' settings, 
> checked correct operation of desktop and of CLI and GUI applications.
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

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

Review Request 125984: kcm_formats: Do not write out an unusable value for the "C" locale

[Jonathan Marten]

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

Review request for Plasma.


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


Repository: plasma-desktop


Description
---

See the referenced bug.  The special locale value "C" needs to be written out 
as simply that and not with an encoding suffix added, otherwise the resulting 
value is not accepted by the locale system.


Diffs
-

  kcms/formats/kcmformats.cpp ea9dece 

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


Testing
---

Built plasma-desktop with these changes, reset 'kcmshell5 formats' settings, 
checked correct operation of desktop and of CLI and GUI applications.


Thanks,

Jonathan Marten

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

plasma-frameworks unittest still broken

[David Faure]

Good morning from the unit test police...

https://build.kde.org/job/plasma-framework%20master%20kf5-qt5/PLATFORM=Linux,Variation=All,compiler=gcc/122/console

03:08:59 FAIL!  : DialogNativeTest::size() Compared values are not the same
03:08:59Actual   (m_dialog->width()): 112
03:08:59Expected (108)  : 108
03:08:59Loc: 
[/home/jenkins/builds/plasma-framework/kf5-qt5/autotests/dialognativetest.cpp(68)]
03:08:59 FAIL!  : DialogNativeTest::position() Compared values are not the same
03:08:59Actual   (m_dialog->x()): 69
03:08:59Expected (71)   : 71
03:08:59Loc: 
[/home/jenkins/builds/plasma-framework/kf5-qt5/autotests/dialognativetest.cpp(86)]
03:08:59 PASS   : DialogNativeTest::cleanupTestCase()
03:08:59 Totals: 2 passed, 2 failed, 0 skipped, 0 blacklisted
03:08:59 * Finished testing of DialogNativeTest *

Please guys keep an eye on CI so I don't have to hunt for broken unittests when 
making a release.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

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

[Powerdevil] [Bug 354623] Full charge not trigger notification

[Martin Klapetek via KDE Bugzilla]

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

--- Comment #7 from Martin Klapetek  ---
Fwiw I'm using kubuntu ci and recently stuff in /opt like kcms and kded modules
stopped being found by the system, I think it has to do with the sycoca changes
and I haven't figured out which paths need adjusting or what, but it's
completely ignoring all of /opt when it comes to sycoca stuff.

So yeah, I haven't bothered, sorry.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

[Powerdevil] [Bug 354623] Full charge not trigger notification

[Martin Klapetek via KDE Bugzilla]

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

--- Comment #8 from Martin Klapetek  ---
But upower -d does report fully-charged.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

[Powerdevil] [Bug 354623] Full charge not trigger notification

[Anthony via KDE Bugzilla]

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

--- Comment #9 from Anthony  ---
Commit it, it will work. I tell you upower -d report fully-charged.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

[Powerdevil] [Bug 354623] Full charge not trigger notification

[Kai Uwe Broulik via KDE Bugzilla]

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

--- Comment #6 from Kai Uwe Broulik  ---
Yes because nobody bothered testing the patch I attached and thus I did not
commit it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 125991: [Widget Explorer] Allow uninstalling user-installed applets

[Kai Uwe Broulik]

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

Review request for Plasma and KDE Usability.


Repository: plasma-desktop


Description
---

The no longer enabled tooltip also contained the uninstall button. This 
restores the uninstall functionality similar to the wallpaper dialog.

User-installed applets get an uninstall button. I chose to always show it 
rather than on hover since only a few applets are likely to be uninstallable 
and I don't want to play the guessing-game, perhaps we should introduce a 
filter for "User-installed applets" similar to the (less useful) "running" 
ones. Clicking the button will queue the uninstallation which can be cancelled 
anytime the dialog is still opened. As soon as the dialog closes, the actual 
uninstall is executed.

Multiple applets can be enqueued simultaneously.


Diffs
-

  desktoppackage/contents/explorer/AppletDelegate.qml 5a81b91 
  desktoppackage/contents/explorer/WidgetExplorer.qml af27229 

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


Testing
---

Installed plasmoid through drag and drop, uninstalled after fixing an uninstall 
bug (Review 125990)

The tooltip says "Undo uninstall". I just noticed I should probably fade the 
applet name too.


File Attachments


Uninstall before
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/11/08/adcb91e0-a8c2-4481-89f1-139b4e716da3__uninstall1.png
Undo uninstall
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/11/08/ebdae6ad-28bd-42f0-bb36-bfa6dfd3cc30__uninstall2.png


Thanks,

Kai Uwe Broulik

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

Review Request 125990: Fix uninstall

[Kai Uwe Broulik]

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

Review request for Plasma.


Repository: plasma-workspace


Description
---

QStandardPaths doesn't add a trailing slash, nor does 
PLASMA_RELATIVE_DATA_INSTALL_DIR have a leading slash. Also, apparently we need 
a structure also for uninstall but then we can remove the setPath because we 
tell that to uninstall...

Not sure if we're doing something wrong to begin with here, though.


Diffs
-

  components/shellprivate/widgetexplorer/widgetexplorer.cpp 0922c18 

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


Testing
---

Works now.


Thanks,

Kai Uwe Broulik

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