D25339: update lineHeight if boundingRect indicates a larger value.

2020-05-09 Thread René J . V . Bertin
rjvbb added a comment. > I assume the lineHeight usages in the renderer are easy to replace with the proper "height()" of the individual lines of the layout. This is what I think could be tricky. One wouldn't want fluctuating line heights when only a single font is used (at least not

D25339: update lineHeight if boundingRect indicates a larger value.

2020-05-09 Thread René J . V . Bertin
rjvbb added a comment. > Looking at the code, might it make more sense to just move away from the fixed height we have? > It isn't used that often and in most cases one could just query the height of the current line. I'd be all for that, provided it doesn't introduce any

D25339: update lineHeight if boundingRect indicates a larger value.

2020-05-07 Thread René J . V . Bertin
rjvbb added a comment. > the code can be smart No, cleverly written at best (don't get me started! ;) ) > but it can't know how a user prefers to read text, there is no one-size fits all. EXACTLY. Words becomes a lot more readable for me when I use 64pt or larger so I read

D25339: update lineHeight if boundingRect indicates a larger value.

2020-05-07 Thread René J . V . Bertin
rjvbb added a comment. This new version does not cause a lineheight regression for me (after backporting it to 5.60.0). However, contrary to what I thought it does not react to emoji F8293110: emoji.txt REPOSITORY R39 KTextEditor REVISION

D25339: update lineHeight if boundingRect indicates a larger value.

2020-05-07 Thread René J . V . Bertin
rjvbb added a comment. With "we've ever seen" you do mean that lineheight only changes when a line that requires it scrolls into view? > Though line height won't shrink during the edit phase, it will back to the actual value upon save. Have you tried to reset the max. lineheight

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

2020-05-05 Thread René J . V . Bertin
rjvbb added a comment. > It's "KTextEditor", not "KCodeEditor". Yes, but look at the traditional meaning of a text editor, which typically means "plain text" editor. KTextEditor's design decision to use a single lineheight puts it squarely in that domain - to reply in style: `It's

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

2020-05-04 Thread René J . V . Bertin
rjvbb added a comment. This patch is only needed when mixing a main Latin1 (like) alphanumeric font with occasional glyphs from a font that have a different, taller height? Am I right that any text that uses only a single font will see some form of significant loss of the number of

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

2020-05-04 Thread René J . V . Bertin
rjvbb requested changes to this revision. rjvbb added a comment. This revision now requires changes to proceed. I can't speak for the special cases where this change would improve matters, but for me it introduces a clear regression (waste of vertical space: 12 lines less) in a basic ascii

Re: building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

2020-04-26 Thread René J . V . Bertin
On Sunday April 26 2020 20:16:33 René J.V. Bertin wrote: >I was referring to code that just rolls their own implementation of the code >in almost an given library. It's a bit of a pathological argument but I'd say >it's beyond argument that you can always do that, whereas even exhaustive

Re: building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

2020-04-26 Thread René J . V . Bertin
On Sunday April 26 2020 18:19:28 David Faure wrote: >> The possibility to use precompiled headers is only an argument here when >> increased compilation times are brought up as an argument against such a >> header. But that's not really an argument if you can continue to use just >> the headers

Re: building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

2020-04-26 Thread René J . V . Bertin
On Sunday April 26 2020 16:58:10 David Faure wrote: >> As a side-note, I'd even argue that >> it would make sense to provide an all-inclusive header per framework, just >> like Apple's frameworks do. > >It's said to improve compilation times with precompiled headers, but in >practice nobody

Re: building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

2020-04-26 Thread René J . V . Bertin
On Sunday April 26 2020 15:46:35 David Faure wrote: >Well, yeah, you can't have it all. I suppose that the appropriate ECM could provide a function that returns the path to the "official" Qt headers (or an expression that evaluates to that path) unless something like QT_HEADER_PATH is defined.

Re: building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

2020-04-26 Thread René J . V . Bertin
On Sunday April 26 2020 14:12:01 David Faure wrote: >> (and possibly LD_PRELOAD). > >I don't see why you would set LD_PRELOAD in this configuration. This may be necessary if even one of the multiple libraries that get loaded has an old-style link to a Qt library which makes it ignore

building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

2020-04-25 Thread René J . V . Bertin
Hi, A priori Qt guarantees that you can run binaries against a different, newer Qt version than they were built against, as long as no private APIs are used. This also works if that newer Qt version is installed elsewhere, provided you set LD_LIBRARY_PATH correctly (and possibly LD_PRELOAD). I

D27065: Fix inverted logic in IOKitStorage::isRemovable

2020-02-15 Thread René J . V . Bertin
rjvbb accepted this revision. rjvbb added a comment. This revision is now accepted and ready to land. Works fine as far as I can tell, amazing no one else noticed this before! REPOSITORY R245 Solid BRANCH master REVISION DETAIL https://phabricator.kde.org/D27065 To: mwolff,

D27065: Fix inverted logic in IOKitStorage::isRemovable

2020-02-15 Thread René J . V . Bertin
rjvbb added a subscriber: kde-mac. rjvbb added a comment. Well, I got as far as confirming you're probably right, when I finally got to sit at my Mac yesterday night ... and then when I woke up it was 3am. I'm tempted to green-light this but I think someone is going to have to verify

D27065: Fix inverted logic in IOKitStorage::isRemovable

2020-02-13 Thread René J . V . Bertin
rjvbb added a comment. Sorry, no. Swamped with last-minute reconstruction efforts in the, erm, structure that's supposed to become my new house next week :-/ REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D27065 To: mwolff, #frameworks, rjvbb, cgilles Cc:

D27065: Fix inverted logic in IOKitStorage::isRemovable

2020-02-10 Thread René J . V . Bertin
rjvbb added a comment. > This is a MacBook Pro Retina, 13-inch, Mid 2014 with macOS 10.15.2 with only this one disk. It's clearly not supposed to get removed, imo :) Indeed. The icon shown is for internal disks. Thanks for drawing attention to this, I'm going to have to look into

D27065: Fix inverted logic in IOKitStorage::isRemovable

2020-02-10 Thread René J . V . Bertin
rjvbb added a comment. How do you connect? The Mac OS has a built-in VNC server but it has to be activated. Once it is you should be able to connect using any VNC client (possibly using ssh tunnelling?). You can configure the Finder to show every connected volume on the desktop, IIRC

D27065: Fix inverted logic in IOKitStorage::isRemovable

2020-02-10 Thread René J . V . Bertin
rjvbb added a comment. > The output comes from running `solid-hardware5 details` and `solid-hardware nonportableinfo` on the UID. That much I got, but on what hardware and with what kind of drive (the summary basically says you don't have a Mac)? Can you add a screenshot of how the

D27065: Fix inverted logic in IOKitStorage::isRemovable

2020-02-10 Thread René J . V . Bertin
rjvbb added a reviewer: cgilles. rjvbb added a comment. In D27065#608746 , @mwolff wrote: > before: [snip] > Note the `Ejectable = false (bool)` vs. `StorageDrive.removable = true (bool)`. The patch here fixes it to yield

D22801: [KIO] silence a QFileInfo warning

2020-02-02 Thread René J . V . Bertin
rjvbb added a comment. Whatever, I'm staying at 5.60.0 anyway, with whatever patches I deem appropriate. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D22801 To: rjvbb, #frameworks, dfaure Cc: dfaure, meven, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham,

D22801: [KIO] silence a QFileInfo warning

2020-02-02 Thread René J . V . Bertin
rjvbb added a comment. I might (if I can find the trouble location) but IMHO KIO should still account for the possibility of this situation too (or more in general, bail out this function for any path that cannot be a mountpoint). REPOSITORY R241 KIO REVISION DETAIL

D22801: [KIO] silence a QFileInfo warning

2019-11-14 Thread René J . V . Bertin
rjvbb added a comment. > I disagree since it will change the current precondition of the function for the need of one of its users. In that case the warning could be made to be a qDebug, and that user can configure his/her logging setting so the category used will always be printed.

D22801: [KIO] silence a QFileInfo warning

2019-11-13 Thread René J . V . Bertin
rjvbb added a comment. Are you not seeing these for instance when browsing an MSWin share in Dolphin (with the same or newer versions of kio-extras, Samba and MSWin)? I tried to figure out where they came from but failed because of the async nature of the chain of events. I presume the

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

2019-10-04 Thread René J . V . Bertin
rjvbb added a comment. A little tinker tool: github.com/RJVB/kbusygadget. An inter-frame freeze duration of 75ms already decreases CPU load (according to `top`) to approx. 3.6% (= almost 4x). I cannot really say if I notice the effect of this short a freeze on the rotation

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

2019-10-03 Thread René J . V . Bertin
rjvbb added a comment. And my point is that you are doing 720 translations and 360 rotations per cycle, with subsequent smoothing of an image, continuously and with sufficient temporal resolution to get a fluid animation that is completely overkill here. Indicating a busy state (a two-state

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

2019-10-03 Thread René J . V . Bertin
rjvbb added a comment. In D22375#541399 , @sitter wrote: > The demo doesn't even use this widget That wasn't the question I answered by referring to oxygen-demo > breeze > F7506213: Peek 2019-10-03 13-46.gif

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

2019-10-03 Thread René J . V . Bertin
rjvbb added a comment. > Does it happen with every code that uses QPropertyAnimation, or just with this KBusyIndicator? I don't know, neither for QVariantAnimation (which is used here). Testing just now (on the N3150 machine) with the Sliders page of the oxygen-demo app I get

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

2019-10-03 Thread René J . V . Bertin
rjvbb added a comment. I'll repeat here what I muttered on the associated commit page: This widget adds a lot of CPU overhead, too much IMHO: the dedicated test tool runs at a bit over 10%CPU, and that is not counting the additional overhead from the displaying layers (X server, the Mac

D22365: KNotification macOS native support by NSNotificationCenter

2019-09-16 Thread René J . V . Bertin
rjvbb added a comment. > Yeah, you're right that we should check system version for back-compatibility. Actually, I didn't want to suggest that. I haven't checked, but I think that the Qt requirement already ensures that the Cocoa APIs that you are using are present. > AFAIU,

D22365: KNotification macOS native support by NSNotificationCenter

2019-09-16 Thread René J . V . Bertin
rjvbb added a comment. I haven't been able to give this much attention, sorry. After backporting the patch to OS X 10.9 it does work so I presume it'll work even better with the full functionality availability. One thing: it has a hardcoded assumption that the Cocoa notication APIs

Re: detect the Qt version KF5 frameworks were built against in CMake?

2019-08-29 Thread René J . V . Bertin
Friedrich W. H. Kossebau wrote: > Can you specifiy some more what you mean by "pull in"? Adding indirect dependencies of the target framework needed for it to link successfully, as you deduced below. > It also assumes you use compatible build flags for Qt and do not have e.g. Of course, the

detect the Qt version KF5 frameworks were build against in CMake?

2019-08-28 Thread René J . V . Bertin
Hi, Some KF5 framework libraries pull in Qt5 headers and/or libraries via their CMake modules, right? Take the somewhat unusual situation where you have your KF5 frameworks built against, say, Qt 5.9 from the system and you want to test an application against a different, newer Qt build

D22365: KNotification macOS native support by NSNotificationCenter

2019-08-01 Thread René J . V . Bertin
rjvbb added a comment. As I thought this needs some hacking on OS X < 10.10 but a priori all one loses is the user notifications. I'm missing a test case in this diff, i.e. a description how you can test these new notifications because I wouldn't know off the top of my head which

D22801: [KIO] silence a QFileInfo warning

2019-07-29 Thread René J . V . Bertin
rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D22801 To: rjvbb, #frameworks Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns

D22801: [KIO] silence a QFileInfo warning

2019-07-29 Thread René J . V . Bertin
rjvbb updated this revision to Diff 62715. rjvbb edited the summary of this revision. rjvbb added a comment. It's probably not a bad idea too to return early if ever the computed `realname` is empty, and avoid the iteration which should be pointless in that case. Right?! CHANGES SINCE LAST

D22801: [KIO] silence a QFileInfo warning

2019-07-29 Thread René J . V . Bertin
rjvbb created this revision. rjvbb added a reviewer: Frameworks. Herald added a project: Frameworks. rjvbb requested review of this revision. REVISION SUMMARY Prevent qWarning() messages that can arise when browsing samba shares: QFileInfo::absolutePath: Constructed with empty filename

metadata editor (and API) for KDE save file dialog?

2019-07-25 Thread René J . V . Bertin
Hi, Has it ever been considered to add a (pop-up) widget to the save file dialog that would allow to specify metadata key/value pairs when saving a file, plus the corresponding API so apps can set it too? For instance, when a browser downloads a file on Mac it will add a Spotlight attribute

D22365: KNotification macOS native support by NSNotificationCenter

2019-07-18 Thread René J . V . Bertin
rjvbb added a comment. Kai Uwe Broulik wrote on 20190718::07:13:16 re: "D22365 : KNotification macOS native support by NSNotificationCenter" > Thanks a lot! Indeed. I've looked a few times in doing this myself but was never convinced that the

Re: kdewebkit tarball gone missing?!

2019-07-17 Thread René J . V . Bertin
Albert Astals Cid wrote: > https://download.kde.org/stable/frameworks/5.60/portingAids/ Ah, thanks, I must have missed the announcement of its promotion. So what kind of tier is kdesigner-plugin now - I still have it categorised as a tier3 but those are supposed to depend only on tier1, tier2

D19986: Install .desktop file for kded5

2019-07-16 Thread René J . V . Bertin
rjvbb added a comment. It seems to me there's a way to tell the translators that a string shouldn't be translated. Why not use that, or put a name that doesn't require translation (like `name=kded5.desktop`)? NB: I replaced "SystemService" with "Service" in my copy, and haven't seen

D19986: Install .desktop file for kded5

2019-07-16 Thread René J . V . Bertin
rjvbb added inline comments. INLINE COMMENTS > dfaure wrote in org.kde.kded5.desktop:4 > What does SystemService mean? > > I'm seeing a warning from the kservice framework: > The desktop entry file "/share/applications/org.kde.kded5.desktop" > has Type= "SystemService" instead of

kdewebkit tarball gone missing?!

2019-07-16 Thread René J . V . Bertin
Hi, A few days ago I downloaded the 5.60.0 kdewebkit tarball from its usual location on downloads.kde.org (= among the other FW tarballs), today it appears to be gone? Has it moved or is this an error? Thanks, R.

D22460: DrKonqi: improved lldb integration

2019-07-14 Thread René J . V . Bertin
rjvbb created this revision. rjvbb added reviewers: kde-frameworks-devel, Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. rjvbb requested review of this revision. REVISION SUMMARY Since a few versions lldb has had a tendency to remain stuck after the initial

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

2019-07-13 Thread René J . V . Bertin
rjvbb added a comment. I could try your solution, of course, but what annoys me is that it comes months after I worked on mine. I currently have too many other things going on in my life to dive in and figure out what on earth was going on again. I do think I outlined it well enough above

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

2019-07-12 Thread René J . V . Bertin
rjvbb added a comment. Please check the earlier discussion; IIRC there is a reliability problem with that signal, and I did try reverting to its use before coming up with the current solution. REPOSITORY R32 KDevelop REVISION DETAIL https://phabricator.kde.org/D16882 To: rjvbb,

D18380: KIO: make file dialog columns resizable again (and movable)

2019-05-09 Thread René J . V . Bertin
rjvbb added a comment. > I didn't read the full encyclopedia of discussions here, I only looked at the patch. It's really a pity that you only did that now, because by now I'll have to reverse-engineer my patch (and read the encyclopedia) to remember how I arrived at the patch.

D20935: sonnet: provide override to force of a specific plugin

2019-05-01 Thread René J . V . Bertin
rjvbb created this revision. rjvbb added a reviewer: Frameworks. Herald added a project: Frameworks. rjvbb requested review of this revision. REVISION SUMMARY This provides an env.var-based mechanism to force the use of a specific spell checking plugin. A warning is printed when a plugin

D18380: KIO: make file dialog columns resizable again (and movable)

2019-03-14 Thread René J . V . Bertin
rjvbb added a comment. > ...aand that's exactly what happened. :( I'm not exhausted, my patch just works good enough (thanks also to feedback on here) to make me more interested in living the rest of my life rather than continue to explore alternatives. Maybe if an alternative

generating emoticon themes from emoji fonts

2019-03-03 Thread René J . V . Bertin
Hi, Is there a tool that can be used to generate themes for KEmoticons from TTF fonts like Apple Colour Emoji or even "mixed" fonts like Segoe UI Symbol, preferably with a selectable resolution/size for the generated images? Thanks, R.

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

2019-03-02 Thread René J . V . Bertin
rjvbb added a comment. On, with the partially reverted patch: CoreText font engine: F6650185: Screen Shot 2019-03-02 at 22.51.09.png FontConfig font engine (= identical to the one on Linux; FreeType & FontConfig have the Infinality patches):

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

2019-03-02 Thread René J . V . Bertin
rjvbb added a comment. > But now the state is just like it was before: If you have a font that does fallbacks for some glyphs It's the fontengine that handles this of course ;) Note that on Unix the fallback mechanism is actually provided by/through fontconfig, so there might be

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

2019-03-02 Thread René J . V . Bertin
rjvbb added a comment. > I would rather keep the current behavior and let people use sane fonts if the want unicode. That's not current acceptable IMHO, not with the cumbersome way of selecting the font. If proper document display depends on "user picking a sane font", they

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

2019-03-01 Thread René J . V . Bertin
rjvbb added a comment. > Just load the XML file from bug 404713. > Before this changes here, you did overpaint the next line randomly with the "oversized one", now you "cut" the oversized line. I don't know where to look specifically in that huge file. I do notice that with the

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

2019-03-01 Thread René J . V . Bertin
rjvbb added a comment. So it seems that the partial revert works; you lost me re: what would break again by doing that? F6646645: Screen Shot 2019-03-01 at 13.07.52.png (background: full revert; foreground partial revert (only

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

2019-03-01 Thread René J . V . Bertin
rjvbb added a comment. > We just can't use one fixed height for such texts. You can, but it'd have to be the maximum of all lineheights (of the entire document or of the visible section). REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19283 To: cullmann,

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

2019-02-28 Thread René J . V . Bertin
rjvbb added a comment. > That is a good observation: @cullmann Could you give this partial revert a try? I'll try to do that too, tomorrow. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19283 To: cullmann, dhaumann Cc: rjvbb, loh.tar, thomassc,

D19416: Restore the search wrapped message to its former type and position.

2019-02-28 Thread René J . V . Bertin
rjvbb added a comment. > @loh.tar The line edit in the search bar was once used before the floating message widgets in the view even existed. I guess it's legacy and possibly can be removed? You don't mean the widget through which you enter the pattern to search on I hope?

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

2019-02-28 Thread René J . V . Bertin
rjvbb added a comment. > Next Frameworks Tag is on Saturday, 2nd, i.e. in 2 days. Revert, or give it a try? TBH I didn't notice any issues with the before code and am pretty certain I couldn't do better. FWIW I noticed that Scribus (1.5.5) filters out pure emoji fonts, from what I

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

2019-02-28 Thread René J . V . Bertin
rjvbb added a comment. Causes https://bugs.kde.org/show_bug.cgi?id=404907 REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19283 To: cullmann, dhaumann Cc: rjvbb, loh.tar, thomassc, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, demsking,

D19416: Restore the search wrapped message to its former type and position.

2019-02-28 Thread René J . V . Bertin
rjvbb created this revision. rjvbb added a reviewer: KTextEditor. rjvbb added a project: KTextEditor. Herald added projects: Kate, Frameworks. Herald added a subscriber: kwrite-devel. rjvbb requested review of this revision. REVISION SUMMARY This fixes issue 398731 by restoring the "search

D18380: KIO: make file dialog columns resizable again (and movable)

2019-02-12 Thread René J . V . Bertin
rjvbb added a comment. > What's the minimum viable change here? I think my the implementation of my solution is pretty minimal by now, no? :) @David Faure: any ideas/suggestions, seems right up your alley? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380

D18380: KIO: make file dialog columns resizable again (and movable)

2019-02-11 Thread René J . V . Bertin
rjvbb added a comment. So this is just going to become a victim of the better-is-the-enemy-of-good principle? I'm not saying there can be better ways to do this but AFAIC this is already pretty good, certainly good enough and even too good in practice to keep me from more interesting

D16894: [ECM] use a macro to add compiler flags conditionally

2019-02-03 Thread René J . V . Bertin
rjvbb updated this revision to Diff 50812. rjvbb added a comment. Now tested more exhaustively and with unittest. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16894?vs=50589=50812 REVISION DETAIL https://phabricator.kde.org/D16894 AFFECTED FILES

D16894: [ECM] use a macro to add compiler flags conditionally

2019-02-03 Thread René J . V . Bertin
rjvbb set the repository for this revision to R240 Extra CMake Modules. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D16894 To: rjvbb, #build_system, kfunk Cc: cgiboudeaux, dfaure, kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system,

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-31 Thread René J . V . Bertin
rjvbb added a comment. > So according to you, this line is useful ? from my point of view, it's needless and just looks like a syntax error. Yes, it shows which compiler is being queried and for what. The line is going to be there anyway because CMake's Check*CompilerFlag macros don't

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-31 Thread René J . V . Bertin
rjvbb added a comment. > Forget that. The syntax is confusing, please remove this HASFLAG That I'm not going to do. The goal is to both to have useful feedback like below, and to avoid caching issues that would cause on the first query to be performed (if you were to use a single result

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-31 Thread René J . V . Bertin
rjvbb added a comment. > There are tests for other ECM modules in the **tests** subdir. That's not the expected answer, so let me rephrase: which existing test can I clone and adapt (which is about the only thing I know how to do in this domain)? I'm going to need a hand here if

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-31 Thread René J . V . Bertin
rjvbb set the repository for this revision to R240 Extra CMake Modules. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D16894 To: rjvbb, #build_system, kfunk Cc: cgiboudeaux, dfaure, kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system,

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-31 Thread René J . V . Bertin
rjvbb updated this revision to Diff 50589. rjvbb marked an inline comment as done. rjvbb added a comment. Updated as requested. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16894?vs=50451=50589 REVISION DETAIL https://phabricator.kde.org/D16894 AFFECTED FILES

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-31 Thread René J . V . Bertin
rjvbb marked 9 inline comments as done. rjvbb added inline comments. INLINE COMMENTS > cgiboudeaux wrote in ECMAddCompilerFlag.cmake:1-25 > Shall be moved after the module documentation I'm guessing you meant "Please move [...]" and not that this would somehow happen automagically(?)

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-30 Thread René J . V . Bertin
rjvbb added a comment. > Right, but I was saying all this because I think IF_SUPPORTED (the keyword in the arguments) should be SUPPORTED_IF. Doh?! I thought that was the case, maybe I just restored the wrong way after the CONDITION experiment. Christophe: can you please be more

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-29 Thread René J . V . Bertin
rjvbb added a comment. > In that sentence, one can read "if supported" for the macro name, ... That was my idea too, and the reason the macro ends in "_if_supported". REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D16894 To: rjvbb, #build_system,

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-28 Thread René J . V . Bertin
rjvbb updated this revision to Diff 50451. rjvbb added a comment. This follows David's suggestion, but using `QUERY_IF` instead of the suggested `TRY_IF` to make it clear that this parameter controls the querying of the compiler. I haven't yet tested the new logic exhaustively but the as

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-28 Thread René J . V . Bertin
rjvbb set the repository for this revision to R240 Extra CMake Modules. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D16894 To: rjvbb, #build_system, kfunk Cc: dfaure, kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system, michaelh, ngraham,

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-28 Thread René J . V . Bertin
rjvbb added a comment. Usually if you have a conditional behaviour the associated condition specifies when to trigger it, no? You're right that the names don't suggest exactly how the condition is being evaluated (with extra checks or not), but that was also a bit the idea. Don't bother

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-27 Thread René J . V . Bertin
rjvbb set the repository for this revision to R240 Extra CMake Modules. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D16894 To: rjvbb, #build_system, kfunk Cc: dfaure, kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system, michaelh, ngraham,

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-27 Thread René J . V . Bertin
rjvbb updated this revision to Diff 50399. rjvbb added a comment. Renamed macro and parameter names as announced in my last comment. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16894?vs=45786=50399 REVISION DETAIL https://phabricator.kde.org/D16894 AFFECTED FILES

D16894: [ECM] use a macro to add compiler flags conditionally

2019-01-27 Thread René J . V . Bertin
rjvbb added a comment. In D16894#400949 , @dfaure wrote: > This makes sense to me. Just the name "SUPPORTED_IF" is strange, when reading that, one thinks "well, if we know the compiler flag is supported, why are we testing that it is?".

D18547: Don't enable -Wzero-as-null-pointer-constant on apple clang

2019-01-27 Thread René J . V . Bertin
rjvbb added a comment. > like René says, this is quite surprising Hmmm, did I say exactly that? :) The surpising bit is that this hasn't been an issue for much longer although maybe even that is not so surprising. I continue to think that the check is not reliable as is. For

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-27 Thread René J . V . Bertin
rjvbb added a comment. > But still, isn't there another way? Now the header and view are locked together. One doesn't work without the other. What's the problem with that? The custom header class isn't public. I did indeed use it for stuff that were not part of a class, or of the

D18547: Don't enable -Wzero-as-null-pointer-constant on apple clang

2019-01-26 Thread René J . V . Bertin
rjvbb added a comment. This is in fact cmake's fault, or ECM's for not taking a cmake quirk into account. If you add an unknown option then all subsequent tests to see if the compiler supports a given argument will fail because cmake does not take into account which of the options

D18547: Don't enable -Wzero-as-null-pointer-constant on apple clang

2019-01-26 Thread René J . V . Bertin
rjvbb requested changes to this revision. rjvbb added a comment. This revision now requires changes to proceed. See also https://phabricator.kde.org/D16894 which (initially) aimed to tackle this in a more general fashion. IMHO the way forward (if my proposal is not acceptable) is to

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-24 Thread René J . V . Bertin
rjvbb updated this revision to Diff 50205. rjvbb added a comment. Use qobject_cast. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18380?vs=50084=50205 REVISION DETAIL https://phabricator.kde.org/D18380 AFFECTED FILES src/filewidgets/kdiroperatordetailview.cpp To: rjvbb,

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-24 Thread René J . V . Bertin
rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham, #frameworks, #dolphin, apol, dfaure, ahartmetz Cc: cfeck, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-24 Thread René J . V . Bertin
rjvbb added a comment. > As far as I know, using qobject_cast is faster than comparing class names, because it only compares metaclass pointers. Additionally, it allows subclasses. Purely academic: that would be true for an ObjC-like construct like `isKindOfClass` but doesn't it

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-24 Thread René J . V . Bertin
rjvbb marked an inline comment as done. rjvbb added inline comments. INLINE COMMENTS > cfeck wrote in kdiroperatordetailview.cpp:54 > Can we use qobject_cast here? Doh, of course. It's probably more expensive (which shouldn't matter here) but also means we have to include `kfilewidget.h`. Is

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-24 Thread René J . V . Bertin
rjvbb added a comment. David, Andreas, any idea why the name column all of a sudden jumps to a larger width when the widget is used in a side-bar and you're making the view narrower and approach the minimum width? It works in our favour here because the end result is that the name column

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-24 Thread René J . V . Bertin
rjvbb added reviewers: dfaure, ahartmetz. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham, #frameworks, #dolphin, apol, dfaure, ahartmetz Cc: kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-23 Thread René J . V . Bertin
rjvbb added a reviewer: Frameworks. rjvbb added a subscriber: kwrite-devel. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham, #frameworks Cc: kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-23 Thread René J . V . Bertin
rjvbb added a comment. > The behavior is better now, thanks. It's back to what you liked before I started tinkering with font squeezing (plus a few fixes to the behaviour in side-bars). Do you know of other applications that use this widget/mode for/in a filebrowser side-bar thingy

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-22 Thread René J . V . Bertin
rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-22 Thread René J . V . Bertin
rjvbb updated this revision to Diff 50084. rjvbb added a comment. the change to the headerfile was now redundant. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18380?vs=50083=50084 REVISION DETAIL https://phabricator.kde.org/D18380 AFFECTED FILES

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-22 Thread René J . V . Bertin
rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-22 Thread René J . V . Bertin
rjvbb updated this revision to Diff 50083. rjvbb added a comment. Well, that was "interesting". It turns out that Qt has what looks like another path through which section sizes are calculated and through which they're shown, which apparently isn't used in file dialogs but which

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-22 Thread René J . V . Bertin
rjvbb added a comment. > I'm still going to try to fix this Good thing I did (am doing), because what you call the jarring exists also without font stretching. It's something in Qt that somehow doesn't occur in file dialogs but only with applications of the widget like in Kate. It is

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-22 Thread René J . V . Bertin
rjvbb added a comment. Yikes, and I can reproduce that. Did you notice this with previous versions that used font stretch? (Probably not if stretch had no effect for the font(s) you tried it with...) I'm still going to try to fix this; even if in the end it doesn't go in there's be a

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-21 Thread René J . V . Bertin
rjvbb updated this revision to Diff 50032. rjvbb added a comment. This version uses letterspacing rather than font stretch, so works regardless of whether a stylename has been set on the font. Letterspacing reduction has a stronger impact on readability than stretch (if you cannot know

D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-21 Thread René J . V . Bertin
rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

  1   2   3   4   5   6   7   8   9   10   >