D13510: Add XDG WM Base support to our XDGShell API

davidedmundson updated this revision to Diff 36127. davidedmundson marked 2 inline comments as done. davidedmundson added a comment. ZZag's comments REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13510?vs=36089=36127 BRANCH master REVISION DETAIL

D13510: Add XDG WM Base support to our XDGShell API

davidedmundson marked 10 inline comments as done. davidedmundson added inline comments. INLINE COMMENTS > zzag wrote in test_xdg_shell.cpp:120 > Missing `default`. Is it okay if version is unknown? IMHO it's better not to as it means we get a compile time warning in the unlikely event that

D13211: Enable comparing KFileItems by url

markg added a comment. While this works, there is a newer en better way for it. It's new in C++14 and called "transparent compare". In "this" case it won't change the resulting code of how you compare, but it might be worth checking that out. Read this:

D13442: Implemented displaying of total lines in kate

shubham added a comment. Since the word count option is similar to line count in the config options, can't I just show word count and line count with a single button option? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13442 To: shubham, #ktexteditor,

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.10 - Build # 50 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.10/50/ Project: Frameworks kio kf5-qt5 FreeBSDQt5.10 Date of build: Wed, 13 Jun 2018 17:32:58 + Build duration: 4 min 18 sec and counting JUnit Tests

D13211: Enable comparing KFileItems by url

This revision was automatically updated to reflect the committed changes. Closed by commit R241:fb3c94ed96c3: Enable comparing KFileItems by url (authored by jtamate). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13211?vs=36037=36112 REVISION DETAIL

D13515: Remove KNS::Engine d-pointer hack

This revision was automatically updated to reflect the committed changes. Closed by commit R304:d659619e0c96: Remove KNS::Engine d-pointer hack (authored by apol). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13515?vs=36098=36105 REVISION DETAIL

D13402: FrameSvg: Simplify updateSizes

zzag added a comment. Ping. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13402 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

zzag added a comment. Ping? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13502: Fix compiler warning about hidden virtual overloads

zzag accepted this revision. zzag added a comment. This revision is now accepted and ready to land. In D13502#277837 , @davidedmundson wrote: > ... > But if this is accepted as a good solution for the problem, ... Well, yeah, that's

D13442: Implemented displaying of total lines in kate

shubham added a comment. In D13442#277786 , @ngraham wrote: > I'm not sure how to do that, and I realize it might be a lot of work. > > In D13442#277600 , @shubham wrote: > > > How can the self

D13502: Fix compiler warning about hidden virtual overloads

davidedmundson added a comment. Yes, but it gets complex with the existing XDGShell having a tonne of other changes on phab. But if this is accepted as a good solution for the problem, I'll fixup XDGShell when we merge that. REPOSITORY R127 KWayland REVISION DETAIL

D13515: Remove KNS::Engine d-pointer hack

mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. this is binary compatible from what I can see REPOSITORY R304 KNewStuff BRANCH master REVISION DETAIL https://phabricator.kde.org/D13515 To: apol, #frameworks, leinir, mwolff Cc:

D13502: Fix compiler warning about hidden virtual overloads

zzag added a comment. In D13502#277828 , @davidedmundson wrote: > I meant the class I changed no longer had warnings. Oh, I thought you were talking about all -Woverloaded-virtual warnings. I think it would be better to fix those

D13502: Fix compiler warning about hidden virtual overloads

davidedmundson added a comment. I meant the class I changed no longer had warnings. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D13502 To: davidedmundson, #kwin Cc: zzag, kde-frameworks-devel, michaelh, ngraham, bruns

D13502: Fix compiler warning about hidden virtual overloads

zzag added a comment. > Compiles with no warnings \o/ Nope. In file included from /home/vlad/KDE/src/frameworks/kwayland/src/client/xdgshell_v6.cpp:20: /home/vlad/KDE/src/frameworks/kwayland/src/client/xdgshell_p.h:49:13: warning: ‘virtual

D13515: Remove KNS::Engine d-pointer hack

leinir added a comment. H... this is tricky, but... yes, it seems reasonable to me that replacing one pointer with another would cause no BIC issues... and i certainly am happy to get rid of the d-pointer hack, so... if we're absolutely super-sure that this is BC, i'm all for it...

D13515: Remove KNS::Engine d-pointer hack

apol created this revision. apol added reviewers: Frameworks, leinir. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. apol requested review of this revision. REVISION SUMMARY Put one of the pointers in KNS::Engine in the

D13442: Implemented displaying of total lines in kate

ngraham added a comment. I'm not sure how to do that, and I realize it might be a lot of work. In D13442#277600 , @shubham wrote: > How can the self toggling be done based on the space available? Conceptually, I would have the status

D13442: Implemented displaying of total lines in kate

shubham added a comment. Oh, didnt come to me that of part when translated to other languages may result in longer texts, so this configure option is to be received and context menu is to be added? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13442 To:

D13442: Implemented displaying of total lines in kate

dhaumann added a comment. +1 for the context menu on the line/column fields. By the way, the argument that " of " is short does not hold, since translations into other languages may result in much longer text. REPOSITORY R39 KTextEditor REVISION DETAIL

D13482: Implement support for the Voice and Call interfaces

jgrulich accepted this revision. jgrulich added a comment. This revision is now accepted and ready to land. Looks good to me. Having a test for this would be nice, but looks that not all interfaces are unit tested. REPOSITORY R281 ModemManagerQt BRANCH master-fixes REVISION DETAIL

D13510: Add XDG WM Base support to our XDGShell API

zzag added inline comments. INLINE COMMENTS > zzag wrote in xdgshell_stable.cpp:525 > Shouldn't it be also static? Ignore it. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D13510 To: davidedmundson, #kwin Cc: zzag, kde-frameworks-devel, michaelh, ngraham, bruns

KDE CI: Frameworks modemmanager-qt kf5-qt5 SUSEQt5.10 - Build # 27 - Fixed!

BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20modemmanager-qt%20kf5-qt5%20SUSEQt5.10/27/ Project: Frameworks modemmanager-qt kf5-qt5 SUSEQt5.10 Date of build: Wed, 13 Jun 2018 11:14:24 + Build duration: 9 min 1 sec and counting JUnit

KDE CI: Frameworks modemmanager-qt kf5-qt5 SUSEQt5.9 - Build # 16 - Fixed!

BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20modemmanager-qt%20kf5-qt5%20SUSEQt5.9/16/ Project: Frameworks modemmanager-qt kf5-qt5 SUSEQt5.9 Date of build: Wed, 13 Jun 2018 11:14:25 + Build duration: 5 min 25 sec and counting JUnit

D13510: Add XDG WM Base support to our XDGShell API

zzag added inline comments. INLINE COMMENTS > test_xdg_shell.cpp:120 > +break; > +} > Missing `default`. Is it okay if version is unknown? > xdgshell_stable.cpp:28 > + > +#include > + Seems like QDebug is not used here. > xdgshell_stable.cpp:307 > +Q_UNUSED(surface) > +

D13510: Add XDG WM Base support to our XDGShell API

davidedmundson created this revision. davidedmundson added a reviewer: KWin. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. davidedmundson requested review of this revision. REVISION SUMMARY This adds XDG WM Base (essentially

D13442: Implemented displaying of total lines in kate

firef added a comment. a line counter as an ECMAscript snippet ships with: https://store.kde.org/p/1240493 REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13442 To: shubham, #ktexteditor, cullmann, brauch Cc: firef, mludwig, zhigalin, ngraham, dhaumann,

D11173: Actually vote when clicking stars in the list view

Restricted Application added a subscriber: kde-frameworks-devel. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D11173 To: leinir, #knewstuff, #frameworks, sitter, apol Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns

D13211: Enable comparing KFileItems by url

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D13211 To: jtamate, dfaure Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D13442: Implemented displaying of total lines in kate

brauch added a comment. I was also thinking about the context menu, yes. That is also where a lot of users would first look for it I think, even before the config dialog. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13442 To: shubham, #ktexteditor, cullmann,

D13442: Implemented displaying of total lines in kate

shubham added a comment. The "of " is mere couple of alphabets, does this takes space on the bar and annoys, I dont think so, in my view it should be by default,what u say cullmann? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13442 To: shubham,

D13442: Implemented displaying of total lines in kate

cullmann added a comment. But then the question is: some want it, some not, and it is always visible, what to do? Perhaps the choice to have a config dialog entry is already bad and we should just have a context menu for the statusbar that allows to configure the appearance like other

D13442: Implemented displaying of total lines in kate

brauch added a comment. I don't like this being user-controllable. We spend so much effort in keeping our option set clearly arranged and limited to useful choices, to me this seems like a very bad opportunity to break that rule. REPOSITORY R39 KTextEditor REVISION DETAIL