D6394: Add KF6-TODO to make KComboBox::minimumSizeHint() public

[David Faure]

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

REPOSITORY
  R284 KCompletion

BRANCH
  addkf5todo

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

To: kossebau, #frameworks, dfaure
Cc: cfeck, apol

D6047: WIP: Support XDG v6

[Martin Flöser]

graesslin added a comment.


  In https://phabricator.kde.org/D6047#119698, @mart wrote:
  
  > trying to implement the kwin part of ping btw, will do in a branch of the 
branch kwin/xdgv6
  
  
  no need to hurry with it. IMHO the way ping works is completely broken and 
won't work. At least Qt holds the connection in a thread, so it will react to 
the ping even if the application is frozen in the main gui thread.
  
  Also please note that ping needs to be different in Wayland compared to X11. 
In X11 we only ping when trying to close the window. In Wayland we should ping 
whenever we interact with the window in some way. E.g. when we pass pointer 
focus, when we pass keyboard focus, maybe even when sending in input events.

REPOSITORY
  R127 KWayland

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

To: davidedmundson, #plasma
Cc: graesslin, mart, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D6047: WIP: Support XDG v6

[David Edmundson]

davidedmundson marked 2 inline comments as done.
davidedmundson added a comment.


  Done all comments except ping/pong.
  Marco, ping me if you ever want this updating.

REPOSITORY
  R127 KWayland

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

To: davidedmundson, #plasma
Cc: graesslin, mart, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D6047: WIP: Support XDG v6

[David Edmundson]

davidedmundson updated this revision to Diff 15889.
davidedmundson marked 2 inline comments as done.
davidedmundson added a comment.
Restricted Application edited projects, added Plasma; removed Plasma on Wayland.


  - cleanups

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6047?vs=15797=15889

BRANCH
  davidedmundson/xdgv6popup

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

AFFECTED FILES
  autotests/client/CMakeLists.txt
  autotests/client/test_xdg_shell.cpp
  autotests/client/test_xdg_shell.h
  autotests/client/test_xdg_shell_v5.cpp
  autotests/client/test_xdg_shell_v6.cpp
  src/client/CMakeLists.txt
  src/client/protocols/xdg-shell-unstable-v6.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/xdgshell.cpp
  src/client/xdgshell.h
  src/client/xdgshell_p.h
  src/client/xdgshell_v5.cpp
  src/client/xdgshell_v6.cpp
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/xdgshell_interface.cpp
  src/server/xdgshell_interface.h
  src/server/xdgshell_interface_p.h
  src/server/xdgshell_v5_interface.cpp
  src/server/xdgshell_v5_interface_p.h
  src/server/xdgshell_v6_interface.cpp
  src/server/xdgshell_v6_interface_p.h
  src/tools/mapping.txt
  tests/CMakeLists.txt
  tests/xdgtest.cpp

To: davidedmundson, #plasma
Cc: graesslin, mart, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D6047: WIP: Support XDG v6

[Marco Martin]

mart added a comment.


  trying to implement the kwin part of ping btw, will do in a branch of the 
branch kwin/xdgv6

REPOSITORY
  R127 KWayland

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

To: davidedmundson, #plasma
Cc: graesslin, mart, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein, lukas

D6394: Add KF6-TODO to make KComboBox::minimumSizeHint() public

[Christoph Feck]

cfeck added a comment.


  +1
  
  That shouldn't have landed in "protected" area.
  
  And no, MSVC has not changed :)

REPOSITORY
  R284 KCompletion

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

To: kossebau, #frameworks
Cc: cfeck, apol

D6394: Add KF6-TODO to make KComboBox::minimumSizeHint() public

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> apol wrote in kcombobox.h:527
> Why KF6? changing visibility doesn't break ABI, AFAIK. We should just not 
> reorder them.

https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B 
hints the access rights are mangled into the symbol signature with some 
compilers. At least MSVC is mentioned here: 
http://blog.qt.io/blog/2009/08/12/some-thoughts-on-binary-compatibility/

Happy to learn that things have changed meanwhile, this is my latest info on 
that matter, never heard of anything else :)

REPOSITORY
  R284 KCompletion

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

To: kossebau, #frameworks
Cc: apol

D6394: Add KF6-TODO to make KComboBox::minimumSizeHint() public

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> kcombobox.h:527
>  
> +// TODO KF6: make public like in base classes, so consumers do not need 
> to cast to base classes
> +// when they have a KComboBox (or subclasses) object and want to access 
> this property

Why KF6? changing visibility doesn't break ABI, AFAIK. We should just not 
reorder them.

REPOSITORY
  R284 KCompletion

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

To: kossebau, #frameworks
Cc: apol

D6394: Add KF6-TODO to make KComboBox::minimumSizeHint() public

[Friedrich W. H. Kossebau]

kossebau created this revision.
Restricted Application added a project: Frameworks.

REPOSITORY
  R284 KCompletion

BRANCH
  addkf5todo

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

AFFECTED FILES
  src/kcombobox.h

To: kossebau, #frameworks

D6390: WIP: Add remote runners over DBus

[David Edmundson]

davidedmundson created this revision.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  This patch allows adding runners which, rather than being in-process
  receive a search term and provide results over DBus.
  
  A metadata file should be installed with metadata for the service and
  path, and the remote app should implement the supplied interface.
  
  By using metadata files all the current krunner architecture and config
  management works seamlessly, and it allows the possibility for a runner
  to be DBus activated if it wants to be.
  
  The goal of this patch is threefold:
  
  - To make it considerably easier to write runners where the data exists
  
  in an already running application. The plasma-browser-integration is a
  good example, it currently has it's own custom protocol, and ends up
  sending data on all the tabs only to filter it inside krunner, somewhat
  wasteful. With this adding searchable tabs to konversation (for example) 
  would be a ten minute patch.
  
  - Make it easy to write a runner in any other language. An app just
  
  needs to speak DBus with no other dependencies. Potentially even shipped
  via the store.
  
  - This approach could work for runners in sandboxes.

TEST PLAN
  Wrote new sample app
  Got my runner showing results
  Brief profiling had roundtrip time as 0ms.

REPOSITORY
  R308 KRunner

BRANCH
  master

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

AFFECTED FILES
  src/CMakeLists.txt
  src/data/org.kde.krunner1.xml
  src/data/servicetypes/plasma-runner.desktop
  src/dbusrunner.cpp
  src/dbusrunner_p.h
  src/dbusutils_p.h
  src/runnermanager.cpp

To: davidedmundson, #plasma
Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart, lukas

Re: Kirigami in Frameworks

[Marco Martin]

On Sat, Jun 24, 2017 at 3:27 PM, David Faure  wrote:
>> the default style for QtQuickControlsStyle1 is "Desktop"
>> we could have called this style "Desktop" as well so all would have
>> aligned nicely, but that could be quite dangerous, as Qt coulddecide
>> any moment that they indeed want to do a style called "Desktop" for
>> qqc2, at which point it would conflict, so having the org.kde prefix
>> was the safe route.
>
> Well, we could also rename ours when/if the conflict occurs?

since the conflict would be of installed files, it would make released
version not installable, which looks to me like too big of a risk.

>> One way to silence this could be when the qtquickcontrols2
>> theme installs into qt's qqc2
>
> If you mean $QTDIR, that's not my case. The plugin is found via
> QML2_IMPORT_PATH I suppose.

qtquickcontrols2 will be installed under QML2_IMPORT_PATH and we need
to install under that folder

>> , to install also an org.kde.desktop
>> symlink in QtQuickControls1 that just points to "Desktop" (note that
>> style is not in the kirigami repository, kirigami has themes with the
>> same name because it's an extension on top of qtquickcontrols2)
>
> That sounds good, if it can be done.

setting a symlink there seems to just work, so i would try to go for that route

>> It has some android specific things, like providing a manifest.xml and
>> some optional qtandroidextras usage for integration when compiled
>> there, but it's intended to be multiplatform, so may make sesie to
>> enable it.
>
> Then I don't think it should be under an android subdir.
> A multiplatform app with extra stuff for android integration is still
> multiplatform in the first place.

sure, renaming it to multipletform then :)

>> talking about examples, do you think is better to build them with a
>> flag on the top cmake as is now, or provide a separate standalone
>> cmake file under examples/ ?
>
> Much better the way you did it, it's how I see it done in most other
> frameworks.
> It makes it easy to enable the building of examples once and for all in
> kdesrc-buildrc for instance, to make sure they keep compiling and so that they
> are available for testing.
> In fact, I wonder if the CI shouldn't set BUILD_EXAMPLES=ON in all frameworks
> that have that option... (4 currently, 5 with kirigami).

ok

--
Marco Martin

D6313: WIP: Support device pixel ratio in icon loader and engine

[Marco Martin]

mart added a comment.


  In https://phabricator.kde.org/D6313#118285, @kvermette wrote:
  
  > Behaviorally speaking there's justification for ensuring SVGs are treated 
the same as PNGs in this case. Looking at the code we aren't shimming the SVGs 
specifically (unless I'm missing something), but I just wanted to chime in with 
this and nip special treatment for SVGs in the bud. In maintaining the Aether 
icon theme I would create links to the higher resolution icons, so you'd see 
something like a "16x16x2" folder point to my "32x32" 'native' folder, and a 
"16x16x3"->"48x48" folder, "32x32x2"->"64x64", etc. Just because we *can* scale 
SVG icons doesn't mean that behavior should be assumed, should the icon set 
have higher fidelity icons ready for HiDPI.
  
  
  i think for svgs 16x16x2 should be preferred over scaling by 2 the 16x16 
version, but still, scaling the 16x16 version should still be preferred over 
the 32x32 version, so this patch is on the right rirection

REPOSITORY
  R302 KIconThemes

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

To: broulik, kde-frameworks-devel, #plasma, #vdg
Cc: mart, kvermette, cfeck, davidedmundson, plasma-devel, #frameworks, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D6376: Fix double delete crash during shutdown

[René J.V. Bertin]

rjvbb added a comment.


  In https://phabricator.kde.org/D6376#119500, @cullmann wrote:
  
  > I don't speak about threading races. The whole class is not thread-safe, if 
threading occurs, all is lost.
  
  
  Really? Only a single thread at a time should trigger audio notifications, is 
that what you're saying? That would surprise me a little, looking at the code 
it seems the class exists so that applications can request to play a 
notification sound by creating an instance and then forget about it. The 
threading I was thinking of is simply when 2 threads queue an audio 
notification at the same time, not the cross-thread use of a NotifyByAudio 
instance.

REPOSITORY
  R289 KNotifications

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

To: cullmann, #frameworks
Cc: aacid, mpyne, rjvbb

D6376: Fix double delete crash during shutdown

[Christoph Cullmann]

cullmann abandoned this revision.

REPOSITORY
  R289 KNotifications

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

To: cullmann, #frameworks
Cc: aacid, mpyne, rjvbb