D29096: Prefix includes and libs dir with QT_SYSROOT

[Erik Schilling]

ablu added a comment.


  Sure!
  
  Though... How does it come that you do not have SYSROOT set in your case? 
Won't that break in other cases?

REPOSITORY
  R240 Extra CMake Modules

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

To: ablu, #build_system, apol, vkrause, kfunk
Cc: dfaure, kossebau, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
bencreasy, michaelh, ngraham, bruns

D29274: ECMGeneratePriFile: make the pri files relocatable

[Erik Schilling]

ablu added a comment.


  The Build worked. So this solves the issue for me!

REPOSITORY
  R240 Extra CMake Modules

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

To: dfaure, vatra, kfunk, apol
Cc: ablu, kossebau, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
bencreasy, michaelh, ngraham, bruns

D29208: [NestedListHelper] Improve indentation code

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> nestedlisthelper.cpp:87
> +QTextBlock nextBlock = block.next();
> +if (!block.isValid()) {
> +return false;

So the last block cannot be unindented? How come?

REPOSITORY
  R310 KTextWidgets

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

To: poboiko, #frameworks, dfaure, mlaurent
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29292: Port KKeySequenceItem to QQC2

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> cblack wrote in KeySequenceItem.qml:57
> Are we able to use some form of units? Hardcoding this seems wrong.

> Can we use the non-attached version here please since it's not likely

It's the worst!

> Are we able to use some form of units? Hardcoding this seems wrong.

It's come up before, this isn't ideal, but there's no other consistent 
alternative. It's the convention followed elsewhere.

Let's follow that up on https://phabricator.kde.org/T10873

> broulik wrote in KeySequenceItem.qml:58
> Is this `_tr` thing we could also improve (separately o/c)?

We need to specify the domain, but you're right i18nd would work just as well 
and save a QObject

REPOSITORY
  R296 KDeclarative

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

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

D29292: Port KKeySequenceItem to QQC2

[Nathaniel Graham]

ngraham added a reviewer: davidre.

REPOSITORY
  R296 KDeclarative

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

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

D29292: Port KKeySequenceItem to QQC2

[Carson Black]

cblack added inline comments.

INLINE COMMENTS

> KeySequenceItem.qml:57
> +ToolTip.visible: hovered
> +ToolTip.delay: 1000
> +ToolTip.text:  _tr.i18n("Click on the button, then enter the 
> shortcut like you would in the program.\nExample for Ctrl+A: hold the Ctrl 
> key and press A.")

Are we able to use some form of units? Hardcoding this seems wrong.

> KeySequenceItem.qml:59
> +ToolTip.text:  _tr.i18n("Click on the button, then enter the 
> shortcut like you would in the program.\nExample for Ctrl+A: hold the Ctrl 
> key and press A.")
> +ToolTip.timeout: 5000
>  

Ditto here, but even hardcoded this looks wrong.

REPOSITORY
  R296 KDeclarative

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

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

D29292: Port KKeySequenceItem to QQC2

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> KeySequenceItem.qml:57
> +ToolTip.visible: hovered
> +ToolTip.delay: 1000
> +ToolTip.text:  _tr.i18n("Click on the button, then enter the 
> shortcut like you would in the program.\nExample for Ctrl+A: hold the Ctrl 
> key and press A.")

Can we use the non-attached version here please since it's not likely (except 
maybe System Tray delegate) to be used in large quantities?

> KeySequenceItem.qml:58
> +ToolTip.delay: 1000
> +ToolTip.text:  _tr.i18n("Click on the button, then enter the 
> shortcut like you would in the program.\nExample for Ctrl+A: hold the Ctrl 
> key and press A.")
> +ToolTip.timeout: 5000

Is this `_tr` thing we could also improve (separately o/c)?

> KeySequenceItem.qml:61
>  
>  onCheckedChanged: {
>  if (checked) {

Probably can now use `onToggled`?

REPOSITORY
  R296 KDeclarative

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

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

D29292: Port KKeySequenceItem to QQC2

[David Edmundson]

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

REVISION SUMMARY
  This implicitly fixes a bug where space is used in the shortcut item.
  Previously as QQC1 button was made up of multiple objects the key
  handling would be handled by an internal object before our filters.
  
  In QQC2 it's all one item, so our Keys handler takes precendence.

TEST PLAN
  Added a test file
  Set the button to control+space, alt+4
  Set the button to alt+space (this correctly showed a warning about a conflict)
  Confirmed tooltip worked correctly
  Used space to activate the button when we weren't recording. This someone 
still worked..

REPOSITORY
  R296 KDeclarative

BRANCH
  master

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

AFFECTED FILES
  src/qmlcontrols/kquickcontrols/KeySequenceItem.qml
  tests/keysequencetest.qml

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

D29281: Deprecate defunct functions

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Looks good to me now, just caring about the use of the deprecation macros (no 
clue about the actual code, has to be Kai or someone else with krunner 
knowledge (if there is) to review).
  
  Discussing this I see a small flaw in the case of retrospective deprecation 
when it comes to the difference of the version number used in the API dox 
comment (`@deprecated Since 5,0, this feature has been defunct`) to the version 
number use with the code deprecation macro (`KRUNNER_DEPRECATED_VERSION(5, 70, 
"No longer use, feature removed")` will resolve to the compiler warning ",Since 
5 70. No longer use, feature removed"). But we cannot also use the true version 
there as well, otherwise might run into people who (in theory at least ) have 
-Werror=deprecated_declarations set and could now suddently trigger over the 
new warning, depending on what they set KF_DISABLE_WARNINGS_SINCE to... have to 
think about that some more how that can be resolved elegantly without more 
complexity, your current patch matches what other code does, so nothing to 
change here for now IMHO. If you have comments given your experience here, 
happy to hear.

REPOSITORY
  R308 KRunner

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

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

Re: Having $(framework)_global.h headers (was: Re: building KF5 projects against a different Qt5 version [...])

[Friedrich W. H. Kossebau]

Am Mittwoch, 29. April 2020, 21:41:37 CEST schrieb David Faure:
> On lundi 27 avril 2020 14:28:42 CEST Friedrich W. H. Kossebau wrote:
> > Am Sonntag, 26. April 2020, 16:12:31 CEST schrieb Friedrich W. H. 
Kossebau:
> > > Am Sonntag, 26. April 2020, 15:46:35 CEST schrieb David Faure:
> > > > On Sunday, April 26, 2020 3:21:34 PM CEST René J.V. Bertin wrote:
> > > > > Talking about version test hacks (or not-so-hacks): why is it that
> > > > > KFOO_VERSION isn't defined systematically when you include any
> > > > > header
> > > > > of
> > > > > the FOO KF5 framework? With Qt you never have to worry about
> > > > > QT_VERSION
> > > > > being defined.
> > > > 
> > > > Well, everything in Qt ends up including global.h
> > > > There's no such central header in KF5 frameworks (which are more
> > > > modular,
> > > > by definition), so you need to include the framework_version.h header.
> > > 
> > > We could perhaps look into extending the export headers into some kind
> > > of
> > > $
> > > (framework)_global.h headers, like we are now doing already a bit for
> > > all
> > > the deprecation definitions when using ecm_generate_export_header, where
> > > all additional stuff is injected via the export headers.
> > > 
> > > I also found a bit annoying to have to add all the explicit includes of
> > > framework_version.h every time one wanted to access KFOO_VERSION.
> > > 
> > > So far had only pondered the idea at that time, no yet investigated
> > > further
> > > given state of own TODO list :) Also low motivation given that there
> > > would
> > > need to be a backward-compatible setup, and usually one uses that very
> > > version header to be compatible to old versions, so there was no
> > > immediate
> > > gain. But long time (at least KF6) I would also prefer to see the
> > > version
> > > macros available by default.
> > 
> > And I meanwhile noticed that the actual version number of a library is
> > already present in the export headers generated with
> > ECMGenerateExportHeader, due to being a possible default number for all
> > the
> > version-dependent calculations for what to show warnings or which API to
> > hide. Not accessible as macro though, so do not have hopes to "abuse" this
> > 
> > :)
> > 
> > Given this kept following me into my shower thoughts this morning, guess I
> > will soonish be working on some ECMGenerateGlobalHeader then, or at least
> > explore more for now in some non-ECM projects the feasibility of semi-
> > generation of such global header carrying all the stuff interesting with
> > all the (public) API of a library. Ping me again in autumn about this.
> I know it's not autumn yet, but isn't this what COMMON_HEADER in
> ecm_generate_headers already does, actually?

No, this is not what I mean here. COMMON_HEADER (from what I understand) 
creates a include-complete-module-headers convenience header, to be used 
instead of including individuals headers as needed. As in: #include 

I meant something similar to qglobal.h, so providing export macros, version 
macros and posible similar module-wide code things which one would expect 
available in all places more or less. And which thus one would expect to 
already be included when using other classes/stuff from a module.

Cheers
Friedrich

Re: Information regarding upcoming Gitlab Migration

[Adriaan de Groot]

On 2020 prilula d. 28id 13:35:22 CEST Ben Cooksley wrote:
> Would some form of git alias/custom command script that works similar
> to the following be suitable?
> 
> git kde-clone skrooge
> 
> That script would then search the appropriate groups (ignoring any
> personal repositories including forks), find the necessary repository
> and perform the clone as appropriate for you. Should it find multiple
> results it would output it's results and then exit with a failure
> (giving you names and the clone urls to pick from manually)

That'd actually be pretty cool.

As a standalone script it'd be useful to migrate existing checkouts, so that's 
shooting two birds in one foot. And then you can have a somewhat structured 
namespace, still with simple lookup and unstructured names (until, as Bhushan 
points out in a different message in this thread, you get non-unique labels 
when decomposing structures names).


[ade]


signature.asc
Description: This is a digitally signed message part.

Re: Having $(framework)_global.h headers (was: Re: building KF5 projects against a different Qt5 version [...])

[David Faure]

On lundi 27 avril 2020 14:28:42 CEST Friedrich W. H. Kossebau wrote:
> Am Sonntag, 26. April 2020, 16:12:31 CEST schrieb Friedrich W. H. Kossebau:
> > Am Sonntag, 26. April 2020, 15:46:35 CEST schrieb David Faure:
> > > On Sunday, April 26, 2020 3:21:34 PM CEST René J.V. Bertin wrote:
> > > > Talking about version test hacks (or not-so-hacks): why is it that
> > > > KFOO_VERSION isn't defined systematically when you include any header
> > > > of
> > > > the FOO KF5 framework? With Qt you never have to worry about
> > > > QT_VERSION
> > > > being defined.
> > > 
> > > Well, everything in Qt ends up including global.h
> > > There's no such central header in KF5 frameworks (which are more
> > > modular,
> > > by definition), so you need to include the framework_version.h header.
> > 
> > We could perhaps look into extending the export headers into some kind of
> > $
> > (framework)_global.h headers, like we are now doing already a bit for all
> > the deprecation definitions when using ecm_generate_export_header, where
> > all additional stuff is injected via the export headers.
> > 
> > I also found a bit annoying to have to add all the explicit includes of
> > framework_version.h every time one wanted to access KFOO_VERSION.
> > 
> > So far had only pondered the idea at that time, no yet investigated
> > further
> > given state of own TODO list :) Also low motivation given that there would
> > need to be a backward-compatible setup, and usually one uses that very
> > version header to be compatible to old versions, so there was no immediate
> > gain. But long time (at least KF6) I would also prefer to see the version
> > macros available by default.
> 
> And I meanwhile noticed that the actual version number of a library is
> already present in the export headers generated with
> ECMGenerateExportHeader, due to being a possible default number for all the
> version-dependent calculations for what to show warnings or which API to
> hide. Not accessible as macro though, so do not have hopes to "abuse" this
> :)
> 
> Given this kept following me into my shower thoughts this morning, guess I
> will soonish be working on some ECMGenerateGlobalHeader then, or at least
> explore more for now in some non-ECM projects the feasibility of semi-
> generation of such global header carrying all the stuff interesting with all
> the (public) API of a library. Ping me again in autumn about this.

I know it's not autumn yet, but isn't this what COMMON_HEADER in 
ecm_generate_headers already does, actually?

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 81546.
alex added a comment.


  Wrap macro about single method, change deprecation text
  
  And thanks for the feedback and explanations :-).

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=81543=81546

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

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

D29281: Deprecate defunct functions

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> kossebau wrote in abstractrunner.cpp:221
> Given EXCLUDE_DEPRECATED_BEFORE_AND_AT is used with KRunner, you want to also 
> wrap all the implementations of the now deprecated API with the BUILD variant 
> of the macro, so here for example:
> 
>   #if KRUNNER_BUILD_DEPRECATED_SINCE(5, 70)
>   void AbstractRunner::createRunOptions(QWidget *parent)
>   {
>   [...]
>   }
>   #endif
> 
> This allows to build KRunner itself without any of the deprecated API 
> actually part of the build.
> Test yourself by setting the cmake var 
> EXCLUDE_DEPRECATED_BEFORE_AND_AT=CURRENT  (e.g. edit the var in the cmake 
> cache).

And best practice seems to be not to do mass wrapping of many methods, but do 
explicitely per method. While more code/lines, has the advantage to be easier 
to read/understand, as usually begin/emd are in view, and also avoids that 
actually wrong code is wrapped (e.g. when methods get added later and by 
accident are placed amid the wrapped ones.

> alex wrote in abstractrunner.h:154
> That makes sense, but I want to express that this method has been defunct  
> since version 5.0
> so that the maintainers of existing plugins can be sure that they can savely 
> remove the method calls.
> Whats the best way to express this?

I would just say in the docs "@deprecated Since 5,0, this feature has been 
defunct".  This is what has been done elsewhere where API got retrroactively 
tagged as deprecated. This is just text in the docs, now with up-to-date 
information, so other than the macros will not affect anything.

REPOSITORY
  R308 KRunner

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

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

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 81543.
alex added a comment.


  Wrap implementations

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=81539=81543

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

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

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> kossebau wrote in abstractrunner.h:154
> The text in the API dox can stay "Since 5.0".
> 
> The only crititical thing where the number of an older, already released 
> version should not be used is the KRUNNER_ENABLE_DEPRECATED_SINCE macro. 
> Because that reacts to KF_DISABLE_DEPRECATED_BEFORE_AND_AT (or 
> KRUNNER_DISABLE_DEPRECATED_BEFORE_AND_AT if set). And someone using, say 
> KF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x05 in their projects because they 
> checked their software and it has not used any deprecated API up to that 
> version, and who still uses this newly deprecated API, would suddenly get no 
> longer building software, which especially is annoying with already released 
> code.
> 
> So: when retroactively tagging deprecated API, in the API dox text mention 
> the correct version where actual deprecation happened, in the macros the 
> version of the upcoming release where the deprecation macros are first 
> existing for API users. Makes sense?

That makes sense, but I want to express that this method has been defunct  
since version 5.0
so that the maintainers of existing plugins can be sure that they can savely 
remove the method calls.
Whats the best way to express this?

REPOSITORY
  R308 KRunner

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

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

D29281: Deprecate defunct functions

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Looks good, almost :) It's a bit complicated on first use (well, also still 
ater, but no better solution was found to get version-controlled API 
deprecation with C++ for now).

INLINE COMMENTS

> abstractrunner.cpp:221
>  
>  void AbstractRunner::createRunOptions(QWidget *parent)
>  {

Given EXCLUDE_DEPRECATED_BEFORE_AND_AT is used with KRunner, you want to also 
wrap all the implementations of the now deprecated API with the BUILD variant 
of the macro, so here for example:

  #if KRUNNER_BUILD_DEPRECATED_SINCE(5, 70)
  void AbstractRunner::createRunOptions(QWidget *parent)
  {
  [...]
  }
  #endif

This allows to build KRunner itself without any of the deprecated API actually 
part of the build.
Test yourself by setting the cmake var EXCLUDE_DEPRECATED_BEFORE_AND_AT=CURRENT 
 (e.g. edit the var in the cmake cache).

> abstractrunner.h:154
>   * is called, the runner should return true
> + * @deprecated since 5.70, feature removed since version 5.0
>   */

The text in the API dox can stay "Since 5.0".

The only crititical thing where the number of an older, already released 
version should not be used is the KRUNNER_ENABLE_DEPRECATED_SINCE macro. 
Because that reacts to KF_DISABLE_DEPRECATED_BEFORE_AND_AT (or 
KRUNNER_DISABLE_DEPRECATED_BEFORE_AND_AT if set). And someone using, say 
KF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x05 in their projects because they 
checked their software and it has not used any deprecated API up to that 
version, and who still uses this newly deprecated API, would suddenly get no 
longer building software, which especially is annoying with already released 
code.

So: when retroactively tagging deprecated API, in the API dox text mention the 
correct version where actual deprecation happened, in the macros the version of 
the upcoming release where the deprecation macros are first existing for API 
users. Makes sense?

REPOSITORY
  R308 KRunner

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

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

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 81539.
alex added a comment.


  Use mordern macros
  
  Thanks :-)

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=81528=81539

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

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

D29281: Deprecate defunct functions

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Be aware that just adding a plain KRUNNER_DEPRECATED is not up-to-modern KF 
standards :)
  Please see cf5f7b4040a77ae69452d58bc189dcc3baaedd92 
 
what macros there are now  and how to apply them. Note especially the 
difference between the ENABLE (for declarations, visible both to library and 
3rd-party API users) & BUILD (visible only to library)), as well as that 5.0 
can be only used in the API dox text, the macros need the version where the 
deprecation tags are first used/released with, to be backward-compatible for 
library consumers which use the KF_DISABLE_DEPRECATED_BEFORE_AND_AT control 
macro.
  
  Please see also 
https://community.kde.org/Policies/Library_Code_Policy#Deprecation_of_API  for 
general info about API deprecation in KF.

REPOSITORY
  R308 KRunner

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

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

D29258: Don't use notifybysnore.h on MSYS2

[Łukasz Wojniłowicz]

wojnilowicz added a comment.


  In D29258#659423 , @brute4s99 
wrote:
  
  > > SnoreToast fails to build on MSYS2 due to missing
  > >  which apparently is not available for this compiler.
  >
  > I'm sorry, missing what exactly?
  
  
  Missing "wrl\wrappers\corewrappers.h".

REPOSITORY
  R289 KNotifications

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

To: wojnilowicz, broulik, brute4s99, vonreth
Cc: vonreth, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29258: Don't use notifybysnore.h on MSYS2

[Łukasz Wojniłowicz]

wojnilowicz updated this revision to Diff 81529.
wojnilowicz added a comment.


  I see. I've updated the patch. Linking to downloaded library is now possible, 
and there is a fallback if no library is present.

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29258?vs=81440=81529

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  src/CMakeLists.txt
  src/config-knotifications.h.cmake
  src/knotificationmanager.cpp

To: wojnilowicz, broulik, brute4s99, vonreth
Cc: vonreth, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 81528.
alex added a comment.


  Add missing KRUNNER_DEPRECATED

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=81527=81528

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

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

D29281: Deprecate defunct functions

[Alexander Lohnau]

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

REVISION SUMMARY
  As in T12163  explained KRunner should be 
ported away from QWidgets and the
  fuctions are defuct anyway.

TEST PLAN
  Compiles

REPOSITORY
  R308 KRunner

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

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

D29096: Prefix includes and libs dir with QT_SYSROOT

[David Faure]

dfaure added a comment.


  Thanks for the quick test!
  
  I'll indeed proceed with the other one, which is more generic (our use case 
isn't related to QT_SYSROOT).
  
  Actually: can you add your comment (that it works for you) in the other 
review request, so people know it's been tested by more than me, and discard 
this one?

REPOSITORY
  R240 Extra CMake Modules

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

To: ablu, #build_system, apol, vkrause, kfunk
Cc: dfaure, kossebau, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
bencreasy, michaelh, ngraham, bruns

D29278: Port KWin to KWaylandServer

[Aleix Pol Gonzalez]

apol created this revision.
apol added reviewers: KWin, Plasma, Frameworks.
Herald added a project: KWin.
Herald added a subscriber: kwin.
apol requested review of this revision.

REVISION SUMMARY
  Away from KWayland::Server and KF5WaylandServer.

TEST PLAN
  Builds, ran nested session

REPOSITORY
  R108 KWin

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  abstract_client.cpp
  abstract_client.h
  abstract_output.cpp
  abstract_output.h
  abstract_wayland_output.cpp
  abstract_wayland_output.h
  autotests/CMakeLists.txt
  autotests/integration/dont_crash_cursor_physical_size_empty.cpp
  autotests/integration/globalshortcuts_test.cpp
  autotests/integration/idle_inhibition_test.cpp
  autotests/integration/input_stacking_order.cpp
  autotests/integration/internal_window.cpp
  autotests/integration/lockscreen.cpp
  autotests/integration/maximize_test.cpp
  autotests/integration/plasmawindow_test.cpp
  autotests/integration/pointer_constraints_test.cpp
  autotests/integration/pointer_input.cpp
  autotests/integration/scene_opengl_shadow_test.cpp
  autotests/integration/scene_qpainter_shadow_test.cpp
  autotests/integration/scene_qpainter_test.cpp
  autotests/integration/test_helpers.cpp
  autotests/integration/transient_placement.cpp
  autotests/integration/xdgshellclient_test.cpp
  autotests/integration/xwayland_input_test.cpp
  autotests/integration/xwayland_selections_test.cpp
  autotests/mock_effectshandler.h
  autotests/test_window_paint_data.cpp
  composite.cpp
  debug_console.cpp
  decorations/decorationbridge.cpp
  effects.cpp
  effects.h
  effects/backgroundcontrast/contrast.cpp
  effects/backgroundcontrast/contrast.h
  effects/blur/blur.cpp
  effects/blur/blur.h
  effects/desktopgrid/desktopgrid.cpp
  effects/slidingpopups/slidingpopups.cpp
  events.cpp
  idle_inhibition.cpp
  idle_inhibition.h
  input.cpp
  keyboard_input.cpp
  keyboard_repeat.cpp
  libkwineffects/CMakeLists.txt
  libkwineffects/kwineffects.cpp
  libkwineffects/kwineffects.h
  linux_dmabuf.cpp
  linux_dmabuf.h
  main.cpp
  main_wayland.cpp
  platform.cpp
  platform.h
  platformsupport/scenes/opengl/CMakeLists.txt
  platformsupport/scenes/opengl/abstract_egl_backend.cpp
  platformsupport/scenes/opengl/abstract_egl_backend.h
  platformsupport/scenes/opengl/egl_dmabuf.cpp
  platformsupport/scenes/opengl/egl_dmabuf.h
  plugins/platforms/drm/drm_backend.cpp
  plugins/platforms/drm/drm_inputeventfilter.cpp
  plugins/platforms/drm/drm_output.cpp
  plugins/platforms/drm/drm_output.h
  plugins/platforms/drm/egl_stream_backend.cpp
  plugins/platforms/drm/egl_stream_backend.h
  plugins/platforms/drm/remoteaccess_manager.cpp
  plugins/platforms/drm/remoteaccess_manager.h
  plugins/platforms/fbdev/fb_backend.cpp
  plugins/platforms/hwcomposer/hwcomposer_backend.cpp
  plugins/platforms/hwcomposer/hwcomposer_backend.h
  plugins/platforms/virtual/virtual_backend.cpp
  plugins/platforms/virtual/virtual_output.cpp
  plugins/platforms/wayland/egl_wayland_backend.cpp
  plugins/platforms/wayland/wayland_backend.cpp
  plugins/platforms/wayland/wayland_output.cpp
  plugins/platforms/x11/windowed/x11windowed_backend.cpp
  plugins/platforms/x11/windowed/x11windowed_output.cpp
  plugins/scenes/opengl/scene_opengl.cpp
  plugins/scenes/opengl/scene_opengl.h
  plugins/scenes/qpainter/scene_qpainter.cpp
  plugins/scenes/qpainter/scene_qpainter.h
  pointer_input.cpp
  pointer_input.h
  scene.cpp
  scene.h
  shadow.cpp
  shadow.h
  tablet_input.cpp
  toplevel.cpp
  toplevel.h
  touch_input.cpp
  virtualdesktops.cpp
  virtualdesktops.h
  virtualkeyboard.cpp
  wayland_cursor_theme.cpp
  wayland_server.cpp
  wayland_server.h
  xdgshellclient.cpp
  xdgshellclient.h
  xkb.cpp
  xkb.h
  xwl/clipboard.cpp
  xwl/clipboard.h
  xwl/databridge.cpp
  xwl/databridge.h
  xwl/dnd.cpp
  xwl/dnd.h
  xwl/drag_wl.cpp
  xwl/drag_wl.h
  xwl/drag_x.cpp
  xwl/drag_x.h
  xwl/selection_source.cpp
  xwl/selection_source.h
  xwl/transfer.cpp
  xwl/transfer.h

To: apol, #kwin, #plasma, #frameworks
Cc: kwin, Orage, cacarry, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, 
ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, 
romangg, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29096: Prefix includes and libs dir with QT_SYSROOT

[Erik Schilling]

ablu added a comment.


  looks good! yours is probably a bit more "CMakeish"! So feel free to continue 
with that one and close this.

REPOSITORY
  R240 Extra CMake Modules

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

To: ablu, #build_system, apol, vkrause, kfunk
Cc: dfaure, kossebau, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
bencreasy, michaelh, ngraham, bruns

D29256: [server] Introduce mapped() signal

[Aleix Pol Gonzalez]

apol accepted this revision.

REPOSITORY
  R127 KWayland

BRANCH
  introduce-mapped-signal

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

To: zzag, #kwin, davidedmundson, apol
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29274: ECMGeneratePriFile: make the pri files relocatable

[David Faure]

dfaure updated this revision to Diff 81519.
dfaure added a comment.


  Fix error: regex "[^/]*" matched an empty string.

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29274?vs=81506=81519

BRANCH
  master

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

AFFECTED FILES
  modules/ECMGeneratePriFile.cmake

To: dfaure, vatra, kfunk, apol
Cc: ablu, kossebau, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
bencreasy, michaelh, ngraham, bruns

D29274: ECMGeneratePriFile: make the pri files relocatable

[Erik Schilling]

ablu added a comment.


  My reason is basically the same. Will try this version too.

REPOSITORY
  R240 Extra CMake Modules

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

To: dfaure, vatra, kfunk, apol
Cc: ablu, kossebau, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
bencreasy, michaelh, ngraham, bruns

D29096: Prefix includes and libs dir with QT_SYSROOT

[Erik Schilling]

ablu added a comment.


  Hi @dfaure. It looks pretty much the same and I think it should work for us. 
I will kick off a build to confirm.

REPOSITORY
  R240 Extra CMake Modules

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

To: ablu, #build_system, apol, vkrause, kfunk
Cc: dfaure, kossebau, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
bencreasy, michaelh, ngraham, bruns

D26604: Check if there is an activatable service when notification service owner changes

[Nicolas Fella]

nicolasfella abandoned this revision.
nicolasfella added a comment.


  I favor of  D29021 

REPOSITORY
  R289 KNotifications

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

To: nicolasfella, #plasma, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D26605: Remove fallback to KPassivePopup

[Nicolas Fella]

nicolasfella abandoned this revision.
nicolasfella added a comment.


  In favor of  D29021 

REPOSITORY
  R289 KNotifications

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

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

D29096: Prefix includes and libs dir with QT_SYSROOT

[David Faure]

dfaure added a comment.


  Hi @ablu,
  
  Could you test if my patch in D29274  
solves your problem? By making these lines relative to the location of the .pri 
file, it should work very well in a sysroot context as well.

REPOSITORY
  R240 Extra CMake Modules

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

To: ablu, #build_system, apol, vkrause, kfunk
Cc: dfaure, kossebau, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
bencreasy, michaelh, ngraham, bruns

D29021: Remove checks for notification service and fallback to KPassivePopup

[Kai Uwe Broulik]

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


  Verified that close signal works and that quitting plasma, sending a 
notification, then has the notification show up when plasma is started.
  Please wait until after Frameworks tagging before pushing this

REPOSITORY
  R289 KNotifications

BRANCH
  nofallback2

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

To: nicolasfella, #frameworks, #plasma, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

KDE CI: Frameworks » kirigami » kf5-qt5 FreeBSDQt5.14 - Build # 55 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kirigami/job/kf5-qt5%20FreeBSDQt5.14/55/
 Project:
kf5-qt5 FreeBSDQt5.14
 Date of build:
Wed, 29 Apr 2020 14:01:04 +
 Build duration:
5 min 55 sec and counting
   JUnit Tests
  Name: projectroot.usr.home.jenkins.workspace.Frameworks.kirigami.kf5-qt5_FreeBSDQt514 Failed: 0 test(s), Passed: 7 test(s), Skipped: 0 test(s), Total: 7 test(s)

D29027: Move document corner fold to top right in two icons

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R266:db3ec8680b35: Move document corner fold to top right in 
two icons (authored by davidhurka, committed by ngraham).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29027?vs=80707=81512

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

AFFECTED FILES
  icons-dark/actions/16/document-close.svg
  icons-dark/actions/16/gnumeric-link-internal.svg
  icons-dark/actions/22/document-close.svg
  icons-dark/actions/22/gnumeric-link-internal.svg
  icons/actions/16/document-close.svg
  icons/actions/16/gnumeric-link-internal.svg
  icons/actions/22/document-close.svg
  icons/actions/22/gnumeric-link-internal.svg

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

D29027: Move document corner fold to top right in two icons

[Nathaniel Graham]

ngraham added a comment.


  You can do a regular old git push if you cherry-pick the commit onto master. 
I'll land it for you though.

REPOSITORY
  R266 Breeze Icons

BRANCH
  update-document-close

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

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

D29274: ECMGeneratePriFile: make the pri files relocatable

[Friedrich W. H. Kossebau]

kossebau added a comment.


  D29096  also wants to touch those 
entries, for different reasons. You may want to align here.

REPOSITORY
  R240 Extra CMake Modules

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

To: dfaure, vatra, kfunk, apol
Cc: kossebau, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
bencreasy, michaelh, ngraham, bruns

D29274: ECMGeneratePriFile: make the pri files relocatable

[David Faure]

dfaure created this revision.
dfaure added reviewers: vatra, kfunk, apol.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
dfaure requested review of this revision.

REVISION SUMMARY
  Instead of generating
  QT.KArchive.includes = /full/path/include/KF5/KArchive
  make it
  QT.KArchive.includes = $$PWD/../../include/KF5/KArchive
  
  This makes the whole install prefix relocatable after the fact,
  the includes will be found based on where the .pri file ended up.
  
  This is especially useful for Conan support, says Bogdan.

TEST PLAN
  After make install in ECM, cd karchive/examples/helloworld && qmake && make

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  modules/ECMGeneratePriFile.cmake

To: dfaure, vatra, kfunk, apol
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, 
michaelh, ngraham, bruns

[sysadmin/repo-metadata] projects: projects: move new wayland repos to their final place

[Bhushan Shah]

Git commit 53d913c3af459024d359c13def87db8913d8ed6b by Bhushan Shah.
Committed on 29/04/2020 at 11:09.
Pushed by bshah into branch 'master'.

projects: move new wayland repos to their final place

- plasma-wayland-protocols -> frameworks
- kwayland-server -> kde/workspace

Ref: T13052

CCMAIL: kde-frameworks-devel@kde.org
CCMAIL: plasma-de...@kde.org

R  +0-0projects/frameworks/plasma-wayland-protocols/i18n.json [from: 
projects/playground/libs/kwayland-server/i18n.json - 100% similarity]
R  +1-1projects/frameworks/plasma-wayland-protocols/metadata.yaml 
[from: projects/playground/libs/plasma-wayland-protocols/metadata.yaml - 080% 
similarity]
R  +0-0projects/kde/workspace/kwayland-server/i18n.json [from: 
projects/playground/libs/plasma-wayland-protocols/i18n.json - 100% similarity]
R  +1-1projects/kde/workspace/kwayland-server/metadata.yaml [from: 
projects/playground/libs/kwayland-server/metadata.yaml - 082% similarity]

https://commits.kde.org/sysadmin/repo-metadata/53d913c3af459024d359c13def87db8913d8ed6b

diff --git a/projects/playground/libs/kwayland-server/i18n.json 
b/projects/frameworks/plasma-wayland-protocols/i18n.json
similarity index 100%
rename from projects/playground/libs/kwayland-server/i18n.json
rename to projects/frameworks/plasma-wayland-protocols/i18n.json
diff --git a/projects/playground/libs/plasma-wayland-protocols/metadata.yaml 
b/projects/frameworks/plasma-wayland-protocols/metadata.yaml
similarity index 80%
rename from projects/playground/libs/plasma-wayland-protocols/metadata.yaml
rename to projects/frameworks/plasma-wayland-protocols/metadata.yaml
index 9b1dba4..c0602cf 100644
--- a/projects/playground/libs/plasma-wayland-protocols/metadata.yaml
+++ b/projects/frameworks/plasma-wayland-protocols/metadata.yaml
@@ -5,7 +5,7 @@ members:
 - displayname: Aleix Pol
   username: apol
 name: Plasma Wayland Protocols
-projectpath: playground/libs/plasma-wayland-protocols
+projectpath: frameworks/plasma-wayland-protocols
 repoactive: true
 repopath: plasma-wayland-protocols
 type: project
diff --git a/projects/playground/libs/plasma-wayland-protocols/i18n.json 
b/projects/kde/workspace/kwayland-server/i18n.json
similarity index 100%
rename from projects/playground/libs/plasma-wayland-protocols/i18n.json
rename to projects/kde/workspace/kwayland-server/i18n.json
diff --git a/projects/playground/libs/kwayland-server/metadata.yaml 
b/projects/kde/workspace/kwayland-server/metadata.yaml
similarity index 82%
rename from projects/playground/libs/kwayland-server/metadata.yaml
rename to projects/kde/workspace/kwayland-server/metadata.yaml
index d6c5cd8..3f8d612 100644
--- a/projects/playground/libs/kwayland-server/metadata.yaml
+++ b/projects/kde/workspace/kwayland-server/metadata.yaml
@@ -5,7 +5,7 @@ members:
 - displayname: Aleix Pol
   username: apol
 name: KWayland Server
-projectpath: playground/libs/kwayland-server
+projectpath: kde/workspace/kwayland-server
 repoactive: true
 repopath: kwayland-server
 type: project

Information regarding upcoming Gitlab Migration: clarifications

[Bhushan Shah]

Good afternoon,

[Please keep sysad...@kde.org list or bs...@kde.org in the CC for
replies]

I want to clarify some bits for which we have gotten a questions about,

- Non unique naming: There's some teams which prefer if we dropped the
  namespace- part from their name which we have added. While currently
  this does not result in the naming conflict right away, we realize
  this will cause it at one point, for example,

  maui-dialer -> maui/dialer
  plasma-dialer -> plasma-mobile/dialer

  To minimize the impact of the Gitlab move we won't be doing such
  renames which introduce non-unique names right away. But we would
  prefer if the existing tooling or infrastructure is ready for this
  kind of cases at later point. Only way to enforce non-unique naming is
  one big KDE/ subgroup which we want to avoid.

  Current naming in the repo-metadata branch[1] I've pointed does not
  reflect those renames, as we are not planning to do those renames
  right away during gitlab move, but at a later stage.

- Existing configurations: we want to reduce impact on existing release
  schedule, and existing developer workflow,  therefore we will continue
  to privide the existing anongit.kde.org and git.kde.org (although this
  will be read-only) with current flat structuring for 3 weeks after
  actual migration, which will keep the existing scripts/clones working
  enough to give developers time to change to the new structure.

  We will also try to provide a script which allows you to migrate your
  existing clones to new repo paths and as mentioned by Ben in other
  message, potentially a git-kde script which would allow you to clone
  individual repository without knowing it's namespace (provided that
  there is no conflict of it's name). like "git kde karchive"

- Translations: we will co-ordinate with the translations team to let
  them adapt their tooling to updated structure as this requires changes
  on their side how translations are stored in svn repository

Thanks!

[1] 
https://cgit.kde.org/sysadmin/repo-metadata.git/tree/projects-invent?h=bshah/invent

-- 
Bhushan Shah
http://blog.bshah.in
IRC Nick : bshah on Freenode
GPG key fingerprint : 0AAC 775B B643 7A8D 9AF7 A3AC FE07 8411 7FBC E11D


signature.asc
Description: PGP signature

D29231: Add keyboard_shortcuts_inhibit protocol

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> zzag wrote in keyboard_shortcuts_inhibit_interface.cpp:21
> You've probably meant Q_DECL_HIDDEN, right?
> 
> On an unrelated note: there are valid arguments against nested private 
> classes so it would be really nice if we revisit this topic in the new repo.

yeah, that one

> there are valid arguments against nested private classes so it would be 
> really nice if we revisit this topic in the new repo.

Sure

REPOSITORY
  R127 KWayland

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

To: bport, zzag, davidedmundson, apol
Cc: romangg, crossi, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29231: Add keyboard_shortcuts_inhibit protocol

[Vlad Zahorodnii]

zzag added inline comments.

INLINE COMMENTS

> davidedmundson wrote in keyboard_shortcuts_inhibit_interface.cpp:21
> Q_DECL_PRIVATE

You've probably meant Q_DECL_HIDDEN, right?

On an unrelated note: there are valid arguments against nested private classes 
so it would be really nice if we revisit this topic in the new repo.

> keyboard_shortcuts_inhibit_interface.h:61
> + */
> +KeyboardShortcutsInhibitorInterface 
> *getShortcutsInhibitor(SurfaceInterface *surface, SeatInterface *seat) const;
> +

We usually don't prefix getters with "get". What about shortcutsInhibitor() or 
findInhibitior()?

REPOSITORY
  R127 KWayland

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

To: bport, zzag, davidedmundson, apol
Cc: romangg, crossi, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D28647: Fix KIO::Scheduler::emitReparseSlaveConfiguration() to work if called twice in same process

[Jonathan Marten]

marten added a comment.


  I've thought about this a bit more and can't find any explanation as to why 
the DBus signal does not loop back to the sending process as well as all other 
listeners.  It may be an unspecified detail of the DBus implementation that 
could possibly change at any time and start happening.  So maybe it would be 
better to retain the boolean flag and fix the ordering - in other words, keep 
things simple and implement the change as in the original diff.

REPOSITORY
  R241 KIO

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

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

D29231: Add keyboard_shortcuts_inhibit protocol

[David Edmundson]

davidedmundson added a comment.


  Let's wait for the new kwaylandserver repo before pushing.
  
  But ++, looks good.

INLINE COMMENTS

> keyboard_shortcuts_inhibit_interface.cpp:21
> +
> +class KeyboardShortcutsInhibitorInterface::Private : public 
> QtWaylandServer::zwp_keyboard_shortcuts_inhibitor_v1
> +{

Q_DECL_PRIVATE

> keyboard_shortcuts_inhibit_interface.cpp:35
> +
> +KeyboardShortcutsInhibitorInterface::Private::Private(wl_resource *resource, 
> int id, SurfaceInterface *surface,
> +  SeatInterface *seat,

It's not clear what resource this argument is referring to.

I think from reading the code it's the resource of the manager? At which point 
"parentResource" would be a better name.

Or separate client, id, version args.

REPOSITORY
  R127 KWayland

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

To: bport, zzag, davidedmundson, apol
Cc: romangg, crossi, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29231: Add keyboard_shortcuts_inhibit protocol

[Cyril Rossi]

crossi added a comment.


  some code style related comments

INLINE COMMENTS

> test_keyboard_shortcuts_inhibitor_interface.cpp:109
> +m_manager = m_display.createKeyboardShortcutsInhibitManager(this);
> + QVERIFY(m_serverCompositor->isValid());
> +connect(m_serverCompositor, ::surfaceCreated, this, 
> [this](SurfaceInterface *surface) {

indentation

> test_keyboard_shortcuts_inhibitor_interface.cpp:154
> +for (int i = 0; i < 3; ++i) {
> +KWayland::Client::Surface *s 
> =m_clientCompositor->createSurface(this);
> +m_clientSurfaces += s->operator wl_surface *();

add space after =

> display.h:328
> + */
> +KeyboardShortcutsInhibitManagerInterface 
> *createKeyboardShortcutsInhibitManager(QObject *object);
> +

parameter should be `QObject *parent = nullptr`

> keyboard_shortcuts_inhibit_interface.h:69
> +friend class KeyboardShortcutsInhibitorInterface;
> +explicit KeyboardShortcutsInhibitManagerInterface(Display *d, QObject 
> *parent);
> +void removeInhibitor(SurfaceInterface *const surface, SeatInterface 
> *const seat);

parent parameter should be defaulted to nullptr

REPOSITORY
  R127 KWayland

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

To: bport, zzag, davidedmundson, apol
Cc: romangg, crossi, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29231: Add keyboard_shortcuts_inhibit protocol

[Benjamin Port]

bport retitled this revision from "[WIP] Add keyboard_shortcuts_inhibit 
protocol" to "Add keyboard_shortcuts_inhibit protocol".

REPOSITORY
  R127 KWayland

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

To: bport, zzag, davidedmundson, apol
Cc: romangg, crossi, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

KDE CI: Frameworks » kwayland » kf5-qt5 FreeBSDQt5.14 - Build # 14 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kwayland/job/kf5-qt5%20FreeBSDQt5.14/14/
 Project:
kf5-qt5 FreeBSDQt5.14
 Date of build:
Wed, 29 Apr 2020 09:02:55 +
 Build duration:
10 min and counting
   JUnit Tests
  Name: projectroot.autotests Failed: 12 test(s), Passed: 31 test(s), Skipped: 0 test(s), Total: 43 test(s)Failed: projectroot.autotests.client.kwayland_testCompositorFailed: projectroot.autotests.client.kwayland_testDataDeviceFailed: projectroot.autotests.client.kwayland_testDataSourceFailed: projectroot.autotests.client.kwayland_testRegionFailed: projectroot.autotests.client.kwayland_testShmPoolFailed: projectroot.autotests.client.kwayland_testSubCompositorFailed: projectroot.autotests.client.kwayland_testSubSurfaceFailed: projectroot.autotests.client.kwayland_testWaylandConnectionThreadFailed: projectroot.autotests.client.kwayland_testWaylandRegistryFailed: projectroot.autotests.client.kwayland_testWaylandShellFailed: projectroot.autotests.client.kwayland_testWaylandSurfaceFailed: projectroot.autotests.server.kwayland_testWaylandServerDisplay

D29254: [RenameDialog] Add an arrow indicating direction from src to dest

[Ahmad Samir]

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

INLINE COMMENTS

> meven wrote in renamedialog.cpp:143
> You don't need a member variable for this.

Indeed, it's only used in one place (too much copy/paste...).

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, meven, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

D29254: [RenameDialog] Add an arrow indicating direction from src to dest

[Ahmad Samir]

ahmadsamir updated this revision to Diff 81492.
ahmadsamir added a comment.


  Don't use a member var for an object that's only used in one function

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29254?vs=81431=81492

BRANCH
  l-srt-to-dest (branched from master)

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

AFFECTED FILES
  src/widgets/renamedialog.cpp

To: ahmadsamir, #frameworks, dfaure, meven, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

[Benjamin Port]

bport updated this revision to Diff 81491.
bport added a comment.


  Simplify code arround QHash and QPair

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29231?vs=81386=81491

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_keyboard_shortcuts_inhibitor_interface.cpp
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/keyboard_shortcuts_inhibit_interface.cpp
  src/server/keyboard_shortcuts_inhibit_interface.h

To: bport, zzag, davidedmundson, apol
Cc: romangg, crossi, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29163: RFC: Add document-replace icon (Like for Overwrite action)

[David Hurka]

davidhurka added a comment.


  Then I will remove the border. I think it looks slightly better, because the 
bottom-left corner is visible this way; and the 16px version looks to chunky 
otherwise.
  
  I will update this revision when I have my system back. Otherwise, anyone 
else may take the template file from this revision and update it. Command would 
be:
  
$ breeze-icon-cleaner -s '16 22 32' -n 'document-replace' 
document-replace.svg

REPOSITORY
  R266 Breeze Icons

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

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

D29027: Move document corner fold to top right in two icons

[David Hurka]

davidhurka added a comment.


  Is it possible to land this without arc? I can’t get arc running on my 
current system. :(
  
  If you want this in the next release, you too may land it for me. Otherwise I 
will land it when I have my own system back, i. e. in a few weeks.
  
  I will change the other gnumeric icons to use Highlight instead of a fixed 
color later.

REPOSITORY
  R266 Breeze Icons

BRANCH
  update-document-close

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

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

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

D29254: [RenameDialog] Add an arrow indicating direction from src to dest

[Méven Car]

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

INLINE COMMENTS

> renamedialog.cpp:143
>  QLabel *m_destPreview;
> +QLabel *m_srcToDestArrow;
>  QScrollArea *m_srcArea;

You don't need a member variable for this.

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, meven, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns