D14397: Support libcanberra for audio notification

rjvbb added a comment. > I really don't see what the big fuss is all about. Did you read Harald's remark (about raising a neon fuss) to which I replied?! REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D14397 To: broulik, #frameworks, dfaure,

D14397: Support libcanberra for audio notification

rjvbb added inline comments. INLINE COMMENTS > sitter wrote in CMakeLists.txt:77 > I'd prefer if this was moved up, before finding Phonon, and then find phonon > in the else branch of CANBERRA_FOUND. > > The rationale is that (e.g.) all of neon's tech which auto detects missing > cmake

D14397: Support libcanberra for audio notification

rjvbb added a comment. I have no other objections than the ones above, as long as there's a fallback to NOT using libcanberra which can be enforced without having to patch the code (= via `CMAKE_DISABLE_FIND_PACKAGE_FOO` or a dedicated option). REPOSITORY R289 KNotifications REVISION

D14397: Support libcanberra for audio notification

rjvbb added a comment. > I don't really care about your artificially created problems. You *have* libcanberra but want to use it for one project and not the other. Sorry, but no. Oh, and do you have to show ignorance? This is a very common problem in software packaging, part of what

D14397: Support libcanberra for audio notification

rjvbb added a comment. > (Oh btw it's not like I didn't try, I originally ported all of this to QtMultimedia but `QAudioEffect` which is for low-latency sound effects only supports WAV sounds and `QMediaPlayer` had significant overhead and initialization times as well which is why I

D14397: Support libcanberra for audio notification

rjvbb added a comment. [I didn't get to send this yesterday] > Not using something because it's from a "rival GUI" is not a valid argument. It is IMVHO if "it" introduces a dependency on another GUI middleware. Libcanberra does that to the best of my knowledge. > Canberra

D14397: Support libcanberra for audio notification

rjvbb added a comment. > That's reinventing the wheel to a point. Creating Qt after Gtk or vice-versa was also reinventing the wheel to a much larger extent - so is adding more and more OS features to Qt as has been going on for years. libcanberra comes from "the other" GUI

D14397: Support libcanberra for audio notification

rjvbb added a comment. > However I rather like the idea of improving QTMultiMedia's sound handling features rather than working around the lack of them by using a different library. Thanks Nathan for rewording my argument better than I managed. > Is there a CMake way to say "one of

D14397: Port audio notification to libcanberra

rjvbb added a comment. Anyway, if using QtMultiMedia cannot be considered please make it a build option to use just QApplication::beep() for sound notifications so that packagers can decide whether or not they want to introduce dependencies on libcanberra and everything it needs. On Mac at

D14397: Port audio notification to libcanberra

rjvbb added a comment. That's a nicety, not necessarily a requirement, at least not for playing notification sounds. Just how often do you get sound alerts at such high rates that caching becomes unavoidable, before you get mad and resolve the issue or yank the plug? I think that proper

D14397: Port audio notification to libcanberra

rjvbb added a comment. > According to its website "It comes with several backends (ALSA, PulseAudio, OSS, GStreamer, null)" Which are all either Linux-specific or *nix-desktop oriented with ties to X11 and probably even GTk. Justifying a change with loss of weight while at the

D14397: Port audio notification to libcanberra

rjvbb added a comment. > - gstreamer is technically x-platform, whether one actually wants to attempt building and using it on these platforms is another question I suppose Gstreamer builds and functions on Mac, but AFAIK comes with a whole bunch of dependencies to install and run.

D14397: Port audio notification to libcanberra

rjvbb added a comment. > We probably lose the ability to play sound on Windows and Mac? Does libcanberra (still) require pulseaudio or can it work with different backends? It not, a move to using it exclusively for sound would get a big fat -1 from me. Pulseaudio is problematic on

D14000: qqc2-desktop-style: basic support for QGuiApplication-based apps (WIP/PoC)

rjvbb updated this revision to Diff 37897. rjvbb added a comment. Using QApplication::style() CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14000?vs=37568=37897 REVISION DETAIL https://phabricator.kde.org/D14000 AFFECTED FILES plugin/kquickstyleitem.cpp

D14000: qqc2-desktop-style: basic support for QGuiApplication-based apps (WIP/PoC)

rjvbb set the repository for this revision to R858 Qt Quick Controls 2: Desktop Style. REPOSITORY R858 Qt Quick Controls 2: Desktop Style REVISION DETAIL https://phabricator.kde.org/D14000 To: rjvbb, #frameworks Cc: alexeymin, davidedmundson, mart, broulik, plasma-devel, ragreen, Pitel,

D13899: KMessageWidget: use theme instead of hardcoded colours

rjvbb added a comment. This is what approach 1 looks like with (clockwise) Breeze Dark, Oxygen and Wonton Soup: F6110913: image.png Positive messages don't have an icon here because currently only the Breeze icon set has a "positive" icon. The

D14000: qqc2-desktop-style: basic support for QGuiApplication-based apps (WIP/PoC)

rjvbb updated this revision to Diff 37568. rjvbb added a comment. build on Mac too CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14000?vs=37439=37568 REVISION DETAIL https://phabricator.kde.org/D14000 AFFECTED FILES plugin/kquickstyleitem.cpp plugin/kquickstyleitem_p.h To:

D14000: qqc2-desktop-style: basic support for QGuiApplication-based apps (WIP/PoC)

rjvbb set the repository for this revision to R858 Qt Quick Controls 2: Desktop Style. REPOSITORY R858 Qt Quick Controls 2: Desktop Style REVISION DETAIL https://phabricator.kde.org/D14000 To: rjvbb, #frameworks Cc: mart, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai,

D13899: KMessageWidget: use theme instead of hardcoded colours

rjvbb marked 6 inline comments as done. rjvbb added a comment. Ping? REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13899 To: rjvbb, #frameworks, #vdg Cc: aacid, broulik, kde-frameworks-devel, michaelh, crozbo, firef, ngraham, bruns, skadinna,

D7448: generate and use a local cdda_interface headerfile copy

This revision was automatically updated to reflect the committed changes. Closed by commit R342:0c31ea5a5373: generate and use a local cdda_interface headerfile copy (authored by rjvbb). REPOSITORY R342 KIO AudioCD CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7448?vs=18476=37541

D13933: [Purpose]: use Arc's status colours for the Phab plugin drop-down list (WIP)

This revision was automatically updated to reflect the committed changes. Closed by commit R495:6d518089aa1c: Prepare to use Arcs status colours in the revision drop-down list (authored by rjvbb). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D13933?vs=37292=37523#toc REPOSITORY R495

D14000: qqc2-desktop-style: basic support for QGuiApplication-based apps (WIP/PoC)

rjvbb edited the summary of this revision. rjvbb edited reviewers, added: Frameworks; removed: Framework: Syntax Highlighting. rjvbb set the repository for this revision to R858 Qt Quick Controls 2: Desktop Style. REPOSITORY R858 Qt Quick Controls 2: Desktop Style REVISION DETAIL

D11193: Sonnet : use current hunspell API

This revision was automatically updated to reflect the committed changes. Closed by commit R246:0a96acf251ba: Use the current hunspell API (authored by rjvbb). REPOSITORY R246 Sonnet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11193?vs=29246=37434 REVISION DETAIL

D13933: [Purpose]: use Arc's status colours for the Phab plugin drop-down list (WIP)

rjvbb created this revision. rjvbb added a reviewer: apol. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. rjvbb requested review of this revision. REVISION SUMMARY I'm trying to get the Purpose Phabricator plugin to list the

D13899: KMessageWidget: use theme instead of hardcoded colours

rjvbb set the repository for this revision to R236 KWidgetsAddons. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13899 To: rjvbb, #frameworks, #vdg Cc: aacid, broulik, kde-frameworks-devel, michaelh, crozbo, firef, ngraham, bruns, skadinna, aaronhoneycutt,

D13899: KMessageWidget: use theme instead of hardcoded colours

rjvbb updated this revision to Diff 37194. rjvbb added a comment. constifies KThemeSettings. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13899?vs=37188=37194 REVISION DETAIL https://phabricator.kde.org/D13899 AFFECTED FILES src/CMakeLists.txt src/kmessagewidget.cpp

D13899: KMessageWidget: use theme instead of hardcoded colours

rjvbb set the repository for this revision to R236 KWidgetsAddons. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13899 To: rjvbb, #frameworks, #vdg Cc: broulik, kde-frameworks-devel, michaelh, crozbo, firef, ngraham, bruns, skadinna, aaronhoneycutt, mbohlender

D13899: KMessageWidget: use theme instead of hardcoded colours

rjvbb updated this revision to Diff 37188. rjvbb marked 3 inline comments as done. rjvbb edited the summary of this revision. rjvbb edited the test plan for this revision. rjvbb added a comment. Patch updated as requested. CHANGES SINCE LAST UPDATE

D13899: KMessageWidget: use theme instead of hardcoded colours

rjvbb marked 15 inline comments as done. rjvbb added inline comments. INLINE COMMENTS > broulik wrote in kthemesettings.cpp:30 > This doesn't cascade to system-wide settings I aligned to what KConfig actually does: it only considers the kdeglobals file in the writable generic config location.

D13899: KMessageWidget: use theme instead of hardcoded colours

rjvbb added a comment. A few snapshots obtained with runtime theme changes (see also D13881 ) Breeze: F6020556: image.png Breeze Dark: F6020561: image.png

D13899: KMessageWidget: use theme instead of hardcoded colours

rjvbb created this revision. rjvbb added reviewers: Frameworks, VDG. rjvbb added projects: Frameworks, VDG. Restricted Application added a subscriber: kde-frameworks-devel. rjvbb requested review of this revision. REVISION SUMMARY This is a split-off of my D13777

D13884: [KMessageWidget] Update stylesheet when palette changes

rjvbb accepted this revision. This revision is now accepted and ready to land. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13884 To: broulik, #frameworks, rjvbb, ngraham, cfeck Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13884: [KMessageWidget] Update stylesheet when palette changes

rjvbb added a comment. Otherwise LGTM (I also checked the in-app palette change myself: seems to work fine). INLINE COMMENTS > kmessagewidget.cpp:57 > void createLayout(); > +void applyStyleSheet(); > void updateSnapShot(); Nitpick: shouldn't this be called `setStyleSheet` or

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb added a comment. Seems my reply per email went AWOL: This is a work-in-progress ticket, but I can change the title because it is indeed not just about reverting a regression. > That just doesn't look good, sorry. Again, that's an argument one should avoid. It's too

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb added a comment. Since there's been mention about aligning Kirigami and re: the (IMHO) controversial approach of using foreground (text) colours for background purposes: Have you guys considered using the 4 colours in question only for the message text and outer frame, keeping

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb set the repository for this revision to R236 KWidgetsAddons. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13777 To: rjvbb, ngraham, #frameworks Cc: cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb updated this revision to Diff 37000. rjvbb added a comment. KThemeSettings moved to its own header+implementation files, and extended so different groups can be read. I don't think it needs much else in terms of functionality or complexity, if anything. There's no point in exporting it

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb marked 8 inline comments as done. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13777 To: rjvbb, ngraham, #frameworks Cc: cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb updated this revision to Diff 36943. rjvbb added a comment. Turns out it's trivial to check if and what custom theme was activated through the KColorSchemeManager. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13777?vs=36939=36943 REVISION DETAIL

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb set the repository for this revision to R236 KWidgetsAddons. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13777 To: rjvbb, ngraham, #frameworks Cc: cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb added a comment. A few examples showing the subtle effect of brightness matching, comparing to the Breeze theme (the brightness reference): Oxygen: F5979829: breeze-vs-oxygen-vs-oxygen-matched.png Obsidian Coast F5979830:

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb set the repository for this revision to R236 KWidgetsAddons. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13777 To: rjvbb, ngraham, #frameworks Cc: cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb updated this revision to Diff 36939. rjvbb added a comment. Using the theme's normal foreground colour for message text for more reliable readability with darker themes. Brightness matching to the fallback colour should never occur here, at least not when the palette text colour

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb updated this revision to Diff 36886. rjvbb added a comment. A new revision: - makes a start with the requested export of the QSettings functionality - drops the alpha tweak and introduces a new experiment: Since background colours are not obtained from theme colours designed

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb set the repository for this revision to R236 KWidgetsAddons. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13777 To: rjvbb, ngraham, #frameworks Cc: cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb set the repository for this revision to R236 KWidgetsAddons. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13777 To: rjvbb, ngraham, #frameworks Cc: cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb updated this revision to Diff 36858. rjvbb added a comment. The need for an extra check in `qColorFromSettings()` meant I changed the API to put all tests in there and added a default return value argument. IOW, basically the same API as KConfig has. I've simplified the

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb added inline comments. INLINE COMMENTS > ngraham wrote in kmessagewidget.cpp:311 > Also, commits should be atomic; even if we want to do this, it should be in > another patch since it represents a separate conceptual change compared to > the status quo, as opposed to simply a bugfix or

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb marked 2 inline comments as done. rjvbb added inline comments. INLINE COMMENTS > ngraham wrote in kmessagewidget.cpp:311 > I don't think all of this complicated code is necessary. The theme itself is > supposed to ensure readability with the colors that it uses. Also, this > results in a

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb updated this revision to Diff 36846. rjvbb added a comment. cleanup REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13777?vs=36845=36846 REVISION DETAIL https://phabricator.kde.org/D13777 AFFECTED FILES src/kmessagewidget.cpp To: rjvbb,

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb set the repository for this revision to R236 KWidgetsAddons. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13777 To: rjvbb, ngraham, #frameworks Cc: cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb updated this revision to Diff 36845. rjvbb added a comment. Version using the suggested colour values from ~/.config/kdeglobals , if the file exists and has a Colors:Window group. In my case this gives informational and error messages almost the same background colour because I

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb added a comment. > Another idea would have been to have the widget style alter the appearance like it already does for things like `KTitleWidget` Care to develop? > - Suspend output in a Konsole window that has a dark background color: the text is unreadable. By hitting

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb added a comment. 5.47.0 (stock) vs. 5.42.0 (stock), QtCurve (top) vs. Macintosh native (below): F5967082: KMessageWidget-styles-before,after.png with revision 2 of my patch: F5967086: Screen Shot 2018-06-28 at 16.04.28.png

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb marked 3 inline comments as done. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13777 To: rjvbb, ngraham, #frameworks Cc: cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb set the repository for this revision to R236 KWidgetsAddons. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13777 To: rjvbb, ngraham, #frameworks Cc: cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb updated this revision to Diff 36841. rjvbb added a comment. This revision incorporates the feedback and implements the approach already evoked: - use the user's highlight colour for positive messages, with a 0.4 alpha value - use the user's tooltip background colour for

D13777: KMessageWidget : revert to using highlight colour for Information style (WIP)

rjvbb created this revision. rjvbb added reviewers: ngraham, Frameworks. Restricted Application added a project: Frameworks. rjvbb requested review of this revision. REVISION SUMMARY A recent commit (00bce130d35e9dc398709e690a05f8dde70f52b3

Re: text search balloon background colour

If we're indeed talking about the change to match Kirigami: why on earth do that? If developers want to achieve that style they can use the framework that provides it. If not, let the style in use handle the way things look, AS PER THE USER'S REQUEST! R.

Re: text search balloon background colour

Kai Uwe Broulik wrote: Hi, > it was adjusted to more closely resemble the Breeze visual style. The colors > are unfortunately hardcoded as KWidgetAddons as a Tier 1 framework cannot > depend on KColorScheme. Yeah, that's not good, hardcoding colours is in fact a bug because it can lead to

text search balloon background colour

Hi, Has the background colour used for text search balloons ("Continuing search from top" , for instance) been changed recently? Idem for the background colour in the search widget itself. I may be imagining this, but I'd swear the former has become some sort of light cobalt blue and the

Re: QStandardPaths::GenericDataLocation on MacOS

Hi, I just remembered that some time ago I wrote a small library that allows me to use binaries compiled against my own patched qt5-kde with an "official" Qt build: it provides the additional symbols that are in qt5-kde but not standard Qt through code injection (DYLD_INSERT_LIBRARIES). That

Re: QStandardPaths::GenericDataLocation on MacOS

On Thursday May 03 2018 21:57:03 Thomas Friedrichsmeier wrote: > _If_ applications were to actually install to the current paths defined > for that on Mac "Library/Application Support", and "~Library/Application > Support", on more, no less, _then_ clashes would start to happen. As A priori no,

Re: QStandardPaths::GenericDataLocation on MacOS

>On Fri, May 4, 2018 at 2:38 AM, Thomas Friedrichsmeier >> homebrew, craft). It is also about the fact that this installation >> option causes problems, e.g. if users are to mix MacPorts and a >> single-app-bundled KF5 app No, that shouldn't happen, and it doesn't with the official Kate bundle.

Re: QStandardPaths::GenericDataLocation on MacOS

On Thursday May 03 2018 15:19:33 Thomas Friedrichsmeier wrote: Hi, >plan is actually to get David Faure to champion a solution (whichever >one) into Qt. I can be wrong, but I don't see that happening beyond his general support. Also, what are the chances that this will be overhauled anyway for

Re: QStandardPaths::GenericDataLocation on MacOS

On Thursday May 03 2018 22:22:30 Ben Cooksley wrote: Hi, > Last time around the Qt Core maintainer vetoed our attempts to get > this changed, and forced us to liase with a Qt Company employee whose > responsiveness was lacking (to say the least). Suffice to say, that > effectively killed the

Re: QStandardPaths::GenericDataLocation on MacOS

On Thursday May 03 2018 10:02:35 Thomas Friedrichsmeier wrote: Hi, I have mostly skimmed over your long email but allow me to warn you firsthand that I've tried to get feedback and constructive collaboration like this before and never got very far because the prevailing attitude is 1) don't

D12221: Fix problem that font/italic/... attributes no longer work with e.g. >= Qt 5.9

rjvbb added a comment. Actually, no. I haven't been following that part at all (and am taking 2 weeks off of intensive dev stuff ;) ) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D12221 To: cullmann, #frameworks, #kate, dhaumann Cc: rjvbb, bcooksley, ngraham,

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

rjvbb added a comment. David, Could you please push it for me? I probably won't be able to do so for at least a week and it'd be a pity if the change just misses a release because of that. Thanks. REPOSITORY R135 Integration for Qt applications in Plasma REVISION DETAIL

D11183: Sonnet: don't impose the default client

This revision was automatically updated to reflect the committed changes. Closed by commit R246:e63a234d55d9: Dont impose using the default client (authored by rjvbb). REPOSITORY R246 Sonnet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11183?vs=31594=31608 REVISION DETAIL

D11183: Sonnet: don't impose the default client

rjvbb set the repository for this revision to R246 Sonnet. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11183 To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause Cc: mwolff, kde-frameworks-devel, michaelh, ngraham, bruns

D11183: Sonnet: don't impose the default client

rjvbb updated this revision to Diff 31594. rjvbb added a comment. Unnecessary rebump, pardon, rebase. What's holding this up? CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11183?vs=30657=31594 REVISION DETAIL https://phabricator.kde.org/D11183 AFFECTED FILES

D11891: Fix crashes in NotifyByAudio when closing applications

rjvbb added a comment. This is about better and more concise English. The queued connection is the indirect explanation why the patch is necessary, and thus comes after the direct explanation (the fact that there may be pending signals). Think of it as a courtesy to people who want to get

D11891: Fix crashes in NotifyByAudio when closing applications

rjvbb added a comment. (Oops, missed that one :-/) REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D11891 To: aacid, #frameworks, cullmann, rjvbb Cc: cfeck, rjvbb, mpyne, michaelh, ngraham

D11891: Fix crashes in NotifyByAudio when closing applications

rjvbb requested changes to this revision. rjvbb added a comment. This revision now requires changes to proceed. (sorry, keep forgetting to set the action. Consider this a change request at least for the typo in the comment ;)) REPOSITORY R289 KNotifications REVISION DETAIL

D11891: Fix crashes in NotifyByAudio when closing applications

rjvbb added a comment. So the difference here is that `finishNotification` isn't called if `notification == nullptr`, with the crucial difference probably being the fact that `m` isn't added multiple times to the list of reusable items? Why isn't that logic part of

D11183: Sonnet: don't impose the default client

rjvbb set the repository for this revision to R246 Sonnet. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11183 To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause Cc: mwolff, kde-frameworks-devel, michaelh, ngraham

D11183: Sonnet: don't impose the default client

rjvbb updated this revision to Diff 30657. rjvbb marked an inline comment as done. rjvbb added a comment. next iteration CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11183?vs=30617=30657 REVISION DETAIL https://phabricator.kde.org/D11183 AFFECTED FILES src/core/loader.cpp

D11183: Sonnet: don't impose the default client

rjvbb marked 4 inline comments as done. rjvbb added a comment. > If I suggest something and Milian suggests something else then Milian is right :-) Unconditionally? :) INLINE COMMENTS > dfaure wrote in loader.cpp:114 > can this ever happen now that you set it if it was empty? If it can

D11183: Sonnet: don't impose the default client

rjvbb added a comment. > ah, then this should be simplified by using `std::any_of` instead I was asked to use std::find_if, and (spicey detail) referred to the KDevelop code for examples how to use it :) REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11183 To:

D11183: Sonnet: don't impose the default client

rjvbb marked an inline comment as done. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11183 To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause Cc: kde-frameworks-devel, michaelh, ngraham

D11183: Sonnet: don't impose the default client

rjvbb set the repository for this revision to R246 Sonnet. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11183 To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause Cc: kde-frameworks-devel, michaelh, ngraham

D11183: Sonnet: don't impose the default client

rjvbb updated this revision to Diff 30617. rjvbb added a comment. Modified as requested. (Please double-check, it works AFAICT but it's the first time I use this construct.) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11183?vs=29244=30617 REVISION DETAIL

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

rjvbb added a comment. That's because of the MSVC remarks you made inline? I don't have a way to test with MSVC, so I could do with some suggestions how to resolve the flagged issues. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D5865 To: rjvbb,

D11193: Sonnet : use current hunspell API

rjvbb added a comment. No problem if there's a good reason to wait! Surprising that OpenSuSE has such an old hunspell but apparently a new enough Qt and other dependencies required for building Plasma5 desktops! (My Ubuntu 14.04 also has hunspell 1.3.2 but is stuck on Qt 5.2 so I'm

D11193: Sonnet : use current hunspell API

rjvbb added a comment. > Which version of hunspell does this raise the requirement to? 1.5.1 it seems: https://github.com/hunspell/hunspell/commit/f90e69759203a945128b704a1b7037697a5113dd Is that acceptable (that release is almost 2 years old by now)? REPOSITORY R246 Sonnet

D11193: Sonnet : use current hunspell API

rjvbb added a comment. Any objections if I commit this by the end of the week? REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11193 To: rjvbb, #frameworks, dfaure, mlaurent, vkrause Cc: kde-frameworks-devel, michaelh, ngraham

D11183: Sonnet: don't impose the default client

rjvbb added a comment. Any objections if I commit this by the end of the week? REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11183 To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause Cc: kde-frameworks-devel, michaelh, ngraham

D11183: Sonnet: don't impose the default client

rjvbb added reviewers: dfaure, mlaurent, vkrause. rjvbb edited subscribers, added: kde-frameworks-devel; removed: Frameworks. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11183 To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause Cc: kde-frameworks-devel,

Re: D11193: Sonnet : use current hunspell API

> You should probably add the sonnet maintainer (or whoever committed > the most to the repository in recent months) as the reviewer. Possibly, but they're all supposed to be notified of this via the frameworks- devel ML already.

D11193: Sonnet : use current hunspell API

rjvbb added reviewers: dfaure, mlaurent, vkrause. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11193 To: rjvbb, #frameworks, dfaure, mlaurent, vkrause Cc: kde-frameworks-devel, michaelh, ngraham

Re: D11183: Sonnet: don't impose the default client

This introduces a subtle change in behaviour so I'd appreciate some feedback!

Re: D11193: Sonnet : use current hunspell API

Silence means acceptance?

D11193: Sonnet : use current hunspell API

rjvbb updated this revision to Diff 29246. rjvbb added a comment. add missing context (patch unchanged) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11193?vs=29116=29246 REVISION DETAIL https://phabricator.kde.org/D11193 AFFECTED FILES src/plugins/hunspell/hunspelldict.cpp

D11193: Sonnet : use current hunspell API

rjvbb set the repository for this revision to R246 Sonnet. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11193 To: rjvbb, #frameworks Cc: kde-frameworks-devel, michaelh

D11183: Sonnet: don't impose the default client

rjvbb added a reviewer: cullmann. rjvbb set the repository for this revision to R246 Sonnet. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D11183 To: rjvbb, #frameworks, cullmann Cc: #frameworks, michaelh

D11183: Sonnet: don't impose the default client

rjvbb updated this revision to Diff 29244. rjvbb added a comment. Add missing context (patch unchanged) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11183?vs=29094=29244 REVISION DETAIL https://phabricator.kde.org/D11183 AFFECTED FILES src/core/loader.cpp To: rjvbb,

D11193: Sonnet : use current hunspell API

rjvbb created this revision. rjvbb added a reviewer: Frameworks. Restricted Application added a project: Frameworks. rjvbb requested review of this revision. REVISION SUMMARY The hunspell backend uses three deprecated hunspell API functions; this patch fixes that. TEST PLAN Works as

D11183: Sonnet: don't impose the default client

rjvbb created this revision. rjvbb added a reviewer: Frameworks. Restricted Application added a project: Frameworks. rjvbb requested review of this revision. REVISION SUMMARY Sonnet has a hidden default client concept (hidden because the standard configuration dialog doesn't expose the

Re: Converting KDE4 projects to KF5

On Wednesday February 21 2018 19:23:06 Luigi Toscano wrote: > Manual inspection is better. Not all of them produces code that can be > compiled immediately. Well, that's something probably a lot easier to see after applying them than before ;) All of them also are not as powerfull as you

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