Re: Collection of packaging notes

[David Redondo]

Am Freitag, 10. November 2023, 12:36:09 CET schrieb Jonathan Riddell:
> On Fri, 10 Nov 2023 at 10:55, Christophe Marin  wrote:
> 
> > - The libksysguard  libraries soversion weren't bumped. It's not only a
> > problem for
> > building plasma 5 and plasma 6. KSysguard is also an optional dependency
> > for kdevelop.
> 
> I've bumped the soversions in libksysguard now. (I'll look at doing the
> KFification for kuserfeedback too.)
> 
> I've also release noted that ksysguard the app is dead and should be
> removed from distros, it hasn't been released for over 2 years and hasn't
> been ported.

The Ksysguard dependency of kdevelop mentioned here is libksysguard which has 
a CMake name of ksysguard confusingly.

> Jonathan

David

Re: Frameworks / Plasma/ Gear Release Schedule Plan

[David Redondo]

Am Dienstag, 12. September 2023, 21:42:37 CEST schrieb christ...@cullmann.io:
> Hi,
> 
> i prepared some merge request for the switch in the meta data:
> 
> https://invent.kde.org/sysadmin/repo-metadata/-/merge_requests/185
> 
> A question about the CMake changes:
> 
> Is there some preferred way to switch?
> 
> I would just alter the defaults to
> 
> set(QT_REQUIRED_VERSION "6.4.0")
> set(QT_MAJOR_VERSION "6")
> set(KF_MIN_VERSION "5.240.0")
> set(KF_MAJOR_VERSION "6")
> 
> in the first step and start later to remove the version ifs.

For Gear the minimum versions are changed by the individual projects, last 
time I looked there was also no consistent naming of these CMake variables, 
not have a  'KF_MAJOR_VERSION' variable at all. (Actually I never seen a 
KF_MAJOR_VERSION variable before, so maybe only Kate?).
For reference Frameworks right now requires at least Qt 6.5.
I think explicitely setting QT_MAJOR_VERSION to 6 is not needed, including ECM 
with 5.240 will enable using Qt 6 install paths. (Of course it doesn't hurt to 
be explicit)

> Greetings
> Christoph

David

Re: kio-extras and the KF5/KF6 period

[David Redondo]

Am Dienstag, 20. Juni 2023, 14:52:44 CEST schrieb David Redondo:
> Harald and I prototyped another solution to build a Qt
> 5 and Qt 6 version out of the same repo and employed it on
> plasma-integration: https://invent.kde.org/plasma/plasma-integration/-/
> merge_requests/91
> This has the advantage of having the code for both versions in
> the same place and the Qt5 version being not stuck on some old
> version. Also the translations can be shared automatically since
> they use the same catalog.
> There is one case  where Qt uses unversioned targets and one
> unversioned ecm variable but cadavidn be easily worked around.

And here is the same idea applied to building both Qt5 and Qt6 versions out of 
a single src directory at the same time. The repo is smaller and nothing 
duplicated so you can get a better idea of what's going on. 
This looks like a good option for me for repos with little divergence between 
both versions.

https://invent.kde.org/plasma/kwayland-integration/-/merge_requests/45

 David

Re: kio-extras and the KF5/KF6 period

[David Redondo]

Am Mittwoch, 21. Juni 2023, 12:31:47 CEST schrieb Ben Cooksley:
> It sounds like we are approaching (or have already hit) a "crossroads"
> where it is time for the Qt 5 codebase to enter a stable release only
> phase, with master becoming Qt 6 exclusive.
> Splitting the code into separate Qt 5 / Qt 6 folders sounds like it will be
> difficult to ensure that all bug fixes are applied equally to both sides.

You are right, that's why in general for Plasma projects the  master branch is 
Qt6 only.
But for some special projects we need to have Qt 5 AND Qt 6 builds in a Plasma 
6 session to support Qt5 apps on Plasma 6  - for example Breeze or plasma-
integration (our QPlatformTheme). Freezing Qt5 at a stable version doesn't 
work quite here as  development happening on master will lead to UX and visual 
divergence compared to the frozen branch.

> Cheers,
> Ben

David

Re: kio-extras and the KF5/KF6 period

[David Redondo]

Am Mittwoch, 17. Mai 2023, 00:02:18 CEST schrieb Albert Astals Cid:
> kio-extras provides plugins for kio.
> 
> So KF5 applications want a KF5 kio-extras and KF6 applications want a KF6
> kio- extras.
> 
> If we're going to support a period on which we ship both Kf5 and KF6 based
> applications we need to:
> 
> Make sure kf5 and kf6 are coinstallable.
> 
> a) release two tarballs, one for each KF
> b) release one tarball that compiles both for kf5 and kf6
> c) just release the kf6 tarball, stop releasing the kf5 tarball but ask
> distributions to still install it
> 
> 

Harald and I prototyped another solution to build a Qt
 5 and Qt 6 version out of the same repo and employed it on
plasma-integration: https://invent.kde.org/plasma/plasma-integration/-/
merge_requests/91
This has the advantage of having the code for both versions in 
the same place and the Qt5 version being not stuck on some old
 version. Also the translations can be shared automatically since 
they use the same catalog.
There is one case  where Qt uses unversioned targets and one
unversioned ecm variable but cadavidn be easily worked around.

David

Re: kio-extras and the KF5/KF6 period

[David Redondo]

Am Mittwoch, 7. Juni 2023, 13:00:13 CEST schrieb Luigi Toscano:
> I think this is a different case: plasma-integration is part of plasma, and
> afaik the maser branch of all repositories which are part of plasma are
> Qt6-only already.

Note that plasma-integration is our Qt Platform Theme and is in the same 
situation as Breeze - we need to have a qt5 version for integration of Qt5 
Apps running under Plasma 6.

David

Re: kio-extras and the KF5/KF6 period

[David Redondo]

Am Donnerstag, 25. Mai 2023, 12:51:32 CEST schrieb Nicolas Fella:
> Hi,
> 
> Am 17.05.23 um 00:02 schrieb Albert Astals Cid:
> > kio-extras provides plugins for kio.
> > 
> > So KF5 applications want a KF5 kio-extras and KF6 applications want a KF6
> > kio- extras.
> 
> This is also the case in more places, e.g. Breeze/Oxygen and
> plasma-integration, so having a unified approach would make sense.
> 
> > If we're going to support a period on which we ship both Kf5 and KF6 based
> > applications we need to:
> > 
> > Make sure kf5 and kf6 are coinstallable.
> > 
> > a) release two tarballs, one for each KF
> 
> In some cases, where there is a large enough divergence between the 5
> and 6 code we might want to have separate branches to maintain those
> (e.g. master is Qt6 and we have a qt5-compat branch).
> 

I think plasma-integration is already at that point where it's very hard to 
maintain and do something because it's becoming a soup of #ifdefs.
Checking 5 vs. 6 but also checking Qt6 Patch levels which is only going to 
become worse in the future. For exmaple if we want to use  QNativeInterface 
API which theoretically would  make the code easier to read or dbus menu 
support which might end up needing to do different thing depending on the Qt 6
version.

So for plasma-integration I would like to no longer have 5 and 6 support in 
master.

Re: Splitting KGlobalAccel interface and runtime

[David Redondo]

Am Montag, 13. Februar 2023, 21:05:33 CET schrieb Nicolas Fella:
> It wouldn't automatically solve the coinstallability problem of KF5 and
> KF6, because a kglobalacceld provided by KF5-KGlobalAccel would still
> conflict with a Plasma-provided kglobalacceld, but it's at least
> conceptually less messy since it's clear that the Plasma-provided one
> would be the preferred one to use.

While running both may never work, I would suggest making the KF6 kglobacceld
register a versioned name (i.e. org.kde.kglobalaccel6) instead of 
org.kde.kglobalaccel and have the interface talk to that name to avoid 
autostarting a KF5 kglobalacceld. 

We can also go further and version the whole API http://0pointer.de/blog/
projects/versioning-dbus.html to make changes easier in the future.

> This also means that a KF6-based kglobalacceld must work with a KF5 
interface library

The KF6 one should also register the old name and support the current DBus API
so KF5 applications will not autostart a KF5 kglobalacceld.

David

Re: Challenge: adding new method overloads when existing consumers use {} with args

[David Redondo]

Am Samstag, 23. Juli 2022, 17:20:08 CEST schrieb Friedrich W. H. Kossebau:
> Hi,
>
> (cc: kde-frameworks-devel for heads-up, please reply to kde-devel only)
Sorry replied wrong first :)
> given a class C with a method foo(A a):
> --- 8< ---
> class C
> {
> public:
> void foo(A a);
> };
> --- 8< ---
>
> Now you want to add an overload, to serve further use-cases as requested by
> API consumers:
> --- 8< ---
> void foo(B b);
> --- 8< ---
>
>
> But there is existing consumer code, making use of C++17, which calls
> --- 8< ---
> C c;
> c.foo({});
> --- 8< ---
>
> So the new overload will not be source-compatible and break existing code,
> for what my WE mood brain tells me.
>
> Which spoils the API evolving we have been doing so far e.g. in KDE
> Frameworks quite a bit.
> So far we seem to just have been lucky, but with more consumer code starting
> to make use of C++17 features, the risk has grown and I just ran into this
> the first time ->
> https://invent.kde.org/frameworks/kwidgetsaddons/-/commit/
> e425aaa3025272cb70169354d04dfb3713f9783a#note_491339
>
> Had not yet thought about this challenge myself before, so curious what
> people think can be done here?
>
> Should perhaps the use of list-initializers with arguments in method calls
> be discouraged, at least for plain {} ones, where no type information is
> provided at all?

Adding such an overload as in your example is source incompatible regardless. 
See also our guidelines.
https://community.kde.org/Policies/
Binary_Compatibility_Issues_With_C%2B%2B#Note_about_ABI

You cannot... 
  For existing functions of any type:
Add an overload (binary compatible, but not source compatible: it makes 
 ambiguous). Adding overloads to already overloaded functions is ok 
(since any use of  already needed a cast).

> Cheers
> Friedrich

David

Re: Challenge: adding new method overloads when existing consumers use {} with args

[David Redondo]

Am Samstag, 23. Juli 2022, 17:20:08 CEST schrieb Friedrich W. H. Kossebau:
> Hi,
> 
> (cc: kde-frameworks-devel for heads-up, please reply to kde-devel only)
> 
> given a class C with a method foo(A a):
> --- 8< ---
> class C
> {
> public:
> void foo(A a);
> };
> --- 8< ---
> 
> Now you want to add an overload, to serve further use-cases as requested by
> API consumers:
> --- 8< ---
> void foo(B b);
> --- 8< ---
> 
> 
> But there is existing consumer code, making use of C++17, which calls
> --- 8< ---
> C c;
> c.foo({});
> --- 8< ---
> 
> So the new overload will not be source-compatible and break existing code,
> for what my WE mood brain tells me.
> 
> Which spoils the API evolving we have been doing so far e.g. in KDE
> Frameworks quite a bit.
> So far we seem to just have been lucky, but with more consumer code starting
> to make use of C++17 features, the risk has grown and I just ran into this
> the first time ->
> https://invent.kde.org/frameworks/kwidgetsaddons/-/commit/
> e425aaa3025272cb70169354d04dfb3713f9783a#note_491339
> 
> Had not yet thought about this challenge myself before, so curious what
> people think can be done here?
> 
> Should perhaps the use of list-initializers with arguments in method calls
> be discouraged, at least for plain {} ones, where no type information is
> provided at all?

Adding such an overload as in your example is source incompatible regardless. 
See also our guidelines.
https://community.kde.org/Policies/
Binary_Compatibility_Issues_With_C%2B%2B#Note_about_ABI

You cannot... 
  For existing functions of any type:
Add an overload (binary compatible, but not source compatible: it makes 
 ambiguous). Adding overloads to already overloaded functions is ok 
(since any use of  already needed a cast).

> Cheers
> Friedrich

David

KWayland 5.94 respin request

[David Redondo]

Please include
https://invent.kde.org/frameworks/kwayland/-/commit/2e94a425120971ec535278629e216f5e632fe9bc
It fixes a crash with a newly added function

Thanks,
David

Re: KF6 meeting notes 2021-12-07

[David Redondo]

Am Dienstag, 7. Dezember 2021, 18:04:19 CET schrieb Volker Krause:


https://phabricator.kde.org/T11587
- there are bigger plans to rework color scheming, this needs more
coordination with the effort to down-tier this for KF6
- would likely mean only storing the color scheme name in kdeglobals 
rather

than actual colors

I actually started a branch to use QSettings instead of KConfig for
KColorScheme with some changes to the public API that does this.
https://invent.kde.org/frameworks/kconfigwidgets/-/commits/work/davidre/
qsettings/

- do we need per color cascading or is that rather a bug? compare e.g. 
to

Kate's syntax highlighting themes.
This does away with per color cascading, and imo there's not much use 
case for
it. When introducing the Header color set extra effort was taken to NOT 
have
cascading for those. Also Plasma will only allow creating full color 
schemes

from the gui iirc.


- should we just put this into KF::Config itself?

The reason I did not finish this because I hit a stumbling block, that
QSettings cannot deal with nested groups (see the latest few commits in 
the
branch), so our current format '[Colors:Button][Inactive]' does not work 
which
we should keep imo and we had to copy the group parsing from KConfig. So 
that

might indeed make sense.

Re: Noteworthy changes when porting to C++17

[David Redondo]

Hi I found these two papers

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

Regards,
David

Re: Including instead of , does it upset POSIX?

[David Redondo]

Looking into the C++ 17 working draft (http://open-std.org/JTC1/SC22/WG21/
docs/papers/2017/n4659.pdf)  is actually the same as the POSIX header.

In section 22.4 (page 517)
>The contents of the headerare the same as the POSIX header, 
except thaterrnoshallbe defined as a macro. [Note:The intent is to remain in 
close alignment with the POSIX standard.— endnote] A separateerrnovalue shall 
be provided for each thread.

And at the bottom of 22.4.1
>The meaning of the macros in this header is defined by the POSIX standard.

Regards,
David

P.S. In the latest working draft this seems now to be section 19.4 instead

Re: KGlobalAccel on non-Plasma systems

[David Redondo]

Hi,

I agree that it's most useful on plasma.

>On systems that do not use X11 (Windows, macOS, Android) it
>is most likely not useful at all.
There exists code for mac and windows but I don't know either  if it's working 
or just a dummy so it compiles. 

Also for everyone interested in the topic, KGlobalAccel will be a topic on 
Saturday's frameworks meeting 15.00 CEST at meet.kde.org.
Seems like it ties into the topic of different frameworks types as observed by 
Nicolas.

Regards,
David

Re: How to sort TableModel QML Type

[David Redondo]

Am Samstag, 17. Oktober 2020, 20:38:43 CEST schrieb chiasa.men:
> Is there a way to sort a TableModel without relying on c++ code?

Hi, 
we have QSortFilterProxyModel bindings in KItemModels, which makes it easy to 
create and use such a proxy model from Qml: https://api.kde.org/frameworks/
kitemmodels/html/ksortfilterproxymodel_8h_source.html

Best Regards,
David

Re: IconDialog customLocation

[David Redondo]

Hi, 
copy pasting my response from https://bugs.kde.org/show_bug.cgi?id=427434 :

I think you  want to set "user: true". The name of this property comes from 
KIconDialog [1] which is wrapped by IconDialog [2]

Best regards,
David

[1] https://api.kde.org/frameworks/kiconthemes/html/classKIconDialog.html
[2] https://invent.kde.org/frameworks/kdeclarative/-/blob/master/src/
qmlcontrols/kquickcontrolsaddons/icondialog.cpp

Re: Breakage in Breeze Icons

[David Redondo]

I fixed the build now. There was a symlink to a removed file which
cmake -E copy_directory didn't like.

Regards,
David

D29281: Deprecate defunct functions

[David Redondo]

davidre added a comment.


  This caused https://bugs.kde.org/show_bug.cgi?id=423003. I removed excluding 
the virtual method from the build in 
  
https://invent.kde.org/frameworks/krunner/commit/8f7ce559b84ee0c21de0256e6591793e4b95f411

REPOSITORY
  R308 KRunner

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

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: davidre, kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29463: Fix Kirigami.Units.devicePixelRatio=1.3 when it should be 1.0 at 96dpi

[David Redondo]

davidre added a comment.


  Ping

REPOSITORY
  R242 Plasma Framework (Library)

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

To: Zren, #kirigami, mart
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29802: Require out-of-source builds

[David Redondo]

davidre accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R266 Breeze Icons

BRANCH
  require-in-source-build (branched from master)

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

To: ngraham, #frameworks, #vdg, ognarb, davidre
Cc: ltoscano, davidre, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29802: Require in-source build

[David Redondo]

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


  I don't think we want to require in source builds

REPOSITORY
  R266 Breeze Icons

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

To: ngraham, #frameworks, #vdg, ognarb, davidre
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28208: Move sni icon handling logic from data engine to applet

[David Redondo]

davidre updated this revision to Diff 82519.
davidre added a comment.


  - fix

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28208?vs=82492=82519

BRANCH
  sni (branched from master)

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

AFFECTED FILES
  applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
  applets/systemtray/systemtraymodel.cpp
  applets/systemtray/systemtraymodel.h
  dataengines/statusnotifieritem/statusnotifieritemsource.cpp

To: davidre, kmaterka, broulik, mart, #plasma, #vdg, #frameworks
Cc: bruns, 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

D28208: Move sni icon handling logic from data engine to applet

[David Redondo]

davidre updated this revision to Diff 82492.
davidre added a comment.


  - Rebase without test

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28208?vs=78982=82492

BRANCH
  sni (branched from master)

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

AFFECTED FILES
  applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
  applets/systemtray/systemtraymodel.cpp
  applets/systemtray/systemtraymodel.h
  dataengines/statusnotifieritem/statusnotifieritemsource.cpp

To: davidre, kmaterka, broulik, mart, #plasma, #vdg, #frameworks
Cc: bruns, 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

D28624: Print a warning if runner is incompatible with KRunner

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R308:e56def21a286: Print a warning if runner is incompatible 
with KRunner (authored by davidre).

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28624?vs=79487=81486

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

AFFECTED FILES
  src/runnermanager.cpp

To: davidre, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28093: [breeze-icons] add TeamViewer tray icons

[David Redondo]

davidre added a comment.


  Couldn't we put the tray icon in the plasma theme?

REPOSITORY
  R266 Breeze Icons

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

To: rocka, #vdg, ngraham, ndavis
Cc: davidre, IlyaBizyaev, ngraham, ndavis, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, bruns

D29232: [WIP][RFC]Introduce the Header color set

[David Redondo]

davidre added inline comments.

INLINE COMMENTS

> ndavis wrote in kcolorscheme.cpp:271
> Why is that? The only one that might is Active, but that's not really used 
> much right now.

Because the new colors are the replacement for theses colors. I thought one of 
those might map to this

REPOSITORY
  R265 KConfigWidgets

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

To: mart, #vdg, #plasma, cblack
Cc: davidre, ndavis, cblack, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns

D29232: [WIP][RFC]Introduce the Header color set

[David Redondo]

davidre added inline comments.

INLINE COMMENTS

> kcolorscheme.cpp:271
>  static const DecoDefaultColors defaultDecorationColors = {
>  {  61, 174, 233 }, // Focus
>  { 147, 206, 233 }, // Hover

I would have expected at least of one the new colors to have the same value as 
this one.

REPOSITORY
  R265 KConfigWidgets

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

To: mart, #vdg, #plasma, cblack
Cc: davidre, ndavis, cblack, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns

D29011: Fix variable mixup

[David Redondo]

davidre abandoned this revision.
davidre added a comment.


  Oh I diffed the wrong repo...

REPOSITORY
  R302 KIconThemes

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

To: davidre, #breeze, #plasma
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29011: Fix variable mixup

[David Redondo]

davidre added reviewers: Breeze, Plasma.

REPOSITORY
  R302 KIconThemes

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

To: davidre, #breeze, #plasma
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29011: Fix variable mixup

[David Redondo]

davidre created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidre requested review of this revision.

REVISION SUMMARY
  Caused icons to be not longer recolored when changing the color scheme until 
an
  application is started again

TEST PLAN
  Change color scheme while having an application open, icons should change 
color correctly

REPOSITORY
  R302 KIconThemes

BRANCH
  typo (branched from master)

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

AFFECTED FILES
  src/kiconloader.cpp

To: davidre
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28856: Save disabling of desktop file components in kglobalshortcutsrc

[David Redondo]

davidre updated this revision to Diff 80265.
davidre added a comment.


  Don't parse the disabledGroup as component

REPOSITORY
  R268 KGlobalAccel

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28856?vs=80206=80265

BRANCH
  disable (branched from master)

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

AFFECTED FILES
  src/runtime/globalshortcutsregistry.cpp
  src/runtime/globalshortcutsregistry.h
  src/runtime/kserviceactioncomponent.cpp

To: davidre, davidedmundson, fvogt, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28856: Save disabling of desktop file components in kglobalshortcutsrc

[David Redondo]

davidre marked 4 inline comments as done.

REPOSITORY
  R268 KGlobalAccel

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

To: davidre, davidedmundson, fvogt, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28856: Save disabling of desktop file components in kglobalshortcutsrc

[David Redondo]

davidre added inline comments.

INLINE COMMENTS

> globalshortcutsregistry.cpp:274
> +auto disabledComponents = KConfigGroup(&_config, 
> "disabledComponents").readEntry("disabled", QStringList());
>  for (const QString  : groupList)
>  {

good point

> globalshortcutsregistry.cpp:333
>  for (const QString  : lstDesktopFiles) {
> -if (_components.contains(desktopFile)) {
> +if (_components.contains(desktopFile) || 
> disabledComponents.contains(desktopFile)) {
>  continue;

It actually is if you follow the constructor chain

REPOSITORY
  R268 KGlobalAccel

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

To: davidre, davidedmundson, fvogt, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28856: Save disabling of desktop file components in kglobalshortcutsrc

[David Redondo]

davidre updated this revision to Diff 80206.
davidre added a comment.


  foo

REPOSITORY
  R268 KGlobalAccel

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28856?vs=80205=80206

BRANCH
  disable (branched from master)

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

AFFECTED FILES
  src/runtime/globalshortcutsregistry.cpp
  src/runtime/globalshortcutsregistry.h
  src/runtime/kserviceactioncomponent.cpp

To: davidre, davidedmundson, fvogt, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28856: Save disabling of desktop file components in kglobalshortcutsrc

[David Redondo]

davidre created this revision.
davidre added reviewers: davidedmundson, fvogt, meven.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidre requested review of this revision.

REVISION SUMMARY
  Works for writable and not writable files. Additional positive we don't have 
to modify the desktop file
  That means a kcm doesn't need to do anything special as the component is 
enabled again automagically upon doRegister().

REPOSITORY
  R268 KGlobalAccel

BRANCH
  disable (branched from master)

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

AFFECTED FILES
  src/runtime/globalshortcutsregistry.cpp
  src/runtime/globalshortcutsregistry.h
  src/runtime/kserviceactioncomponent.cpp

To: davidre, davidedmundson, fvogt, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D24956: Consider desktop files with NoDisplay attribute

[David Redondo]

davidre added a comment.


  In D24956#648991 , @meven wrote:
  
  > In D24956#648968 , 
@davidedmundson wrote:
  >
  > > > [14:12]  DavidRedondo1: my understanding is that a system might 
ship "konsole opens with control+t". The UI allows you to remove that. This 
would remove the entry in kglobalshortcutsrc, but because it's still  in the 
system defaults file as soon as you log in again it'll add it back
  > >
  > > [14:25]  d_ed, fvogt Apparently the runtime writes the 
hidden thing when a component is cleanedUp 
https://cgit.kde.org/kglobalaccel.git/tree/src/runtime/kserviceactioncomponent.cpp#n135
  >
  >
  > This is addressed in D25088 
  
  
  This has the same failure that it only works for writable files. I have the 
idea to save in kglobalshortcutsrc if a desktopfile is disabled

REPOSITORY
  R268 KGlobalAccel

BRANCH
  master

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

To: meven, mart, #plasma, fvogt, apol, davidedmundson
Cc: davidedmundson, davidre, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D28699: Add startCapture method

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R296:5eb3c795a8f2: Add startCapture method (authored by 
davidre).

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28699?vs=79691=79764

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

AFFECTED FILES
  src/qmlcontrols/kquickcontrols/KeySequenceItem.qml

To: davidre, #frameworks, davidedmundson, broulik, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28699: Add startCapture method

[David Redondo]

davidre created this revision.
davidre added reviewers: Frameworks, davidedmundson, broulik.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidre requested review of this revision.

REVISION SUMMARY
  Starts capturing as if the user clicked on the main button. Useful if one
  wants to start capturing upon other user input than clicking on the item.

TEST PLAN
  Call the method

REPOSITORY
  R296 KDeclarative

BRANCH
  startCpature (branched from master)

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

AFFECTED FILES
  src/qmlcontrols/kquickcontrols/KeySequenceItem.qml

To: davidre, #frameworks, davidedmundson, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28697: Also relase the window in the destructor

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R296:6c5619ffebae: Also relase the window in the destructor 
(authored by davidre).

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28697?vs=79688=79690

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

AFFECTED FILES
  src/qmlcontrols/kquickcontrols/private/keysequencehelper.cpp

To: davidre, broulik, #frameworks, davidedmundson
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28697: Also relase the window in the destructor

[David Redondo]

davidre created this revision.
davidre added reviewers: broulik, Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidre requested review of this revision.

REVISION SUMMARY
  Otherwise the keyboard is never released when the item is destroyed while
  capturing is active

REPOSITORY
  R296 KDeclarative

BRANCH
  release (branched from master)

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

AFFECTED FILES
  src/qmlcontrols/kquickcontrols/private/keysequencehelper.cpp

To: davidre, broulik, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27848: Remove the panel tooltip icon

[David Redondo]

davidre added a comment.


  In D27848#643045 , @ngraham wrote:
  
  > See the parent task.
  >
  > This component doesn't only darw system tray tooltips but rather tooltips 
for all panel widgets. The idea was that we don't want icons in *any* of these 
tooltips, because they're either redundant or inconsistent with the icon that 
you're hovering the mouse over. For this patch, I guess I should have marked 
the `icon` parameter as deprecated. I can do that in a follow-up patch.
  >
  > I didn't remove the display of a custom image because I figured that in 
this case, the designer was specifically trying to set something different. But 
maybe that should be deprecated too. Open to opinions.
  >
  > On another note, it would have been nice if these concerns had been brought 
up during the month when the patch was open for review.
  
  
  Wouldn't then be the solution to remove icons to wherever the panel widgets 
set them? And why would a designer only explicitely set images but not icons? 
Also this component does not only draw panel tooltips but also other tooltips, 
for example on the widget edit handle things. 
  Sorry for not noticing earlier

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #vdg, #plasma, cblack, niccolove, apol
Cc: broulik, davidre, cblack, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D28624: Print a warning if runner is incompatible with KRunner

[David Redondo]

davidre created this revision.
davidre added a reviewer: broulik.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidre requested review of this revision.

REVISION SUMMARY
  Useful for finding out why runner isn't loaded even though it's enabled in
  settings. Can for example happen if a runner is installed by an application
  that's compiled against a newer KRunner version than Plasma.

TEST PLAN
  Try to load an incompatible runner plugin

REPOSITORY
  R308 KRunner

BRANCH
  warning (branched from master)

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

AFFECTED FILES
  src/runnermanager.cpp

To: davidre, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27848: Remove the panel tooltip icon

[David Redondo]

davidre added a comment.


  I think this was done at the wrong level. If we don't want icons in the 
systray  then the way would be to net set the property in the systray and not 
ignore the property in the generic component but keep the property for api 
stability purposes. Now we have a component with a documented property that 
doesn't work. People trying to use this will think it's broken while we can't 
use it in any context where we want an icon in the future. Furthermore it's 
inconsistent that one can still set an icon via image strengthening the broken 
look from the outside.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #vdg, #plasma, cblack, niccolove, apol
Cc: davidre, cblack, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D28203: Move corner fold to top right in 24 icons

[David Redondo]

davidre accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis, ngraham, davidre
Cc: davidre, kossebau, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D28203: Move corner fold to top right in 24 icons

[David Redondo]

davidre closed this revision.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis, ngraham, davidre
Cc: davidre, kossebau, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D28470: [PlasmaCore.IconItem] Refactor source handling for different types

[David Redondo]

davidre added a comment.


  In D28470#640468 , @cblack wrote:
  
  > Splitting into multiple classes seems like a good idea, but "strategy"? 
Seems like an odd choice of name to me.
  
  
  I had assumed it's because of https://en.m.wikipedia.org/wiki/Strategy_pattern

REPOSITORY
  R242 Plasma Framework (Library)

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

To: kmaterka, #plasma, broulik, apol, davidedmundson
Cc: davidre, cblack, kde-frameworks-devel, #plasma, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D28203: Move corner fold to top right in 24 icons

[David Redondo]

davidre added a comment.


  Seems @kossebau was faster ;)

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis, ngraham, davidre
Cc: davidre, kossebau, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D28203: Move corner fold to top right in 24 icons

[David Redondo]

davidre requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis, ngraham, davidre
Cc: davidre, kossebau, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D28203: Move corner fold to top right in 24 icons

[David Redondo]

davidre reopened this revision.
davidre added a comment.


  This broke the build. Please note that tagging is today

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis, ngraham
Cc: davidre, kossebau, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D28223: Add "Stat" prefix to StatDetails Enum entries

[David Redondo]

davidre added inline comments.

INLINE COMMENTS

> global.h:322
>  /// No field returned, useful to check if a file exists
> -NoDetails = 0x0,
> +StatNoDetails = 0x0,
>  /// Filename, access, type, size, linkdest

I think this enum was not released yet

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, kossebau, dfaure
Cc: davidre, broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27617: [breeze-icons] Add telegram-desktop tray icons

[David Redondo]

davidre added a comment.


  In D27617#630440 , @ilya-fedin 
wrote:
  
  > In D27617#629987 , @ngraham 
wrote:
  >
  > > Would it help this use case if we moved the Telegram icon into the Plasma 
theme for now, pending a fix in Telegram itself?
  >
  >
  > There is nothing to do on Telegram side. Everything that Telegram can get 
is icon theme name (sometime), does the icon exist, the icon itself and its 
sizes. Adding a dependency on kde frameworks is not a way, of course.
  >
  > In D27617#630392 , @IlyaBizyaev 
wrote:
  >
  > > Btw that overlay is always as tiny as on your screenshot, so not usable 
for a counter
  >
  >
  > +1. Moreover, icon is not recolored even with IconName + OverlayIconPixmap.
  
  
  Oh yes that's true :/. It's a similiar bug to the one I fixed not long ago. 
It seems the SNI desperately needs the change as outlined in the other diff by 
removing all the rendering from the data engine. This way also the sizing of 
the overlay, I think I noticed it being inconsistent between pixmap and 
overlayname path.

REPOSITORY
  R266 Breeze Icons

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

To: rocka, #vdg, Fuchs, ndavis
Cc: davidre, ilya-fedin, broulik, alexeymin, IlyaBizyaev, ndavis, ngraham, 
kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns

D27617: [breeze-icons] Add telegram-desktop tray icons

[David Redondo]

davidre added a comment.


  In D27617#630336 , @IlyaBizyaev 
wrote:
  
  > In D27617#630228 , @ngraham 
wrote:
  >
  > > So... how do we fix this so that you can use a nice monochrome Breeze 
icon in your system tray? Whose code needs to change?
  >
  >
  >
  >
  > 1. SNI implementation in Plasma needs to be fixed to properly support 
OverlayIconPixmap 

 with IconName. From what I understand, since it affects a data engine and that 
is considered public API, this might have to wait for Plasma 6
  > 2. Since old versions of Plasma will still have the issue, and other SNI 
implementations likely don't support this either, this can't become the default 
behavior for Telegram; so there has to be a way for Telegram to check if an SNI 
implementation supports what it needs
  > 3. Then Telegram code needs to add a special case for that.
  >
  >   Since I feel like a proxy at this point, tagging @davidre and @ilya-fedin.
  
  
  Huh that case is working what I found that `IconName` and `OverlayIconName` 
is currently broken, see D28107 .
  `IconName and `OverlayIconPixmap` work fine  here F8184158: 
Screenshot_20200319_102827.PNG 
  
  If it doesn't for you please file a bug so we can investigate that.

REPOSITORY
  R266 Breeze Icons

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

To: rocka, #vdg, Fuchs, ndavis
Cc: davidre, ilya-fedin, broulik, alexeymin, IlyaBizyaev, ndavis, ngraham, 
kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns

D28130: Introduce api for passive notifications

[David Redondo]

davidre added a comment.


  I wonder if it's for the use case wehere an InlineMessage doesn't work does 
it need to contain actions?

REPOSITORY
  R296 KDeclarative

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

To: mart
Cc: davidre, broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D28033: Create ExpandableListItem

[David Redondo]

davidre added a comment.


  In D28033#627192 , @ngraham wrote:
  
  > In D28033#627184 , @cblack wrote:
  >
  > > In D28033#627180 , @davidre 
wrote:
  > >
  > > > I would love if we could find a way to show that the item is expandable
  > >
  > >
  > > I would probably use a pointing down arrow without a stem when collapsed, 
and pointing up when open.
  >
  >
  > Makes sense to me. Where would the arrow go though?
  >
  > In D28033#627186 , @niccolove 
wrote:
  >
  > > Strongly agree on having a single component. By the way, all these lists 
have the issue that the button only appearing on hover is very 
touch-unfriendly. This does not have to be addressed in this patch, but it 
having it implemented in only one place makes it much easier to fix.
  >
  >
  > Yeah, I could have the highlight remain in place if Qt detects that the 
click was actually a touch, and then the button would appear when the list item 
it touched.
  >
  > However the expanded view itself is not very touch-friendly since the 
individual list items are really too short to be easily touchable.
  
  
  I would put it in a flat button on the right

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #vdg, #plasma
Cc: bruns, niccolove, cblack, davidre, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham

D28033: Create ExpandableListItem

[David Redondo]

davidre added a comment.


  I would love if we could find a way to show that the item is expandable

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #vdg, #plasma
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27940: Add a KCapacityBar::State enum and attribute to allow display different status

[David Redondo]

davidre added a comment.


  Yes that's a problem that KColorScheme currently lives in KConfigWidgets. 
Maybe similiar to  D27038  and D27039 
?  Hopefully we can achieve T12218 


REPOSITORY
  R236 KWidgetsAddons

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

To: meven, cfeck, #frameworks
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27912: Draw inlineNotes after drawing word wrap marker

[David Redondo]

davidre added a comment.


  Seems in 
https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5.12/314/
 the test passed again?

REPOSITORY
  R39 KTextEditor

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

To: davidre, #ktexteditor, brauch
Cc: cullmann, brauch, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
cblack, GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D27912: Draw inlineNotes after drawing word wrap marker

[David Redondo]

davidre added a comment.


  In D27912#623891 , @brauch wrote:
  
  > I also don't understand this. Even if the painting somehow changes, e.g. 
because some painter state is set which wasnt set before (which I do not see to 
be the case here), that should not affect the line layout, as that is computed 
separately.
  >
  > Very strange.
  
  
  Yes I have the same thoughts. Maybe someone could trigger a rebuild, maybe it 
was just a fluke?

REPOSITORY
  R39 KTextEditor

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

To: davidre, #ktexteditor, brauch
Cc: cullmann, brauch, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
cblack, GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D27912: Draw inlineNotes after drawing word wrap marker

[David Redondo]

davidre added a comment.


FAIL!  : InlineNoteTest::testInlineNote() Compared values are not the same
Actual   (newCoordCol04): QPoint(51,1)
Expected (coordCol04)   : QPoint(33,1)
  
  I have node idea why the test would be failing. It complains that a 
coordinateToCursor changed. My idea what could cause this that an inlineNote is 
inserted at not the exprected place so column 4 shifts to the right. However 
the change in the painting order shouldn't  cause this as far as I understand 
it. To make matters worse the test passes for me locally. I would need help 
here to invesitagte or I would revert  this for the moment.

REPOSITORY
  R39 KTextEditor

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

To: davidre, #ktexteditor, brauch
Cc: cullmann, brauch, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
cblack, GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D27912: Draw inlineNotes after drawing word wrap marker

[David Redondo]

davidre added a comment.


  In D27912#623871 , @cullmann wrote:
  
  > Hi, could it be that killed the inline notes autotest?
  >
  > 
https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5.12/313/
  
  
  Sorry I will take a look

REPOSITORY
  R39 KTextEditor

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

To: davidre, #ktexteditor, brauch
Cc: cullmann, brauch, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
cblack, GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D27912: Draw inlineNotes after drawing word wrap marker

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:875d4da5475f: Draw inlineNotes after drawing word wrap 
marker (authored by davidre).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27912?vs=77168=77173

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: davidre, #ktexteditor, brauch
Cc: brauch, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, 
GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27912: Draw inlineNotes after drawing word wrap marker

[David Redondo]

davidre retitled this revision from "Draw inlineNotes after drawing caret" to 
"Draw inlineNotes after drawing word wrap marker".
davidre edited the summary of this revision.
davidre edited the test plan for this revision.
davidre added a reviewer: KTextEditor.

REPOSITORY
  R39 KTextEditor

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

To: davidre, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27912: Draw inlineNotes after drawing word wrap marker

[David Redondo]

davidre edited the summary of this revision.

REPOSITORY
  R39 KTextEditor

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

To: davidre, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27912: Draw inlineNotes after drawing caret

[David Redondo]

davidre created this revision.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
davidre requested review of this revision.

REVISION SUMMARY
  Normal text is column inlined so the caret even though drawn last doesn't go
  over anything. Inline notes on the other hand can contain arbitrary conent
  especially not column aligned text and background. Drawing the caret over them
  looks ugly.

REPOSITORY
  R39 KTextEditor

BRANCH
  caret

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: davidre
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27589: Try to apply the colorscheme of the current theme to QIcons

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:b7fa6e0e916b: Try to apply the colorscheme of the current 
theme to QIcons (authored by davidre).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27589?vs=76284=76760

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

AFFECTED FILES
  src/declarativeimports/core/iconitem.cpp
  src/plasma/private/theme_p.cpp
  src/plasma/private/theme_p.h
  src/plasma/theme.cpp
  src/plasma/theme.h

To: davidre, #plasma, cblack, ngraham, mart
Cc: mart, wbauer, cblack, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27039: [KStyle] Set the color of KMessageWidgets to the correct one from the current color scheme

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R252:d3a1b5148008: [KStyle] Set the color of KMessageWidgets 
to the correct one from the current… (authored by davidre).

REPOSITORY
  R252 Framework Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27039?vs=74713=76410

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

AFFECTED FILES
  src/kstyle/kstyle.cpp

To: davidre, #frameworks, aacid, anthonyfieroni, apol
Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27038: [KMessageWidget] Allow the style to change our palette

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R236:d946fb71ea22: [KMessageWidget] Allow the style to change 
our palette (authored by davidre).

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27038?vs=76408=76409

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

AFFECTED FILES
  src/kmessagewidget.cpp

To: davidre, #frameworks, apol
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27038: [KMessageWidget] Allow the style to change our palette

[David Redondo]

davidre updated this revision to Diff 76408.
davidre added a comment.


  Rebase

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27038?vs=74701=76408

BRANCH
  polish

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

AFFECTED FILES
  src/kmessagewidget.cpp

To: davidre, #frameworks, apol
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27540: KCModule: Indicate when a setting has been changed from the default value

[David Redondo]

davidre added a comment.


  That title is now not optimal isn't it? 1. It's not only in KCModule 2. It 
also shows unsaved settings

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg
Cc: iasensio, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27589: Try to apply the colorscheme of the current theme to QIcons

[David Redondo]

davidre updated this revision to Diff 76284.
davidre added a comment.


  - Introduce Plasma::Theme::palette

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27589?vs=76197=76284

BRANCH
  qiconcolor (branched from master)

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

AFFECTED FILES
  src/declarativeimports/core/iconitem.cpp
  src/plasma/private/theme_p.cpp
  src/plasma/private/theme_p.h
  src/plasma/theme.cpp
  src/plasma/theme.h

To: davidre, #plasma, cblack, ngraham, mart
Cc: mart, wbauer, cblack, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27557: Auto-generate 24px monochrome icons

[David Redondo]

davidre added a comment.


  I disagree but it's not my call

REPOSITORY
  R266 Breeze Icons

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

To: ngraham, #vdg, ndavis, #frameworks, sitter
Cc: davidre, bcooksley, kossebau, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

D27557: Auto-generate 24px monochrome icons

[David Redondo]

davidre added a comment.


  Also we need to have this on Windows, too.
  Breeze-icons is part of Frameworks and these were exported/installed files. A 
user could easily have a reference to one of the files somewhere.

REPOSITORY
  R266 Breeze Icons

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

To: ngraham, #vdg, ndavis, #frameworks, sitter
Cc: davidre, bcooksley, kossebau, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, ngraham, bruns

D27617: [breeze-icons] Add telegram-desktop tray icons

[David Redondo]

davidre edited the summary of this revision.
davidre added a reviewer: Fuchs.

REPOSITORY
  R266 Breeze Icons

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

To: rocka, #vdg, Fuchs
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27589: Try to apply the colorscheme of the current theme to QIcons

[David Redondo]

davidre edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  qiconcolor (branched from master)

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

To: davidre, #plasma, cblack
Cc: wbauer, cblack, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D27589: Try to apply the colorscheme of the current theme to QIcons

[David Redondo]

davidre added a comment.


  In D27589#616106 , @wbauer wrote:
  
  > Sounds like it would fix https://bugs.kde.org/show_bug.cgi?id=417780 ?
  
  
  Yes seems like the exact bug

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  qiconcolor (branched from master)

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

To: davidre, #plasma, cblack
Cc: wbauer, cblack, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D27038: [KMessageWidget] Allow the style to change our palette

[David Redondo]

davidre added a comment.


  In D27038#616095 , @apol wrote:
  
  > But then now we are not refreshing the palette when it changes outside, no?
  >
  > How about adding a boolean value when it's already happening to prevent the 
infinite recursion?
  
  
  I refresing the palette when the application palette changes. But yes could 
also do it this way

REPOSITORY
  R236 KWidgetsAddons

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

To: davidre, #frameworks
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27589: Try to apply the colorscheme of the current theme to QIcons

[David Redondo]

davidre edited the test plan for this revision.
davidre added a reviewer: Plasma.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: davidre, #plasma
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27589: Try to apply the colorscheme of the current theme to QIcons

[David Redondo]

davidre created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidre requested review of this revision.

REVISION SUMMARY
  Before icons loaded internally with QIcon::fromTheme were being colored with 
the
  colors from the current global color scheme instead of the ones from the 
current
  Plasma Theme. Leading to visual bugs when the two differ. This happened 
because
  KIconLoader uses the global color scheme by default.
  A prominent case is the notification send by the network plasmoid when one
  successfully connected to a wireless network. It sets the icon
  "network-wireless-on" which is not included in Breeze icons (but is included 
in
  Breeze Plasma Theme). If the current icon theme is indeed Breeze, IconItem
  resorts to using QIcon::fromTheme and we end up with a wrong colored
  "network-wireless" icon.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  qiconcolor (branched from master)

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

AFFECTED FILES
  src/declarativeimports/core/CMakeLists.txt
  src/declarativeimports/core/iconitem.cpp

To: davidre
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27039: [KStyle] Set the color of KMessageWidgets to the correct one from the current color scheme

[David Redondo]

davidre added a comment.


  Ping

REPOSITORY
  R252 Framework Integration

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

To: davidre, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27038: [KMessageWidget] Allow the style to change our palette

[David Redondo]

davidre added a comment.


  Ping

REPOSITORY
  R236 KWidgetsAddons

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

To: davidre, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R236:835c64d1b693: [KMessageWidget] Draw it with QPainter 
instead of using stylesheet (authored by davidre).

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27035?vs=76018=76174

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

AFFECTED FILES
  autotests/kmessagewidgetautotest.cpp
  src/kmessagewidget.cpp

To: davidre, #frameworks, ngraham, cfeck
Cc: apol, cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, bruns

D27540: Status indicator for individual widgets in KCModule

[David Redondo]

davidre added a comment.


  In D27540#615129 , @ervin wrote:
  
  > In D27540#615105 , @davidre 
wrote:
  >
  > > In D27540#615092 , @ervin 
wrote:
  > >
  > > > In D27540#615061 , @davidre 
wrote:
  > > >
  > > > > How does it look? Will this be opt in for other users of 
KConfigDialogManager?
  > > >
  > > >
  > > > I'd say you should try it. ;-)
  > > >  I really need wider feedback on how it behaves in different context. 
Currently the patch makes it mandatory for everyone.
  > >
  > >
  > > I use KConfigDialogManager to manage the settings in Spectacle MainWindow 
which are instant apply by 
  > >  `connect(mConfigManager, ::widgetModified, 
mConfigManager, ::updateSettings);` and I don't like it 
that they appear there too
  >
  >
  > You mean they appear and stick around? Or they appear/disappear immediately?
  >  If they stick around I'd indeed have to check why the state isn't 
recomputed on the updateSettings call...
  >
  > > F8117881: grafik.png 
  > > 
  > >> a setting which is currently dirty or which differs from default value.
  > > 
  > > What is your idea behind this? After first seeing this patch my intuition 
was that it would show for unsaved changes. But then I was confused when I open 
a SettingsDialog that there were already marks even though I changed nothing! I 
would expect them only for unsaved changes, I don't know how other platforms 
handle this if they have something similiar?
  >
  > Currently it helps the user find out the unsaved changes (the least useful 
in some way, or your immediate memory is not good) but it also helps the user 
find out which of her settings differ from the default values (and that's 
indeed something you will forget over time and wonder "why the hell is the 
defaults button enabled").
  >
  > Hope the extra context helps.
  
  
  They stick around because as you said the settings differ from the defaults.
  
  The pen for me conveys the meaning that something was edited that's the 
reason I would only show it for unsaved changes. But let's others weigh in what 
they think

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27540: Status indicator for individual widgets in KCModule

[David Redondo]

davidre added a reviewer: VDG.

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport, #vdg
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27540: Status indicator for individual widgets in KCModule

[David Redondo]

davidre added a comment.


  In D27540#615092 , @ervin wrote:
  
  > In D27540#615061 , @davidre 
wrote:
  >
  > > How does it look? Will this be opt in for other users of 
KConfigDialogManager?
  >
  >
  > I'd say you should try it. ;-)
  >  I really need wider feedback on how it behaves in different context. 
Currently the patch makes it mandatory for everyone.
  
  
  I use KConfigDialogManager to manage the settings in Spectacle MainWindow 
which are instant apply by 
  `connect(mConfigManager, ::widgetModified, 
mConfigManager, ::updateSettings);` and I don't like it 
that they appear there too
  F8117881: grafik.png 
  
  > a setting which is currently dirty or which differs from default value.
  
  What is your idea behind this? After first seeing this patch my intuition was 
that it would show for unsaved changes. But then I was confused when I open a 
SettingsDialog that there were already marks even though I changed nothing! I 
would expect them only for unsaved changes, I don't know how other platforms 
handle this if they have something similiar?

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27540: Status indicator for individual widgets in KCModule

[David Redondo]

davidre added a comment.


  The editmarks should probably respect the visibility of the associated 
widget. In this picture I use a invisble lineedit to manage the settings of the 
QButtonGroup beneath (QButtonGroup isn't supported by KCOnfigDialogManager)
  F8117874: grafik.png 

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27540: Status indicator for individual widgets in KCModule

[David Redondo]

davidre added a comment.


  How does it look? Will this be opt in for other users of KConfigDialogManager?

REPOSITORY
  R265 KConfigWidgets

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

To: ervin, ngraham, davidedmundson, meven, crossi, bport
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[David Redondo]

davidre added a comment.


  So I think this is ready to be merged and I would do so this weekend

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  no-css (branched from master)

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

To: davidre, #frameworks, ngraham, cfeck
Cc: apol, cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh, bruns

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[David Redondo]

davidre updated this revision to Diff 76018.
davidre marked 4 inline comments as done.
davidre added a comment.


  Code style and typos

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27035?vs=75717=76018

BRANCH
  no-css (branched from master)

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

AFFECTED FILES
  autotests/kmessagewidgetautotest.cpp
  src/kmessagewidget.cpp

To: davidre, #frameworks, ngraham
Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D27463: KconfigXT: Add a value attribute to Enum field choices

[David Redondo]

davidre added a comment.


  Could you also add documentation to whatever  
https://api.kde.org/frameworks/kconfig/html/kconfig_compiler.html is generated 
from?

REPOSITORY
  R237 KConfig

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

To: meven, ervin, bport, crossi, #frameworks
Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns

D27463: KconfigXT: Add a value attribute to Enum field choices

[David Redondo]

davidre added a comment.


  If you touch the xsd do you neeed to increase the version number?

REPOSITORY
  R237 KConfig

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

To: meven, ervin, bport, crossi, #frameworks
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[David Redondo]

davidre updated this revision to Diff 75717.
davidre added a comment.


  remove debug remains

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27035?vs=75716=75717

BRANCH
  no-css (branched from master)

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

AFFECTED FILES
  autotests/kmessagewidgetautotest.cpp
  src/kmessagewidget.cpp

To: davidre, #frameworks, ngraham
Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[David Redondo]

davidre updated this revision to Diff 75716.
davidre added a comment.


  Explicitly set TextColor and palettes of the labels and react to parent 
changes . For example necessary for konsole 
  that uses stylesheet which break palette propagation and reparents the widget 
after creation.

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27035?vs=74766=75716

BRANCH
  no-css (branched from master)

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

AFFECTED FILES
  autotests/kmessagewidgetautotest.cpp
  src/kmessagewidget.cpp

To: davidre, #frameworks, ngraham
Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D27315: Fix size of the header in GridViewKCM

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R296:d9b52d900471: Fix size of the header in GridViewKCM 
(authored by davidre).

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27315?vs=75445=75642

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

AFFECTED FILES
  src/qmlcontrols/kcmcontrols/qml/GridViewKCM.qml

To: davidre, mart, broulik, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns

D27315: Fix size of the header in GridViewKCM

[David Redondo]

davidre edited the test plan for this revision.
davidre added a reviewer: broulik.

REPOSITORY
  R296 KDeclarative

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

To: davidre, mart, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27315: Fix size of the header in GridViewKCM

[David Redondo]

davidre created this revision.
davidre added a reviewer: mart.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidre requested review of this revision.

REVISION SUMMARY
  The same as in SimpleKCM and ScrollviewKCM

REPOSITORY
  R296 KDeclarative

BRANCH
  gridviewkcm (branched from master)

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

AFFECTED FILES
  src/qmlcontrols/kcmcontrols/qml/GridViewKCM.qml

To: davidre, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27272: Add status icons: data-error, data-warning, data-information

[David Redondo]

davidre added a comment.


  In D27272#608602 , @ngraham wrote:
  
  > I think the emblem icons look great in your "After" screenshot.
  >
  > Wouldn't it be easier to have KDevelop use these emblem icons from their 
current names rather than symlinking the icons you want to use to new files 
with different names? Semantically it seems wrong; status icons are monochrome 
and use the action icon style. See 
https://hig.kde.org/style/icons/action_status.html.
  
  
  The reason for this patch is the same concern. Using the emblem icons here is 
also semantically wrong[1]. While it gives good results with Breeze and other 
themes like Oxygen or Adwaita don't have them and we can use the fallback of 
`dialog-*` we don't know if it's true for every third party theme.
  [1] 
https://specifications.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html
  
  > Emblems Icons for tags and properties of files, that are displayed in 
the file manager. This context contains emblems for such things as “read-only” 
or “photos”.

REPOSITORY
  R266 Breeze Icons

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

To: kossebau, #vdg, ndavis
Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

[David Redondo]

davidre added a comment.


  Ping, how should I proceed?

REPOSITORY
  R236 KWidgetsAddons

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

To: davidre, #frameworks, ngraham
Cc: cfeck, dhaumann, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns

D25123: New yakuake logo/icon

[David Redondo]

davidre added a comment.


  If you land this please update the summary before. No need to have all the 
variants  in the commit message it should be about the actual change. Thanks :)

REPOSITORY
  R266 Breeze Icons

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

To: mglb, #vdg, #yakuake, hein
Cc: davidre, GB_2, ndavis, ognarb, ngraham, hein, kde-frameworks-devel, 
numerodix, vmarinescu, ryanmccoskrie, LeGast00n, cblack, michaelh, bruns

D26951: Remove old code that is not used anymore

[David Redondo]

davidre added a comment.


  I just took a look at KSelector and it actually inherits minimum and maximum 
properties from QAbstractSlider so we could just drop these custom properties 
for KF6

REPOSITORY
  R265 KConfigWidgets

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

To: davidre, dfaure, ervin, #frameworks
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns