D24945: [ksmserver] Signal session management state to kwin directly

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> davidedmundson wrote in org.kde.KWin.Session.xml:7
> There was a third option I considered. Have N methods
> 
> startSave
> startQuit
> saveCancelled
> 
> and then make the state change an implicit part of the invokable action we 
> need to do on save.
> 
> I went this way as I felt it was nice to have this acting more like a 
> property independently, but what do you think.

I guess it doesn't matter ultimately. I won't advocate for a header with an 
enum, don't worry.
Logically, having an int is simpler than a string and we will have to document 
both and have an implicit tie in both implmentations.

Having 3 methods seems quite elegant as in it's a more semantic API.

As you put it (t, the 3rd seems simpler to grasp, not that everyone should be 
looking at it.

REPOSITORY
  R120 Plasma Workspace

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

To: davidedmundson, #kwin
Cc: apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, ahiemstra, mart

D24953: [applets/weather] Use unit description from KUnitConversion

[Ismael Asensio]

iasensio created this revision.
iasensio added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
iasensio requested review of this revision.

REVISION SUMMARY
  Use unit description directly from KUnitConversion instead of making a new 
string.
  This avoids duplicating translation strings and improves consistency.
  The units name is now `description` (`symbol`) also for consistency with 
krunner

TEST PLAN
  No behavior changes within the applet
  `Configure` -> `Units` shows translated `description` (`symbol`) for every 
unit

REPOSITORY
  R114 Plasma Addons

BRANCH
  weather_use_unit_description

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

AFFECTED FILES
  applets/weather/plugin/abstractunitlistmodel.h
  applets/weather/plugin/plugin.cpp
  applets/weather/plugin/util.cpp
  applets/weather/plugin/util.h

To: iasensio, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24952: Add mmHg to pressure options in weather applet

[Ismael Asensio]

iasensio created this revision.
iasensio added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
iasensio requested review of this revision.

REVISION SUMMARY
  Add millimeters of mercury (mmHg) to pressure options in weather applet, 
which is a common pressure unit in some countries
  
  FEATURE: 401734
  FIXED-IN: 5.18

TEST PLAN
  You can select 'mmHg' as the pressure unit and it displays correctly in the 
widget

REPOSITORY
  R114 Plasma Addons

BRANCH
  weather_mmHg

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

AFFECTED FILES
  applets/weather/plugin/plugin.cpp

To: iasensio, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24945: [ksmserver] Signal session management state to kwin directly

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> davidedmundson wrote in org.kde.KWin.Session.xml:7
> It also advocates for use of {sv} in your spec which is inherently string 
> based.
> 
> I'm happy to use ints, as long as we then don't get into that trap of saying; 
> "now we have an enum to sync in two places, we must create a header file and 
> a dependency "

There was a third option I considered. Have N methods

startSave
startQuit
saveCancelled

and then make the state change an implicit part of the invokable action we need 
to do on save.

I went this way as I felt it was nice to have this acting more like a property 
independently, but what do you think.

REPOSITORY
  R120 Plasma Workspace

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

To: davidedmundson, #kwin
Cc: apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, ahiemstra, mart

D24945: [ksmserver] Signal session management state to kwin directly

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> apol wrote in org.kde.KWin.Session.xml:7
> Here it recommends using uint for such things:
> https://dbus.freedesktop.org/doc/dbus-api-design.html#interface-files

It also advocates for use of {sv} in your spec which is inherently string based.

I'm happy to use ints, as long as we then don't get into that trap of saying; 
"now we have an enum to sync in two places, we must create a header file and a 
dependency "

REPOSITORY
  R120 Plasma Workspace

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

To: davidedmundson, #kwin
Cc: apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, ahiemstra, mart

D24947: Install krunner desktop file to ${KDE_INSTALL_APPDIR}

[Fabian Vogt]

fvogt added a comment.


  In D24947#554005 , @meven wrote:
  
  > In D24947#554001 , @fvogt wrote:
  >
  > > IMO it's a good thing to have KRunner as application in the menu anyway
  >
  >
  > It was not in the application menu before
  >  With this change (and without NoDisplay) it will be displayed.
  
  
  IMO an improvement.
  
  > So I really think we should change kglobalaccel behavior ignoring 
application with NoDisplay like it should.
  
  Yup, it should read `Hidden` instead.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: meven, fvogt, apol, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24786: [GTK3] Add styles for libhandy widgets

[Noah Davis]

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


  I guess it doesn't matter that much right now.

REPOSITORY
  R98 Breeze for Gtk

BRANCH
  hdy-styles (branched from master)

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

To: cblack, #breeze, #vdg, ndavis
Cc: ngraham, ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24947: Install krunner desktop file to ${KDE_INSTALL_APPDIR}

[Méven Car]

meven added a comment.


  In D24947#554001 , @fvogt wrote:
  
  > IMO it's a good thing to have KRunner as application in the menu anyway
  
  
  It was not in the application menu before
  With this change (and without NoDisplay) it will be displayed.
  
  So I really think we should change kglobalaccel behavior ignoring application 
with NoDisplay like it should.
  
  >   NoDisplay means "this application exists, but don't display it in the 
menus". This can be useful to e.g. associate this application with MIME types, 
so that it gets launched from a file manager (or other apps), without having a 
menu entry for it (there are tons of good reasons for this, including e.g. the 
netscape -remote, or kfmclient openURL kind of stuff). "
  
  
https://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: meven, fvogt, apol, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24947: Install krunner desktop file to ${KDE_INSTALL_APPDIR}

[Fabian Vogt]

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


  IMO it's a good think to have KRunner as application in the menu anyway

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: meven, fvogt, apol, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24947: Install krunner desktop file to ${KDE_INSTALL_APPDIR}

[Méven Car]

meven created this revision.
meven added reviewers: fvogt, apol, Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
meven requested review of this revision.

REVISION SUMMARY
  Redo of D24857  afetr 
a53e8065fbce3de526781e236014404a7d4fa84f 


REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  krunner/CMakeLists.txt

To: meven, fvogt, apol, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24945: [ksmserver] Signal session management state to kwin directly

[Aleix Pol Gonzalez]

apol added a comment.


  Looks good to me overall. It's better to use dbus.

INLINE COMMENTS

> org.kde.KWin.Session.xml:7
> +
> +
> +

Here it recommends using uint for such things:
https://dbus.freedesktop.org/doc/dbus-api-design.html#interface-files

REPOSITORY
  R120 Plasma Workspace

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

To: davidedmundson, #kwin
Cc: apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Cyril Rossi]

crossi updated this revision to Diff 68755.
crossi added a comment.


  qml style

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24916?vs=68748=68755

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

AFFECTED FILES
  kcms/style/package/contents/ui/EffectSettingsPopup.qml

To: crossi, #plasma, ervin, mart, bport
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Kai Uwe Broulik]

broulik added a comment.


  > It's likely we'll encounter that ComboBox breakage at other places.
  
  we do :) 
https://cgit.kde.org/kscreen.git/tree/kcm/package/contents/ui/Panel.qml?id=7811411c6425dd52bcd732a5910d177fee4a89d2#n42

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, mart, bport
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Kevin Ottens]

ervin added a comment.


  Style wise, functions come before children objects. Other than that looks 
good to me.
  
  Through Kai comments, the question also becomes, wouldn't it be better to fix 
the root cause? It's likely we'll encounter that ComboBox breakage at other 
places.

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, mart, bport
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Kevin Ottens]

ervin added a comment.


  In D24916#553958 , @broulik wrote:
  
  > > Ah somehow I thought we had a third more "clever" case of currentIndex 
moving.
  >
  > Heh, yea, there's also some more elaborate "change index while moving the 
mouse" going on :(
  >  So best hack would be to have a c++ thing that sets the property without 
going through QML's binding system :D
  
  
  So we're back on the "but it's not going to be pretty" department. ;-)

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, mart, bport
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Kai Uwe Broulik]

broulik added a comment.


  > Ah somehow I thought we had a third more "clever" case of currentIndex 
moving.
  
  Heh, yea, there's also some more elaborate "change index while moving the 
mouse" going on :(
  So best hack would be to have a c++ thing that sets the property without 
going through QML's binding system :D

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, mart, bport
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24945: [ksmserver] Signal session management state to kwin directly

[David Edmundson]

davidedmundson updated this revision to Diff 68751.
davidedmundson added a comment.


  update

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24945?vs=68750=68751

BRANCH
  origin-master (branched from master)

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

AFFECTED FILES
  ksmserver/CMakeLists.txt
  ksmserver/logout.cpp
  ksmserver/org.kde.KWin.Session.xml
  ksmserver/server.cpp
  ksmserver/server.h

To: davidedmundson, #kwin
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24945: [ksmserver] Signal session management state to kwin directly

[David Edmundson]

davidedmundson created this revision.
davidedmundson added a reviewer: KWin.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  Kwin had to have two ICE connections in order to track state
  indepdendently of it's session saving.
  
  This replaces that with a more direct DBus protocol allowing for both
  simplification on the kwin side as well as comunicating the logout state
  better for effects.
  
  Whilst this code temporarily complicates things, now we have this
  interface the next step is drop all the isWM() stuff and do kwin
  specific session management also over this interface. See T11882 


TEST PLAN
  Added qdebug into kwin
  started logging out with an unsaved file, cancelled shutdown
  started logging out with, discarded file

REPOSITORY
  R120 Plasma Workspace

BRANCH
  origin-master (branched from master)

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

AFFECTED FILES
  ksmserver/CMakeLists.txt
  ksmserver/logout.cpp
  ksmserver/server.cpp
  ksmserver/server.h

To: davidedmundson, #kwin
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Cyril Rossi]

crossi updated this revision to Diff 68748.
crossi added a comment.


  refactor code into a function

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24916?vs=68670=68748

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

AFFECTED FILES
  kcms/style/package/contents/ui/EffectSettingsPopup.qml

To: crossi, #plasma, ervin, mart, bport
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Kevin Ottens]

ervin added a comment.


  In D24916#553919 , @broulik wrote:
  
  > > AFAICT it won't be pretty though. :-)
  >
  > I think it's pretty straightforward
  >  1.) the delegate must not explicitly set a `currentIndex` because Qt does 
it for us, see D18299 
  >  2.) the wheel handler must be changed to call `decrementCurrentIndex()` 
and `incrementCurrentIndex()` (hopefully they wrap...) instead of changing the 
index manually, so it doesn't break the binding.
  
  
  Ah somehow I thought we had a third more "clever" case of currentIndex 
moving. Then indeed if that's just those two we should be fine.

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, mart, bport
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Kai Uwe Broulik]

broulik added a comment.


  > AFAICT it won't be pretty though. :-)
  
  I think it's pretty straightforward
  1.) the delegate must not explicitly set a `currentIndex` because Qt does it 
for us, see D18299 
  2.) the wheel handler must be changed to call `decrementCurrentIndex()` and 
`incrementCurrentIndex()` (hopefully they wrap...) instead of changing the 
index manually, so it doesn't break the binding.

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, mart, bport
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Kevin Ottens]

ervin added a comment.


  In D24916#553886 , @broulik wrote:
  
  > Hm I think qqc2-desktop-style breaks the binding on `currentIndex`, 
otherwise this would not be neccessary...
  
  
  Good point... What about fixing that? AFAICT it won't be pretty though. :-)

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, mart, bport
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24917: KCM Style port to KConfigXT

[Kevin Ottens]

ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kcmstyle.cpp:317
> +
> +m_model->setSelectedStyle(m_settings->widgetStyle());
>  

That line and the metaenum wrangling could likely be factored out somehow and 
shared with load(). Especially the enum part is uncommon enough that we 
probably want to hide it behind a nicer name (also would help a bit having a 
more consistent level of abstraction in the code of those functions).

> kcmstyle.h:48
>  Q_PROPERTY(StylesModel *model READ model CONSTANT)
> -
> -Q_PROPERTY(bool iconsOnButtons READ iconsOnButtons WRITE 
> setIconsOnButtons NOTIFY iconsOnButtonsChanged)
> -Q_PROPERTY(bool iconsInMenus READ iconsInMenus WRITE setIconsInMenus 
> NOTIFY iconsInMenusChanged)
> +Q_PROPERTY(StyleSettings* styleSettings READ styleSettings CONSTANT)
>  Q_PROPERTY(ToolBarStyle mainToolBarStyle READ mainToolBarStyle WRITE 
> setMainToolBarStyle NOTIFY mainToolBarStyleChanged)

Please have the space before the * and not after to follow the style of that 
file (see the Q_PROPERTY just above).

> kcmstyle.h:66
>  
> -bool iconsOnButtons() const;
> -void setIconsOnButtons(bool enable);
> -Q_SIGNAL void iconsOnButtonsChanged();
> -
> -bool iconsInMenus() const;
> -void setIconsInMenus(bool enable);
> -Q_SIGNAL void iconsInMenusChanged();
> +StyleSettings* styleSettings() const;
>  

ditto

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, ervin, mart, bport, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24853: [DigitalClock] Fix layout and QML warnings

[Konrad Materka]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:82cfabd0a5c8: [DigitalClock] Fix layout and QML warnings 
(authored by kmaterka).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24853?vs=68743=68745

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

AFFECTED FILES
  applets/digital-clock/package/contents/ui/configAppearance.qml
  applets/digital-clock/package/contents/ui/configTimeZones.qml

To: kmaterka, #plasma, #plasma_workspaces, ngraham
Cc: ngraham, davidedmundson, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Kai Uwe Broulik]

broulik added a comment.


  Hm I think qqc2-desktop-style breaks the binding on `currentIndex`, otherwise 
this would not be neccessary...

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, mart, bport
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24916: KCM style : fix combobox that weren't updated after user made change

[Kevin Ottens]

ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> EffectSettingsPopup.qml:72
> +onMainToolBarStyleChanged: {
> +mainToolBarStyleCombo.currentIndex = 
> mainToolBarStyleCombo.model.findIndex(function (item) {
> +return item.value === kcm.mainToolBarStyle

To de-duplicate a bit, because now that's 4 times (almost) the same piece of 
code. What about having a function resetIndex() declared in that combo and 
another one in the other combobox which would do the currentIndex + findIndex 
dance.

This way in both the code would become in both cases:

  currentIndex: resetIndex()
  ...
  onFooStyleChanged: fooStyleCombo.resetIndex()

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, mart, bport
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24853: [DigitalClock] Fix layout and QML warnings

[Nathaniel Graham]

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


  LGTM.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: kmaterka, #plasma, #plasma_workspaces, ngraham
Cc: ngraham, davidedmundson, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24942: Require and use the new NewStuffQuick Button component

[Nathaniel Graham]

ngraham added a comment.


  Nice. However it doesn't seem to work for me. If I install a theme using the 
dialog, it doesn't show up in the Global Themes KCM. Could it because of a typo 
(`changedEntryes.count()`) ?
  
  I've filed a bunch of bug reports regarding polishing the dialog itself:
  
  - https://bugs.kde.org/show_bug.cgi?id=413433
  - https://bugs.kde.org/show_bug.cgi?id=413434
  - https://bugs.kde.org/show_bug.cgi?id=413435
  - https://bugs.kde.org/show_bug.cgi?id=413436
  - https://bugs.kde.org/show_bug.cgi?id=413437
  - https://bugs.kde.org/show_bug.cgi?id=413439
  - https://bugs.kde.org/show_bug.cgi?id=413440
  - https://bugs.kde.org/show_bug.cgi?id=413441

REPOSITORY
  R119 Plasma Desktop

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

To: leinir, #plasma, #knewstuff, #vdg, broulik
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24853: [DigitalClock] Fix layout and QML warnings

[Konrad Materka]

kmaterka updated this revision to Diff 68743.
kmaterka added a comment.


  Remove hardcoded margins, these are not needed anymore.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24853?vs=68524=68743

BRANCH
  master

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

AFFECTED FILES
  applets/digital-clock/package/contents/ui/configAppearance.qml
  applets/digital-clock/package/contents/ui/configTimeZones.qml

To: kmaterka, #plasma, #plasma_workspaces
Cc: ngraham, davidedmundson, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24853: [DigitalClock] Fix layout and QML warnings

[Konrad Materka]

kmaterka added inline comments.

INLINE COMMENTS

> ngraham wrote in configTimeZones.qml:58
> Don't use hardcoded margins. This should be `Kirigami.Units.smallSpacing`

I just copied values form the original Rectangle. I will remove it entirely, 
probably it's not needed anymore.

REPOSITORY
  R120 Plasma Workspace

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

To: kmaterka, #plasma, #plasma_workspaces
Cc: ngraham, davidedmundson, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24846: Port kcm icons to kconfigxt

[Kevin Ottens]

ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> broulik wrote in main.cpp:141
> Does this `KIconTheme` instance need caching? I recall creating those parses 
> a tonne of files and directories and is quite slow

Agreed, it'll hit the disk quite a bit, this might be a problem in a function 
which takes part in bindings (more than one which will change around the same 
time).

> main.cpp:176
> +{
> +   return m_model->selectedTheme() != KIconTheme::current();
> +}

Indentation looks wrong here.

> bport wrote in IconSizePopup.qml:47-53
> I wiil look if I can simplify that

I like the intent of trying to have names instead of just indices everywhere... 
but that might not be feasible without extra complexity we'll want to avoid 
indeed.

I think the more fundamental problem is that the settings are based on those 
names, while the invokables (most notably availableIconSizes) are based on 
indices. Probably something to address, like have names everywhere and just a 
function to map from index to name in the iconTypeList.

REPOSITORY
  R119 Plasma Desktop

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

To: bport, ervin, mart, #plasma, crossi
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24942: Require and use the new NewStuffQuick Button component

[Dan Leinir Turthra Jensen]

leinir created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
leinir requested review of this revision.

REVISION SUMMARY
  This removes the need for the custom hack for spawning a dialog,
  and is intended to function as a sample for how to implement this
  in other KCMs.
  
  Also use the big, pretty Preview mode (because super-visual content)
  
  This could be done wholesale for the entire bunch of KCMs, but i post
  this on its own in an attempt to gain feedback on a self contained
  example of how to do this porting step.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  use-newstuffquick-in-lookandfeel-kcm (branched from master)

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

AFFECTED FILES
  CMakeLists.txt
  kcms/lookandfeel/CMakeLists.txt
  kcms/lookandfeel/kcm.cpp
  kcms/lookandfeel/kcm.h
  kcms/lookandfeel/package/contents/ui/main.qml

To: leinir
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24934: [notifications] Center no notification text on mobile

[Marco Martin]

mart added a comment.


  I'm fine with it, however...
  there must be a bug somewhere in the shell, because the whole notification 
applet is supposed to be invisible when there are none, i have to investigate 
into that

REPOSITORY
  R120 Plasma Workspace

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

To: nicolasfella, #plasma, #vdg, broulik
Cc: mart, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra

D24810: Ensure defintion of XDG_DATA_DIRS

[Adriaan de Groot]

adridg added inline comments.

INLINE COMMENTS

> dfaure wrote in startplasma.cpp:197
> Wait, where does /usr/local/etc/xdg come from? 
> https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html 
> says the default is only /etc/xdg.

On FreeBSD (other BSD's, too), `/etc` is for the **base** system only, and all 
user-add-ons go in *$LOCALBASE*, usually `/usr/local` .. so XDG stuff, which is 
a user-add-on, goes into `/usr/local/etc`. Even our `/etc/os-release` is in 
`/usr/local/etc/os-release` (and Frameworks are suitably patched for that).

**Some** BSD derivatives are less dogmatic about `/etc`, but not us.

REPOSITORY
  R120 Plasma Workspace

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

To: tcberner, #freebsd, dfaure, apol
Cc: adridg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

Plasma 5.18 Kickoff Meeting Wednesday 14:00UTC/19500CET

[Jonathan Riddell]

Let's have a Plasma 5.18 Kickoff meeting on Wednesday evening.  Remember
the clocks go back as you wake up on Sunday.  14:00UTC/GMT/British Isles
and 15:00CET

Meet in #plasma:kde.org Matrix room or #plasma freenode IRC channel

Things to discuss and confirm:
Review successes and failures of 5.17
Schedule for 5.18
LTS status for 5.18
What versions of Qt and KF5 we'll support
Propose new Todo items for 5.18

Jonathan

D24940: Enchanting/Showing more informations on the info section of plasma-nm plasmoid

[Francesco Bonanno]

mibofra added a comment.


  I've updated the diff!

REPOSITORY
  R116 Plasma Network Management Applet

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

To: mibofra, jgrulich
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24940: Enchanting/Showing more informations on the info section of plasma-nm plasmoid

[Francesco Bonanno]

mibofra updated this revision to Diff 68739.

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24940?vs=68729=68739

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

AFFECTED FILES
  libs/models/networkmodelitem.cpp

To: mibofra, jgrulich
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24856: Add a .desktop file to ksplashqml

[Méven Car]

meven marked an inline comment as done.
meven added inline comments.

INLINE COMMENTS

> ksplashqml.desktop.cmake:8
> +X-DBUS-StartupType=Unique
> +X-DBUS-ServiceName=org.kde.ksplashqml
> +X-KDE-StartupNotify=false

I think X-DBUS-* may not be necessary here.

> ngraham wrote in ksplashqml.desktop.cmake:3
> Human readable name please

I have added GenericName which serves this purpose.
But this desktop files won't be visible by users thanks to "NoDisplay".

REPOSITORY
  R120 Plasma Workspace

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

To: meven, apol, #plasma
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24856: Add a .desktop file to ksplashqml

[Méven Car]

meven updated this revision to Diff 68738.
meven added a comment.


  Rebase on master

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24856?vs=68737=68738

BRANCH
  arcpatch-D24856_1

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

AFFECTED FILES
  ksplash/ksplashqml/CMakeLists.txt
  ksplash/ksplashqml/ksplashqml.desktop.cmake

To: meven, apol, #plasma
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24856: Add a .desktop file to ksplashqml

[Méven Car]

meven updated this revision to Diff 68737.
meven added a comment.


  Add GenericName

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24856?vs=68520=68737

BRANCH
  arcpatch-D24856_1

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

AFFECTED FILES
  krunner/CMakeLists.txt
  ksplash/ksplashqml/CMakeLists.txt
  ksplash/ksplashqml/ksplashqml.desktop.cmake

To: meven, apol, #plasma
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24925: KCM KSplash port to ManagedConfigModule

[Cyril Rossi]

crossi updated this revision to Diff 68734.
crossi added a comment.


  Remove unnecessary override

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24925?vs=68699=68734

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

AFFECTED FILES
  kcms/ksplash/kcm.cpp
  kcms/ksplash/kcm.h

To: crossi, #plasma, ervin, mart, bport
Cc: davidedmundson, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24798: Migrate QQC1 to QQC2

[Kai Uwe Broulik]

broulik added a comment.


  That new time zone view lacks any form of keyboard navigation. I cannot use 
arrow keys to navigate it and neither can I hit space to toggle a checkbox. It 
also lacks a frame around its content area like the text field has.

REPOSITORY
  R120 Plasma Workspace

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

To: guoyunhe, #plasma, #plasma_workspaces, ngraham
Cc: broulik, kmaterka, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24798: Migrate QQC1 to QQC2

[Yunhe Guo]

guoyunhe updated this revision to Diff 68733.
guoyunhe added a comment.


  Merge kmaterka's patch D24853 

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24798?vs=68345=68733

BRANCH
  master

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

AFFECTED FILES
  applets/appmenu/package/contents/ui/main.qml
  applets/digital-clock/package/contents/ui/configAppearance.qml
  applets/digital-clock/package/contents/ui/configTimeZones.qml
  applets/digital-clock/plugin/timezonesi18n.cpp
  applets/systemmonitor/common/contents/ui/ConfigGeneral.qml
  sddm-theme/BreezeMenuStyle.qml
  sddm-theme/KeyboardButton.qml
  sddm-theme/Main.qml
  sddm-theme/SessionButton.qml
  wallpapers/image/imagepackage/platformcontents/phone/ui/config.qml
  wallpapers/image/imagepackage/platformcontents/phone/ui/customwallpaper.qml

To: guoyunhe, #plasma, #plasma_workspaces, ngraham
Cc: kmaterka, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24810: Ensure defintion of XDG_DATA_DIRS

[David Faure]

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


  Please make sure the first line of the commit log says CONFIG and not DATA, 
too (the phab title still says DATA).

INLINE COMMENTS

> startplasma.cpp:197
> +if (!qEnvironmentVariableIsSet("XDG_CONFIG_DIRS")) {
> +qputenv("XDG_CONFIG_DIRS", KDE_INSTALL_FULL_CONFDIR 
> ":/etc/xdg:/usr/local/etc/xdg");
> +}

Wait, where does /usr/local/etc/xdg come from? 
https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html 
says the default is only /etc/xdg.

REPOSITORY
  R120 Plasma Workspace

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

To: tcberner, #freebsd, dfaure, apol
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24810: Ensure defintion of XDG_DATA_DIRS

[Tobias C. Berner]

tcberner updated this revision to Diff 68732.
tcberner edited the summary of this revision.
tcberner added a comment.


  Update commit message in diff.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24810?vs=68731=68732

BRANCH
  master

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

AFFECTED FILES
  startkde/config-startplasma.h.cmake
  startkde/startplasma.cpp

To: tcberner, #freebsd, dfaure, apol
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24810: Ensure defintion of XDG_DATA_DIRS

[Tobias C. Berner]

tcberner updated this revision to Diff 68731.
tcberner added a comment.


  Fix comment and commit message.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24810?vs=68379=68731

BRANCH
  master

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

AFFECTED FILES
  startkde/config-startplasma.h.cmake
  startkde/startplasma.cpp

To: tcberner, #freebsd, dfaure, apol
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart