D14143: Make the action selector OSD independent of the other OSDs

[David Edmundson]

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

INLINE COMMENTS

> osd.cpp:144
>  Q_EMIT osdActionSelected(static_cast(action));
>  hideOsd();
>  }

I don't understand how this hideOsd() will work, it only updates m_osdObject 
which is the other OSD.

> osd.cpp:137
> +auto *rootObject = m_osdActionSelector->rootObject();
> +connect(rootObject, SIGNAL(clicked(int)),
>  this, SLOT(onOsdActionSelected(int)));

This will reconnect on the second invocation

REPOSITORY
  R104 KScreen

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

To: gladhorn, #plasma, davidedmundson
Cc: davidedmundson, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14143: Make the action selector OSD independent of the other OSDs

[Frederik Gladhorn]

gladhorn updated this revision to Diff 37984.
gladhorn added a comment.


  return qstring without ref

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14143?vs=37923=37984

BRANCH
  arcpatch-D14143_2

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

AFFECTED FILES
  kded/osd.cpp
  kded/osd.h
  kded/qml/OsdSelector.qml

To: gladhorn, #plasma
Cc: zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D14165: Handle keyboard in action selector OSD

[Frederik Gladhorn]

gladhorn added a comment.


  Awesome! I'll incorporate the ideas, that's great. Any chance to get 
https://phabricator.kde.org/D14143 in? It's required for this since it changes 
the window type. I'd also like to make the global shortcut show and hide the 
osd (right now pressing it again makes the osd fade out and in again).

REPOSITORY
  R104 KScreen

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

To: gladhorn, #plasma
Cc: broulik, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14152: Remove ref from returned QString

[Frederik Gladhorn]

This revision was automatically updated to reflect the committed changes.
Closed by commit R104:97f7b9cff782: Remove ref from returned QString (authored 
by gladhorn).

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14152?vs=37922=37983

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

AFFECTED FILES
  kded/osd.cpp

To: gladhorn, #plasma, davidedmundson
Cc: zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13874: textplugin: Fix missing QTextStream include

[Volker Krause]

This revision was automatically updated to reflect the committed changes.
Closed by commit R112:6b2036cb8a1d: textplugin: Fix missing QTextStream include 
(authored by alistairf, committed by vkrause).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D13874?vs=37123=37982#toc

REPOSITORY
  R112 Milou

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13874?vs=37123=37982

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

AFFECTED FILES
  lib/previews/textplugin.cpp

To: alistairf, kossebau
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D14056: Remove FileItemActionPlugin in favor of Purpose plugin

[Nicolas Fella]

This revision was automatically updated to reflect the committed changes.
Closed by commit R97:6fa60e4bf363: Remove FileItemActionPlugin in favor of 
Purpose plugin (authored by nicolasfella).

REPOSITORY
  R97 Bluedevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14056?vs=37586=37970

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

AFFECTED FILES
  src/CMakeLists.txt
  src/fileitemactionplugin/CMakeLists.txt
  src/fileitemactionplugin/bluedevilfileitemaction.desktop
  src/fileitemactionplugin/sendfileitemaction.cpp
  src/fileitemactionplugin/sendfileitemaction.h

To: nicolasfella, broulik, drosca
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D14186: Show software notifier only once startup has completed

[David Edmundson]

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


  No.
  
  If we explicitly set the backend before calling ::sceneGraphBackend then it 
will have the correct result. 
  When we're using the software renderer from the config that will have 
happened.
  
  It doesn't cover the using an env var case, but then the KCM will be of no 
use.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D14186: Show software notifier only once startup has completed

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Plasma, davidedmundson.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  `QQuickWindow::sceneGraphBackend()` is empty until the first `QQuickWindow` 
was created, so doing this in `main` effectively did nothing.

TEST PLAN
  Ran `QT_QUICK_BACKEND=software plasmashell` and got the software SNI in my 
tray

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  shell/main.cpp
  shell/shellcorona.cpp

To: broulik, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D14174: Fix blur behind folderview context menus

[Nathaniel Graham]

ngraham added a comment.


  In D14174#293468 , @hein wrote:
  
  > We have blurred context menus now? Wtf.
  >
  > But ok.
  
  
  It's an optional setting in the Breeze theme. Not really my thing, but I have 
to admit that user feedback has been overwhelmingly positive so far!

REPOSITORY
  R119 Plasma Desktop

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

To: davidedmundson, #plasma, hein
Cc: ngraham, hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14185: Disable script execution over DBus when scripting console is disabled

[Kai Uwe Broulik]

broulik created this revision.
broulik added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  There's no point in disabling the GUI for this but then still allowing stuff 
via DBus

TEST PLAN
  Put the following in `kdeglobals`
  
[KDE Action Restrictions]
plasma-desktop/scripting_console=false
  
  did a `qdbus` call
  
Error: org.freedesktop.DBus.Error.Failed
Administrative policies prevent script execution
  
  - Verified that `unlockedDesktop` (system immutable) also blocks it
  - Verified that it still works when none of those restrictions apply

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  shell/shellcorona.cpp

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

Re: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

> Couldn't we teach each Swoosh how to have its own set of favorites,
> recents etc, but also how to inherit the "standard" or "default" set?
> Then a Swoosh could be either an Activity or a Virtual Desktop.

So, configuration? That would work if it can be made pretty.

Cheers,
Ivan

Re: Discussion for Virtual Desktops and Activities future

[Nate Graham]

Couldn't we teach each Swoosh how to have its own set of favorites, 
recents etc, but also how to inherit the "standard" or "default" set? 
Then a Swoosh could be either an Activity or a Virtual Desktop.


Nate



On 07/17/2018 07:03 AM, Ivan Čukić wrote:

UI-wise:

We currently (let's pretend) have two options for the users (I've
replaced the terms activity and VD with 'swoosh' inspired by the
former Mozilla problem):
- have multiple swooshes where favourites, recents etc. are shared
- have multiple swooshes where favorties, recents are per-swoosh

Marco's proposal, for the sake of simplicity, wants to
- have multiple swooshes where favourits, recents etc. are per-swoosh,
just prettier

What's the benefit then? How would the concept be made clearer by this change?

Even pretending we have just two cases (since everyone thinks that the
group C does not exist), the proposed solution just erases one of
them.

I don't think that a bad implementation of something in kwin that was
created by a former Plasma developer and that none of us want to touch
is a good enough reason for removing a group of users.

I really don't see this as a concept simplification. Especially since
we tried to make no VDs, only activities to be the default. If the aim is
to force the users to use activities because they are cool, I think
we need a different aim.

If the problem is only that switching activities is not pleasant - no
desktop effects, etc. this is IMO the wrong way to tackle it.


Implementation-wise

In Plasma 5, KAMD is the only entity that manages activities for a
reason. We have had so many problems in Plasma 4 when Plasma wanted to
do the same thing that KAMD does. Just remember the 'let's create a
new activity for every user login' bug that we had.

Having KWin control the activities, while KAMD is managing activities
is a *bad* idea.

Cheers,
Ivan

Re: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

UI-wise:

We currently (let's pretend) have two options for the users (I've
replaced the terms activity and VD with 'swoosh' inspired by the
former Mozilla problem):
- have multiple swooshes where favourites, recents etc. are shared
- have multiple swooshes where favorties, recents are per-swoosh

Marco's proposal, for the sake of simplicity, wants to
- have multiple swooshes where favourits, recents etc. are per-swoosh,
just prettier

What's the benefit then? How would the concept be made clearer by this change?

Even pretending we have just two cases (since everyone thinks that the
group C does not exist), the proposed solution just erases one of
them.

I don't think that a bad implementation of something in kwin that was
created by a former Plasma developer and that none of us want to touch
is a good enough reason for removing a group of users.

I really don't see this as a concept simplification. Especially since
we tried to make no VDs, only activities to be the default. If the aim is
to force the users to use activities because they are cool, I think
we need a different aim.

If the problem is only that switching activities is not pleasant - no
desktop effects, etc. this is IMO the wrong way to tackle it.


Implementation-wise

In Plasma 5, KAMD is the only entity that manages activities for a
reason. We have had so many problems in Plasma 4 when Plasma wanted to
do the same thing that KAMD does. Just remember the 'let's create a
new activity for every user login' bug that we had.

Having KWin control the activities, while KAMD is managing activities
is a *bad* idea.

Cheers,
Ivan

Re: Virtual Desktop and Activities future: Partitioning proposal

[Roman Gilg]

On Tue, Jul 17, 2018 at 11:01 AM Marco Martin  wrote:
>
> On lunedì 16 luglio 2018 20:07:40 CEST Roman Gilg wrote:
> > --
> > libtaskmanager implementation:
> > --
> > * Listens for VD, active VD and VD identifier changes.
> > * Associates internally VDs with Activities.
>
> hmm, i feel this association would be more robust if it's in kamd?
True. libtaskmanager still needs to know which VD is associated with
which Activity (or at least with the current one) I assume. But maybe
kamd could tell this to libtaskmanager. One would probably need to
create a full data flow diagram to know if and how this could work out
in practice.

D14165: Handle keyboard in action selector OSD

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> OsdSelector.qml:136
>  Keys.onEscapePressed: clicked("dialog-cancel")
> +Keys.onReturnPressed: {
> +clicked(buttonRow.checkedButton.action)

Also do `Keys.onEnterPressed` so that enter on numpad works

> OsdSelector.qml:148
> +// check for checked property, the repeater is also a child 
> and must be skipped
> +if (event.key === Qt.Key_Left) {
> +do {

Make sure you take into account right-to-left languages (not sure if the layout 
mirrors then but it should, check by running the app with `-reverse` argument)

REPOSITORY
  R104 KScreen

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

To: gladhorn, #plasma
Cc: broulik, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14174: Fix blur behind folderview context menus

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:42a74c9a79ba: Fix blur behind folderview context menus 
(authored by davidedmundson).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14174?vs=37915=37940

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

AFFECTED FILES
  containments/desktop/plugins/folder/foldermodel.cpp

To: davidedmundson, #plasma, hein
Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

Re: Discussion for Virtual Desktops and Activities future

[David Edmundson]

On Tue, Jul 17, 2018 at 8:55 AM, Marco Martin  wrote:

> On lunedì 16 luglio 2018 12:44:13 CEST David Edmundson wrote:
> > As for my mod, in my head it's basically what you just said above.
> Clicking
> > + creates an activity which means KAMD then creates a N desktops.
> Clicking
> > - tells KAMD to delete the activity which in turn deletes N desktops.
> Might
>
> I think the reason is that i still don't understand the proposal
> completely.
> N desktop you mean if i have 4 virtual desktop it creates 4 desktops per
> activity, or it creates one and then is up to the user adding more?
>
> Yes, if you were to configure it to have 4 desktops per activity and you
add an activity you get 4 more desktops.

David

Re: Virtual Desktop and Activities future: Partitioning proposal

[Marco Martin]

On lunedì 16 luglio 2018 20:07:40 CEST Roman Gilg wrote:
> --
> libtaskmanager implementation:
> --
> * Listens for VD, active VD and VD identifier changes.
> * Associates internally VDs with Activities.

hmm, i feel this association would be more robust if it's in kamd?

-- 
Marco Martin

Re: Discussion for Virtual Desktops and Activities future

[Marco Martin]

On lunedì 16 luglio 2018 12:44:13 CEST David Edmundson wrote:
> As for my mod, in my head it's basically what you just said above. Clicking
> + creates an activity which means KAMD then creates a N desktops. Clicking
> - tells KAMD to delete the activity which in turn deletes N desktops. Might

I think the reason is that i still don't understand the proposal completely.
N desktop you mean if i have 4 virtual desktop it creates 4 desktops per 
activity, or it creates one and then is up to the user adding more?




-- 
Marco Martin

D14147: Port from GConf to GSettings

[David Rosca]

drosca added inline comments.

INLINE COMMENTS

> CMakeLists.txt:46
> +
> +if(USE_GCONF AND USE_GSETTINGS)
> +message(FATAL_ERROR "USE_GCONF and USE_GSETTINGS cannot be used at the 
> same time")

This should probably use cache variables:

  set(USE_GCONF FALSE CACHE STRING "Build with GConf")
  set(USE_GSETTINGS TRUE CACHE STRING "Build with GSettings")

> gsettingsitem.cpp:22
> +#include 
> +#include 
> +

not needed

> gsettingsitem.cpp:26
> +
> +#include "gsettingsitem.h"
> +#include "debug.h"

local includes should be the first in file

> gsettingsitem.cpp:40
> +case G_VARIANT_CLASS_STRING:
> +toReturn = QVariant(QString::fromUtf8(g_variant_get_string(gvalue, 
> NULL)));
> +break;

nullptr

> gsettingsitem.cpp:53
> +{
> +
> +// It might be hard to detect the right GVariant type from

newline

> gsettingsitem.cpp:63
> +switch (g_variant_type_peek_string(g_variant_get_type(oldValue))[0]) {
> +case G_VARIANT_CLASS_BOOLEAN:
> +newValue = g_variant_new_boolean(val.toBool());

indentation

> gsettingsitem.cpp:73
> +
> +if (newValue)
> +g_settings_set_value(m_settings, key.toLatin1().data(), newValue);

braces

> gsettingsitem.cpp:82
> +{
> +
> +m_settings = 
> g_settings_new_with_path("org.freedesktop.pulseaudio.module-group", 
> key.toLatin1().data());

newline

> gsettingsitem.cpp:91
> +g_settings_sync();
> +if (m_settings)
> +g_object_unref(m_settings);

braces

> gsettingsitem.h:34
> + public:
> +
> +explicit GSettingsItem(const QString , QObject *parent = nullptr);

newline

> gsettingsitem.h:41
> +
> +
> +Q_SIGNALS:

newline

> gsettingsitem.h:48
> +
> +static void settingChanged(GSettings *settings, const gchar *key, gpointer 
> data)
> +{

move to .cpp

> modulemanager.cpp:93
> +#if USE_GCONF || USE_GSETTINGS
>  
> +m_combineSinks = new ConfigModule(QStringLiteral("combine"), 
> QStringLiteral("module-combine"), this);

newline

REPOSITORY
  R115 Plasma Audio Volume Applet

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

To: nicolasfella, drosca, davidedmundson
Cc: rikmills, broulik, asturmlechner, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart