D6800: Add template for a simple kpart-based application

[Friedrich W . H . Kossebau]

kossebau added a subscriber: ltoscano.
kossebau added a comment.


  In https://phabricator.kde.org/D6800#128256, @aacid wrote:
  
  > At least there is a problem l10n wise in which there is a Messages.sh so i 
guess it will be run and create a "fake" .pot file (and something similar for 
the .desktop file)
  >
  > Though probably this is already happening?
  
  
  Good question, had not thought about before. Indeed the existing template 
elsewhere also have pure Messages.sh in the tree, e.g.
  https://cgit.kde.org/kapptemplate.git/tree/src/templates/C++/kde-frameworks5
  https://cgit.kde.org/kapptemplate.git/tree/src/templates/C++/kpartapp/src
  https://cgit.kde.org/kapptemplate.git/tree/src/templates/C++/akonadiresource
  as well as
  https://cgit.kde.org/plasma-framework.git/tree/templates/cpp-plasmoid
  https://cgit.kde.org/plasma-framework.git/tree/templates/plasma-wallpaper
  
  Looking at
  https://websvn.kde.org/trunk/l10n-kf5/templates/messages/kdesdk/
  https://websvn.kde.org/trunk/l10n-kf5/templates/messages/frameworks/
  I cannot see though any catalogs which would match the template-string 
catalog names, so seems there is some magic already to prevent that?
  
  @ltoscano can you raise the curtain on this matter? :)

REPOSITORY
  R306 KParts

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

To: kossebau, #frameworks, dfaure
Cc: ltoscano, aacid

D6772: Fix usage of query_qmake: differ between calls expecting qmake or not

[Friedrich W . H . Kossebau]

kossebau added a comment.


  In https://phabricator.kde.org/D6772#129531, @apol wrote:
  
  > Works for me.
  >  Are you sure we don't need to include the `TRY` on other calls?
  
  
  Any calls you would be thinking off? The ones where this patch does not add 
`TRY` are in code which expects qmake to exist and the method to return a 
proper value, and which do not have a fallback plan if it doesn't. Which boils 
down to the case where the user explicitly set `KDE_INSTALL_USE_QT_SYS_PATHS` 
to `TRUE` but no qmake-qt5 is around.
  
  These are the calls I am aware off, and where TRY is only passed if failing 
is handled directly or rather indirectly):
  kde-modules/KDEInstallDirs.cmake:query_qmake(qt_install_prefix_dir 
QT_INSTALL_PREFIX TRY)
  kde-modules/KDEInstallDirs.cmake:query_qmake(qt_plugins_dir 
QT_INSTALL_PLUGINS)
  kde-modules/KDEInstallDirs.cmake:query_qmake(qt_imports_dir 
QT_INSTALL_IMPORTS)
  kde-modules/KDEInstallDirs.cmake:query_qmake(qt_qml_dir QT_INSTALL_QML)
  kde-modules/KDEInstallDirs.cmake:query_qmake(qt_docs_dir QT_INSTALL_DOCS)
  modules/ECMAddQch.cmake:query_qmake(qt_docs_dir QT_INSTALL_DOCS TRY)
  modules/ECMGeneratePriFile.cmake:query_qmake(qt_install_prefix_dir 
QT_INSTALL_PREFIX TRY)
  modules/ECMGeneratePriFile.cmake:  query_qmake(qt_host_data_dir QT_HOST_DATA)

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  handlenoqmakefound

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

To: kossebau, #frameworks, ltoscano, rdieter, apol
Cc: #build_system

D6772: Fix usage of query_qmake: differ between calls expecting qmake or not

[Friedrich W . H . Kossebau]

kossebau marked an inline comment as done.
kossebau added a comment.


  If there are no objections, will push on Saturday, 29th July

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, ltoscano, rdieter, apol
Cc: #build_system

D6800: Add template for a simple kpart-based application

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R306:089284db0c0d: Add template for a simple kpart-based 
application (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D6800?vs=16946=17264#toc

REPOSITORY
  R306 KParts

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6800?vs=16946=17264

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

AFFECTED FILES
  CMakeLists.txt
  ExtraDesktop.sh
  templates/CMakeLists.txt
  templates/kpartsapp/CMakeLists.txt
  templates/kpartsapp/README
  templates/kpartsapp/icons/16-apps-%{APPNAMELC}.png
  templates/kpartsapp/icons/32-apps-%{APPNAMELC}.png
  templates/kpartsapp/icons/48-apps-%{APPNAMELC}.png
  templates/kpartsapp/icons/CMakeLists.txt
  templates/kpartsapp/kpartsapp.kdevtemplate
  templates/kpartsapp/kpartsapp.png
  templates/kpartsapp/src/CMakeLists.txt
  templates/kpartsapp/src/part/%{APPNAMELC}part.cpp
  templates/kpartsapp/src/part/%{APPNAMELC}part.desktop
  templates/kpartsapp/src/part/%{APPNAMELC}part.h
  templates/kpartsapp/src/part/%{APPNAMELC}partui.rc
  templates/kpartsapp/src/part/CMakeLists.txt
  templates/kpartsapp/src/part/Messages.sh
  templates/kpartsapp/src/shell/%{APPNAMELC}.desktop
  templates/kpartsapp/src/shell/%{APPNAMELC}shell.cpp
  templates/kpartsapp/src/shell/%{APPNAMELC}shell.h
  templates/kpartsapp/src/shell/%{APPNAMELC}ui.rc
  templates/kpartsapp/src/shell/CMakeLists.txt
  templates/kpartsapp/src/shell/Messages.sh
  templates/kpartsapp/src/shell/main.cpp

To: kossebau, #frameworks, dfaure
Cc: lueck, ltoscano, aacid

D6772: Fix usage of query_qmake: differ between calls expecting qmake or not

[Friedrich W . H . Kossebau]

kossebau added a comment.


  A regression due to 
https://phabricator.kde.org/R240:8ac7abb78d97210c5cbbc87fba83d58d7b843a8d Seems 
the "it allows for modules using it to decide what they should do" was never 
told the modules using it ;)

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, ltoscano, rdieter, apol
Cc: #build_system

D6772: Fix usage of query_qmake: differ between calls expecting qmake or not

[Friedrich W . H . Kossebau]

kossebau added a reviewer: apol.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, ltoscano, rdieter, apol
Cc: #build_system

D6772: Fix usage of query_qmake: differ between calls expecting qmake or not

[Friedrich W . H . Kossebau]

kossebau added a comment.


  In https://phabricator.kde.org/D6772#126799, @apol wrote:
  
  > What's the background for the change?
  
  
  When KDE_INSTALL_USE_QT_SYS_PATHS is explicitely set to ON but no qmake-qt5 
executable found (like @rdieter reported to be possible on fedora packaging), 
query_qmake will only emit a warning and then return an empty string.
  Just, e.g. KDEInstallDirs code does not check for that empty string/error, 
but assumes a proper value to be returned when using it for estimating the Qt 
system paths, e.g. like this:
  
query_qmake(qt_plugins_dir QT_INSTALL_PLUGINS)

_define_absolute(QTPLUGINDIR ${qt_plugins_dir}
"Qt plugins"
 QT_PLUGIN_INSTALL_DIR)
  
  And given there is no sane default here, but instead we expect to get the 
paths from qmake, these kinds of calls to query_qmake should fail hard if there 
no executable.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, ltoscano, rdieter, apol
Cc: #build_system

D6782: Remove unused init() declaration from K_PLUGIN_FACTORY_DECLARATION_WITH_BASEFACTORY_SKEL

[Friedrich W . H . Kossebau]

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

REVISION SUMMARY
  Method implementation was removed in 
https://phabricator.kde.org/R244:b6c7c9b93b8dbb24c4a60e7fe72bb1d561d968a0
  so this declaration just adds dead code to the macro expansion.

TEST PLAN
  Unit tests still work, KDevelop (K_PLUGIN_FACTORY_WITH_JSON) still builds
  fine.

REPOSITORY
  R244 KCoreAddons

BRANCH
  removeUnusedFactoryInitDeclaration

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

AFFECTED FILES
  src/lib/plugin/kpluginfactory.h

To: kossebau, #frameworks

D6772: Fix usage of query_qmake: differ between calls expecting qmake or not

[Friedrich W . H . Kossebau]

kossebau updated this revision to Diff 16912.
kossebau added a comment.


  - use cmake_parse_arguments
  - fix english grammar in dox

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6772?vs=16880=16912

BRANCH
  handlenoqmakefound

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

AFFECTED FILES
  kde-modules/KDEInstallDirs.cmake
  modules/ECMAddQch.cmake
  modules/ECMGeneratePriFile.cmake
  modules/ECMQueryQmake.cmake

To: kossebau, #frameworks, ltoscano, rdieter, apol
Cc: #build_system

D6789: API dox: add note about moc include need with K_PLUGIN_FACTORY(_WITH_JSON)

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R244:f0a17af5abd5: API dox: add note about moc include need 
with K_PLUGIN_FACTORY(_WITH_JSON) (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D6789?vs=16929=16951#toc

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6789?vs=16929=16951

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

AFFECTED FILES
  src/lib/plugin/kpluginfactory.h

To: kossebau, #frameworks, mpyne

D6773: Add API dox for KDEInstallDirs' KDE_INSTALL_USE_QT_SYS_PATHS

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:f5c11c00198e: Add API dox for KDEInstallDirs' 
KDE_INSTALL_USE_QT_SYS_PATHS (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D6773?vs=16881=16952#toc

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6773?vs=16881=16952

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

AFFECTED FILES
  kde-modules/KDEInstallDirs.cmake

To: kossebau, #frameworks, apol
Cc: #build_system

D6789: API dox: add note about moc include need with K_PLUGIN_FACTORY(_WITH_JSON)

[Friedrich W . H . Kossebau]

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

REPOSITORY
  R244 KCoreAddons

BRANCH
  addnoteaboutmocincludewithpluginfactory

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

AFFECTED FILES
  src/lib/plugin/kpluginfactory.h

To: kossebau, #frameworks

D6338: DOXYGEN_PREPROC -> DOXYGEN_SHOULD_SKIP_THIS, to standardize within KF5

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R249:c0445f0c536b: DOXYGEN_PREPROC -> 
DOXYGEN_SHOULD_SKIP_THIS, to standardize within KF5 (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D6338?vs=15738=15940#toc

REPOSITORY
  R249 KI18n

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6338?vs=15738=15940

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

AFFECTED FILES
  src/CMakeLists.txt
  src/klocalizedstring.h

To: kossebau, #frameworks, ilic

D6339: Remove custom doxygen config files

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R249:f9bd662f22d8: Remove custom doxygen config files 
(authored by kossebau).

REPOSITORY
  R249 KI18n

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6339?vs=15739=15939

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

AFFECTED FILES
  doxygen.cfg
  doxygen.css

To: kossebau, #frameworks, ilic

D5455: Extend Programmer's Guide with notes about influence of setlocale()

[Friedrich W. H. Kossebau]

kossebau added a comment.


  ping?

REPOSITORY
  R249 KI18n

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

To: kossebau, #frameworks, ilic

D5439: API dox: more info about KAboutData's orgDomain/desktopFileName properties

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R244:f01e8b41f622: API dox: more info about KAboutData's 
orgDomain/desktopFileName properties (authored by kossebau).

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5439?vs=13439=13779

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

AFFECTED FILES
  src/lib/kaboutdata.cpp
  src/lib/kaboutdata.h

To: kossebau, #frameworks, aacid, mpyne, ltoscano, stikonas
Cc: dfaure

D6772: Fix usage of query_qmake: differ between calls expecting qmake or not

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:03fc1de44ef9: Fix usage of query_qmake: differ between 
calls expecting qmake or not (authored by kossebau).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6772?vs=16912=17344

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

AFFECTED FILES
  kde-modules/KDEInstallDirs.cmake
  modules/ECMAddQch.cmake
  modules/ECMGeneratePriFile.cmake
  modules/ECMQueryQmake.cmake

To: kossebau, #frameworks, ltoscano, rdieter, apol
Cc: #build_system

D7011: Extract lineedit password

[Friedrich W . H . Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> mlaurent wrote in knewpasswordwidgettest.cpp:63
> Nope as setPassword not authorize to see icon (it's a security when we 
> setPassword from apps you don't want to see it)

Could you add this as comment to this line? it is not directly obvious, so 
would be good to store this knowledge directly in the code, so the next code 
reader gets why the code is like this.

> passwordlineedit.h:24
> +#include 
> +class QLineEdit;
> +class QAction;

Can be removed, given the QLineEdit include (for QLineEdit::EchoMode)

> mlaurent wrote in passwordlineedit.h:27
> Just for info LineEditUrlDropEventFilter  has not K prefix
> but ok I will rename it

Thanks :) Yes, seems LineEditUrlDropEventFilter slipped in without somebody 
catching that.

When you do could you please also adapt PasswordLineEditPrivate? Even the test 
best would follow the naming pattern, so when scanning the dir with tests to 
find the matching test, it is found where expected (with default sorting by 
names).

> passwordlineedit.h:27
> +class PasswordLineEditPrivate;
> +class KWIDGETSADDONS_EXPORT PasswordLineEdit : public QWidget
> +{

Please also add API dox comment in front of the class, otherwise kapidox and 
ecm_add_qch will not pick up this class, given their doxygen settings.

> passwordlineedit.h:33
> + * Constructs a lineedit password widget.
> + * @since 5.37
> + *

@since should go to central class api dox comment (see other comment), or be 
behind the @param (see 
https://community.kde.org/Frameworks/Frameworks_Documentation_Policy#Document_Public_and_Protected_Members).

> passwordlineedit.h:61
> + */
> +void setClearButtonEnabled(bool clear);
> +

For a complete API this would also have `isClearButtonEnabled() const` here, 
and making this a  `clearButtonEnabled` property.

With these properties e.g. integration in Qt Designer can be improved.

> passwordlineedit.h:66
> + */
> +void setEchoMode(QLineEdit::EchoMode mode);
> +/**

getter missing as well for balanced api, and should become a property as well.

> passwordlineedit.h:94
> +Q_SIGNALS:
> +void echoModeTriggered();
> +

Please add api dox for this signal. Which echo mode is triggered here? and 
can't it be reverted?

> passwordlineedit.h:97
> +private:
> +class PasswordLineEditPrivate;
> +PasswordLineEditPrivate *const d;

can be removed, class already forward-declared at the beginning, as normal 
non-nested one (as helpful for avoing visibility inheritance).

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

To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: kossebau, elvisangelaccio, #frameworks

D7011: Extract lineedit password

[Friedrich W . H . Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> kpasswordlineedit.cpp:30
> +
> +class KPasswordLineEdit::KPasswordLineEditPrivate : public QObject
> +{

Make this a normal class KPasswordLineEditPrivate here, following fixes 
proposed above for the header.

> kpasswordlineedit.h:25
> +class QAction;
> +class PasswordLineEditPrivate;
> +

change to KPasswordLineEditPrivate, so it is the forward declaration of the 
non-nested private class which KPasswordLineEdit should use.

> kpasswordlineedit.h:49
> +Q_PROPERTY(QString password READ password WRITE setPassword NOTIFY 
> passwordChanged)
> +Q_PROPERTY(bool clearButton READ isClearButtonEnabled WRITE 
> setClearButtonEnabled)
> +public:

better name this property `clearButtonEnabled` instead of `clearButton`, 
following the property name in qlinedit

> kpasswordlineedit.h:128
> + */
> +void echoModeTriggered();
> +void passwordChanged();

Thanks for the docs. Hm, the name "echoModeTriggered" by itself is telling me 
that some "echomode" is triggered. While actually the signal tells that the 
echomode has been toggled/changed. So I would propose to rename it.

And ideally the signal would also carry the new echo mode, so the slot does not 
need to query again what the new mode is.
Just an suggestion while you expose this as public API, I see the old internal 
code was fine to just query again. But not really matching the typical Qt-style 
API that we all so like :)

Perhaps consider changing this to
`void echoModeChanged(QLineEdit::EchoMode echoMode);`

> kpasswordlineedit.h:133
> +private:
> +class KPasswordLineEditPrivate;
> +KPasswordLineEditPrivate *const d;

Remove this embedded class forward declaration -> results in leaked symbols due 
to visibility inherited.

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

To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: kossebau, elvisangelaccio, #frameworks

D6841: Use "document-close" as icon for KStandardAction::close

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R265:89b36b21701c: Use "document-close" as icon for 
KStandardAction::close (authored by kossebau).

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6841?vs=17039=17894

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

AFFECTED FILES
  src/kstandardaction_p.h

To: kossebau, #vdg, dfaure
Cc: #frameworks

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

[Friedrich W . H . Kossebau]

kossebau added a comment.


  +1 as well. Please also add a note in the API dox what BUILD_TESTING will do 
on this macro, so this is not magic behaviour and people can plan with this 
(like making sure to not `set target_link_libraries(... Qt5::Tests)` 
separately, but only pass as args in the macro *cough*kdevelop*cough*)

REPOSITORY
  R240 Extra CMake Modules

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

To: kfunk
Cc: kossebau, vkrause, elvisangelaccio, asturmlechner, apol, #frameworks, 
#build_system

D5866: ecm_qt_declare_logging_category(): more unique include guard for header

[Friedrich W. H. Kossebau]

kossebau created this revision.
Restricted Application added projects: Frameworks, Build System.

REVISION SUMMARY
  The old guard was created just from the identifier + _H, which runs the
  chance to clash in projects which use an identifier matching the project
  name and which also have a class or central file header which is named
  by the project and then has an include guard matching the filename.
  Example:
  project ABC -> abc.h with ABC_H guard
  identifier ABC, header debug.h -> debug.h with ABC_H guard
  any.cpp including both abc.h and debug.h will see only one content
  
  Using both the header file name and identifier for the guard name
  and prefixing it additionally with a macro specific term should make
  the guard both follow the usual pattern for guards matching the
  file name and also add some namespacing to allow for similar named
  header files in bigger projects (e.g. "debug.h") which could be
  included in the same include tree.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  improveqtlogdeclareheaderguard

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

AFFECTED FILES
  modules/ECMQtDeclareLoggingCategory.cmake

To: kossebau, #frameworks, #build_system

D5866: ecm_qt_declare_logging_category(): more unique include guard for header

[Friedrich W. H. Kossebau]

kossebau added a comment.


  A different reasoning for why not yet using `#pragma once`: this macro 
targets users of projects with at least cmake and Qt. Unless Qt itself does not 
use that pragma, let's not risk to screw over people who try to reuse ECM for 
some non-mainstream setup, unless we have no other choice.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, #build_system
Cc: elvisangelaccio

D5866: ecm_qt_declare_logging_category(): more unique include guard for header

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Yes, `#pragma once` might be the nicer solution here.
  
  I stayed away from proposing it though, as for one it is not a real standard 
by specifications and also by KDE coding traditions.
  And I would not like to be the one adding (and thus being responsible) the 
guinea pig to find out if that pragma is good enough for the real world ;)
  
  Given the generated file is build-specific, one could write a configure test 
to check if the compiler supports it and then use that.
  Left for the next person though, the patch as-is solves the issues I hit 
already ;)

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, #build_system
Cc: elvisangelaccio

D5867: Add or improve "Generated. Don't edit" messages and make consistent

[Friedrich W. H. Kossebau]

kossebau created this revision.
Restricted Application added projects: Frameworks, Build System.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  improvegeneratednote

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

AFFECTED FILES
  modules/ECMQmLoader.cpp.in
  modules/ECMQtDeclareLoggingCategory.cpp.in
  modules/ECMQtDeclareLoggingCategory.h.in
  modules/ECMVersionHeader.h.in

To: kossebau, #frameworks, #build_system

D6279: API dox: add notes to KActionCollection about QAction ownership handling

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
kossebau marked 2 inline comments as done.
Closed by commit R263:951bc8a9b3c9: API dox: add notes to KActionCollection 
about QAction ownership handling (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D6279?vs=15611=15637#toc

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6279?vs=15611=15637

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

AFFECTED FILES
  src/kactioncollection.h

To: kossebau, #frameworks, dfaure
Cc: apol

D6338: DOXYGEN_PREPROC -> DOXYGEN_SHOULD_SKIP_THIS, to standardize within KF5

[Friedrich W. H. Kossebau]

kossebau added a comment.


  See current output of kapidox, which has no clue about `DOXYGEN_PREPROC`: 
https://api.kde.org/frameworks/ki18n/html/klocalizedstring_8h.html
  
  ((sadly doxygen has no proper support for global methods, only documents them 
as part of the file documentation, but that is another unrelated issue))

REPOSITORY
  R249 KI18n

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

To: kossebau, #frameworks, ilic

D6339: Remove custom doxygen config files

[Friedrich W. H. Kossebau]

kossebau added a comment.


  For some background, files had been added in 
https://cgit.kde.org/kdelibs.git/commit/staging/ki18n/?h=frameworks=67a945a51440a67e40cf7d0dbde93c5ecc5118c2

REPOSITORY
  R249 KI18n

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

To: kossebau, #frameworks, ilic

D6338: DOXYGEN_PREPROC -> DOXYGEN_SHOULD_SKIP_THIS, to standardize within KF5

[Friedrich W. H. Kossebau]

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

REVISION SUMMARY
  The other KF5 modules use DOXYGEN_SHOULD_SKIP_THIS, picking up the example
  from the doxygen documentation.
  So for consistency and current expectations of kapidox (and built-in support
  of ECMAddQCH) switch to use that preprocessor macro as well.

TEST PLAN
  Library still builds and works, generated QCH file still same content.

REPOSITORY
  R249 KI18n

BRANCH
  standardizedoxygenppmacro

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

AFFECTED FILES
  doxygen.cfg
  src/CMakeLists.txt
  src/klocalizedstring.h

To: kossebau, #frameworks, ilic

D6339: Remove custom doxygen config files

[Friedrich W. H. Kossebau]

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

REVISION SUMMARY
  Are not integrated wih kapidox or ECMAddQch, and the other
  KF5 modules do not have such files.
  
  For custom generation of API dox instead the QCH generation
  can be used, by the target KF5I18n_QCH:
  
make KF5I18n_QCH
  
  And either the created QCH file be used
  or the intermediate html files in
  src/KF5I18n_ECMQchDoxygen/html/ below the build dir
  be looked at with a normal web browser.
  
  By using some official API dox generation method this will
  also ensure the output uses official configurations and any
  issues might not be missed.

REPOSITORY
  R249 KI18n

BRANCH
  removecustomdoxygencfg

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

AFFECTED FILES
  doxygen.cfg
  doxygen.css

To: kossebau, #frameworks, ilic

D6249: FindQHelpGenerator: avoid picking up Qt4 version

[Friedrich W. H. Kossebau]

kossebau added a comment.


  @palimaka You have KDE push rights, correct? Will you have time this week to 
push this, or do you want someone/me to do that for you?
  Would be good to have this in as soon as possible, given tagging release is 
<2 weeks away :)

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: palimaka, #frameworks, kossebau, kfunk
Cc: alexeymin, asturmlechner, #build_system

D6249: FindQHelpGenerator: avoid picking up Qt4 version

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:a13f1dcd2e7b: FindQHelpGenerator: avoid picking up Qt4 
version (authored by palimaka, committed by kossebau).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6249?vs=15514=15796

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

AFFECTED FILES
  find-modules/FindQHelpGenerator.cmake

To: palimaka, #frameworks, kossebau, kfunk
Cc: alexeymin, asturmlechner, #build_system

D6249: FindQHelpGenerator: avoid picking up Qt4 version

[Friedrich W. H. Kossebau]

kossebau added a comment.


  So pushing now myself, so this todo can be checked off :)

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: palimaka, #frameworks, kossebau, kfunk
Cc: alexeymin, asturmlechner, #build_system

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

[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

D6279: API dox: add notes to KActionCollection about QAction ownership handling

[Friedrich W. H. Kossebau]

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

REPOSITORY
  R263 KXmlGui

BRANCH
  apidoxQActionOwnership

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

AFFECTED FILES
  src/kactioncollection.h

To: kossebau, #frameworks, dfaure

D6279: API dox: add notes to KActionCollection about QAction ownership handling

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 15611.
kossebau added a comment.


  note auto-removal on destroying the action

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6279?vs=15594=15611

BRANCH
  apidoxQActionOwnership

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

AFFECTED FILES
  src/kactioncollection.h

To: kossebau, #frameworks, dfaure
Cc: apol

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

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R284:1910d3ac9b1e: Add KF6-TODO to make 
KComboBox::minimumSizeHint() public (authored by kossebau).

REPOSITORY
  R284 KCompletion

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6394?vs=15882=15902

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

AFFECTED FILES
  src/kcombobox.h

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

D2854: New: ECMAddQch, for generating qch & doxygen tag files

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 14671.
kossebau added a comment.
Restricted Application added projects: Frameworks, Build System.


  Some minor improvements, prepare for release with 5.36

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D2854?vs=8863=14671

BRANCH
  addApiDox

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

AFFECTED FILES
  docs/module/ECMAddQch.rst
  find-modules/FindQHelpGenerator.cmake
  kde-modules/KDEInstallDirs.cmake
  modules/ECMAddQch.cmake
  modules/ECMQchDoxygen.config.in
  modules/ECMQchDoxygenLayout.xml

To: kossebau, #frameworks, #build_system, staniek
Cc: shumski, kfunk, staniek, winterz, ochurlaud, #kdevelop

D3438: Create a QCH file with the API dox, optionally, using ECMAddQCH

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 14667.
kossebau added a comment.
Restricted Application added a project: Frameworks.


  Update to latest ECMAddQch

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3438?vs=8791=14667

BRANCH
  addQCHBuild

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

AFFECTED FILES
  CMakeLists.txt
  KF5CoreAddonsConfig.cmake.in
  src/lib/CMakeLists.txt

To: kossebau, #frameworks, ochurlaud

D3458: Create a QCH file with the API dox, optionally, using ECMAddQCH

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 14668.
kossebau added a comment.
Restricted Application added a project: Frameworks.


  Update to latest ECMAddQch

REPOSITORY
  R243 KArchive

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3458?vs=8789=14668

BRANCH
  addQCHBuild

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

AFFECTED FILES
  CMakeLists.txt
  KF5ArchiveConfig.cmake.in
  src/CMakeLists.txt

To: kossebau, #frameworks

D3439: Create a QCH file with the API dox, optionally, using ECMAddQCH

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 14669.
kossebau added a comment.
Restricted Application added a project: Frameworks.


  Update to latest ECMAddQch

REPOSITORY
  R288 KJobWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3439?vs=8792=14669

BRANCH
  addQCHBuild

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

AFFECTED FILES
  CMakeLists.txt
  KF5JobWidgetsConfig.cmake.in
  src/CMakeLists.txt

To: kossebau, #frameworks, ochurlaud
Cc: shumski

D3609: Create a QCH file with the API dox, optionally, using ECMAddQch

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 14670.
kossebau added a comment.
Restricted Application added a project: Frameworks.


  Update to latest ECMAddQch

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3609?vs=8818=14670

BRANCH
  addQCHBuild

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

AFFECTED FILES
  CMakeLists.txt
  KF5SyntaxHighlightingConfig.cmake.in
  src/lib/CMakeLists.txt

To: kossebau, #frameworks

D2854: New: ECMAddQch, for generating qch & doxygen tag files

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 14672.
kossebau added a comment.


  fix lett old "since 5.30"

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D2854?vs=14671=14672

BRANCH
  addApiDox

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

AFFECTED FILES
  docs/module/ECMAddQch.rst
  find-modules/FindQHelpGenerator.cmake
  kde-modules/KDEInstallDirs.cmake
  modules/ECMAddQch.cmake
  modules/ECMQchDoxygen.config.in
  modules/ECMQchDoxygenLayout.xml

To: kossebau, #frameworks, #build_system, staniek
Cc: shumski, kfunk, staniek, winterz, ochurlaud, #kdevelop

D2854: New: ECMAddQch, for generating qch & doxygen tag files

[Friedrich W. H. Kossebau]

kossebau added a comment.


  If there are no further comments or other feedback, I would like to commit 
this once 5.35 is tagged.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, #build_system, staniek
Cc: shumski, kfunk, staniek, winterz, ochurlaud, #kdevelop

D5866: ecm_qt_declare_logging_category(): more unique include guard for header

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:a959e34971f9: ecm_qt_declare_logging_category(): more 
unique include guard for header (authored by kossebau).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5866?vs=14552=14780

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

AFFECTED FILES
  modules/ECMQtDeclareLoggingCategory.cmake

To: kossebau, #frameworks, #build_system
Cc: elvisangelaccio

D5866: ecm_qt_declare_logging_category(): more unique include guard for header

[Friedrich W. H. Kossebau]

kossebau added a comment.


  If noone objects or has further comments, will commit this Wednesday, 24th.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, #build_system
Cc: elvisangelaccio

D5914: Add template for Plasma QML Applet with QML extension

[Friedrich W. H. Kossebau]

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

REVISION SUMMARY
  Should help people to kick-off plasmoids which also
  use custom QML classes.

TEST PLAN
  Created new plasmoid from template using kdevelop.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  addtemplateforplasmoidwithqmlplugin

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

AFFECTED FILES
  templates/CMakeLists.txt
  templates/qml-plasmoid-with-qml-extension/CMakeLists.txt
  templates/qml-plasmoid-with-qml-extension/Messages.sh
  templates/qml-plasmoid-with-qml-extension/README
  templates/qml-plasmoid-with-qml-extension/package/contents/ui/main.qml
  templates/qml-plasmoid-with-qml-extension/package/metadata.desktop
  templates/qml-plasmoid-with-qml-extension/plugin/%{APPNAMELC}plugin.cpp
  templates/qml-plasmoid-with-qml-extension/plugin/%{APPNAMELC}plugin.h
  templates/qml-plasmoid-with-qml-extension/plugin/CMakeLists.txt
  templates/qml-plasmoid-with-qml-extension/plugin/qmldir
  
templates/qml-plasmoid-with-qml-extension/qml-plasmoid-with-qml-extension.kdevtemplate

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

D5914: Add template for Plasma QML Applet with QML extension

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Get created template bundle here: 
https://share.kde.org/index.php/s/eHJGEku7pw7DotV

REPOSITORY
  R242 Plasma Framework (Library)

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

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

D2854: New: ECMAddQch, for generating qch & doxygen tag files

[Friedrich W. H. Kossebau]

kossebau edited the summary of this revision.
kossebau edited the test plan for this revision.

REPOSITORY
  R240 Extra CMake Modules

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

To: kossebau, #frameworks, #build_system, staniek
Cc: shumski, kfunk, staniek, winterz, ochurlaud, #kdevelop

D5867: Add or improve "Generated. Don't edit" messages and make consistent

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:b58820c4af64: Add or improve "Generated. Don't edit" 
messages and make consistent (authored by kossebau).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5867?vs=14553=14600

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

AFFECTED FILES
  modules/ECMQmLoader.cpp.in
  modules/ECMQtDeclareLoggingCategory.cpp.in
  modules/ECMQtDeclareLoggingCategory.h.in
  modules/ECMVersionHeader.h.in

To: kossebau, #frameworks, #build_system, apol

D6053: Use explicit flag values or explicit constructor instead of nullptr

[Friedrich W. H. Kossebau]

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

REVISION SUMMARY
  The constructor of QFlags which was intended to handle literal "0"
  as commonly used indicator of no-flags-set has been done with a
  trick based on pointer types. Which these days of nullptr existing and
  compilers pushing to use that sadly fired back and, instead of
  allowing convenient code with commonly understood literal "0" values,
  resulted in quite some code using "nullptr" to indicate a QFLags-based
  type value with no flags set, which can be puzzling for humans
  reading the code.
  
  To improve the code again, instead of "nullptr" are used:
  
  - enum item representing the 0=no-flag-set value, if existing
  - explicit constructor

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  nonullptrforflagsplease

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

AFFECTED FILES
  src/kassistantdialog.h
  src/keditlistwidget.cpp
  src/kfontrequester.cpp
  src/kpagedialog.h
  src/kpagewidgetmodel.cpp
  src/kpassworddialog.h
  src/kruler.h
  src/kseparator.h

To: kossebau, #frameworks, cfeck

D6054: Use explicit flag values or explicit constructor instead of nullptr

[Friedrich W. H. Kossebau]

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

REVISION SUMMARY
  The constructor of QFlags which was intended to handle literal "0"
  as commonly used indicator of no-flags-set has been done with a
  trick based on pointer types. Which these days of nullptr existing and
  compilers pushing to use that sadly fired back and, instead of
  allowing convenient code with commonly understood literal "0" values,
  resulted in quite some code using "nullptr" to indicate a QFLags-based
  type value with no flags set, which can be puzzling for humans
  reading the code.
  
  To improve the code again, instead of "nullptr" are used:
  
  - enum item representing the 0=no-flag-set value, if existing
  - explicit default constructor

REPOSITORY
  R278 KWindowSystem

BRANCH
  nonullptrforflagsplease

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

AFFECTED FILES
  autotests/kstartupinfo_unittest.cpp
  autotests/kwindowinfox11test.cpp
  autotests/kwindowsystemx11test.cpp
  autotests/netwininfotestwm.cpp
  src/kstartupinfo.cpp
  src/kwindowinfo.cpp
  src/kwindowinfo.h
  src/kwindowsystem.h
  src/platforms/xcb/kwindowinfo.cpp
  src/platforms/xcb/kwindowsystem.cpp
  src/platforms/xcb/netwm.cpp
  src/platforms/xcb/netwm.h

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

D3458: Create a QCH file with the API dox, optionally, using ECMAddQCH

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
kossebau marked an inline comment as done.
Closed by commit R243:42292d5214c6: [FEATURE] Option to build & install QCH 
file with the public API dox (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D3458?vs=14668=15151#toc

REPOSITORY
  R243 KArchive

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3458?vs=14668=15151

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

AFFECTED FILES
  CMakeLists.txt
  KF5ArchiveConfig.cmake.in
  src/CMakeLists.txt

To: kossebau, #frameworks
Cc: dfaure

D3438: Create a QCH file with the API dox, optionally, using ECMAddQCH

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R244:f1ccb8877e78: [FEATURE] Option to build & install QCH 
file with the public API dox (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D3438?vs=14667=15152#toc

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3438?vs=14667=15152

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

AFFECTED FILES
  CMakeLists.txt
  KF5CoreAddonsConfig.cmake.in
  src/lib/CMakeLists.txt

To: kossebau, #frameworks, ochurlaud

D3609: Create a QCH file with the API dox, optionally, using ECMAddQch

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:6891e5a13848: [FEATURE] Option to build & install QCH 
file with the public API dox (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D3609?vs=14670=15153#toc

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3609?vs=14670=15153

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

AFFECTED FILES
  CMakeLists.txt
  KF5SyntaxHighlightingConfig.cmake.in
  src/lib/CMakeLists.txt

To: kossebau, #frameworks

D3439: Create a QCH file with the API dox, optionally, using ECMAddQCH

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R288:66510d55dbe0: [FEATURE] Option to build & install QCH 
file with the public API dox (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D3439?vs=14669=15154#toc

REPOSITORY
  R288 KJobWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3439?vs=14669=15154

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

AFFECTED FILES
  CMakeLists.txt
  KF5JobWidgetsConfig.cmake.in
  src/CMakeLists.txt

To: kossebau, #frameworks, ochurlaud
Cc: shumski

D3458: Create a QCH file with the API dox, optionally, using ECMAddQCH

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> dfaure wrote in CMakeLists.txt:55
> slightly cryptic for someone who wouldn't know what QCH is. You could add 
> (for opening in Qt Assistant) or something similar?
> 
> Then again these days people don't know what Assistant is, they use the 
> embedded help in QtCreator ;)

What about "Build API documentation in QCH format (for e.g. Qt Assistant, Qt 
Creator & KDevelop)"

REPOSITORY
  R243 KArchive

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

To: kossebau, #frameworks
Cc: dfaure

D6053: Use explicit flag values or explicit constructor instead of nullptr

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> cfeck wrote in kfontrequester.cpp:187
> Does removing the default value mean the flags would be now uninitialized?

No, the constructor called would be (as before) 
`QFlags::QFlags(Zero zero = Q_NULLPTR)`, so 
still default to 0 internally, so with no flag set.
(cmp. also 
https://code.woboq.org/qt5/qtbase/src/corelib/global/qflags.h.html#_ZN6QFlagsC1EMNS_7PrivateEi)

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  nonullptrforflagsplease

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

To: kossebau, #frameworks, cfeck, kfunk
Cc: kfunk

D6054: Use explicit flag values or explicit constructor instead of nullptr

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Checking the diff another time I find it would be even nicer to extend 
`NET::Property`, `NET::Property2`, `NET::Action` & Co. to have an entry for `0` 
value, named `NoProperties`, `NoProperties2`, `NoActionFlags` etc. (cmp. e.g. 
`Qt::ItemFlags`), so those enum values could be used instead of the default 
constructors.
   Would both be more expressive and also more balanced when used next to other 
flag sets assembled by the enum values (similar to
  
  Something to consider for the future perhaps...

REPOSITORY
  R278 KWindowSystem

BRANCH
  nonullptrforflagsplease

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

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

D6053: Use explicit flag values or explicit constructor instead of nullptr

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R236:ff2e1d8e22fe: Use explicit flag values or explicit 
constructor instead of nullptr (authored by kossebau).

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6053?vs=15033=15047

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

AFFECTED FILES
  src/kassistantdialog.h
  src/keditlistwidget.cpp
  src/kfontrequester.cpp
  src/kpagedialog.h
  src/kpagewidgetmodel.cpp
  src/kpassworddialog.h
  src/kruler.h
  src/kseparator.h

To: kossebau, #frameworks, cfeck, kfunk
Cc: kfunk

D6054: Use explicit flag values or explicit constructor instead of nullptr

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R278:ccbad906db1b: Use explicit flag values or explicit 
constructor instead of nullptr (authored by kossebau).

REPOSITORY
  R278 KWindowSystem

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6054?vs=15034=15063

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

AFFECTED FILES
  autotests/kstartupinfo_unittest.cpp
  autotests/kwindowinfox11test.cpp
  autotests/kwindowsystemx11test.cpp
  autotests/netwininfotestwm.cpp
  src/kstartupinfo.cpp
  src/kwindowinfo.cpp
  src/kwindowinfo.h
  src/kwindowsystem.h
  src/platforms/xcb/kwindowinfo.cpp
  src/platforms/xcb/kwindowsystem.cpp
  src/platforms/xcb/netwm.cpp
  src/platforms/xcb/netwm.h

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

RFC: Packaging of KF5 API dox QCH files and their devel helper files

[Friedrich W. H. Kossebau]

Hi packagers,

(cc: to kde-frameworks-devel only for heads-up)

with ECM/KF5 5.35.0 now snapshot and being prepared for release, a new feature 
has already been pushed to the master branches of ECM and almost all the non-
deprecated KF5 modules, which might need some more fine-tuning in the few 
weeks until it then ships with next release of ECM/KF5, 5.36.0, and after that 
will be harder to improve.

And your, the packagers/distributors', input here is very much needed!
Because this might need new packaging patterns.


NEW BUILD ARTIFACTS INSTALLED: A QCH FILE...

The new feature is the option to create and install documentation of the 
public API in QCH format ("Qt compressed help", for e.g. Qt Assistant, Qt 
Creator & KDevelop) during the build of all the KDE Frameworks modules with 
public API.
By passing -DBUILD_QCH=ON to cmake, and with the tools doxygen & 
qhelpgenerator around (and perl needed as well currently by doxygen), 
additionally a QCH file will be generated and installed.
The advantage to do it at build time, as opposed to run it over a pure 
checkout of the sources, is that also all generated source files are visible 
to the documentation generator (as well as build options), so the 
documentation can be more complete. And it matches the exact version of the 
installed headers and library automatically.

Example:
KCoreAddons built with cmake -DBUILD_QCH=ON
-> installs file KF5CoreAddons.qch to ${KDE_INSTALL_QTQCHDIR}


... A DOXYGEN TAG FILE, ...

Just, things are slightly more complex. Because for libraries whose API makes 
use of classes etc. from other libraries (like when subclassing from their 
classes or using their types for own method arguments), the documentation only 
gets really useful if it includes information about the things from the other 
libraries or at least has cross-links to it. The Qt help system supports such 
cross-linking across documentation for different libraries, using a qthelp:// 
url scheme.
Doxygen supports those cross-linking as well when generating the QCH files, as 
well as picking up documentation for inherited things. For that it needs a 
separate metadata file about a given QCH file, so called "tag file", which 
contains all the info needed to create qthelp:// links into a given QCH file 
for its documented classes etc. as well as getting documentation for inherited 
class methods etc.
For the different QCH files of the Qt modules those are installed in a subdir 
next to the QCH files (see "qmake-qt5 -query QT_INSTALL_DOCS"). E.g. on 
openSUSE the files are located in this pattern, by example of qtcore:
/usr/share/doc/packages/qt5/qtcore.qch
/usr/share/doc/packages/qt5/qtcore/qtcore.tags

So to allow the same cross-linking also for the KF5 QCH files, for each QCH 
file also a doxygen tag file is created and installed.

Example:
KCoreAddons built with cmake -DBUILD_QCH=ON
-> installs file KF5CoreAddons.qch to ${KDE_INSTALL_QTQCHDIR}
-> installs file KF5CoreAddons.tags to ${KDE_INSTALL_QTQCHDIR}


... & MORE CMAKE CONFIG

Just, when we want doxygen to create a QCH file with cross-linking to another 
QCH file, how do we know where the respective tag file is exactly, so we can 
tell it to doxygen? And how do we know some other info bits needed to generate 
proper qthelp:// urls (like correct org domain and path)?

For the use with KF5 libraries, there are two different cases interesting: 
linking to Qt QCH files, and linking to KF5 QCH files.

For location of Qt QCH files, we can get some info from "qmake-qt5 -query 
QT_INSTALL_DOCS" and then look at the known subpath. The other data needed for 
qthelp:// urls can be guessed to follow given patterns.

For KF5 QCH files, as we have more control about the buildsystem, the solution 
is to simply store all the details in the CMake config files installed for a 
given library. The current approach simply follow the modern cmake way for 
definition of the linking of code libraries, where one only needs to list the 
target names of the libraries in target_link_libraries() and things like 
include paths are automatically pulled in:
for each QCH library a cmake target is provided in the additionally installed 
CMake config files, which has all the needed information set as properties. So 
when defining the generation of a QCH file with the new ECM macro 
ecm_add_qch(), the QCH libraries to link to can be simply noted by their 
target name, in the parameter group LINK_QCHS, and everything else is handled 
automatically.
And to make things more simple for now, the additionally installed CMake 
config files are included automatically by the normal CMake config files. So 
currently no separate "find_package(KF5MyLib_QCH)" or similar is needed, but 
the QCH target is automatically available (unless build was done without QCH).

While there are no such QCH targets available for the Qt QCH files with the 
existing Qt CMake config files, the ECM macro simply generates them on the 
fly, so for linking to some qtxyz.qch 

D2854: New: ECMAddQch, for generating qch & doxygen tag files

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:c837f58d505b: New: ECMAddQch, for generating qch & 
doxygen tag files (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D2854?vs=14672=15148#toc

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D2854?vs=14672=15148

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

AFFECTED FILES
  docs/module/ECMAddQch.rst
  find-modules/FindQHelpGenerator.cmake
  kde-modules/KDEInstallDirs.cmake
  modules/ECMAddQch.cmake
  modules/ECMQchDoxygen.config.in
  modules/ECMQchDoxygenLayout.xml
  tests/CMakeLists.txt
  tests/KDEInstallDirsTest/var_list.cmake

To: kossebau, #frameworks, #build_system, staniek
Cc: shumski, kfunk, staniek, winterz, ochurlaud, #kdevelop

D6249: FindQHelpGenerator: avoid picking up Qt4 version

[Friedrich W. H. Kossebau]

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


  Thanks for the fix.  Sadly this fails on my openSUSE TW system with system 
Qt5 packages, where _path is /usr/lib64/qt5/bin, and while that dir has both 
qhelpgenerator-qt5 and qhelpgenerator files, they are links pointing to the 
actual executable file located at /usr/bin/qhelpgenerator-qt5
  
lrwxrwxrwx 1 root root 31 28. Mai 12:40 
/usr/lib64/qt5/bin/qhelpgenerator-qt5 -> ../../../bin/qhelpgenerator-qt5
lrwxrwxrwx 1 root root 31 28. Mai 12:40 /usr/lib64/qt5/bin/qhelpgenerator 
-> ../../../bin/qhelpgenerator-qt5
  
  Still had no time to investigate some more why this fails.
  
  (Actually it is not such an issue if the Qt4 version is used, it's working as 
well. But still using the latest version of qhelpgenerator, as provided by qt5, 
feels better -> "newer is better!")

REPOSITORY
  R240 Extra CMake Modules

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

To: palimaka, #frameworks, kossebau, kfunk
Cc: alexeymin, asturmlechner, #build_system

D6249: FindQHelpGenerator: avoid picking up Qt4 version

[Friedrich W. H. Kossebau]

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


  Ah, PEBKAC, I only added NO_DEFAULT_PATH to the code when manually applying 
the patch. Fixing also PATH->PATHS improved things, and the executable now is 
found in ${_path}.
  
  No more objection from my side :)

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: palimaka, #frameworks, kossebau, kfunk
Cc: alexeymin, asturmlechner, #build_system

D5455: Extend Programmer's Guide with notes about influence of setlocale()

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 14158.
kossebau added a comment.


  rework text follwing feedback

REPOSITORY
  R249 KI18n

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5455?vs=13447=14158

BRANCH
  addNoteAboutGettextLocaleIssuse

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

AFFECTED FILES
  docs/programmers-guide.md

To: kossebau, #frameworks, ilic, aacid
Cc: aacid

D5455: Extend Programmer's Guide with notes about influence of setlocale()

[Friedrich W. H. Kossebau]

kossebau added a comment.


  @ilic Thanks for the comments, agreed. Actually the initial text had some 
"really?" across it, as I was surprised about the findings stated in there and 
wanted (your) confirmation.
  So made this info bit a subsection of the "Writing Messages" part, and 
changed it to first be handling instructions (given this is a guide), then have 
some explanation for the curious. Also dumped any proposals for custom usage of 
setlocale().

REPOSITORY
  R249 KI18n

BRANCH
  addNoteAboutGettextLocaleIssuse

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

To: kossebau, #frameworks, ilic, aacid
Cc: aacid

D5455: Extend Programmer's Guide with notes about influence of setlocale()

[Friedrich W. H. Kossebau]

kossebau added a comment.


  BTW, the `` and `\ref x` seems not to work (only reused that for 
consistency), will do a separate patch later which follows instructions from 
http://doxygen.10944.n7.nabble.com/Creating-links-in-and-between-Markdown-files-pages-td6689.html

REPOSITORY
  R249 KI18n

BRANCH
  addNoteAboutGettextLocaleIssuse

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

To: kossebau, #frameworks, ilic, aacid
Cc: aacid

D5455: Extend Programmer's Guide with notes about influence of setlocale()

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In https://phabricator.kde.org/D5455#107095, @kossebau wrote:
  
  > BTW, the `` and `\ref x` seems not to work (only reused that 
for consistency), will do a separate patch later which follows instructions 
from 
http://doxygen.10944.n7.nabble.com/Creating-links-in-and-between-Markdown-files-pages-td6689.html
  
  
  Hmpf, seems there is a bug in doxygen about section creation, as actually 
also stated by the doxygen author in the linked thread above. Guess that is 
also why currently the `` are used. Giving up on that bug for now.
  
  For the given patch in this review, will commit next WE unless someone has 
further comments :)(or before if you ilic give your okay)

REPOSITORY
  R249 KI18n

BRANCH
  addNoteAboutGettextLocaleIssuse

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

To: kossebau, #frameworks, ilic, aacid
Cc: aacid

D5455: Extend Programmer's Guide with notes about influence of setlocale()

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R249:db45cf7242d7: Extend Programmer's Guide with notes about 
influence of setlocale() (authored by kossebau).

REPOSITORY
  R249 KI18n

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5455?vs=14158=14214

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

AFFECTED FILES
  docs/programmers-guide.md

To: kossebau, #frameworks, ilic, aacid
Cc: aacid

D5914: Add template for Plasma QML Applet with QML extension

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:4d5793c28a3f: Add template for Plasma QML Applet with QML 
extension (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D5914?vs=14681=14687#toc

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5914?vs=14681=14687

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

AFFECTED FILES
  templates/CMakeLists.txt
  templates/qml-plasmoid-with-qml-extension/CMakeLists.txt
  templates/qml-plasmoid-with-qml-extension/Messages.sh
  templates/qml-plasmoid-with-qml-extension/README
  templates/qml-plasmoid-with-qml-extension/package/contents/ui/main.qml
  templates/qml-plasmoid-with-qml-extension/package/metadata.desktop
  templates/qml-plasmoid-with-qml-extension/plugin/%{APPNAMELC}plugin.cpp
  templates/qml-plasmoid-with-qml-extension/plugin/%{APPNAMELC}plugin.h
  templates/qml-plasmoid-with-qml-extension/plugin/CMakeLists.txt
  templates/qml-plasmoid-with-qml-extension/plugin/qmldir
  
templates/qml-plasmoid-with-qml-extension/qml-plasmoid-with-qml-extension.kdevtemplate

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

D7580: Support loading by stream and restoring state on reload

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R383:d1ee5a0e3908: Support loading by stream and restoring 
state on reload (authored by kossebau).

REPOSITORY
  R383 SVGPart

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7580?vs=19446=19814

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

AFFECTED FILES
  CMakeLists.txt
  svgbrowserextension.cpp
  svgbrowserextension.h
  svgpart.cpp
  svgpart.h

To: kossebau, #frameworks, dfaure

D7580: Support loading by stream and restoring state on reload

[Friedrich W . H . Kossebau]

kossebau added a comment.


  Thanks for quick reaction :) Will push now, even if we discovered another 
item which needs some more clarification. Would do a follow-up fix then if we 
find one is needed,

INLINE COMMENTS

> dfaure wrote in svgpart.cpp:191
> When I reload in konqueror (with KHTML or WebEngine), the yOffset is 
> preserved.
> Looking at KonqView::restoreHistory this is because restoreState is only 
> called when reload==false.
> 
> It might be a good idea for kate/kdevelop to do the same, they have the 
> actual information of whether reload was pressed, while parts can only guess.

This is actually another item I am slightly unsure about and you now added to 
this unsureness. The API dox of OpenUrlArguments::reload() 

 flag says:
"part should reload the URL, i.e. the cache shouldn't be used (forced reload). "

So far I asked I would have said this flag responds to whether 
"If-Modified-Since" should be added to the HTTP request header or that ETag be 
used (`reload=false`) or whether not (`reload=true`). And that "reload" should 
have rather been named "forcedReload".

So in the case of Kate/KDevelop (or rather the ktexteditor preview plugin, as 
this is the one who cares about feeding the new document data to the kparts 
plugin), setting the reload flag to `true` would mean the forced reload also of 
embedded document resources, like images, fonts or whatever external resources 
a document might include, right?
But actually we just want the new version of the text document to be loaded 
again. Which either

- is passed via a file, which has a newer filestamp then, thus indicates there 
is newer data inside, or
- is feed via a data stream, where one would assume the pushing side already 
made sure only an updated data version is pushed.

So previously I would have thought `reload=false` would be the correct thing to 
do, as there is no need for forced reload. Have I been missing something?

REPOSITORY
  R383 SVGPart

BRANCH
  supportstreamandreload

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

To: kossebau, #frameworks, dfaure

D7580: Support loading by stream and restoring state on reload

[Friedrich W . H . Kossebau]

kossebau edited the summary of this revision.
kossebau edited the test plan for this revision.

REPOSITORY
  R383 SVGPart

BRANCH
  supportstreamandreload

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

To: kossebau, #frameworks, dfaure

D7580: Support loading by stream and restoring state on reload

[Friedrich W . H . Kossebau]

kossebau added a comment.


  In https://phabricator.kde.org/D7580#145144, @kossebau wrote:
  
  > With the latest update then this patch would represent the blue-print for 
KParts plugins as I see it when it comes to supporting both state restoring and 
support for streams, at least for what is possible currently with the existing 
KParts API.
  >
  > Any patterns which you see running against the ideas of the API? Or can 
this patch be applied as is, and be pointed to as template?
  
  
  @dfaure Ping?
  
  Sorry to be a bit pushy, but begin of upcoming week I plan to release 0.1.0 
of the ktexteditor preview plugin, and I would like to have the svgpart code in 
master usable as reference for hopefully further kpart adaptions or even new 
implementations inspired by the preview plugin  (and as reference for a new 
kpart plugin kapptemplate I want to create).

REPOSITORY
  R383 SVGPart

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

To: kossebau, #frameworks, dfaure

D3977: Fix memleak in KDynamicJobTracker, KWidgetJobTracker needs QApplication

[Friedrich W . H . Kossebau]

kossebau added a comment.


  In https://phabricator.kde.org/D3977#155721, @elvisangelaccio wrote:
  
  > @kossebau 
  >  After this change we get the
  >
  >  Tried to unregister a kio job that hasn't been registered.
  >   
  >   warning with a `KCompositeJob` in Ark (batchextract.cpp).
  >   
  >
  > The job is now automatically unregistered and the `unregisterJob(this)` 
call in the job destructor is causing this warning.
  >  Are KCompositeJobs still supposed to unregister themselves?
  
  
  KJob related knowledge sadly completely swapped out of my brain, would need 
to investigate myself. No idea if there should be done anything special for 
KCompositeJobs.
  
  On a quick look the automatic unregistering based on the finished signal 
seems to make sense for any KJob subclass. Perhaps the change in this commit 
conflicts with some older code in Ark to work around the old behaviour and 
which does some additional manual unregistration? Or perhaps some slot in 
accidentally invoked 2x so the unregistration happens more than once, with the 
second try than failing? Wild guesses is all I have to offer, sorry. I cannot 
remember anything questionable about this patch, so no pointers into it, 
instead my initial reaction would be to look at the consumer side.

REPOSITORY
  R241 KIO

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

To: kossebau, #frameworks, kfunk
Cc: elvisangelaccio, kfunk

D3977: Fix memleak in KDynamicJobTracker, KWidgetJobTracker needs QApplication

[Friedrich W . H . Kossebau]

kossebau added a comment.


  In https://phabricator.kde.org/D3977#155973, @elvisangelaccio wrote:
  
  > From what I can see, after this change any job registered with
  >
  >   KIO::getJobTracker()->registerJob(job);
  >   
  >
  > no longer needs to be manually unregistered with
  >
  >   KIO::getJobTracker()->unregisterJob(job);
  
  
  Looking at the history, `KJobTrackerInterface::registerJob(KJob *job)`has 
connected the job's `finished(KJob*)` signal to the tracker's 
`unregisterJob(KJob*)` since ages, 2008 and possibly longer (cmp. 
https://phabricator.kde.org/R446:7308fa7e6b756f5c6fe1b8adbcc6095e3bb5b995)
  
  And this commit here now uncovered that somehow for Ark as well.
  
  > This is a minor thing, but should probably be documented somewhere.
  
  Agreed, by just looking at the API (dox) one would assume one had to possibly 
also unregister manually. You proposed it -> you do it? :)

REPOSITORY
  R241 KIO

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

To: kossebau, #frameworks, kfunk
Cc: elvisangelaccio, kfunk

Re: Running applications and unittests without "make install"

[Friedrich W. H. Kossebau]

Hi Ben & all,

Am Samstag, 19. August 2017, 02:24:21 CEST schrieb Ben Cooksley:
> On Wed, Aug 16, 2017 at 3:17 AM, David Faure  wrote:
> > Hi everyone,
> 
> Hi David,
> 
> > The documentation for how to do that is now up at
> > 
> >  https://blogs.kde.org/2017/08/15/running-applications-and-unittests-witho
> >  ut-make-install> 
> > I have ported/fixed all frameworks, except for kirigami, and there are
> > still pending merge requests for kpackage.
> > 
> > I'm wondering if we could change the CI scripts so that they run the 
tests
> > before doing make install? For frameworks only, for now.
> 
> I've now done this, for Linux at least.

Any chance the bits about "For frameworks only, for now." was missed here? 
E.g. for kdevelop stable/master builds I now also see tests been run before 
install happens (which fails many of them).

Seems to me those bits were important, if looking at the code:

that code in ECM to support running tests uninstalled right now is only 
triggered by some "find_package(ECM x.y.z)", with x.y.z >= 5.38.0

Cmp. the
NOT ("${ECM_GLOBAL_FIND_VERSION}" VERSION_LESS "5.38.0")
in https://cgit.kde.org/extra-cmake-modules.git/tree/kde-modules/
KDECMakeSettings.cmake#n252 , where ${ECM_GLOBAL_FIND_VERSION} is from
set(ECM_GLOBAL_FIND_VERSION "${ECM_FIND_VERSION}")
in https://cgit.kde.org/extra-cmake-modules.git/tree/ECMConfig.cmake.in#n11

And that find_package of ECM with 5.38.0 is something only the KDE Frameworks 
repos do right now. Where other projects, like kdevelop, are fine with less 
recent versions (e.g. kdevelop master only expects ECM 5.14.0 as minimum), 
and thus would not trigger that new build layout needed for uninstalled 
testing.

So is it possible to limit that new build steps order to just the Frameworks 
product, and restore old behaviour for anyone else?

Perhaps in the long term also the logic in ECM needs to be changed, so the 
new build artifact placement can be triggered by other means from cmake code 
which is prepared for the optional new feature (by testing the version of the 
found ECM or some set feature flag variable). It will be quite some time for 
many application projects to jump to 5.38.0 as minimum ECM, given developers 
are sometimes bound to LTS distributions.

Cheers
Friedrich

D7580: Support loading by stream and restoring state on reload

[Friedrich W . H . Kossebau]

kossebau updated this revision to Diff 19446.
kossebau added a comment.


  add support for state restoring via BrowserExtension

REPOSITORY
  R383 SVGPart

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7580?vs=19091=19446

BRANCH
  supportstreamandreload

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

AFFECTED FILES
  CMakeLists.txt
  svgbrowserextension.cpp
  svgbrowserextension.h
  svgpart.cpp
  svgpart.h

To: kossebau, #frameworks, dfaure

D7580: Support loading by stream and restoring state on reload

[Friedrich W . H . Kossebau]

kossebau added a comment.


  In https://phabricator.kde.org/D7580#144557, @dfaure wrote:
  
  > In https://phabricator.kde.org/D7580#144518, @kossebau wrote:
  >
  > > When I read this initially, I guessed this method is just about the view 
state. But is also bound to data-pulling by the kpart, given that the default 
implementation explicitely calls openUrl() with the url stored in the 
datastream. Which might make sense for simple-to-use API with the non-stream 
use cases. But leaves out the stream-based data-pushing usage.
  >
  >
  > It does indeed. Two incompatible features...
  >
  > I think all we need is for the part to remember that it opened the URL via 
the stream api, and add that to the data saved by saveState(). Then in 
restoreState() we can skip openUrl() when that bool is true. It'll be up to the 
caller to redo the openStream/writeStream/closeStream sequence.
  
  
  Might work, yes. One issue: changing the content layout of the state 
QDataStream is currently not protected by any versioning, so existing stored 
history (like from session restore) would be parsed wrongly if suddenly the 
base implementation of restore also tries to extract such a bool? Might need 
some handling, though not sure if serious data would be lost.
  
  As said, myself might look into improving this only later this year, so if 
you (or someone else) feels inspired to play with this, please go ahead already.
  
  >>   BTW: can you tell if Falkon will/does support kparts?
  > 
  > It doesn't, and I don't think it will, but you can try to convince David 
Rosca ;)
  
  Too bad. Having a cross-media/format navigator like with Konqueror had been 
one of the things that attracted me to KDE.
  Okay, so nothing where I should extend my testing coverage to.
  
  With the latest update then this patch would represent the blue-print for 
KParts plugins as I see it when it comes to supporting both state restoring and 
support for streams, at least for what is possible currently with the existing 
KParts API.
  
  Any patterns which you see running against the ideas of the API? Or can this 
patch be applied as is, and be pointed to as template?

REPOSITORY
  R383 SVGPart

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

To: kossebau, #frameworks, dfaure

D7580: Support loading by stream and restoring state on reload

[Friedrich W . H . Kossebau]

kossebau created this revision.

REVISION SUMMARY
  The KTextEditor preview plugin* repeatedly feeds new
  versions to the same kpart instance, to allow instant
  preview of changes. To avoid stressing of the filesystem
  the stream API of the kpart is used if available.
  *https://frinring.wordpress.com/2017/08/21/look-what-you-have-donewwdo/
  
  This patch adds support for the stream API.
  Additionally it remembers the view state on closing an url,
  and if the same url is load again, the view state is restored.
  The latter allows continuous display of the same, but
  updated file as e.g. happening with the preview plugin.
  
  Open questions:
  a) use of the streaming API matching intentions?
  b) Restoring of view state overlaps with support in browser history,
  
for which there is the KParts::OpenUrlArguments property of ReadOnlyPart.
That one only supports x/y offset, so how would zoom and other custom
state properties be save and retrieved again, if they should?
  
  This patch should help to solve this questions in general, so it is
  known what to do for other kparts.

TEST PLAN
  Editing SVG files in Kate/Kdevelop using the preview plugin* will
  no longer reset the offset to 0,0 each time the view is updated on
  changes, also is the filesystem no longer used on updates.
  
  - kde:scratch/kossebau/ktexteditorpreviewplugin

REPOSITORY
  R383 SVGPart

BRANCH
  supportstreamandreload

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

AFFECTED FILES
  svgpart.cpp
  svgpart.h

To: kossebau, #frameworks, dfaure

D7580: Support loading by stream and restoring state on reload

[Friedrich W . H . Kossebau]

kossebau updated this revision to Diff 19091.
kossebau added a comment.


  handle repeated closeUrl call

REPOSITORY
  R383 SVGPart

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7580?vs=18868=19091

BRANCH
  supportstreamandreload

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

AFFECTED FILES
  svgpart.cpp
  svgpart.h

To: kossebau, #frameworks, dfaure

D7580: Support loading by stream and restoring state on reload

[Friedrich W . H . Kossebau]

kossebau added a comment.


  > As I see it, it's *either* openUrl/closeUrl *or* 
openStream/writeStream/closeStream.
  
  Hm, when I before did a quick search for how to use the stream API, I came 
across
  https://api.kde.org/frameworks/khtml/html/classKHTMLPart.html#details
  which made me think the closeStream is more in the idea of QIODevice::close, 
to hint that there is no further data coming in.
  
  The API dox of ReadOnlyPart enforced that idea with me:
  
  - ReadOnlyPart::closeStream() 
:
 "Terminate the sending of data to the part."
  - ReadOnlyPart::doCloseStream() 
:
 "This is called by closeStream(), to indicate that all the data has been sent. 
Parts should ensure that all of the data is displayed at this point."
  
  And with that in mind the existing ReadOnlyPart code matched my expectations, 
as `ReadOnlyPart::openStream`calls closeUrl, similar to how 
`ReadOnlyPart::openUrl` calls it.
  
  So openUrl would be only matched by openStream/writeStream/closeStream, with 
the latter triple being the push variant to the first. And closeUrl would be 
used with both, to tell the part to unload the current content, disconnect from 
the content source if needed and display some void, with the url being reset to 
invalid/none. So the whole purpose of closeStream would be to just signal that 
no more data will come in, so e.g. any "loading more" indicator can be hidden.
  
  I now found the original discussion thread for the stream API (KParts API: 
thinking about the future... , 
just 15 years old :) ) which does not shed that much more light onto the idea 
of the API, though I could read into it my existing understanding of the API :)
  
  But that's why I included you @dfaure into this review, as there is no 
current implementation/usage of the API, so chance I just see what I need here 
:) And hope you(r memories) could correct me were wrong. Though now I see your 
initial memories conflicting with the API dox & code? Meh...

REPOSITORY
  R383 SVGPart

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

To: kossebau, #frameworks, dfaure

D7580: Support loading by stream and restoring state on reload

[Friedrich W . H . Kossebau]

kossebau edited the summary of this revision.

REPOSITORY
  R383 SVGPart

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

To: kossebau, #frameworks, dfaure

D7198: Set CMAKE_*_OUTPUT_DIRECTORY to run tests without installing.

[Friedrich W . H . Kossebau]

kossebau reopened this revision.
kossebau added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> KDECMakeSettings.cmake:46-47
> +#
> +# When find_package(ECM 5.38) or higher is called, this also selects
> +# a layout for the build dir that helps running executables without 
> installing.
>  #

Trying to fix things for kdevelop tests, I have to report that as user of ECM 
this note does not really assist me, besides hinting there is random magic 
happening :(

So could you please add a comment telling what is that "layout" like, how does 
it help exactly running executables without installing? What other 
pre-conditions are need to be met? How should one rely on that layout in own 
cmake code?

Commits like https://phabricator.kde.org/D7347 which make use of the here 
undocumented `${CMAKE_LIBRARY_OUTPUT_DIRECTORY}` and then also point to yet 
another documentation source, on community.kde.org, instead of these docs, 
indicate things need some work here.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: dfaure, cgiboudeaux, kfunk
Cc: kossebau, aacid, kfunk, #frameworks, #build_system

D8351: API dox: add note about calling setApplicationDomain after QApp creation

[Friedrich W . H . Kossebau]

kossebau added a comment.


  Another option might be to split off a data structure where the application 
domain string is stored separately, and thus delay the initialization of the 
KLocalizedStringPrivateStatics instance to the first i18n call. Though given 
that the initialisation sets global values which then are const for the rest of 
the runtime based on the environment at the time, that would be slightly random.
  Having straight control about the time when the init is done feels better to 
me. Possibly that should be even more emphasized in the API dox?

REPOSITORY
  R249 KI18n

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

To: kossebau, #frameworks, ilic, ltoscano

D8351: API dox: add note about calling setApplicationDomain after QApp creation

[Friedrich W . H . Kossebau]

kossebau created this revision.
kossebau added reviewers: Frameworks, ilic, ltoscano.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  The current implementation of KLocalizedString::setApplicationDomain()
  will trigger the creation of the KLocalizedStringPrivateStatics instance and
  thus also the calculation of the locale languanges from the env vars
  as set at that time, and QLocale::system() on some platforms.
  So the claim that "This call can be made at any place in the code" does not
  hold, instead API consumers should be instructed about when to call
  the method ideally.

REPOSITORY
  R249 KI18n

BRANCH
  addNoteAboutCallingSetAppDomainAfterQApp

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

AFFECTED FILES
  docs/programmers-guide.md
  src/klocalizedstring.h

To: kossebau, #frameworks, ilic, ltoscano

D9012: Revert "Detach before setting the d pointer"

[Friedrich W . H . Kossebau]

kossebau added a comment.


  In https://phabricator.kde.org/D9012#172387, @leinir wrote:
  
  > Quicker is better here, i think... Perhaps it is worth adding the 
documentation we discussed as well in this review? Thinking about making it 
easier to track the history and whatnot of what happened and why...
  
  
  Myself I have no time left to invest into this. And would be happy to see at 
least the regression finally fixed, before slipping in yet another release in 
some days. Remember, this breaks core functionality of knewstuff across 
multiple applications (everywhere at least where files are uncompressed after 
the download). Rendering KNewStuff completely useless in those apps, and also 
making the apps themselves look bad and broken.
  
  I would propose to have those improve the API documentation who introduced 
the regression. As they know best why they missed the bit about things being 
explicitly-shared by design.
  
  Actually I am already a bit annoyed that I have to push for fixing a 
regression I did not introduce myself.

REPOSITORY
  R304 KNewStuff

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

To: kossebau, whiting, leinir, apol
Cc: #frameworks

D9012: Revert "Detach before setting the d pointer"

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R304:bcb7dd628811: Revert Detach before setting the d 
pointer (authored by kossebau).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9012?vs=22979=23055

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

AFFECTED FILES
  src/core/entryinternal.cpp

To: kossebau, whiting, leinir, apol
Cc: #frameworks

D9012: Revert "Detach before setting the d pointer"

[Friedrich W . H . Kossebau]

kossebau created this revision.
kossebau added reviewers: whiting, leinir, apol.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  This reverts commits 
https://phabricator.kde.org/R304:04cc49c71bdb948e06ccae2d97d7cc1a1d2f62af as 
well
  as the follow-up partial fix 
https://phabricator.kde.org/R304:c32c8d002e1216373560c94738841a7a5e1b976b
  
  The whole internal data sync'ing design of the KNewStuff core library
  relies on EntryInternal instances explicitely sharing the data.
  Changing only EntryInternal to implicitly shared data broke things.
  And changed behaviour of that class also for any 3rd-party consumers.
  
  BUG: 386156

REPOSITORY
  R304 KNewStuff

BRANCH
  unbreakEntryInternalDataSyncing

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

AFFECTED FILES
  src/core/entryinternal.cpp

To: kossebau, whiting, leinir, apol
Cc: #frameworks

D9012: Revert "Detach before setting the d pointer"

[Friedrich W . H . Kossebau]

kossebau added a comment.


  See also discussion at https://phabricator.kde.org/D7194#159565 ff.

REPOSITORY
  R304 KNewStuff

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

To: kossebau, whiting, leinir, apol
Cc: #frameworks

D8336: Improve apidox of KJobTrackerInterface

[Friedrich W . H . Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in kjobtrackerinterface.h:73
> Probably yes, should we add a TODO comment for KF6?

And for the protected, not sure, it feels unbalanced to me. But something to 
think about at time of KF6, so yes, add a (non-doxygen) comment somewhere (e.g. 
at method line end).

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

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

D8336: Improve apidox of KJobTrackerInterface

[Friedrich W . H . Kossebau]

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


  (Sorry, forgot about this one, subscribed too to many diffs in the 
phabricator view)
  
  Looks good to me in general. Added some comments you might want to consider, 
but are free to ignore :)

INLINE COMMENTS

> kjobtrackerinterface.h:54
>   * Register a new job in this tracker.
> + * The default implementation connects all the KJob signals
> + * to the protected slots of this class.

IMHO this should have an explicit listing of the signals which are connected, 
so the API consumer exactly knows what to rely on. A few of the "all the KJob 
signals" are not wired up.

> kjobtrackerinterface.h:73
>   * Unregister a job from this tracker.
> + * You need to manually call this method only if you re-implemented
> + * registerJob() without connecting KJob::finished to this slot.

I would propose to make this a "@note ", given this is no description of the 
normal behaviour, but some additional info. It might also catch more attention.

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

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

D8728: Install mimetype definitions for kcfg/kcfgc/ui.rc/knotify & qrc files

[Friedrich W . H . Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R244:dbde1029b3b7: Install mimetype definitions for 
kcfg/kcfgc/ui.rc/knotify  qrc files (authored by kossebau).

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8728?vs=22746=22750

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

AFFECTED FILES
  src/mimetypes/kde5.xml

To: kossebau, #frameworks, dfaure
Cc: ngraham

D8728: Install mimetype definitions for kcfg/kcfgc/ui.rc/knotify & qrc files

[Friedrich W . H . Kossebau]

kossebau updated this revision to Diff 22746.
kossebau added a comment.


  - drop qrc mimetype proposal, qt types are to be dealt with upstream
  - x-vnd -> vnd

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8728?vs=22095=22746

BRANCH
  mimetypesforkfdevelopmentfiles

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

AFFECTED FILES
  src/mimetypes/kde5.xml

To: kossebau, #frameworks, dfaure
Cc: ngraham

D8728: Install mimetype definitions for kcfg/kcfgc/ui.rc/knotify & qrc files

[Friedrich W . H . Kossebau]

kossebau added a comment.


  @dfaure thanks for review.
  
  Given these are kf5-specific mimetypes I am unsure whether to propose them 
also for inclusion in shared-mime-info. What do you think?
  
  I have been tempted to also add mimetypes for all the special scripty files 
(Messages.sh, ExtraDeskto.sh, XmlMessages.,sh). Having those defined might help 
both newcomers, but also oldies who have no clue about scripty details get some 
hint from their filemanager/IDE what kind of special files those are. Any 
opinion here as well?

REPOSITORY
  R244 KCoreAddons

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

To: kossebau, #frameworks, dfaure
Cc: ngraham

D8947: Expose KFilePlacesModel 'iconName' rule

[Friedrich W . H . Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> kfileplacesmodel.h:52
> +GroupRole = 0x0a5b64ee,
> +// @since 5.41
> +IconNameRole = 0x00a45c00

+1 for adding `@since`. But needs  three slashes`///` for doxygen to pick up 
this comment

REPOSITORY
  R241 KIO

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

To: renatoo, dfaure
Cc: kossebau, #frameworks

D9182: return nullptr -> return {} for QFlags

[Friedrich W . H . Kossebau]

kossebau added a comment.


  Code change looks good to me. Think I missed that in my clean-up commits to 
@kfunk 's initial commit as I only compiled the library parts of KDE 
Frameworks, not the plugin modules.
  
  No maintainer of this module though, so just a +1 for now.
  
  Commit message could perhaps be extended to quickly note why the nullptr has 
been there at all, so future commit history readers get a clue, which they do 
not from a "irritating" attribution :)

REPOSITORY
  R287 KImageFormats

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

To: mkoller, kfunk, kossebau
Cc: #frameworks

D8728: Install mimetype definitions for kcfg/kcfgc/ui.rc/knotify & qrc files

[Friedrich W . H . Kossebau]

kossebau added a subscriber: ltoscano.
kossebau added a comment.


  In https://phabricator.kde.org/D8728#174635, @dfaure wrote:
  
  > I agree that those are really KF5 specific and developer-oriented, so 
rather unlikely to be e.g. sent by email to other people.
  
  
  Okay, so for now not looking into pushing them upstream (too bad we gave up 
on "turning every user into a developer" ;) )
  
  > I'm unsure about mimetypes for special files by name, that seems too 
specialized. Mimetypes are for *types* of files, not for each specific file ;)
  >  I mean, Messages.sh is a shell script, that's its type. Its specific 
purpose is something else, of course, but still you would use a viewer or 
editor for shell scripts to open it (which is the main reason for mimetypes: 
app associations).
  
  But isn't Messages.sh a sub-type to shell-script, as it needs to match some 
further requirements, as defined by scripty expectations? And while there might 
be no specific viewer/editor/etc as handler assigned, it still a type of its 
own as handled by humans (outside of scripty server). So IMHO it is helpful to 
also have these files displayed annotated as such in filemanagers/dialogs, 
ideally both by icon and type tooltip. With such annotated presentation to 
humans for me being another raison d’être for mimetypes :)
  So IMHO "Messages,sh" & Co match things already in shared-mime-info like 
"AUTHORS", "COPYING", "CREDITS", "LICENSE", where there also usually is no 
specific handler, still it is helpful when browsing files to have these special 
files annotated.
  But given @ltoscano's plan to overhaul string extraction, I currently will 
not invest more into the scripty-specific file types, would rather first see to 
find someone who could find some Qt developer to pursue 
https://bugreports.qt.io/browse/QTBUG-64435 given this is another set of modern 
Qt file types which are badly presented when browsing files in file managers or 
IDEs. Any hint who could be motivated there?

REPOSITORY
  R244 KCoreAddons

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

To: kossebau, #frameworks, dfaure
Cc: ltoscano, ngraham