Re: Plasma repositories sprint cleaning

[Ivan Čukić]

On Friday, 3 March 2023 13:45:27 CET Nicolas Fella wrote:
> - lancelot: An alternative Plasma launcher. No releases, not much
> activity in recent years. If anyone cares about it it might be better of
> maintained outside of KDE to avoid impression that it is "officially"
> supported

This one can be moved to my personal (names)space on invent.

Lancelot stopped being developed since Plasma 4, with some ideas of a
revival in some form, which didn't happen. The repository contains a
few infrastructure things meant for it, but it has stagnated for quite some 
time now.

Cheers,
Ivan



-- 
dr Ivan Čukić
KDE e.V., ivan.cu...@kde.org, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

Re: C++/library requirements for Plasma 5.26

[Ivan Čukić]

> You might very well not be reading this correctly, as it's phrased very
> ambiguously:

Very likely. It is just that the ambiguous wording with the conclusion
didn't give me the confidence of the quality of implementation.

> However, C++20 also brought dozens of smaller language and library
> improvements that have been working just fine in both Clang and GCC even

Yes, many library features C++20 brought are nice gems. :)

> Ok, building and possibly also *testing* ;-)

Tests for async things are usually not that good/exhaustive even
for normal code that doesn't deal with experimental compiler
features.

If we go for coroutines, I'd just advise a slower and well-planned
adoption then - use it in a smaller, but often used part, and then
go from there. (I hate advocating slow adoption of something this
cool, but... :) )

Cheers,
Ivan

-- 
dr Ivan Čukić
ivan.cu...@kde.org, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

Re: C++/library requirements for Plasma 5.26

[Ivan Čukić]

Hi Nicolas,

While I've been known to request newer and newer compilers
for everything, and while I do support this request for GCC 10
and Clang 11, I'm not sure I'd support requiring QCoro and
coroutines.

Not because of QCoro itself, but that clang doesn't yet officially
support coroutines. According to [1] they have a partial support
since version 8, and according to [2] they might lead to problems
when coroutines might be resumed on a wrong thread (if I'm
reading this correctly), with a comment 'This feature requires
further analysis of the C++ Standard to determine what work is
necessary for conformance.'

Clang development seems to have slowed down a bit so I'm
not sure how viable is pushing C++20 to Plasma just yet.
Especially since most of us compile with GCC - if we are able
to overlook bugs that our patches introduce for dark-theme
users because we use and test on the light one, I can't dare
to imagine issues we can introduce to Clang-based distros. :)

Cheers,
Ivan

p.s. From a personal standpoint, yes, I'd really like to be able to
use QCoro in Vault...

[1] https://en.cppreference.com/w/cpp/compiler_support/20
[2] https://clang.llvm.org/cxx_status.html

-- 
dr Ivan Čukić
ivan.cu...@kde.org, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

Re: Request on the Kickoff replacement

[Ivan Čukić]

Hi David,

> The KickOff replacement is a big change. I adapted to KickOff and now I
> have to adapt to the new one.

Just a small note - you don't have to - the old one is now on
store.kde.org as Legacy Kickoff [1]

Cheers,
Ivan

[1] https://store.kde.org/p/1468103/

-- 
dr Ivan Čukić
i...@cukic.co, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

Re: Activities (problems & proposals)

[Ivan Čukić]

is an action too annoying to do every
> time, so the users ends up ignoring that.

With the issues of start/stop, I actually like most users ignoring this.


>A new widget that
> 
>a) Displays all user activities and up to four app for each one (after
> which the list becomes scrollable or visible on hover)

Can you provide a mockup of this?


> f) Hovering on activity icon would expose its name on a tooltip, and
> hovering an app would show its title in a tooltip

Just in the case this is not known - for the default activity applet, you can
configure it to show the current activity icon and name in the panel (if it is 
a horizontal panel).


>This widget could be shipped by default between kickoff and TM. It would
> take almost no space by default, since there would only be the default
> activity and a + button. There would also be a ? button that, when pressed,

Don't count on this. People thought that even a single icon is too much to
show by default.


Martin wrote:
> The window menu approach is quite awkward. I wonder whether dragging
> a window onto the widget which then would open a sidebar with all 

Dragging the window was not possible so the dragging from task manager to
the applet was introduced. But, it seems to have problems for Niccolo.


Cheers,
Ivan

-- 
dr Ivan Čukić
i...@cukic.co, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

Re: Gitlab and plasma dev

[Ivan Čukić]

> Somehow it's possible to create a label that covers all of Plasma as the
> VDG have one already.
> 
> Would that workflow work for other devs?

Sounds cool.

The thing that I found annoying is that only one person can be assigned to 
review. One thing that I was on the verge of doing is to @people that I'd like 
to notice the review in the comment to the MR.

Cheers,
Ivan


-- 
dr Ivan Čukić
i...@cukic.co, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

Re: Plasma Firewall

[Ivan Čukić]

Hi Kai,


This is something that Plasma has been missing for a long time. Thanks to 
everyone involved!


A few comments from a quick run-through over kcm/core:

# Style issues

- Some enums use MACRO_STYLE, some NormalStyle

- Some one-line if bodies in the same file have {}, some do not

- Double plural interfaces_names 
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/firewallclient.cpp#L56

- Personal preference - error handling in the if instead of out. This:
  if (!m_currentBackend) { return nullptr; }
  return proper;
instead of this:
  if (m_currentBackend) { return proper; }
  return nullptr;

- Missing m_ prefix for member variables:
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
profile.h#L108

- s/getRule/ruleAt/ - we don't usually have 'get' in the API if we can avoid 
it: https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
ifirewallclientbackend.h#L50

- Could use a pipe through clang-format
   

# Non-style issues:

- Should be ||, not && in:
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
rulelistmodel.cpp#L37

- Can be a const or a ref-to-const, no need for qAsConst after:
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
firewallclient.cpp#L58

- Should add m_currentBackend = nullptr; after delete
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
firewallclient.cpp#L248
(otherwise you get UB on 270 if no plugins have been found)

- Logic-wise - it would be better to redesign (if possible) not to allow 
m_currentBackend == nullptr instead of asserting and checking in each function
(for example, to show a UI that can not call anything in the backend is no 
backend is loaded)

- Wrong order - sort m_profiles and then replace it with a new contents?
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
ifirewallclientbackend.cpp#L36

- cbegin/cend instead of begin/end:
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
ifirewallclientbackend.cpp#L42 

- [] instead of [name]
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
ifirewallclientbackend.cpp#L43

- mark member functions that can be const as const
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
ifirewallclientbackend.h#L89

- else if for chained checks of reader.name() == something
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
profile.cpp#L163

- !...isEmpty is not needed - adds.contains(":") covers !...isEmpty
https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
rule.cpp#L46

- Assigning long-lived parents is naughty: https://invent.kde.org/tcanabrava/
plasma-firewall/-/blob/master/kcm/backends/ufw/ufwclient.cpp#L318



So much from me at this point.

Cheers,
Ivan



-- 
dr Ivan Čukić
i...@cukic.co, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

Re: Gitlab and plasma dev

[Ivan Čukić]

On Monday, 18 May 2020 16:46:57 CEST Harald Sitter wrote:
> On Mon, May 18, 2020 at 3:48 PM Nate Graham  wrote:
> > Personally I would prefer it if we don't automatically send everything
> > to the mailing list. That way people can subscribe to only what they
> > really care about using the web UI, which is very simple and
> > straightforward--and then the mailing list can just be used for
> > discussions and announcements. That would help the signal-to-noise ratio
> > stay high for the mailing list.
> 
> +1

A long long time ago, this (the current setup) was chosen because everyone 
ignored that 'other' mailing list with the idea that at least one person on 
the list should always react to some mail (bug or a review request).

It, obviously, didn't work (*), so +1 from me as well for abandoning the 
current behavior.

Cheers,
Ivan

(*) for me at least, it had an opposite effect to the desired


-- 
dr Ivan Čukić
i...@cukic.co, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

D29679: Dedicated drop areas for assigning tasks to activities

[Ivan Čukić]

ivan closed this revision.

REPOSITORY
  R119 Plasma Desktop

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

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

D29595: Add gocryptfs support

[Ivan Čukić]

ivan added a comment.


  Done. Thanks again for the patch!

REPOSITORY
  R845 Plasma Vault

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

To: mpilia, ivan, #plasma
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29595: Add gocryptfs support

[Ivan Čukić]

ivan closed this revision.

REPOSITORY
  R845 Plasma Vault

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

To: mpilia, ivan, #plasma
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29679: Dedicated drop areas for assigning tasks to activities

[Ivan Čukić]

ivan created this revision.
ivan added reviewers: Plasma, ngraham, mart.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ivan requested review of this revision.

REVISION SUMMARY
  Instead of using a difficult to discover 'move to activity by default,
  copy to activity when Control is pressed', show two drop areas when the
  mouse hovers an activity item in the switcher.
  
  If the user drops the task onto the left area, the task will be moved
  to that activity. If the user drops the task onto the right area, the
  task will be assigned to the new activity without removing it from the
  current one.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

AFFECTED FILES
  desktoppackage/contents/activitymanager/ActivityItem.qml
  desktoppackage/contents/activitymanager/TaskDropArea.qml
  imports/activitymanager/switcherbackend.cpp
  imports/activitymanager/switcherbackend.h

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

D29595: Add gocryptfs support

[Ivan Čukić]

ivan added inline comments.

INLINE COMMENTS

> mpilia wrote in vaultwizardbase.h:65
> Of course, sorry. For some reason I though 1 was highest priority.

Heh, it was my first impression when the priority patch was contributed to 
Vaults. Now I'm extra careful :)

REPOSITORY
  R845 Plasma Vault

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

To: mpilia, ivan, #plasma
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29595: Add gocryptfs support

[Ivan Čukić]

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


  Great work!

INLINE COMMENTS

> vaultwizardbase.h:65
> +{ "cryfs", 2 },
> +{ "gocryptfs", 3 },
>  };

Lower the priority to 1.

I don't want to have gocryptfs as the default. At least for the time being.

REPOSITORY
  R845 Plasma Vault

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

To: mpilia, ivan, #plasma
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27353: Allow Activity Switcher to move/add windows to activities by drag and drop from the taskbar

[Ivan Čukić]

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


  Superseeded by https://phabricator.kde.org/D29548 since there was no activity 
for >2 months.

REPOSITORY
  R119 Plasma Desktop

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

To: wstephenson, #plasma, ivan, davidedmundson
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29548: Drag and drop assigning windows to activities in the default switcher

[Ivan Čukić]

ivan closed this revision.

REPOSITORY
  R119 Plasma Desktop

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

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

D29548: Drag and drop assigning windows to activities in the default switcher

[Ivan Čukić]

ivan created this revision.
ivan added reviewers: Plasma, mart, davidedmundson, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ivan requested review of this revision.

REVISION SUMMARY
  When dragging a window from the taskbar onto the activity switcher
  applet, the switcher will pop up showing all the activities. When the
  user drops the task onto an activity, it will be moved to said activity.
  
  Alternatively, if the Control key is pressed, the window will be
  assigned to the destination activity as well as the current one (copy).

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

AFFECTED FILES
  applets/showActivityManager/package/contents/ui/main.qml
  desktoppackage/contents/activitymanager/ActivityItem.qml
  imports/activitymanager/CMakeLists.txt
  imports/activitymanager/switcherbackend.cpp
  imports/activitymanager/switcherbackend.h

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

D29379: Unassign activity from vault on activity deletion

[Ivan Čukić]

ivan closed this revision.

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

To: ivan, ngraham, davidedmundson, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29379: Unassign activity from vault on activity deletion

[Ivan Čukić]

ivan updated this revision to Diff 81780.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29379?vs=81779=81780

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

AFFECTED FILES
  kded/service.cpp
  kded/service.h

To: ivan, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29379: Unassign activity from vault on activity deletion

[Ivan Čukić]

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

REVISION SUMMARY
  CCBUG: 420784

REPOSITORY
  R845 Plasma Vault

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  kded/service.cpp
  kded/service.h

To: ivan, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

Re: Information regarding upcoming Gitlab Migration: clarifications

[Ivan Čukić]

> We have made a big fuss in the past about having different projects
> that do the same thing and now we'll have that but also we'll have
> several projects with the same name?
> It really feels off to me and I wonder if this is related to the move to
> gitlab.

+1 to both sentiments - that projects should have different names and that 
this is a bit off topic for the gitlab migration.

Cheers,
Ivan



-- 
dr Ivan Čukić
i...@cukic.co, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

D29110: [applet] Port to Kirigami.PlaceholderMessage

[Ivan Čukić]

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


  Fair enough.

REPOSITORY
  R845 Plasma Vault

BRANCH
  port-to-PlaceholderMessage (branched from master)

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

To: ngraham, #vdg, #plasma, ivan, niccolove
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29130: [Applet] Show in system tray iff at least one vault is open

[Ivan Čukić]

ivan added a comment.


  And I'l just be using my keyboard shortcut ;)

REPOSITORY
  R845 Plasma Vault

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

To: ksmanis, #plasma, #vdg, ivan
Cc: kmaterka, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D29130: [Applet] Show in system tray iff at least one vault is open

[Ivan Čukić]

ivan added a comment.


  I'm not fond of this change, but I agree with Nate's comment about the 
possibility of setting 'always visible' for users that wish to keep the current 
behaviour, so I'm not going to give this a -1.
  
  I'd still like the current behaviour kept, maybe with a different icon to 
show when there is a vault that is currently open to remedy one of the reasons 
behind this patch.
  
  But, again, this is a personal preference and I'm not going to block the 
patch if everybody else decides it is a good idea.

REPOSITORY
  R845 Plasma Vault

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

To: ksmanis, #plasma, #vdg, ivan
Cc: kmaterka, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D29110: [applet] Port to Kirigami.PlaceholderMessage

[Ivan Čukić]

ivan added a comment.


  Not sure I like pulling in Kirigami just for this. Are there other places 
where Kirigami could improve the Vaults applet?

REPOSITORY
  R845 Plasma Vault

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

To: ngraham, #vdg, #plasma, ivan, niccolove
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28333: Add a new daemon for stats monitoring

[Ivan Čukić]

ivan added inline comments.

INLINE COMMENTS

> client.cpp:65-66
> +m_connections.insertMulti(sensor, connect(sensor, 
> ::sensorInfoChanged, this, [this, sensor]() {
> +auto newInfo = sensor->info();
> +m_pendingMetaDataChanges[sensor->path()] = newInfo;
> +}));

... = sensor->info();

> client.cpp:79-80
> +for (const QString  : sensorPaths) {
> +auto sensor = m_subscribedSensors.take(sensorPath);
> +if (sensor) {
> +disconnect(m_connections.take(sensor));

if (auto sensor = ...)

> ksysguarddaemon.cpp:74
> +auto pluginObjects = KPluginLoader::instantiatePlugins("ksysguard", 
> nullptr, this);
> +std::for_each(pluginObjects.constBegin(), pluginObjects.constEnd(), 
> [=](QObject *plugin) {
> +auto factory = qobject_cast(plugin);

`[=]` is evil

> ksysguarddaemon.cpp:111-113
> +for (auto c : m_containers) {
> +for(auto o : c->objects()) {
> +for (auto p : o->sensors()) {

clang-format

> ksysguarddaemon.cpp:161
> +if (SensorProperty *sensorProperty = findSensor(sensorId)) {
> +const QVariant value = sensorProperty->value();
> +if (value.isValid()) {

I'm not going to be bold enough to propose

  if (const auto variant = ...; variant.isValid()) { }

> ksysguarddaemon.cpp:172-173
> +{
> +int subsystemIndex = path.indexOf('/');
> +int propertyIndex = path.lastIndexOf('/');
> +

const int, const int

REPOSITORY
  R106 KSysguard

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

To: davidedmundson, #plasma
Cc: ivan, mart, zzag, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra

D28333: Add a new daemon for stats monitoring

[Ivan Čukić]

ivan added inline comments.

INLINE COMMENTS

> main.cpp:45
> +m_property1->setMin(0);
> +m_property1->setMin(100);
> +m_property1->setShortName("Some Sensor 1");

`setMax`?

> client.cpp:38
> +{
> +connect(m_daemon, ::sensorRemoved, this, [=](const 
> QString ) {
> +m_subscribedSensors.remove(sensor);

s/=/this/

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0806r2.html

> client.cpp:56
> +auto sensor = m_daemon->findSensor(sensorPath);
> +if (sensor) {
> +m_connections[sensor] = connect(sensor, 
> ::valueChanged, this, [this, sensor]() {

Maybe:

  if(auto sensor = m_daemon->findSensor(sensorPath)) {
  ...
  }

REPOSITORY
  R106 KSysguard

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

To: davidedmundson, #plasma
Cc: ivan, mart, zzag, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra

Re: Update on Status of Gitlab Migration

[Ivan Čukić]

Hi all,

While I do like the invent name, I agree there should be a redirect of some 
sort from git.kde.org to it.

The aforementioned Debian salsa server has one as well - https://
git.debian.org/ - a message stating that the new server is 'salsa.debian.org'

Cheers,
Ivan


-- 
dr Ivan Čukić
i...@cukic.co, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

D28798: Make warning and error messages grammatically correct

[Ivan Čukić]

ivan added a comment.


  Maybe "Unable to close the vault; it is being used by %1" -> "Unable to close 
the vault as it is being used by %1" ?
  
  "Unable to close the vault; an application is using it" -> "Unable to close 
the vault because an application is using it"
  
  While writing for Manning, I was told to avoid semicolons and try to rephrase 
the sentence instead.

REPOSITORY
  R845 Plasma Vault

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

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

D27353: Allow Activity Switcher to move/add windows to activities by drag and drop from the taskbar

[Ivan Čukić]

ivan added a comment.


  Any news on this?

REPOSITORY
  R119 Plasma Desktop

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

To: wstephenson, #plasma, ivan, davidedmundson
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

ivan added a comment.


  > used Layouts instead of Anchors
  
  I know, I'm oldschool - when I was young, we didn't have QML, Kirigami and 
... :)

REPOSITORY
  R119 Plasma Desktop

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

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

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:36e322792002: Add the option to show the current activity 
name and icon (authored by ivan).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28136?vs=78018=78021

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

AFFECTED FILES
  applets/showActivityManager/package/contents/config/config.qml
  applets/showActivityManager/package/contents/config/main.xml
  applets/showActivityManager/package/contents/ui/ConfigAppearance.qml
  applets/showActivityManager/package/contents/ui/main.qml

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

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

ivan added a comment.


  Last change - use "activities" icon if no icon is set (though this should not 
happen if the user creates the activity through legal ways)

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

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

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

ivan updated this revision to Diff 78018.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28136?vs=78014=78018

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

AFFECTED FILES
  applets/showActivityManager/package/contents/config/config.qml
  applets/showActivityManager/package/contents/config/main.xml
  applets/showActivityManager/package/contents/ui/ConfigAppearance.qml
  applets/showActivityManager/package/contents/ui/main.qml

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

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

ivan marked 3 inline comments as done.
ivan added a comment.


  Done

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

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

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

ivan updated this revision to Diff 78014.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28136?vs=78006=78014

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

AFFECTED FILES
  applets/showActivityManager/package/contents/config/config.qml
  applets/showActivityManager/package/contents/config/main.xml
  applets/showActivityManager/package/contents/ui/ConfigAppearance.qml
  applets/showActivityManager/package/contents/ui/main.qml

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

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

ivan added a comment.


  Kirigami has quite interesting features I must say :)

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

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

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

ivan updated this revision to Diff 78006.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28136?vs=77985=78006

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

AFFECTED FILES
  applets/showActivityManager/package/contents/config/config.qml
  applets/showActivityManager/package/contents/config/main.xml
  applets/showActivityManager/package/contents/ui/ConfigAppearance.qml
  applets/showActivityManager/package/contents/ui/main.qml

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

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

ivan added a comment.


  The reason for the icon setting is to have full backwards compatibility. 
Maybe someone likes the static icon better than random icons they set for each 
activity. I don't  like to have everything configurable, but here it is not a 
significant maintainance overhead, and the configuration dialogue would be 
quite dull with a single checkbox :)

REPOSITORY
  R119 Plasma Desktop

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

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

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

ivan added a comment.


  Screenshot:
  
  F8184234: Screenshot_20200319_112003.png 


REPOSITORY
  R119 Plasma Desktop

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

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

D28136: Add the option to show the current activity name and icon

[Ivan Čukić]

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

REVISION SUMMARY
  The current activity switching applet shows a generic icon that
  opens the activity switcher which does not offer any information
  on the current activity. This does not go well with other applets
  like desktop pager.
  
  This patch adds options (on by default) to show the current activity
  icon and name in the applet.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  ivan/activity-switcher-applet-name-and-icon

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

AFFECTED FILES
  applets/showActivityManager/package/contents/config/config.qml
  applets/showActivityManager/package/contents/config/main.xml
  applets/showActivityManager/package/contents/ui/ConfigAppearance.qml
  applets/showActivityManager/package/contents/ui/main.qml

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

D28099: Show command output on unknown failure

[Ivan Čukić]

This revision was automatically updated to reflect the committed changes.
Closed by commit R845:1e2988d7d3ca: Show command output on unknown failure 
(authored by ivan).

REPOSITORY
  R845 Plasma Vault

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28099?vs=77828=77836

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

AFFECTED FILES
  .gitignore
  kded/engine/backends/cryfs/cryfsbackend.cpp
  kded/engine/commandresult.cpp
  kded/engine/commandresult.h
  kded/engine/fusebackend_p.cpp
  kded/ui/mountdialog.cpp
  kded/ui/mountdialog.h

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

D28099: Show command output on unknown failure

[Ivan Čukić]

ivan updated this revision to Diff 77828.

REPOSITORY
  R845 Plasma Vault

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28099?vs=77822=77828

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

AFFECTED FILES
  .gitignore
  kded/engine/backends/cryfs/cryfsbackend.cpp
  kded/engine/commandresult.cpp
  kded/engine/commandresult.h
  kded/engine/fusebackend_p.cpp
  kded/ui/mountdialog.cpp
  kded/ui/mountdialog.h

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

D28099: Show command output on unknown failure

[Ivan Čukić]

ivan added a comment.


  Of course... I need to reboot my brain it seems.

REPOSITORY
  R845 Plasma Vault

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

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

D28099: Show command output on unknown failure

[Ivan Čukić]

ivan created this revision.
ivan added reviewers: Plasma, davidedmundson, ngraham, mart.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ivan requested review of this revision.

REVISION SUMMARY
  If the error is unknown (all errors in encfs), show the 'details...' button
  which shows the command line output to the user when activated.
  
  BUG:412779

REPOSITORY
  R845 Plasma Vault

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  kded/engine/backends/cryfs/cryfsbackend.cpp
  kded/engine/commandresult.cpp
  kded/engine/commandresult.h
  kded/engine/fusebackend_p.cpp
  kded/ui/mountdialog.cpp
  kded/ui/mountdialog.h

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

D28069: Support execution of shell scripts on activity events

[Ivan Čukić]

This revision was automatically updated to reflect the committed changes.
Closed by commit R161:9a36c4da7d7a: Support execution of shell scripts on 
activity events (authored by ivan).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D28069?vs=77722=77724#toc

REPOSITORY
  R161 KActivity Manager Service

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28069?vs=77722=77724

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

AFFECTED FILES
  src/service/plugins/runapplication/RunApplicationPlugin.cpp

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

D28069: Support execution of shell scripts on activity events

[Ivan Čukić]

ivan updated this revision to Diff 77722.

REPOSITORY
  R161 KActivity Manager Service

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28069?vs=77715=77722

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

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/CheckCxxFeatures.cmake
  cmake/modules/c++-test-override-attr-none-fail.cpp
  cmake/modules/c++-test-override-attr-none.cpp
  cmake/modules/c++11-test-auto-N2546.cpp
  cmake/modules/c++11-test-initializer-lists-N2672.cpp
  cmake/modules/c++11-test-lambda-N2927.cpp
  cmake/modules/c++11-test-nullptr-N2431-fail.cpp
  cmake/modules/c++11-test-nullptr-N2431.cpp
  cmake/modules/c++11-test-override-N3206-fail.cpp
  cmake/modules/c++11-test-override-N3206.cpp
  cmake/modules/c++11-test-unique_ptr-none.cpp
  cmake/modules/c++11-test-variadic-templates-N2242.cpp
  src/CMakeLists.txt
  src/service/plugins/runapplication/RunApplicationPlugin.cpp

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

D28069: Support execution of shell scripts on activity events

[Ivan Čukić]

ivan created this revision.
ivan added reviewers: davidedmundson, mart.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ivan requested review of this revision.

REVISION SUMMARY
  In addition to opening a file and starting an application specified
  by a .desktop file, allow execution of shell scripts when activity
  is started/stopped/switched to/switched from.
  

  
  This patch also removes CMake checks for C++11 features and increases
  the compiler requirements to C++17 since this repository is not a
  part of KDE Frameworks.

REPOSITORY
  R161 KActivity Manager Service

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

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/CheckCxxFeatures.cmake
  cmake/modules/c++-test-override-attr-none-fail.cpp
  cmake/modules/c++-test-override-attr-none.cpp
  cmake/modules/c++11-test-auto-N2546.cpp
  cmake/modules/c++11-test-initializer-lists-N2672.cpp
  cmake/modules/c++11-test-lambda-N2927.cpp
  cmake/modules/c++11-test-nullptr-N2431-fail.cpp
  cmake/modules/c++11-test-nullptr-N2431.cpp
  cmake/modules/c++11-test-override-N3206-fail.cpp
  cmake/modules/c++11-test-override-N3206.cpp
  cmake/modules/c++11-test-unique_ptr-none.cpp
  cmake/modules/c++11-test-variadic-templates-N2242.cpp
  src/CMakeLists.txt
  src/service/plugins/runapplication/RunApplicationPlugin.cpp

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

D28035: [Applet] Port to ExpandableListItem

[Ivan Čukić]

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


  I assume people tell you that you're awesome all the time, so I'm not going 
to ;)

REPOSITORY
  R845 Plasma Vault

BRANCH
  port-to-ExpandableListItem (branched from master)

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

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

D28035: [Applet] Port to ExpandableListItem

[Ivan Čukić]

ivan added a comment.


  I like it. Is the device notifier going to look like this as Vaaults tried to 
mimic that one?

REPOSITORY
  R845 Plasma Vault

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

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

D27353: Allow Activity Switcher to move/add windows to activities by drag and drop from the taskbar

[Ivan Čukić]

ivan added a comment.


  This looks cool. The thing I'm missing (correct me if I'm wrong - I'm yet to 
test the patch) is for it to open the switcher when dragging the window over 
the switcher applet icon.

REPOSITORY
  R119 Plasma Desktop

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

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

D27322: Allow move or add window to Activities during Dnd using Ctrl modifier

[Ivan Čukić]

ivan added a comment.


  Cool idea, I'll have to think of something similar for the regular activity 
switcher

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

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

D26447: Dynamically show and hide based on whether or not any vaults are configured

[Ivan Čukić]

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


  That is why I put the "or in addition to" part.
  
  Anyhow, I don't mind this to go in as is.

REPOSITORY
  R845 Plasma Vault

BRANCH
  dynamically-show-and-hide (branched from master)

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

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

D26111: KRunner add a recentlyused runner accessing KActivityStats data

[Ivan Čukić]

ivan added a comment.


  > Why not change the recentdocuments runner?
  
  Looks fine to me, I just don't see a response to this comment.

REPOSITORY
  R120 Plasma Workspace

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

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

D26438: Runners: Convert foreach to for

[Ivan Čukić]

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


  @meven
  
  I hate Qt... defining `operator*` without `operator->` ... ugh
  
  I've approved the patch, sorry for the bad nitpick :)

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

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

D26438: Runners: Convert foreach to for

[Ivan Čukić]

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


  @davidedmundson
  
  Yes, the wrapper just serves for keyval iterators to be usable with a 
range-based for loop. Quite useful if there are a lot of these. In this case, 
where it is only one loop I agree it might be an overkill.
  
  @meven
  
  Just a single nit-pick left :)

INLINE COMMENTS

> fetchsqlite.cpp:101
> +for (auto entry = bindObjects.constKeyValueBegin(); entry != 
> bindObjects.constKeyValueEnd(); ++entry) {
> +query.bindValue((*entry).first, (*entry).second);
>  //qDebug() << "* Bound " << variableName << " to " << 
> query.boundValue(variableName);

`entry->first` and `entry->second`

REPOSITORY
  R120 Plasma Workspace

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

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

D26438: Runners: Convert foreach to for

[Ivan Čukić]

ivan added a comment.


  Something like this:
  
template 
class asRangeImpl
{
public:
asRange(const T )
: m_data{data}
{
}

auto begin() const { return m_data.keyValueBegin(); }
auto end() const { return m_data.keyValueEnd(); }

private:
const T _data;
};

template 
class asRange(const T& map)
{
return asRangeImpl(map);
}

REPOSITORY
  R120 Plasma Workspace

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

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

D26438: Runners: Convert foreach to for

[Ivan Čukić]

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

INLINE COMMENTS

> davidedmundson wrote in fetchsqlite.cpp:100
> There's an optimisation available here

This can be made much better by providing a custom range object to iterate from 
`keyValueBegin()` to `keyValueEnd()` - converting to `std::map` is quite 
inefficient.

REPOSITORY
  R120 Plasma Workspace

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

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

D26447: Dynamically show and hide based on whether or not any vaults are configured

[Ivan Čukić]

ivan added a comment.


  For some reason, hiding this icon when there are no vaults was deemed 
undesired before. I don't recall why as I think it is a good idea. :)
  
  Maybe it was always shown for discoverability purposes... don't know.
  
  Is there a reason why this is a `count` instead of (or in addition to) just 
being `isEmpty` or something similar since the actual count is not used?

REPOSITORY
  R845 Plasma Vault

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

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

D22005: Ensure url from KActivity resources are properly built

[Ivan Čukić]

ivan added inline comments.

INLINE COMMENTS

> backend.cpp:322
>  while (actionCount < 5 && resultIt != results.end()) {
> -const QString resource = (*resultIt).resource();
> +QString resource = (*resultIt).resource();
> +if (QDir::isAbsolutePath(resource)) {

`resultIt->resource()` (when you are at it :) )

REPOSITORY
  R119 Plasma Desktop

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

To: meven, #plasma, hein, broulik
Cc: ivan, meven, 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

D22382: Add global shortcuts for switching to the previous/next activity

[Ivan Čukić]

ivan added a comment.


  I'm not thrilled about having default shortcuts set. We have the shortcuts 
space quite polluted as it is.
  
  Especially since we have the meta+tab with different semantics.
  
  Maybe the text should explicitly state what 'next' means in this case.

REPOSITORY
  R120 Plasma Workspace

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

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

D22381: Add previous-/nextActivity methods

[Ivan Čukić]

ivan added inline comments.

INLINE COMMENTS

> Activities.cpp:56
> +inline
> +bool nameBaseOrdering(const ActivityInfo , const ActivityInfo 
> )
> +{

Nitpick Base -> Based

> Activities.cpp:251
> +
> +for (int i = 0; i < a.count(); ++i) {
> +if (a[i] == currentActivity) {

Can be done with `std::find_if` as well

> Activities.cpp:333
>  
> +for (int i = 0; i < sortedActivities.count(); ++i) {
> +if (sortedActivities[i].id == activity) {

This can be done with `std::find_if` and then remove with iterator.

REPOSITORY
  R161 KActivity Manager Service

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

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

D23737: Extract mimetype from recentlyused.xbel

[Ivan Čukić]

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

INLINE COMMENTS

> GtkEventSpy.cpp:207
>  
> -void GtkEventSpyPlugin::addDocument(const QUrl , const QString 
> )
> +void GtkEventSpyPlugin::addDocument(const QUrl , const QString 
> , const QString mimetype)
>  {

const-ref not const val - `const QString& mimetype`

REPOSITORY
  R161 KActivity Manager Service

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

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

D23112: Add a event Spy for GtkFileChooser recent files

[Ivan Čukić]

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


  Another tiny change, and I think it is ready to land. Unless someone else 
sees other issues.

INLINE COMMENTS

> GtkEventSpy.cpp:126
> +
> +if (exec.at(0) == QStringLiteral("'") && exec.at(exec.size() - 1) == 
> QStringLiteral("'")) {
> +// remove "'" caracters wrapping the command

No need to compare chars with strings:

  if (!exec.isEmpty() && exec[0] == '\'' && exec.back() == '\'')

REPOSITORY
  R161 KActivity Manager Service

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

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

D23112: Add a event Spy for GtkFileChooser recent files

[Ivan Čukić]

ivan added inline comments.

INLINE COMMENTS

> GtkEventSpy.cpp:68
> +QDateTime visited;
> +QList *applications;
> +

No need for this to be a pointer to a list. Make it just `QList 
applications`.

> GtkEventSpy.cpp:70
> +
> +Bookmark(){
> +applications = new QList();

The constructor will not be needed once `applications` stops being a pointer.

> GtkEventSpy.cpp:76
> +
> +QString Bookmark::latestApplication() const {
> +Application current = applications->first();

`{` which starts a function should be on a new line (I don't care much about 
this, but let's follow the KF5 style)

> GtkEventSpy.cpp:77-78
> +QString Bookmark::latestApplication() const {
> +Application current = applications->first();
> +for (const Application  : qAsConst(*applications)) {
> +if (app.modified > current.modified) {

When you make `applications` not to be a pointer `qAsConst` will not be needed 
as this is a `const` member function.

> GtkEventSpy.cpp:89
> +public:
> +BookmarkHandler(){
> +current = nullptr;

Replace with:

  BookmarkHandler()
  : current(nullptr)
  {
  }

> GtkEventSpy.cpp:91
> +current = nullptr;
> +marks = QList();
> +}

No need for this - marks are already default-constructed.

> GtkEventSpy.cpp:113
> +if (qName == QStringLiteral("bookmark")) {
> +current = new Bookmark();
> +current->href = QUrl(attributes.value("href"));

No need for dynamic allocation. Make it a normal variable instead of a pointer.

> GtkEventSpy.cpp:185
> +reader.setErrorHandler();
> +QXmlInputSource *source = new QXmlInputSource();
> +

No need for dynamic allocation. Make it a normal variable instead of a pointer.

> meven wrote in GtkEventSpy.cpp:143
> It is to just extract the executable name, we don't want to have an exploding 
> number of initiatingAgent for every argument and parameter that might come 
> through here.

I meant it will be a problem if someone decides to have a space in the 
executable like `my\ aweomse\ binary`. But this should not be an issue at the 
moment.

REPOSITORY
  R161 KActivity Manager Service

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

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

D23112: Add a event Spy for GtkFileChooser recent files

[Ivan Čukić]

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


  Thanks for geting involved this much!

INLINE COMMENTS

> GtkEventSpy.cpp:129
> +// remove "'" caracters wrapping the command
> +exec = exec.mid(1, exec.size() -2 );
> +

It might be dangerous to assume that the command is wrapped with single quotes

> GtkEventSpy.cpp:133
> +// See 
> https://techbase.kde.org/Development/Tutorials/Services/Traders#The_KTrader_Query_Language
> +QString query = QString("exist Exec and ((Exec ~~ '%1') or (exist 
> GenericName and '%1' ~~ GenericName) or (exist Name and '%1' ~~ 
> Name))").arg(exec);
> +KService::List services = 
> KServiceTypeTrader::self()->query(QStringLiteral("Application"), query);

const auto query

> GtkEventSpy.cpp:134
> +QString query = QString("exist Exec and ((Exec ~~ '%1') or (exist 
> GenericName and '%1' ~~ GenericName) or (exist Name and '%1' ~~ 
> Name))").arg(exec);
> +KService::List services = 
> KServiceTypeTrader::self()->query(QStringLiteral("Application"), query);
> +

This should also be `const`, other local variables as well

> GtkEventSpy.cpp:143
> +// remove space and any caracter after
> +int spaceIdex = exec.indexOf(" ");
> +if (spaceIdex != -1) {

Let's hope commands will never have spaces in them :)

REPOSITORY
  R161 KActivity Manager Service

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

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

D22381: Add previous-/nextActivity methods

[Ivan Čukić]

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


  Inserting/Removing/Updating a sorted list does not need to resort every time 
- removing is easy, adding a new item is std::lower_bound (a binary search), 
and updating is a combination of the two.

INLINE COMMENTS

> Activities.cpp:54
>  
> +static
> +bool infoLessThan(const ActivityInfo , const ActivityInfo )

You can use anonymous namespace for this (instead of `static`) or just make it 
a non-static function.

It can be marked as `inline`, although the compiler will probably do that 
regardless of you saying so.

You can rename it to something like `nameBasedOrdering` - better communicates 
what it does.

> Activities.cpp:349
>  }
> +updateSortedActivityList();
>  

You can just find the activity in the list, and remove it - the order for the 
rest will not change.

> Activities_p.h:76
>  QHash activities;
> +QList sortedActivities;
>  QReadWriteLock activitiesLock;

`QList` -> `QVector`.

`QList` is an evil and slow class :)

REPOSITORY
  R161 KActivity Manager Service

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

To: muesli, ivan
Cc: ivan, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22381: Add previous-/nextActivity methods

[Ivan Čukić]

ivan added a comment.


  You are right, they are randomized.
  
  And, yes, that is what I meant on IRC - that redundancy due to keeping a 
sorted list will likely lead to future issues, but I see no other way if we 
want to be consistent regarding listing and switching.
  
  As it is required to have fast access by UUID, the hash needs to stay. So, 
I'd say we will need the redundancy. Maybe there is some multi-key sorted 
multi-map in boost - kamd already uses boost::flat_map, so this would be fine. 
:)

REPOSITORY
  R161 KActivity Manager Service

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

To: muesli, ivan
Cc: ivan, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22381: Add previous-/nextActivity methods

[Ivan Čukić]

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

INLINE COMMENTS

> Activities.cpp:218
> +auto a = q->ListActivitiesWithInformation();
> +std::sort(a.begin(), a.end(), );
> +

I don't like the fact that it constantly resorts the activities.

The second problem is that ListActivities returns a list in one order, and this 
traverses activities in another order. If this will work by name, probably the 
List functions should as well.

REPOSITORY
  R161 KActivity Manager Service

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

To: muesli, ivan
Cc: ivan, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D19997: Change http URLs to https

[Ivan Čukić]

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


  Accepted, but incomplete ;)

REPOSITORY
  R845 Plasma Vault

BRANCH
  master

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

To: vkrause, ivan
Cc: ivan, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19991: FileItem plugin for opening and closing Vaults from Dolphin

[Ivan Čukić]

ivan closed this revision.

REPOSITORY
  R845 Plasma Vault

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

To: ivan, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19991: FileItem plugin for opening and closing Vaults from Dolphin

[Ivan Čukić]

ivan updated this revision to Diff 54609.

REPOSITORY
  R845 Plasma Vault

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19991?vs=54607=54609

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

AFFECTED FILES
  CMakeLists.txt
  fileitemplugin/CMakeLists.txt
  fileitemplugin/plasmavaultfileitemaction.cpp
  fileitemplugin/plasmavaultfileitemaction.h
  fileitemplugin/plasmavaultfileitemaction.json

To: ivan, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19991: FileItem plugin for opening and closing Vaults from Dolphin

[Ivan Čukić]

ivan updated this revision to Diff 54607.

REPOSITORY
  R845 Plasma Vault

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19991?vs=54599=54607

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

AFFECTED FILES
  CMakeLists.txt
  fileitemplugin/CMakeLists.txt
  fileitemplugin/plasmavaultfileitemaction.cpp
  fileitemplugin/plasmavaultfileitemaction.h
  fileitemplugin/plasmavaultfileitemaction.json

To: ivan, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19991: FileItem plugin for opening and closing Vaults from Dolphin

[Ivan Čukić]

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

REVISION SUMMARY
  Adds a context menu item when the user right-clicks a directory that is a 
registered vaults mount point.
  
  BUG:388602

REPOSITORY
  R845 Plasma Vault

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

AFFECTED FILES
  CMakeLists.txt
  fileitemplugin/CMakeLists.txt
  fileitemplugin/plasmavaultfileitemaction.cpp
  fileitemplugin/plasmavaultfileitemaction.h
  fileitemplugin/plasmavaultfileitemaction.json

To: ivan, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

Reverted "[Activities KCM] vertically center the buttons"

[Ivan Čukić]

Hi all,

Just to notify you that I've reverted commit 
9cdfe25910cbbfbef09364ce3adba53bcc292853 because it broke the layout (see [1]) 
of the items in the Activities KCM.

Cheers,
Ivan

[1] https://ibin.co/4W7JHc5mZ64Q.jpg

-- 
dr Ivan Čukić
KDE, ivan.cu...@kde.org, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12

D17039: ASAN: Fix leak in KAStatsFavoritesModel

[Ivan Čukić]

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


  Ok, +2 from me then

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

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

D17039: ASAN: Fix leak in KAStatsFavoritesModel

[Ivan Čukić]

ivan added a comment.


  Using a shared pointer seems like a pessimization. Could this be done with 
std::unique_ptr?

REPOSITORY
  R119 Plasma Desktop

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

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

D16257: Use correct wording, since it is an action word(not a noun)

[Ivan Čukić]

ivan requested changes to this revision.
ivan added a comment.


  It is (also) a noun, though MW spells it as "pop-up" [1]
  
  [1] https://www.merriam-webster.com/dictionary/pop-up
  
  edit: Heh, Nate was faster :)

REPOSITORY
  R120 Plasma Workspace

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

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

D15011: [Kickoff] Make the search field always look like a search field

[Ivan Čukić]

ivan added a comment.


  Different search bars are on different levels of usefulness.
  
  Use-cases for searching activities are much rarer than, for example, 
searching widgets. The current activity switcher UI clearly shows the search 
button, so there's no problem with discoverability. While the switcher also 
provides a clickless-UI for people who like to keyboard-navigate.

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #plasma, #vdg, abetts
Cc: ivan, urohan, cfeck, acrouthamel, fabianr, huftis, rooty, sharvey, romangg, 
broulik, safaalfulaij, oysteins, filipf, abetts, davidedmundson, 
michaeltunnell, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D16087: RFC: Store full URL as URI

[Ivan Čukić]

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


  > Not sure what the implications are for this change, it does fix Dolphin 
showing folders in its
  
  This I don't like.
  
  KAMD stores file paths, not URIs for local files in the database.
  
  This needs more investigation of the consequences.

REPOSITORY
  R161 KActivity Manager Service

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

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

Re: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

> shouldn't be more, add an activity you get one new virtual desktop,
> then you can add more for that activity if you want?

+1 I guess that original David's comment was about when we thought all
activities would have the same number of VDs.

Cheers,
Ivan

-- 
KDE, ivan.cu...@kde.org, http://cukic.co/
gpg key fingerprint: 292F 9B5C 5A1B 2A2F 9CF3  45DF C9C5 77AF 0A37 240A

Re: Virtual Desktop and Activities future: Merging proposal

[Ivan Čukić]

What to do with X11?

I don't like the idea that we have two completely different behaviours
for W and X.

Cheers,
Ivan

Re: Virtual Desktop and Activities future: Merging proposal

[Ivan Čukić]

So, if I understood this correctly, KAMD keeps a list of activities,
and a list of VDs which are usually mapped 1-1, but could also be 1-n.

Adding an activity would mean adding a VD (just one by default).
Removing an activity would mean removing all VDs for that activity.

This could be (I think) easily done as another KAMD plugin. The plugin
could also provide the info about VDs that belong to the current
activity for 'advanced' versions of pager (if they get implemented by
a third party) etc.

This could work. Might end up even being better feature-wise than our
current setup. There were quite a few users who wanted different
number of VDs per-activity.

I think that all group "C" problems could be remedied with custom
Plasma applets.

There is one question, though. Do the new VDs support showing a window
on two different VDs?


Cheers,
Ivan




On Tue, Jul 24, 2018 at 1:40 PM Marco Martin  wrote:
>
> On Mon, Jul 16, 2018 at 11:47 PM David Edmundson
>  wrote:
> >  - If someone else wants to add new effects/pagers that group/filter VDs 
> > they should be able to.
> > (like Michails quite cool mockups and stuff) I don't think that requires 
> > any changes in the VD protocol to do so, and we definitely shouldn't block 
> > on it nor make it a default.
>
> I like this proposal.. after a basic version of virtual desktops
> working for 5.14 we can start with prototypes
>
> --
> Marco Martin



-- 
KDE, ivan.cu...@kde.org, http://cukic.co/
gpg key fingerprint: 292F 9B5C 5A1B 2A2F 9CF3  45DF C9C5 77AF 0A37 240A

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

[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: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

>  Work "stack" Home "stack"
> ||   ||
> |VD 1|   |VD 3|
> | Web browser|   | Web browser|
> | IDE|   |Music player|
> ||   ||
>
> ||   ||
> |VD 2|   |VD 4|
> |Email   |   |Konversation|
> |chat|   |  Telegram  |
> ||   ||

This has the same problem David's proposal has - if you have 5
activities and 3 VD for each, you get a pager with 15 different VDs.

> With a user interface that explicitly supports and encourages this sort of
> thing (e.g. the ability to apply current activity-specific features such to
> multiple Virtual Desktops; "next" and "previous" keyboard shortcuts
> automatically bound to the top row of each column), it might be sufficient.

This is really hackish. Currently, we have people who tie activity
switching to 'restore last used VD for an activity' and people who
just want to remain on the same VD.

Cheers,
Ivan


-- 
KDE, ivan.cu...@kde.org, http://cukic.co/
gpg key fingerprint: 292F 9B5C 5A1B 2A2F 9CF3  45DF C9C5 77AF 0A37 240A

Re: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

> I think kwin shouldn't filter them. Pager lists all, effects and shortcuts
> cover all.

Ugh... I think that will kill the group 'C'*. While it /would/ be
possible to to use multi-VD-multi-activity setup, it would be far from
convenient.

Or it would require an alternative set of applets that behave in a
more sane way then the default ones :)

Cheers,
Ivan

Re: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

@David

Regarding the backend (wherever the actual logic ends up in - kwin or
kamd), your idea does sound nice. It would also provide the ability
that some of the 'C' category users wanted - to have variable number
of VDs per activity.

But the problem is not primarily how to handle the 'unification'
implementation-wise, but UI-wise. UI is what we primarily need to be
concerned with.

One potential UI approach that came to me, which might be able to
cover all categories of users and which would fit David's proposal, is
the following,:

- Activities and VDs get mostly merged. The current activity switcher
+ VD animations end up there (with some improvements).

- Window organization part of VDs gets changed into "window groups" or
"window sets" where the windows can be grouped together. Switching the
groups could use the same effect as minimize/restore, but with a group
of windows. Or something along those lines.

This would provide the more obvious distinction - "groups" are for
window management and "activities" are about providing different
workspaces. "groups" would be volatile in the sense that their number
can vary a lot - quick creation, automatic destruction etc. (like
activities in gnome shell)

Now, this would require *a lot* of work, and while supporting all the
use-cases for activities/VDs I know of, it would change workflows of
many users.

OTOH, these components would need significant **coordinated** changes:
- Task manager
- Pager
- Activity-related applets
- KWin
- KWin rules (we still need to be able that certain applications need
to be always in same 'group')
- Plasma

And I'm not sure we could have a slow roll-out of the features across
several releases. This would need to be done all at once.

Cheers,
Ivan

Re: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

> And there would be quite a nice opportunity for a nice pr campaign.

IF (and that is a big "if") we can create a new feature (just don't
call it "Virtual Activated Desktops" :D) that will merge activities
and VDs in a sane way, which would not be overly limiting, I would be
more than happy.

It is the proposal 'make VDs and activities the same thing' that I
have a problem with.

Cheers,
Ivan

-- 
KDE, ivan.cu...@kde.org, http://cukic.co/
gpg key fingerprint: 292F 9B5C 5A1B 2A2F 9CF3  45DF C9C5 77AF 0A37 240A

Re: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

> Personally i always used virtual desktops exactly for that, as "activities",
> even before activities were a thing, "i need a new physical space to stuff
> new windows" is not an use case that ever occurred to me in any way
> noted that there are people which see virtual desktops like that.. will have
> to be taken into account somehow, still consider it a pretty crazy use
> case tough.

I'm going to be evil now. Wikipedia definition (not because W is
flawless, but mostly because the 'crazy use case' comment ;)

https://en.wikipedia.org/wiki/Virtual_desktop
In computing, a virtual desktop is a term used with respect to user
interfaces, usually within the WIMP paradigm, to describe ways in
which the virtual space of a computer's desktop environment is
expanded beyond the physical limits of the screen's display area
through the use of software. This compensates for a limited desktop
area and can also be helpful in reducing clutter.


> Ironically, to me activities never were really able to replace virtual
> desktops as "activities", because its window management part never
> got the love it deserved nad always lookedlike a kinda broken virtual
> desktop implementation, so always falled back to the "multiple
> workspaces" implementation kwin was really built around.. which is
> virtual desktops.

Now, while I agree with this statement, there is an "upside" to the
problematic WM-presentation of activity switching.

Activities are not meant to be switched overly often. In the way that
I see activities, you switch to an activity and work for at least half
an hour. If a project deserves less time to focus on than half an
hour, it is either a one of a kind project, or it does not deserve to
have a dedicated activity.

With this in mind, having a non-pretty switching (though, I did try to
make it prettier to an extent that kwin allows with the following
script https://store.kde.org/p/1110510/) is not a huge problem. But I
agree - it should be prettier.


>> Previously, we had problems when we tried to equate activities with any
>> specific thing. As in, 'an activity is a group of Plasma widgets'. The
>> same will happen if the activity becomes 'a group of windows'.
>>
>> This has been discussed quite a few times before.
>
> mostly because i think the whole concept of activities always was a
> blurry thing, because each one working on them had a different idea
> about them to begin with

This is also the truth - on two levels:
- Everyone in the Plasma team who dipped their toes into activities
had (at least slightly) different visions of what they are, I think
this is the case because of the second level:
- Activities *are* a blurry thing. I've seen so many different
workflows that people created with them.

While I have a clear vision of what activities are, I don't think we
can make them exactly that. The same goes for the vision that anyone
in Plasma team has.

We need to try to make one or two more focused usage patterns more
streamlined, but we can not make them *be* only that.

As an analogy, imagine what would happen if we said 'Dolphin is too
powerful, it allows users to have a really weird filesystem layouts,
lets force them to have 5 directories Pictures, Videos, ...'. Even if
this would fit a significant percentage of our user base, this is not
something that we can do.

We could make the main UI of Dolphin present the files like that while
still allowing normal file management (like most Android file managers
do), but we can not remove the 'powerful when needed' features
altogether.

Cheers,
Ivan

--
KDE, ivan.cu...@kde.org, http://cukic.co/
gpg key fingerprint: 292F 9B5C 5A1B 2A2F 9CF3  45DF C9C5 77AF 0A37 240A

Re: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

David> Are you (Michail and Ivan) at Akademy?

Not this year, I'm taking a rest from socializing ;)

Nate> very different things: Virtual Desktops are for window organization
Nate> within the current set of tasks, and Activities are for higher-level

This is exactly my view of VD vs activities.

Nate> both have similar animated transitions; one has an accessible user
Nate> interface and a keyboard shortcut but the other one doesn't; one can

Can you elaborate? With transitions, you mean
plasma-widgets-and-wallpaper-slide in activities vs windows-slide
transition with VDs?

That is the only similarity between the UIs as far as I see.

> I'm willing to experiment with combining them to improve the user

Experimenting is good. I'd love to see a unified UI that would make
activities and VDs 'obvious' while still being good for
only-activities or only-vd setups.

We had an experiment (and a full implementation) back in the 5.1 days
[1] that was meant to provide a nicer UI for activity handling (which
might have confused people regarding VD relation to activities even
more :) ). But the ideas we had to add VD support to it were quite
problematic. The UI was quite busy as it was.

Since then, I got a bit disillusioned wrt the unified UI idea, and I'm
currently leaning more to an activity UI that is document-focused
(favorites, recents, ...) instead of the window-focused one.

But I haven't come up yet with anything that looks manageable.

Michail>

Backend-wise, B1 sounds like a great idea. I see not technical reason
why it would need to be either-or. If a window can have a list of VDs
assigned, that list might contain activities as well. The matching
would be as simple as 'current VD is in the list' && 'current activity
is in the list'.

Or, everything could share the same code, while there would be two
separate lists (this would be cleaner IMO).

Cheers,
Ivan

[1] https://youtu.be/uxaDaXW67Oo?t=41s

-- 
KDE, ivan.cu...@kde.org, http://cukic.co/
gpg key fingerprint: 292F 9B5C 5A1B 2A2F 9CF3  45DF C9C5 77AF 0A37 240A

Re: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

> I think this is a fair recap so far:

The recap sounds great. 

> When Martin and I started talking about the
> Wayland protocol, we
> were keen to do work that would reusable 
> for either use case. This

I especially like this part. As I said, merging the implementation (at least to 
some extent) is a great idea.


> I think it's a reality that we have users who
> use both together, [...]

Yes, we have users that do that, we have users that only use one or the other, 
and we have users that don't use either.

It is the same with any more advanced feature we have. We give users the 
choice, and the users are smart enough to pick what they need.

We have several launchers, some use multiple ones, some a single one, and some 
don't use a launcher.

> It's also reality that we have critics who
> complain the two are
> redundant and it's all a big mess.

This part I don't care about. I care about users, not critics. Trolls will 
always find something to complain about.

> It's not a surprise to me that the idea of 
> merging them has come up,

I'm not surprised either, the same idea has been popping up every few years.

The problem with it is that the proposals are usually about modelling a 
particular person's view of activities. 

> It does mean
> being willing to take some people's existing 
> workflows away, though.

I'm not willing. It goes against Plasma design principle of 'simple by default, 
powerful when needed'.

I'm all for designing a new UI for all of this (though I'd like to see it based 
on the current one which I find quite pretty :) ) which would communicate 
clearly what activities are for and what VDs are for.

> I also think merging has technical risks, e.g. 

Not only the speed. It would also make problems for the Plasma - the current 
activities design is quite baked in the underlying organisation of the shell. 
What would this change require, and how would it handle different behaviours on 
X11 and Wayland which was mentioned in Phab.

> e.g. the VDG's
> mulled Activity overview/dashboard thingie.

This thing https://cukic.co/2014/07/01/just-a-teaser/ ?

Cheers,
Eike

Re: Discussion for Virtual Desktops and Activities future

[Ivan Čukić]

 Just read the phab discussion. If I misunderstood the situation, please correct me. Wondering where the original discussion happened where 'we' decided to merge two orthogonal concepts into one. Can you point me to the relevant thread on plasma-devel?If this is about providing a unified implementation in kwin for both VDs and activities, I'm fine with that. If not, then continue reading. VDs are for managing windows - a solution for the 'I don't have a screen large enough for all these windows'.Activities are for managing work - a solution to the 'I don't want documents/files/etc. from separate projects to get jumbled up together'.Previously, we had problems when we tried to equate activities with any specific thing. As in, 'an activity is a group of Plasma widgets'. The same will happen if the activity becomes 'a group of windows'.This has been discussed quite a few times before. Has anyone collected the use-cases of activities before coming up with this idea?Cheers, Ivan Sent via the BlackBerry Hub for Android   From: mvourla...@gmail.comSent: 3 July 2018 5:36 pmTo: plasma-devel@kde.orgReply to: plasma-devel@kde.orgSubject: Discussion for Virtual Desktops and Activities future  A discussion started by me at: https://phabricator.kde.org/D13745 about concerns related to future Activities / Virtual Desktops merge (I will call it MERGE in the future). As proposed the discussion can be moved here.In my opinion having a concrete draft how things are going to work from a user point view it might bring design issues that can be solved earlier than later. My opinion in the matter can be found at: https://psifidotos.blogspot.com/2012/03/activities-and-workareas-draft.html1. I think that the previous draft can be used in order to identify any users workflow breakage from MERGE.2. A new draft describing MERGE from a user point of view should be created in order for everyone (plama team and VDG) to understand what they are trying to create.By reading comments I identified a internal decision in order for MERGE to be based more in Activites infrastructure  for Plasma product and to VDs infrastructure for KWin product. I would propose to forget the technical approach and focus more on the user point of view. This could help avoid the VDs/Activities debate that started in Plasma 4/5 era and it is still present at a smaller degree.I will start with one concern based on the comments I read even though it would be better for everyone to describe first what is expecting from the MERGE.Concern [A]If MERGE creates a new Activity each time the user needs more space for its windows, doesnt that break totally the VDs users workflow? Example: I am working on my current actitivity and I am writing a note in a plasma widget. I am creating a new Activity, should that Activity look the same as the previous one and if I change the note in the first should it look the same and in the second? (at this example a current VDs user would answer should be in sync and always the same, a current
Activity user would answer it doesnt matter, it is unrelated)

D13255: Make dependency on KF5NetworkManagerQt optional

[Ivan Čukić]

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


  Thanks!

REPOSITORY
  R845 Plasma Vault

BRANCH
  arcpatch-D13255

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

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

D13255: Make dependency on KF5NetworkManagerQt optional

[Ivan Čukić]

ivan added a comment.


  Very nice, thanks. If you want I can accept this (after the `Q_UNUSED` 
change) and I'll do the UI patch separately.

INLINE COMMENTS

> service.cpp:53
> +{
> +(void)enabled;
> +}

You should use `Q_UNUSED` instead.

REPOSITORY
  R845 Plasma Vault

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

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

D13255: Make dependency on KF5NetworkManagerQt optional

[Ivan Čukić]

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


  - Would rather have a separate NetworkManager wrapper class implementation (a 
dummy implementation for when NM is disabled) than this.
  - Also, instead of a cmake flag, it is more common to find_package and define 
the flag depending on whether it was found or not.
  - What about the configuration UI?

REPOSITORY
  R845 Plasma Vault

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

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

Re: KDE CI: Plasma plasma-vault stable-kf5-qt5 FreeBSDQt5.10 - Build # 4 - Still Failing!

[Ivan Čukić]

> In regards to NetworkManagerQt, that is unavailable as
> NetworkManager itself isn't available on FreeBSD.

Ok, didn't know that. NM will be an optional dep once I get to it (probably
not in the next release). I guess after that PV will be buildable again on
BSD.

Cheers,
Ivan


-- 
KDE, ivan.cu...@kde.org, http://cukic.co/
gpg key fingerprint: 292F 9B5C 5A1B 2A2F 9CF3  45DF C9C5 77AF 0A37 240A

Re: Plasma Vaults by Default

[Ivan Čukić]

Yes, that is what I meant.

Cheers,
Ivan

On Wed, May 23, 2018 at 7:14 PM Marco Martin <notm...@gmail.com> wrote:

> On domenica 20 maggio 2018 17:28:08 CEST Ivan Čukić wrote:
> > Hi Jonathan,
> >
> > One possibility is to hide the icon if the user does not use Vaults (if
the
> > user has no vaults configured) similar to how the pager applet behaves.

> how one would add one then?
> should it be visible but in the systray popup area by default?

> --
> Marco Martin



-- 
KDE, ivan.cu...@kde.org, http://cukic.co/
gpg key fingerprint: 292F 9B5C 5A1B 2A2F 9CF3  45DF C9C5 77AF 0A37 240A

D13042: Do not show activity switcher during fast switches

[Ivan Čukić]

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


  Looks ok to me.

REPOSITORY
  R119 Plasma Desktop

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

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

Re: KDE CI: Plasma plasma-vault stable-kf5-qt5 FreeBSDQt5.10 - Build # 4 - Still Failing!

[Ivan Čukić]

Hi all,

I was on a trip.

P-V is meant to be linux-only. If someone to make it work on BSD, I'm open
to that.

Though, it seems like a missing dep only - Could NOT find KF5 (missing:
NetworkManagerQt)

I'll skip commenting other parts of this discussion.

Cheers,
Ivan