- User Since
- Apr 15 2016, 12:33 PM (100 w, 2 d)
Wed, Mar 14
Sun, Mar 11
add missing context (patch unchanged)
Add missing context (patch unchanged)
Fri, Mar 9
Wed, Mar 7
Qt Assistant also rolls its own Window (sans s...) menu.
I mean this patch D11075 <https://phabricator.kde.org/D11075> (the Dock menu). setAsDockMenu() is a Mac-only function.
Tue, Mar 6
Would not it end with having #ifdefs scattered all over the QApplication anyway?
Actually, I do not know, whether there is an API that allows to force Mac OS to create such list in the Dock menu, but if an app implements "Window" menu (systemMenu="window" in the menu xib file), then this list is generated automatically.
In addition to the inline comments:
I'm not sure if all Mac applications always show the open documents under their Dock tile (that menu can be used for anything, of course), and I also think that it would be more useful to have a Windows menu like (most) native Mac apps have.
Other than that, LGTM.
Mon, Mar 5
One could try to reimplement this with Objective-C, I do not know whether it makes sense though.
The legacy argc,argv is handled by QCommandLineParser.
Isn't this another case of "should be fixed in QApplication or another lower level library"?
Thu, Mar 1
Sun, Feb 25
Then again, all "not non-gui" applications need an application icon, and adding one is simple enough that it can be done in cmake with a few simple enough operations. And a little bit of extra care in the source code to avoid undoing the effect of ecm_add_app_icon.
My original implementation is used in conjunction with a build system that ensures all dependencies are available.
I can hardly NOT accept this, given I'm the original author :)
Wed, Feb 21
Sat, Feb 17
Feb 11 2018
Curious, I cannot give a green light (too), please consider that done...
Sorry, I didn't quite get what you mean. Your patch just makes realDpiX and realDpiY global, but the rest is the same. Am I missing something?
I agree that your patch is the minimal way to have Okular compiled on Mac.
On a sideways related note:
I have at least one patch that improves Okular/Mac integration, allowing it to act as an alternative or complement to the native Preview.app .
Feb 10 2018
This is hardly acceptable.
Feb 7 2018
Feb 6 2018
Jan 29 2018
Jan 28 2018
In the code shown by this patch ;) Line 80.
Unfortunately, this patch doesn't fix it. The text still isn't bold.
Jan 27 2018
I can confirm that the patch appears to have no (side)effect whatsoever when using the Freetype fontengine (available also on Mac with -cocoa:fontengine=freetype!)
OK, testing. I'll let you know if I notice anything off.
Thanks for the info! So if/hen this goes in, what are the next steps?
A diagonally related anecdote that shows this location is a more central point in the font selection process than I thought first:
Is this change purely a conversation of what developers use in code to call up fonts in their applications?
Jan 26 2018
Jan 25 2018
The examples seem to have a circular runtime dependency on (at least) plasma-desktop. Is that just an opportunistic dependency (= they'd work fine you'd only just installed plasma-framework)?
Jan 24 2018
Jan 23 2018
updated as requested/discussed.
Jan 22 2018
Jan 21 2018
Jan 19 2018
David Faure wrote on 20180119::08:33:32 re: "D9824: Optimize inotify KDirWatch backend: map inotify wd to Entry"
That's not applicable.
Jan 18 2018
Also we know at least one person that isn't going to forget about this, don't we?
I don't see anything unreasonable to my request, which can also be satisfied by a argued demonstration why the same approach/fix doesn't apply to other backends. AFAIAC I'm just doing my reviewers' service like it's been shown to me how that's done.
Jan 16 2018
i'm assuming the cost would be bigger then the benefit. (just an assumption here, i could be wrong)
I am going to assume you would have said so by now if the fix is not applicable to the QFSW backend because it doesn't do the same costly walk.
But I won't write mega patches like you seem to prefer.
@rjvbb why did you request changes to proceed with this patch? The fact that there are more issues in KDirWatch does not mean we should hold up this approach.
Jan 12 2018
Each hash node will be approx 4+8=12 bytes, and something must point to it, so another 8 bytes, 20 in total. No big deal.
Jan 11 2018
Please test this for the QFSW backend too and post a benchmark result comparison (and include the change if beneficial).
@rjvbb this patch just shows that KDirWatch has tons of performance issues that need to be fixed here.