Re: KInit - Current state and benchmarks

On Montag, 25. November 2019 22:57:11 CET Albert Astals Cid wrote: > El dissabte, 23 de novembre de 2019, a les 11:47:40 CET, Milian Wolff va escriure: > > On Mittwoch, 19. Juni 2019 19:57:56 CET Albert Astals Cid wrote: > > > El dimarts, 18 de juny de 2019, a les 12:04:38 CES

Re: KInit - Current state and benchmarks

ne compared to more modern machines? May I ask how old this machine is and what the speed of the HDD is? Thanks -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part.

D12694: Properly fix the native messaging host file

mwolff added a comment. @bshah ping? will you push this? REPOSITORY R856 Plasma Browser Integration BRANCH master REVISION DETAIL https://phabricator.kde.org/D12694 To: bshah, mwolff, broulik, davidedmundson Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed,

D12694: Properly fix the native messaging host file

mwolff added a comment. It compiles, how do I test it? Can you please also add a `README` to the root of this project that explains what this is, does and how to use it? thanks REPOSITORY R856 Plasma Browser Integration BRANCH master REVISION DETAIL

D10195: DrKonqi: Enable DBus-Interface for debugging with KDevelop

mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. minor style stuff, otherwise lgtm, esp. if it works now INLINE COMMENTS > README:5 > > -[Drkonqi] > +[DrKonqi] > ShowDebugButton=true could be a separate commit >

D10697: Make the badge text readable on dark color schemes

This revision was automatically updated to reflect the committed changes. Closed by commit R119:00f113eacb03: Make the badge text readable on dark color schemes (authored by mwolff). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10697?vs=27637=27725

D10697: Make the badge text readable on dark color schemes

mwolff added a comment. I did not test it with a light color scheme yet, will do that eventually and commit it then to the branch you indicated REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D10697 To: mwolff, broulik, hein Cc: ngraham,

D10697: Make the badge text readable on dark color schemes

mwolff added a comment. using breeze high contrast here, before: F5720670: Screenshot_20180220_211956.png after: F5720669: Screenshot_20180220_212030.png REPOSITORY R119 Plasma Desktop REVISION

D10697: Make the badge text readable on dark color schemes

mwolff created this revision. mwolff added reviewers: broulik, hein. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. mwolff requested review of this revision. REVISION SUMMARY The background color should not be used for the foreground

D10670: Reduce plasmashell frozen time

mwolff added a comment. ok, looking at the reasoning in the other commit: - you need to extend the commit message here - you need to provide a comment in the code the clarifies what's going on here in general, I don't see how such a comparison can be so costly - the real problem

D10670: Reduce plasmashell frozen time

mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. what? this is totally unsafe - the reinterpret_cast below shouldn't be done on anything that has event names other than the previously used one... REPOSITORY R242 Plasma

D10342: From 1.03% to 0.08% cpu usage moving 50.000 files

mwolff added inline comments. INLINE COMMENTS > xwindowtasksmodel.cpp:516 > + > +QUrl url= windowUrlFromMetadata(info->windowClassClass(), > NETWinInfo(QX11Info::connection(), window, > QX11Info::appRootWindow(), NET::WMPid, 0).pid(), `const auto url = ...`; note the space after

D10024: Add supportedSchemes feature

This revision was automatically updated to reflect the committed changes. Closed by commit R241:6ab218dba91f: Add supportedSchemes feature (authored by mwolff). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D10024?vs=25891=26632#toc REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D10024: Add supportedSchemes feature

mwolff added a comment. I've committed this now. If someone tells me how to add KIOSK support, I can add that later on as needed REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10024 To: mwolff, #plasma, dfaure, mart Cc: ngraham, mart, apol,

D10025: Use KFileWidget::setSupportedSchemes when available

This revision was automatically updated to reflect the committed changes. Closed by commit R135:a7d62a329994: Use KFileWidget::setSupportedSchemes when available (authored by mwolff). REPOSITORY R135 Integration for Qt applications in Plasma CHANGES SINCE LAST UPDATE

D10024: Add supportedSchemes feature

mwolff added a comment. @apol any hint what to do about KIOSK? any resources I should have a look at? REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D10024 To: mwolff, #plasma, dfaure, mart Cc: ngraham, mart, apol, plasma-devel, #frameworks, michaelh,

D9325: Use QUrl in the ScreenMapper API

mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. two questions, otherwise lgtm INLINE COMMENTS > foldermodel.cpp:291 > > -const auto oldUrl = m_url; > +const auto oldUrl = this->resolvedUrl(); > could you call this at the

D10024: Add supportedSchemes feature

mwolff added a comment. In https://phabricator.kde.org/D10024#194708, @mart wrote: > ship it with the extra check Aleix noted Can you guys explain me more what would be required for KIOSK support? REPOSITORY R241 KIO BRANCH master REVISION DETAIL

D10024: Add supportedSchemes feature

mwolff updated this revision to Diff 25891. mwolff added a comment. extend apidox, mention empty list == everything allowed REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10024?vs=25764=25891 BRANCH master REVISION DETAIL

D10024: Add supportedSchemes feature

mwolff added a comment. In https://phabricator.kde.org/D10024#194431, @apol wrote: > Makes sense I guess. > > Sounds like it would be useful to set this through kiosk, would that work? Probably, but I have zero experience about kiosk. I'd have to read the defaults from a

D10024: Add supportedSchemes feature

mwolff added a dependent revision: D10025: Use KFileWidget::setSupportedSchemes when available. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10024 To: mwolff, #plasma, dfaure Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg,

D10025: Use KFileWidget::setSupportedSchemes when available

mwolff created this revision. mwolff added a reviewer: dfaure. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. mwolff requested review of this revision. REVISION SUMMARY This makes QFileDialog::setSupportedSchemes work for the KDE file

D10024: Add supportedSchemes feature

mwolff created this revision. mwolff added reviewers: Plasma, dfaure. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. mwolff requested review of this revision. REVISION SUMMARY This addds a whitelist of supported URL

D9812: [Icon Item] Treat sources starting with a slash as local file

mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. lgtm REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D9812 To: broulik, #plasma, hein, mwolff Cc: mwolff, plasma-devel, #frameworks, ZrenBot,

D9812: [Icon Item] Treat sources starting with a slash as local file

mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. lgtm in general, but can be cleaned up INLINE COMMENTS > iconitem.cpp:154 > +// If a file:// URL or a absolute path is passed, take the image > pointed by that from disk

D9325: Use QUrl in the ScreenMapper API

mwolff added a comment. I'd personally prefer to use an API with less conversions if possible. The fact that the test code becomes (marginally) more complex doesn't really count in my opinion. INLINE COMMENTS > foldermodel.cpp:291 > > -const auto oldUrl = m_url; > +const auto

D9313: Don't move back items to second screen if they were manually moved

mwolff added inline comments. INLINE COMMENTS > screenmapper.cpp:177 > +{ > +for (auto it = m_itemsOnDisabledScreensMap.begin(); > + it != m_itemsOnDisabledScreensMap.end(); ++it) { doesn't this work too: for (auto : m_itemsOnDisabledScreensMap) { names.removeAll(name);

D9325: Use QUrl in the ScreenMapper API

mwolff requested changes to this revision. mwolff added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > foldermodel.cpp:1063 > +auto mappableUrl = [this, dropTargetFolderUrl](const QUrl ) -> QUrl { > QString mappedUrl = url.toString(); > if

D9323: Use ScreenMapper everywhere as a singleton

mwolff accepted this revision. This revision is now accepted and ready to land. BRANCH master REVISION DETAIL https://phabricator.kde.org/D9323 To: amantia, #plasma, mwolff, hein Cc: mlaurent, broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts,

D8444: Define default Action when we plug unknown monitor

mwolff added a comment. In https://phabricator.kde.org/D8444#173635, @davidedmundson wrote: > I left a comment a month ago. It has not been addressed. > > > there's one part that's going to appear very confusing. > > > > > I have my (small) laptop. Lets say I want any new

D8850: Support drag and drop between shared folder view containments

mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. thanks for the clarification BRANCH master REVISION DETAIL https://phabricator.kde.org/D8850 To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff,

D8598: FolderView: position files at drop event target position

mwolff updated this revision to Diff 22887. mwolff edited the summary of this revision. mwolff added a comment. remove TODOs REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8598?vs=22715=22887 BRANCH wip/lim-2 REVISION DETAIL

D8850: Support drag and drop between shared folder view containments

mwolff requested changes to this revision. mwolff added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > foldermodel.cpp:1071 > +m_dropTargetPositions.insert(url.fileName(), dropPos); > +m_screenMapper->addMapping(mappableUrl(url),

D8598: FolderView: position files at drop event target position

mwolff added inline comments. INLINE COMMENTS > foldermodel.cpp:155 > + * > + * TODO: push this somehow into the Positioner > + */ here as well then REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8598 To: mwolff, hein, amantia Cc: plasma-devel,

D8598: FolderView: position files at drop event target position

mwolff updated this revision to Diff 22715. mwolff added a comment. remove unused param REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8598?vs=22458=22715 BRANCH wip/lim-2 REVISION DETAIL https://phabricator.kde.org/D8598 AFFECTED FILES

D8850: Support drag and drop between shared folder view containments

mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. lgtm in general, but a unit test for the positioner would be good to have INLINE COMMENTS > foldermodel.cpp:1053 > +if (isDropBetweenSharedViews(mimeData->urls(),

D8850: Support drag and drop between shared folder view containments

mwolff requested changes to this revision. mwolff added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > foldermodel.cpp:1126 > +// the targetUrl file:// path to a desktop:/ path for mapping > +auto destPath = dropTargetUrl.path();

D8915: Update the proxy to source and source to proxy mapping correctly

mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. lgtm, but @hein should have a look too BRANCH master REVISION DETAIL https://phabricator.kde.org/D8915 To: amantia, #plasma, hein, mwolff Cc: plasma-devel, ZrenBot, progwolff,

D8850: WIP: support drag and drop between shared folder view containments

mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. I guess we should split this patch up into three parts: a) add unit test + the fix you found b) (if still required) enforce uniqueness in `Positoiner::updateMaps` + unit

D8850: WIP: support drag and drop between shared folder view containments

mwolff planned changes to this revision. mwolff added a comment. andras, please comandeer this change. and I feel your pain re `arc` usability... phabricator with patch series is a nightmare for developers INLINE COMMENTS > FolderViewLayer.qml:192 > + > +onScreenMappingChanged: { >

D8850: WIP: support drag and drop between shared folder view containments

mwolff created this revision. mwolff added a reviewer: amantia. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY This is still super buggy. From what I've understood so far, the problem seems to be the positioner acting

D8598: FolderView: position files at drop event target position

mwolff updated this revision to Diff 22458. mwolff edited the summary of this revision. mwolff added a comment. update message REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8598?vs=22457=22458 REVISION DETAIL https://phabricator.kde.org/D8598

D8598: FolderView: position files at drop event target position

mwolff updated this revision to Diff 22457. mwolff added a comment. use filename in mapping to support desktop:/ REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8598?vs=21775=22457 REVISION DETAIL https://phabricator.kde.org/D8598 AFFECTED FILES

D8493: Make Folder View screen aware

mwolff added a comment. [14:24] andris: milian umm, how do I actually move files between screens? [14:26] the fact that you can actually resize it with Alt+right-click is a bug [14:27] kbroulik: dnd, which is WIP [14:28] milian: okay because right now the screen-aware FV

D8493: Make Folder View screen aware

mwolff accepted this revision. mwolff added a comment. lgtm from my side. Anyone from the plasma team care to chime in? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8493 To: amantia, #plasma, ervin, mlaurent, dvratil, hein, aacid, davidedmundson, apol,

D8493: Make Folder View screen aware

mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. some more nits, sorry for that ;-) INLINE COMMENTS > screenmapper.cpp:69 > +auto adjustFirstScreen = [this, ](const QString ) { > +int firstScreen =

D8493: Make Folder View screen aware

mwolff added inline comments. INLINE COMMENTS > screenmapper.cpp:82 > +// the screen got completely removed, not only its path changed > +pathIt = m_screensPerPath.begin(); > +while (pathIt != m_screensPerPath.end()) { personally I'd make this code explicit. i.e. this

D8493: Make Folder View screen aware

mwolff added inline comments. INLINE COMMENTS > foldermodeltest.cpp:48 > + > +void FolderModelTest::initTestCase() > +{ remove the init and cleanup if both are empty? > screenmappertest.cpp:36 > + > +void ScreenMapperTest::cleanupTestCase() > +{ remove if empty > screenmappertest.cpp:45 > +

D8597: Cleanup: use temp variables instead of querying properties repeatedly

This revision was automatically updated to reflect the committed changes. Closed by commit R119:b985b7432ef9: Cleanup: use temp variables instead of querying properties repeatedly (authored by mwolff). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D8638: Remove DataModel::roleNameToId

mwolff accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D8638 To: broulik, #plasma, mwolff Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed,

D8639: Optimize SortFilterModel role names

mwolff accepted this revision. mwolff added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > datamodel.h:141 > protected: > -int roleNameToId(const QString ); //FIXME TODO KF6: This should > have been const. > +int roleNameToId(const QString ) const;

D8493: Make Folder View screen aware

mwolff requested changes to this revision. mwolff added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > foldermodel.cpp:535 > +m_screenMapper->addScreen(screen); > +invalidateFilter(); > +} this should not be required, since `addScreen`

D8598: FolderView: position files at drop event target position

mwolff retitled this revision from "WIP: position files at drop target position in folder" to "FolderView: position files at drop event target position". mwolff edited the summary of this revision. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8598 To: mwolff,

D8598: WIP: position files at drop target position in folder

mwolff updated this revision to Diff 21775. mwolff added a comment. use copyjob signals to map the target urls, not the source urls, which resolves a lot of the brittleness of my previous approach. overall, this works quite nicely, imo and is already in an acceptable state (to me).

D8596: Fix memory leak in FolderModel::drop

mwolff abandoned this revision. mwolff added a comment. oh of course, the dropJob is the sender, stupid me. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8596 To: mwolff, #plasma Cc: hein, ngraham, broulik, anthonyfieroni, plasma-devel, ZrenBot, progwolff,

D8598: WIP: position files at drop target position in folder

mwolff added a comment. So far, mostly food for thought. It kinda works but I see lots of problems with this approach... I'll investigate other approaches tomorrow REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8598 To: mwolff, hein, amantia Cc:

D8598: WIP: position files at drop target position in folder

mwolff added a reviewer: amantia. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8598 To: mwolff, hein, amantia Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D8598: WIP: position files at drop target position in folder

mwolff added a reviewer: hein. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8598 To: mwolff, hein Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D8598: WIP: position files at drop target position in folder

mwolff created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY Remember the drop target position and move the items once they become available. This requires us to allow moving while no drag image is

D8597: Cleanup: use temp variables instead of querying properties repeatedly

mwolff created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REPOSITORY R119 Plasma Desktop BRANCH wip/lim-2 REVISION DETAIL https://phabricator.kde.org/D8597 AFFECTED FILES

D8596: Fix memory leak in FolderModel::drop

mwolff created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY When handling a drop event, a drop job was created and a Qt signal/slot connection created. This connection was only freed once the model

D8493: Make Folder View screen aware

mwolff added inline comments. INLINE COMMENTS > amantia wrote in foldermodel.cpp:1671 > Done, although the screenmapper will never change. an assert would be valid in such a scenario > amantia wrote in foldermodel.cpp:1704 > From my understanding, this setAppletInterface is called only once

D8567: Emit signals when a screen is added or removed

mwolff added a comment. lgtm, but the plasma people should chime in now, esp. regarding what @apol said REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D8567 To: amantia, #plasma, ervin, dvratil, mlaurent, hein Cc: mwolff, apol, mart, davidedmundson,

D8493: Make Folder View screen aware

mwolff added inline comments. INLINE COMMENTS > foldermodel.cpp:1671 > +return; > + > +m_screenMapper = screenMapper; missing disconnect on the old mapper for the invalidate filter > foldermodel.cpp:1704 > +setScreen(containment->screen()); > +

D8493: Make Folder View screen aware

mwolff added a comment. some code review from my side INLINE COMMENTS > foldermodel.cpp:77 > #include > +#include > +#include wrong location of include, and the KF5 prefix is wrong too, I guess? > foldermodel.cpp:1314 > + > +const QString name = item.url().toString(); > +if

D8567: Return the screen id for a screen name.

mwolff added inline comments. INLINE COMMENTS > shellcorona.cpp:1161 > > -if (DesktopView* v = desktopForScreen(screen)) > +if (DesktopView* v = desktopForScreen(screen)) { > removeDesktop(v); now you can undo this hunk REPOSITORY R120 Plasma

D8567: Return the screen id for a screen name.

mwolff added inline comments. INLINE COMMENTS > shellcorona.cpp:1162 > removeDesktop(v); > +emit screenRemoved(m_screenPool->id(screen->name())); > +} shouldn't this be done from within `removeDesktop`? considering this duplicates the change above?

Re: Phabricator: All repositories registered - upcoming workflow changes

in > their archives, so those will continue to be accessible through list > archives in the long run. And how do you find the corresponding mail archive thread based on a reviewboard URL? Will there be auto-forwarding in-place? Bye -- Milian Wolff m...@milianw.de http://milianw.de

Re: Review Request 128949: Add monitors for Linux memory infos dirty and writeback

tps://git.reviewboard.kde.org/r/128949/#comment66982> unrelated to the actual change set here, someone should cleanup this file to use Q_UNUSED instead of manually casting to void. - Milian Wolff On Sept. 22, 2016, 7:24 p.m., Pascal VITOUX

Re: Review Request 128892: Open baloo lmdb database read-only beside in baloo_file/baloo_file_extractor + balooctl (for some commands) + unit tests

> On Sept. 11, 2016, 9:35 p.m., Vishesh Handa wrote: > > This is awesome. Ship it. > > Christoph Cullmann wrote: > Lets hope it is awesome correct. :D - Milian --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 127779: use system colors for monochrome icons

(or digicam) and their builtin functionality to change the app's color scheme? I.e. what about per-app changes vs. system-wide changes? - Milian Wolff On April 29, 2016, 9:25 a.m., Marco Martin wrote: > > --- > This is an automatically ge

Re: Review Request 127779: use system colors for monochrome icons

> On April 28, 2016, 1:08 p.m., Milian Wolff wrote: > > src/kiconloader.cpp, line 66 > > <https://git.reviewboard.kde.org/r/127779/diff/5/?file=461490#file461490line66> > > > > this and the line below should be rewritten to > > > > namesp

Re: Review Request 127779: use system colors for monochrome icons

String NULL_EFFECT_FINGERPRINT() { return QStringLiteral("noeffect"); } QString STYLESHEET_TEMPLATE() { return QStringLiteral("..."); } } - Milian Wolff On April 28, 2016, 12:5

Re: Review Request 127079: fix uninitialised var

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127079/#review92574 --- Ship it! Ship It! - Milian Wolff On Feb. 19, 2016, 11

Re: Review Request 127091: Fix IconItem regression in 5.19

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127091/#review92467 --- Ship it! Ship It! - Milian Wolff On Feb. 16, 2016, 4

Re: QML support for kdevelop and plasmate

it here. If you want to get something done fast, i.e. somehow useable in a few months, you really should get a team of at least two working on the plugin. bye -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part