D10078: Add separate lib KF5::DBusRunner

2018-04-23 Thread Friedrich W . H . Kossebau
kossebau updated this revision to Diff 32916. kossebau added a comment. Updating with proposed further massaging of the API - changed some class/method names to closely follow KRunner terms: (RunnerContext, query) - moved RunnerContext into separate header (for one per class) -

D10078: Add separate lib KF5::DBusRunner

2018-04-23 Thread Friedrich W . H . Kossebau
kossebau commandeered this revision. kossebau edited reviewers, added: davidedmundson; removed: kossebau. REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D10078 To: kossebau, broulik, davidedmundson Cc: bruns, michaelh, ngraham, #frameworks

D10078: Add separate lib KF5::DBusRunner

2018-04-24 Thread Friedrich W . H . Kossebau
kossebau added a comment. In D10078#252871 , @davidedmundson wrote: > This makes it quite easy for a developer to screw up and block krunner. > The RAII approach makes it very very hard for a developer to screw up with any of the multiple

D12355: [API dox] New UI marker @info:placeholder

2018-04-19 Thread Friedrich W . H . Kossebau
kossebau updated this revision to Diff 32604. kossebau added a comment. add also in kuit handling code REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12355?vs=32572=32604 BRANCH addinfoplaceholderuimarker REVISION DETAIL

D12353: [API dox] New UI marker @item:valuesuffix

2018-04-19 Thread Friedrich W . H . Kossebau
kossebau updated this revision to Diff 32605. kossebau added a comment. add also in kuit handling code REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12353?vs=32571=32605 BRANCH additemvaluesuffixuimaker REVISION DETAIL

D12353: [API dox] New UI marker @item:valuesuffix

2018-04-19 Thread Friedrich W . H . Kossebau
kossebau added a comment. In D12353#249996 , @aacid wrote: > All the other elements of that list have an entry like > > ./src/kuitmarkup.cpp:281:SET_CUE(InrangeCue, QStringLiteral("inrange")); > > Do we need that too for valuesuffix?

D11675: use KDE_INSTALL_DATADIR instead of FULL_DATADIR

2018-04-19 Thread Friedrich W . H . Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R290:ee1ea06d964c: use KDE_INSTALL_DATADIR instead of FULL_DATADIR (authored by bshah, committed by kossebau). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D11675?vs=30487=32610#toc REPOSITORY

D10078: Add separate lib KF5::DBusRunner

2018-04-25 Thread Friedrich W . H . Kossebau
kossebau added a comment. Good, seems we found agreed-on codebase :) Will brush over the API dox/code comments some more for a final thumb-up. Though after yesterday's prototyping of further dbusrunner plugins there is another thing where I might want to suggest some API change to be

D11716: Fix build on FreeBSD broken by d7cce9937d5e9af2753fadb82d11f308b58bb8fa

2018-03-26 Thread Friedrich W . H . Kossebau
kossebau added a comment. Thanks for coming up so quick with a fix :) You surely wanted to also wrap the include of the "sys/xattr.h" header in the new condition? Missing from the patch. INLINE COMMENTS > config-kioslave-file.h.cmake:15 > +#cmakedefine01 HAVE_SYS_XATTR_H > \ No newline

D11716: Fix build on FreeBSD broken by d7cce9937d5e9af2753fadb82d11f308b58bb8fa

2018-03-26 Thread Friedrich W . H . Kossebau
kossebau added a comment. From quick pure code reading this looks fine to me. +1 Have not tested though right now, so have to leave the ship-it to actual Dolphin/KIO maintainers. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D11716 To: rominf, #dolphin, kossebau,

D11716: Fix build on FreeBSD broken by d7cce9937d5e9af2753fadb82d11f308b58bb8fa

2018-03-26 Thread Friedrich W . H . Kossebau
kossebau added inline comments. INLINE COMMENTS > config-kioslave-file.h.cmake:14 > +/* Defined if system has extended file attributes support. */ > +#cmakedefine01 HAVE_SYS_XATTR_H While FindACL.cmake sets internally the cmake variable HAVE_SYS_XATTR_H, it is not part of the officially set

D11204: Support NTFS hidden files

2018-03-26 Thread Friedrich W . H . Kossebau
kossebau added inline comments. INLINE COMMENTS > file_unix.cpp:40 > #include > +#include > #include This unconditional include sadly breaks the build at least on FreeBSD (see https://build.kde.org/view/Frameworks/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.9/164/console ) and other

D11642: Fix the rcc binary package generation

2018-03-24 Thread Friedrich W . H . Kossebau
kossebau added inline comments. INLINE COMMENTS > KF5PackageMacros.cmake:164 > + DEPENDS ${GENERATED_RCC_CONTENTS} > ${component}-${root}-metadata-json ${kpkgqrc}) > +install(FILES ${GENERATED_RCC_CONTENTS} DESTINATION >

D11675: use KDE_INSTALL_DATADIR instead of FULL_DATADIR

2018-03-25 Thread Friedrich W . H . Kossebau
kossebau added a comment. While myself I have never made use of it, the idea of using a relative path & not an absolute path with the DESTINATION argument is to allow overriding the install prefix at install time. Or in words of https://cmake.org/cmake/help/v3.0/command/install.html sa

D11414: [xcb] Fix implementation of _NET_WM_FULLSCREEN_MONITORS

2018-03-23 Thread Friedrich W . H . Kossebau
kossebau added inline comments. INLINE COMMENTS > netwm.cpp:2845-2847 > +const uint32_t data[5] = { > +topology.top, topology.bottom, topology.left, topology.right, 1 > +}; Seems clang (at least as of FreeBSD with -Wc++11-narrowing) does not like this narrowing from

D10849: Add template for Plasma wallpaper with QML extension

2018-04-03 Thread Friedrich W . H . Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R242:354bd71296b5: Add template for Plasma wallpaper with QML extension (authored by kossebau). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE

D11414: [xcb] Fix implementation of _NET_WM_FULLSCREEN_MONITORS

2018-03-24 Thread Friedrich W . H . Kossebau
kossebau added inline comments. INLINE COMMENTS > graesslin wrote in netwm.cpp:2845-2847 > I guess we need to fix this Thanks for having done that :) REPOSITORY R278 KWindowSystem REVISION DETAIL https://phabricator.kde.org/D11414 To: graesslin, #frameworks, #kwin, #plasma,

D11392: sanitizer: Move implementation into Baloo namespace

2018-03-16 Thread Friedrich W . H . Kossebau
kossebau accepted this revision. kossebau added a comment. This revision is now accepted and ready to land. Only looked at the diff, but nothing suspicious seen. If it builds and works, should be fine :) Perhaps mention in the commit message also that this moving into the namespace

D10749: Add ECMSetupQtPluginMacroNames

2018-03-17 Thread Friedrich W . H . Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R240:36d42640576e: Add ECMSetupQtPluginMacroNames (authored by kossebau). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D10749?vs=29426=29791#toc REPOSITORY R240 Extra CMake Modules CHANGES

D11294: Use ecm_setup_qtplugin_macro_names

2018-03-20 Thread Friedrich W . H . Kossebau
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R244:8b7db6c41328: Use ecm_setup_qtplugin_macro_names (authored by kossebau). REPOSITORY R244 KCoreAddons CHANGES SINCE

D11295: Use ecm_setup_qtplugin_macro_names

2018-03-20 Thread Friedrich W . H . Kossebau
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R242:f018779459e0: Use ecm_setup_qtplugin_macro_names (authored by kossebau). REPOSITORY R242 Plasma Framework (Library)

D11296: Use ecm_setup_qtplugin_macro_names

2018-03-20 Thread Friedrich W . H . Kossebau
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R290:eb91dd14aee1: Use ecm_setup_qtplugin_macro_names (authored by kossebau). REPOSITORY R290 KPackage CHANGES SINCE

D11642: Fix the rcc binary package generation

2018-03-24 Thread Friedrich W . H . Kossebau
kossebau added inline comments. INLINE COMMENTS > kossebau wrote in KF5PackageMacros.cmake:158 > This "Generating xyz" might be still good to have. Please consider picking > this up with a > > COMMENT "Generating: ${xyz}" > > added to `add_custom_command`, where ${xyz} gets a proper

D11642: Fix the rcc binary package generation

2018-03-24 Thread Friedrich W . H . Kossebau
kossebau added inline comments. INLINE COMMENTS > KF5PackageMacros.cmake:158 > -include(${kpackagedir}/qrc.cmake) > -message(STATUS \"Generating: > ${KDE_INSTALL_FULL_DATADIR}/${install_dir}/${root}/${component}/contents.rcc\") > -execute_process(COMMAND ${KPACKAGE_RCC}

D11019: Remove left-over code from kdelibs splitting time

2018-03-04 Thread Friedrich W . H . Kossebau
kossebau created this revision. kossebau added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. kossebau requested review of this revision. TEST PLAN Building/installing kinit still works, projects using

D11019: Remove left-over code from kdelibs splitting time

2018-03-04 Thread Friedrich W . H . Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R303:3bcf7a1b3421: Remove left-over code from kdelibs splitting time (authored by kossebau). REPOSITORY R303 KInit CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11019?vs=28595=28599 REVISION

D10848: Templates: consistent naming, fix translation catalog names & more

2018-02-26 Thread Friedrich W . H . Kossebau
kossebau added inline comments. INLINE COMMENTS > apol wrote in metadata.desktop:88 > I'm not sure, if a 3rd party makes a plasmoid they would call it otherwise. > Are we assuming these templates are only for KDE? Ideally the kapptemplate system would have support for an organization domain

D10078: Add separate lib KF5::DBusRunner

2018-04-25 Thread Friedrich W . H . Kossebau
kossebau updated this revision to Diff 33114. kossebau added a comment. - change #pragma once to traditional include guards, not yet part of kf policies - adapt all file names to class names - brush over API docs REPOSITORY R308 KRunner CHANGES SINCE LAST UPDATE

D10078: Add separate lib KF5::DBusRunner

2018-04-25 Thread Friedrich W . H . Kossebau
kossebau added a comment. In D10078#253730 , @davidedmundson wrote: > > What do you think? Would give this a separate try tonight, to get some idea. > > Forwarding AbstractRunner::teardown is something I'd fully support. Forwarding

D10078: Add separate lib KF5::DBusRunner

2018-04-25 Thread Friedrich W . H . Kossebau
kossebau added a comment. In D10078#253982 , @davidedmundson wrote: > > From my prototyping experiments I can tell that the current approach of recommending in API docs to

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-10-12 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 43516. kossebau added a comment. Updating: - bump since to 5.53 - remove todo question about background paiting, assumed not to be required task of the delegate given the current code - implement todo about catching destroyed user delegate (not

D16300: Remove double underscore (__) from header include guards

2018-10-18 Thread Friedrich W. H. Kossebau
kossebau created this revision. kossebau added a reviewer: Kate. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. kossebau requested review of this revision. REVISION SUMMARY Names containing a double underscore (__) are reserved to the

D16300: Remove double underscore (__) from header include guards

2018-10-18 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D16300#345405 , @broulik wrote: > It could have just gone `#pramga once` conjunctive "could" -> not specified in KF policies if possible. Seems even Qt devs are undecided about it and rejected it again,

D16300: Remove double underscore (__) from header include guards

2018-10-18 Thread Friedrich W. H. Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R39:38a9214aaa0e: Remove double underscore (__) from header include guards (authored by kossebau). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-10-18 Thread Friedrich W. H. Kossebau
kossebau marked 2 inline comments as done. kossebau added a comment. In D8708#344872 , @dhaumann wrote: > Feel free to push this for 5.52 (you'd have to adapt the @since 5.53 again). Thanks for review again :) Would prefer waiting for

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-10-18 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 43868. kossebau marked an inline comment as done. kossebau added a comment. update to Dominik's last review: - remove __ from include guard - add comment that Option::view is always set REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE

D16347: Expose KTextEditor::ViewPrivate:setInputMode(InputMode) to KTextEditor::View

2018-10-28 Thread Friedrich W. H. Kossebau
kossebau resigned from this revision. kossebau added a comment. Not my domain, so resigning as reviewer :) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D16347 To: demsking, mlaurent, vkrause, dhaumann, cullmann Cc: cullmann, kwrite-devel, kde-frameworks-devel,

D5802: ViewPrivate, KateSearchBar, KateVi::MatchHighlighter: use selection foreground for search highlights

2018-10-28 Thread Friedrich W. H. Kossebau
kossebau added a comment. No insight into the topic, but seeing this in my review request list (KDevelop): Given KTextEditor was ported to KSyntaxHighlighting now, what can be said about this patch? Does it still apply? What foreground color options are there now for search highlights?

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-11-03 Thread Friedrich W. H. Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R39:9a0505af2dbf: Introduce AbstractAnnotationItemDelegate for more control by consumer (authored by kossebau). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D8708?vs=43868=44806#toc REPOSITORY

D16770: [ftp kio-slave] Fix deletion of directory with non-latin1/utf8 parent path

2018-11-09 Thread Friedrich W. H. Kossebau
kossebau retitled this revision from "[ftp kio-slave] Fix deletion of directory with non-latin parent path" to "[ftp kio-slave] Fix deletion of directory with non-latin1/utf8 parent path". kossebau edited the summary of this revision. kossebau edited the test plan for this revision. REPOSITORY

D16770: [ftp kio-slave] Fix deletion of directory with non-latin1/utf8 parent path

2018-11-09 Thread Friedrich W. H. Kossebau
kossebau added a subscriber: aacid. kossebau added a comment. @aacid Thanks for providing your ftp server, served its purpose. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D16770 To: kossebau, dfaure Cc: aacid, kde-frameworks-devel, michaelh, ngraham, bruns

D16770: [ftp kio-slave] Fix deletion of directory with non-latin parent path

2018-11-08 Thread Friedrich W. H. Kossebau
kossebau created this revision. kossebau added a reviewer: dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. kossebau requested review of this revision. REVISION SUMMARY KRemoteEncoding::directory() returns the encoded path of the directpry, while

D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

2018-11-15 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D16882#359595 , @rjvbb wrote: > > Please, let's find the root causes and fix things at the base instead of adding such > > "we" (tried to not make this a "you" vs. "others" thing in the language, but

D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

2018-11-15 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D16882#359888 , @rjvbb wrote: > > _If_ it is found that the root bug is in KTextEditor, sure. > See https://bugs.kde.org/show_bug.cgi?id=401069#c2 That looks like good work onto finding the

D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

2018-11-14 Thread Friedrich W. H. Kossebau
kossebau requested changes to this revision. kossebau added a comment. This revision now requires changes to proceed. Oh, I missed the latter "For an as-yet unknown reason populateContextMenu() will be called with every view in which the contextmenu has been opened when the CTags plugin is

D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

2018-11-14 Thread Friedrich W. H. Kossebau
kossebau added a comment. > TextDocument::populateContextMenu() is called when the user opens the contextmenu but this can happen for more than just the view currently active. What does "more than just the view currently active" mean? How can that happen? From what I see

D16466: [KSambaShare] Add unit test for "net usershare info" parser

2018-11-08 Thread Friedrich W. H. Kossebau
kossebau added a comment. Only saw the note from @bcooksley now, pushed an intermediate disabling of the new test for windows, so the build at least passes: 160df8ed7b49f68e74d30cf4343a6251ed4085d8 Seems some

D16832: fix empty runner list after switching to new locale

2018-11-14 Thread Friedrich W. H. Kossebau
kossebau resigned from this revision. kossebau added a comment. No time and insight, sorry. REPOSITORY R308 KRunner REVISION DETAIL https://phabricator.kde.org/D16832 To: shaforostoff, #plasma_workspaces, davidedmundson, #frameworks Cc: kde-frameworks-devel, michaldybczak, sdepiets,

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-10-04 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 42903. kossebau added a comment. Improve rendering in scaled mode REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8708?vs=41828=42903 BRANCH addAnnotationItemDelegate REVISION DETAIL

D15582: Reference product "KF5" in widget metadata, instead of "KDE"

2018-10-08 Thread Friedrich W. H. Kossebau
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R298:9c34333131b8: Reference product KF5 in widget metadata, instead of KDE (authored by kossebau). REPOSITORY R298

D14530: Fix crash when save a QImage to the eps format file

2018-10-08 Thread Friedrich W. H. Kossebau
kossebau abandoned this revision. kossebau added a comment. Closing this review request as the author has not reacted. D15405 should be serving already as more proper fix for the referenced bug. REPOSITORY R287 KImageFormats REVISION DETAIL

D14530: Fix crash when save a QImage to the eps format file

2018-10-08 Thread Friedrich W. H. Kossebau
kossebau commandeered this revision. kossebau edited reviewers, added: zccrs; removed: kossebau. REPOSITORY R287 KImageFormats REVISION DETAIL https://phabricator.kde.org/D14530 To: kossebau, mlaurent, dfaure, zccrs Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-10-10 Thread Friedrich W. H. Kossebau
kossebau added a comment. @cullmann @dhaumann So, what to do? :) Do you think if we delay one more month you will find time to give this the wanted deeper review? Or will this continue to (understandable) lack your motivation given you are so far not a consumer of this new API?

D14111: Install MIME type definition for text/x-rst ourselves for now

2018-10-10 Thread Friedrich W. H. Kossebau
kossebau abandoned this revision. kossebau added a comment. I see how this is no simple matter. Given I could solve my needs locally, no need to add things to maintain where no-one else seems to have a need. So discarding, no bad feelings. Given that rst is around since ages and yet nobody

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-10-04 Thread Friedrich W. H. Kossebau
kossebau added a comment. Thanks everyone who so far looked at this, especially @dhaumann for detailed comments. I am tempted to interpret the lack of further comments, especially the lack of principal objections as an implicit +1 on this patch as it is now :) While I still am

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-10-06 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D8708#337408 , @dhaumann wrote: > Hm, given the size of the patch, and given it introduces new public API we cannot easily change, I think a silent +1 since noone reacts is not good enough. I agree that the

D14530: Fix crash when save a QImage to the eps format file

2018-09-23 Thread Friedrich W. H. Kossebau
kossebau added a comment. @zccrs Hi. Does D15405 fix things for you, so is this patch here no longer needed? If so, please close this review request by selecting and submitting the action "Abandon Revision", so it does no longer appear in our "Please

D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

2018-11-18 Thread Friedrich W. H. Kossebau
kossebau added a comment. For the record, related commits are: Adding the principal clean-up logic: R32:b837392d5f05394794a813afb7ca94e54650fcff Changing it from clean-on-hide to clean-on-about-to-show-again:

D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

2018-11-18 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D16882#361733 , @rjvbb wrote: > > `KTextEditor::View::contextMenu()` talks about "the xmlgui menu" or custom set "context menu object", but no hint/promise whether there is any instance sharing done, e.g. between

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

2019-01-21 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D18167#397319 , @graesslin wrote: > For done code this warning is pointless and negative. I invite you to work with a code base like KWin where it is more important to have a working git blame than protection for

D18434: exiv2extractor: add support for bmp, gif, webp, tga

2019-01-21 Thread Friedrich W. H. Kossebau
kossebau created this revision. kossebau added a reviewer: Baloo. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. kossebau requested review of this revision. REVISION SUMMARY Gives only minimal info for bmp, gif & tga, but knowing the size can be

D18450: Add extractor for AppImage files

2019-01-22 Thread Friedrich W. H. Kossebau
kossebau added a comment. Example screenshot: F6561839: Screenshot_20190122_102745.png There seems to be some bug with the Comment field though, somehow in Dolphin the comment is not shown, where "dump" displays it as existing. REPOSITORY R286

D18450: Add extractor for AppImage files

2019-01-22 Thread Friedrich W. H. Kossebau
kossebau created this revision. kossebau added a reviewer: Baloo. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. kossebau requested review of this revision. REVISION SUMMARY Only a few properties currently can be mapped more or less to the existing

D18450: Add extractor for AppImage files

2019-01-23 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 50146. kossebau added a comment. add unit test REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18450?vs=50077=50146 BRANCH addappimageextractor REVISION DETAIL https://phabricator.kde.org/D18450 AFFECTED

D18450: Add extractor for AppImage files

2019-01-23 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D18450#398143 , @astippich wrote: > Can you add a test please? Done. The sample file sadly is some 2xx KB big, but the AppImage devs could not help to get it smaller without no longer being a real appimage,

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

2019-01-23 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D18167#398076 , @graesslin wrote: > The human error exists as long as clang-tidy is not used. What I fear is that someone does a hand porting - we have seen several attempts to do that in KWin from various

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

2019-01-23 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D18167#398360 , @aacid wrote: > In D18167#398343 , @kossebau wrote: > > > only 3(?) days between proposal and commit was also a very rushy > > > Check your dates

D17086: Add icons for application-vnd.appimage

2018-11-21 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D17086#363855 , @ngraham wrote: > Thanks for the icon! I'd recommend mimicking the style of the other package icons that have a symbol: > > In other words, add a zipper towards the right-side,, put the AppImage

D17086: Add icons for application-vnd.appimage

2018-11-21 Thread Friedrich W. H. Kossebau
kossebau added a comment. Open questions: - how to support the dark icon set? - what would be better colors? Example of icon use with 64 & 16 pixel size, and bigger in preview: F6435751: Screenshot_20181121_220325.png The icon also

D17086: Add icons for application-vnd.appimage

2018-11-25 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 46231. kossebau added a comment. - add symlinks for application-x-iso9660-appimage to application-vnd.appimage REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17086?vs=46229=46231 BRANCH addappimageiconv2

D17086: Add icons for application-vnd.appimage

2018-11-25 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D17086#363977 , @ngraham wrote: > We have two options here: > > 1. Follow the style and make the AppImage icon look like a document with a folded-over corner that has an the AppImage logo in the middle > 2.

D17086: Add icons for application-vnd.appimage/x-iso9660-appimage

2018-11-25 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D17086#366120 , @ngraham wrote: > Looks great to me then! > > But how do I make this work? I compiled and installed the icon and verified that it's installed in the right place. But my AppImage isn't getting the

D17086: Add icons for application-vnd.appimage/x-iso9660-appimage

2018-11-25 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D17086#366111 , @ngraham wrote: > Thanks, that looks great! As for the background color, I'd say it's okay to use a different shade of blue if it would better match the AppImage branding. For now I favor

D17086: Add icons for application-vnd.appimage/x-iso9660-appimage

2018-11-25 Thread Friedrich W. H. Kossebau
kossebau retitled this revision from "Add icons for application-vnd.appimage" to "Add icons for application-vnd.appimage/x-iso9660-appimage". REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D17086 To: kossebau, #vdg, ngraham Cc: TheAssassin, ngraham,

D17086: Add icons for application-vnd.appimage

2018-11-25 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 46229. kossebau added a comment. - use document-style icon shape, to match other existing icons for executables - use Breeze palette color that is more close to AppImage logo one REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE

D17086: Add icons for application-vnd.appimage

2018-11-25 Thread Friedrich W. H. Kossebau
kossebau added a comment. F6441665: Screenshot_20181126_014024.png for an updated example usage. @ngraham How do you create those summary pictures you used above for the comparison previews? In D17086#366097

D17086: Add icons for application-vnd.appimage/x-iso9660-appimage

2018-11-27 Thread Friedrich W. H. Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R266:e3266db59d2f: Add icons for application-vnd.appimage/x-iso9660-appimage (authored by kossebau). REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE

D17086: Add icons for application-vnd.appimage/x-iso9660-appimage

2018-11-27 Thread Friedrich W. H. Kossebau
kossebau added a comment. Thanks for feedback/review :) REPOSITORY R266 Breeze Icons BRANCH addappimageiconv2 REVISION DETAIL https://phabricator.kde.org/D17086 To: kossebau, #vdg, ngraham, TheAssassin Cc: TheAssassin, ngraham, kde-frameworks-devel, michaelh, bruns

D17088: [thumbnailer appimage] Fix building with libappimage not in system path

2018-11-21 Thread Friedrich W. H. Kossebau
kossebau created this revision. kossebau added reviewers: broulik, TheAssassin, azubieta. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. kossebau requested review of this revision. REVISION SUMMARY The current CMake Config file of

D17088: [thumbnailer appimage] Fix building with libappimage not in system path

2018-11-22 Thread Friedrich W. H. Kossebau
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R320:6c2ebbb5b853: [thumbnailer appimage] Fix building with libappimage not in system path (authored by kossebau).

D17044: Add FindExiv2.cmake to ECM

2018-11-20 Thread Friedrich W. H. Kossebau
kossebau added inline comments. INLINE COMMENTS > FindExiv2.cmake:31 > +# Since 5.53.0. > +# TODO KF6: Rename to FindLibExiv2.cmake > +# Could you not already do this now, before it's first released? Any potential users currently have their own copy, they will not use this version. Once they

D17015: Fix the Qt doc creation with Qt 5.12.

2018-11-20 Thread Friedrich W. H. Kossebau
kossebau added a comment. Good to see you caring for ECM documentation not getting broken with Qt 5.12 :) Any idea how we could perhaps deduplicate the FindQHelpGenerator.cmake with the one from find-modules (which is a helper for runtime with the ECMAddQch macro)? No instant idea

D16938: FindQHelpGenerator: try to find Qt5Help instead of Qt5Core

2018-11-17 Thread Friedrich W. H. Kossebau
kossebau accepted this revision. kossebau added a comment. This revision is now accepted and ready to land. Good find. No idea why it was not like this from the start. Untested (besides grepping my local Qt 5.11 CMake Config files to confirm myself that's really the place where the

D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

2018-11-18 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D16882#360857 , @rjvbb wrote: > Re-opening because I found an actual flaw in KDevelop after noticing that context menu duplication still occurred when only the active view receives the aboutToShowContextMenu signal.

D18088: FindGperf: in ecm_gperf_generate set SKIP_AUTOMOC for generated file

2019-01-07 Thread Friedrich W. H. Kossebau
kossebau created this revision. kossebau added reviewers: Frameworks, pino. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. kossebau requested review of this revision. REVISION SUMMARY Avoids to have manually set the property on

D18088: FindGperf: in ecm_gperf_generate set SKIP_AUTOMOC for generated file

2019-01-07 Thread Friedrich W. H. Kossebau
kossebau added a comment. Not sure if there ever s a chance somebody would inject QObject code into such a generated file? REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D18088 To: kossebau, #frameworks, pino Cc: kde-frameworks-devel, kde-buildsystem,

D15513: Fix dangling reference with "auto" becoming "QStringBuilder"

2018-09-14 Thread Friedrich W. H. Kossebau
kossebau created this revision. kossebau added reviewers: ivan, mlaurent. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. kossebau requested review of this revision. REVISION SUMMARY With QT_USE_QSTRINGBUILDER being enabled, code best is checked with clazy

D15513: Fix dangling reference with "auto" becoming "QStringBuilder"

2018-09-14 Thread Friedrich W. H. Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R6:ea5416f8f99a: Fix dangling reference with auto becoming QStringBuilder (authored by kossebau). REPOSITORY R6 KActivities CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15513?vs=41661=41668

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-09-15 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 41694. kossebau added a comment. fix crash due to (no longer needed) circular dependencies on view object constrctions REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8708?vs=41225=41694 BRANCH

D15582: Reference product "KF5" in widget metadata, instead of "KDE"

2018-09-17 Thread Friedrich W. H. Kossebau
kossebau created this revision. kossebau added a reviewer: Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. kossebau requested review of this revision. REPOSITORY R298 KDesignerPlugin BRANCH kdeiskf5now REVISION DETAIL

D15591: Add Open Document thumbnailer

2018-09-18 Thread Friedrich W. H. Kossebau
kossebau added a comment. Just FYI, as I was added, I currently have no time reserved for document related code work: There is such a thumbnailer for OpenDocument Format document which exposes the preview/thumbnail picture from the data since ages on what now is store.kde.org, here my

D15405: [EPS] Fix crash at app shutdown (being tried to persist clipboard image)

2018-09-17 Thread Friedrich W. H. Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R287:98c65a438dfb: [EPS] Fix crash at app shutdown (being tried to persist clipboard image) (authored by kossebau). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D15405?vs=41341=41808#toc

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-09-16 Thread Friedrich W. H. Kossebau
kossebau marked 15 inline comments as done. kossebau added inline comments. INLINE COMMENTS > dhaumann wrote in abstractannotationitemdelegate.h:52 > I am asking myself: Is wrappedLineCount >= 1 always true? If so, can you add > this as documentation? wrappedLineCount == 1 means no wrapping

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-09-16 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 41778. kossebau marked 3 inline comments as done. kossebau added a comment. Update to Dominik's first review REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8708?vs=41694=41778 BRANCH addAnnotationItemDelegate

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-09-17 Thread Friedrich W. H. Kossebau
kossebau added a comment. In D8708#326590 , @dhaumann wrote: > What I would like to see is a comment // KF6: Merge KTextEditor::AnnotationViewInterfaceV2 into KTextEditor::AnnotationViewInterface (kossebau). > For me, this comment is really

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

2018-09-17 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 41828. kossebau added a comment. also add todo about merging KTextEditor::AnnotationViewInterfaceV2 into KTextEditor::AnnotationViewInterface REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8708?vs=41778=41828

D18434: exiv2extractor: add support for bmp, gif, webp, tga

2019-01-24 Thread Friedrich W. H. Kossebau
kossebau added a reviewer: Dolphin. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D18434 To: kossebau, #baloo, #dolphin Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D18450: Add extractor for AppImage files

2019-01-22 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 50073. kossebau added a comment. - switch and use any localized versions found matching the current system locale, should be more expected - also extract appdata and expose as plain text, even though that can be quite some data, but that's the UI's

D18450: Add extractor for AppImage files

2019-01-22 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 50077. kossebau added a comment. - skip getting the unlocalized description if there is a localized one REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18450?vs=50073=50077 BRANCH addappimageextractor

D18434: exiv2extractor: add support for bmp, gif, webp, tga

2019-01-27 Thread Friedrich W. H. Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R286:6bc922351db0: exiv2extractor: add support for bmp, gif, webp, tga (authored by kossebau). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18434?vs=50016=50400

<    1   2   3   4   5   6   7   8   9   10   >