D5562: Fish ioslave: Port away from KDELibs4Support

2017-04-30 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Nice work. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D5562 To: marten, #plasma, dfaure Cc: plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, jensr

D5561: Filter ioslave: Port away from KDELibs4Support

2017-04-30 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks! REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D5561 To: marten, #plasma, dfaure Cc: plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, jensreut

D5560: Archive ioslave: Port away from KDELibs4Support

2017-04-30 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D5560 To: marten, #plasma, dfaure Cc: plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D5446: Implement support for selected mime type filters

2017-04-30 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R135 Integration for Qt applications in Plasma BRANCH master REVISION DETAIL https://phabricator.kde.org/D5446 To: elvisangelaccio, #plasma, dfaure Cc: anthonyfieroni, plasma-devel, spstarr, progwol

D5446: Implement support for selected mime type filters

2017-04-15 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > elvisangelaccio wrote in kfiledialog_unittest.cpp:116 > No, selectedMimeTypeFilter() is only in 5.9 so that wouldn't compile Oh I see. The QCOMPARE is just so that a failure happens. Pretty pointless, it's not testing anything from Qt. I would jus

D5446: Implement support for selected mime type filters

2017-04-15 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfiledialog_unittest.cpp:84 > + > +QMimeType headerMime = > QMimeDatabase().mimeTypeForName(QStringLiteral("text/x-chdr")); > +QMimeType jsonM

D5221: [desktop:/ KIO] Add descriptive name for root item

2017-03-31 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D5221 To: broulik, #plasma, hein, elvisangelaccio, dfaure Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts,

D5221: [desktop:/ KIO] Add descriptive name for root item

2017-03-29 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kio_desktop.cpp:171 > +// Set a descriptive display name for the root item > +if (entry.stringValue(KIO::UDSEntry::UDS_NAME) == QLatin1String(".")) { >

D5112: Make archiver ioslave extensible

2017-03-28 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Looks OK. It would be good to port this away from kdelibs4support though, to avoid propagating that dependency on users of the lib -- making it impossible to remove later. At least it sho

D4614: [Baloo Widgets] Add KPropertiesDialog Plugin with file metadata

2017-03-27 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > broulik wrote in baloofilepropertiesplugin.desktop:8 > I went through all extractors in KFileMetaData and looked at what they > support. > > Unfortunately KP

D5112: Make archiver ioslave extensible

2017-03-27 Thread David Faure
dfaure added a comment. Yeah KF5Konq is a bug, it shouldn't be named that way. I don't really care about casing, the cmake code will refer to the imported target (which shouldn't have a KF5:: prefix either), not to the library name anyway. REPOSITORY R320 KIO Extras REVISION DETAIL

D5112: Make archiver ioslave extensible

2017-03-27 Thread David Faure
dfaure added a comment. The idea sounds OK to me. INLINE COMMENTS > CMakeLists.txt:32 > +FILE KF5KioArchiveTargets.cmake > +NAMESPACE KF5:: > +) The KF5 prefix is wrong, this isn't part of KF5. > CMakeLists.txt:44 > + > +add_library(KF5KioArchive kio_archivebase.cpp kio_archive_debu

D5013: Truncate url string in KIO::job description to avoid freezing up plasma notification

2017-03-12 Thread David Faure
dfaure added a comment. Ship it ;) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D5013 To: yuenlim, #plasma, dfaure Cc: dfaure, plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D5013: Truncate url string in KIO::job description to avoid freezing up plasma notification

2017-03-12 Thread David Faure
dfaure added a comment. Yes; looks much nicer, doesn't it? INLINE COMMENTS > job_p.h:76 > Job *q_ptr; > +static const int DescriptionUrlMaxLength = 100; > This can now be moved to the helper function. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D5013 To:

D5013: Truncate url string in KIO::job description to avoid freezing up plasma notification

2017-03-11 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Extra credit for factorizing this into a file-static function and handling data: URLs separately (no point in seeing 100 chars of data crap, we could do data:[...] or something like that).

D5013: Truncate url string in KIO::job description to avoid freezing up plasma notification

2017-03-11 Thread David Faure
dfaure added a comment. Can you use KStringHandler::csqueeze ? This way filenames are still visible. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D5013 To: yuenlim, #plasma Cc: dfaure, plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abet

D4979: Add places:/ KIO slave

2017-03-08 Thread David Faure
dfaure added a comment. "I didn't find signals to notify changes" --> you mean how to tell apps that something changed in a given protocol? That's what KDirNotify is for, but you can't do the monitoring from a short-lived slave, you need something that stays alive for the whole session, and

D4923: [desktop:/ KIO] Strip superfluous slashes and fixup local root url

2017-03-07 Thread David Faure
dfaure added a comment. KFileItem::localPath() should not ever ever be a URL like desktop:/. That would be very wrong. It just returns what you set in UDS_LOCAL_PATH, so it's kio_desktop you should debug, not kio ;) Well except for KIO::ForwardingSlaveBase, called by kio_desktop -- note t

D4839: kio_mtp: add write permissions to root storage folder

2017-03-05 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. If you say it works... ;) REPOSITORY R320 KIO Extras BRANCH Applications/16.12 REVISION DETAIL https://phabricator.kde.org/D4839 To: elvisangelaccio, #plasma, dfaure Cc: plasma-deve

[kio-extras] [Bug 376128] SMB://server/dir Timeout on Server server in Dolphin

2017-03-04 Thread David Faure
https://bugs.kde.org/show_bug.cgi?id=376128 David Faure changed: What|Removed |Added Component|general |default Product|frameworks-kio

D4923: [desktop:/ KIO] Strip superfluous slashes and fixup local root url

2017-03-03 Thread David Faure
dfaure added a comment. QDir::cleanPath or QUrl::NormalizePathSegments can help making this fix more generic. But I'm curious, where does the "/." come from in the first place? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D4923 To: broulik, #plasma,

[Differential] [Accepted] D4739: make sure the cancel action is last

2017-02-28 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH arcpatch-D4739 REVISION DETAIL https://phabricator.kde.org/D4739 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: mart, #plasma, #frameworks, dfa

[Differential] [Commented On] D4739: make sure the cancel action is last

2017-02-28 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > mart wrote in dropjob.h:38 > used for class KIO::DropMenu : public QMenu in dropjob.cpp, > doesn't compile otherwise Not a good enough reason to have it in the public header (which doesn't use it anywhere), you can move that fwd decl to the .cpp f

[Differential] [Commented On] D4739: make sure the cancel action is last

2017-02-23 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > dropjob.h:38 > class DropJobPrivate; > +class DropMenu; > still here? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D4739 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: mart, #plasm

[Differential] [Updated] D4739: make sure the cancel action is last

2017-02-23 Thread David Faure
dfaure added a comment. I like the encapsulation into a different class. INLINE COMMENTS > dropjob.cpp:63 > +public: > +DropMenu(QWidget *parent = 0); > +~DropMenu(); nullptr > dropjob.cpp:167 > + > +void DropMenu::addExtraActions(QList appActions, QList > pluginActions) > +{ unne

[Differential] [Updated] D4667: [applet] Let specify a version for applets private plugins

2017-02-19 Thread David Faure
dfaure added a comment. I was always told "plugins should not get versioned at the .so level". Not sure why exactly. The more usual solution was version numbers in .desktop files, to query for things with the right version. I guess this can be transposed to json since Qt reads the json data

[Differential] [Commented On] D4600: TaskManager: add icon to cache after painfully querying it over XCB/NETWM.

2017-02-18 Thread David Faure
dfaure added a comment. Looking at the code in qxcbintegration.cpp, it seems that WM_CLASS is in fact derived from 1. -name xcb-specific commandline argument, let's ignore that 2. $RESOURCE_NAME env var, no idea what that is, let's ignore that 3. the basename of argv[0], i.e. the name

[Differential] [Accepted] D4620: allow to add application actions on an open menu

2017-02-18 Thread David Faure
dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > dropjob.cpp:330 > +} > +//separator at last position and not needed anymore? > +if (d->m_appActions.isEmpty() && d->m_pluginActions.isEmpty() &&

[Differential] [Commented On] D4614: [Baloo Widgets] Add KPropertiesDialog Plugin with file metadata

2017-02-16 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > Messages.sh:6 > + > +$XGETTEXT `find . -name \*.cc -o -name \*.cpp -o -name \*.h -o -name \*.qml > | grep -v "/src/filepropertiesplugin/"` -o $podir/baloowidgets.pot > rm -f rc.cpp This is obviously wrong, it would duplicate a lot of stuff that t

[Differential] [Changed Subscribers] D4620: allow to add application actions on an open menu

2017-02-15 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > hein wrote in dropjob.cpp:324 > Should this be && instead of ||? "Add actions if either of those is not > empty" seems weird, what's the reasoning? It's a copy of the if() on line 311. But there it makes sense. Here not so much ;) What if there

[Differential] [Closed] D4600: TaskManager: add icon to cache after painfully querying it over XCB/NETWM.

2017-02-13 Thread David Faure
dfaure closed this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D4600 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: dfaure, hein Cc: plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

[Differential] [Commented On] D4600: TaskManager: add icon to cache after painfully querying it over XCB/NETWM.

2017-02-13 Thread David Faure
dfaure added a comment. It is on my TODO list now. It will happen. REPOSITORY R120 Plasma Workspace BRANCH Plasma/5.9 REVISION DETAIL https://phabricator.kde.org/D4600 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: dfaure, hein Cc: plasma-devel,

[Differential] [Commented On] D4600: TaskManager: add icon to cache after painfully querying it over XCB/NETWM.

2017-02-13 Thread David Faure
dfaure added a comment. So what's the suggested fix for kmail? (I sense a communication issue here rather than bad will). WM_CLASS is obscure to most devels including me. How do we set it, in case renaming the desktop file isn't an option? REPOSITORY R120 Plasma Workspace BRANCH Plasm

[Differential] [Request, 2 lines] D4600: TaskManager: add icon to cache after painfully querying it over XCB/NETWM.

2017-02-13 Thread David Faure
dfaure created this revision. dfaure added a reviewer: hein. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY Hitting again a bug where switching desktops shows a delay of several seconds before the taskbar updates, I not

[Differential] [Accepted] D4534: [Folder View] Don't show script execution prompt on desktop:/

2017-02-09 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Oh I see. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D4534 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: broulik

[Differential] [Commented On] D4534: [Folder View] Don't show script execution prompt on desktop:/

2017-02-09 Thread David Faure
dfaure added a comment. The code that creates the application shortcuts that you mention, should make them executable. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D4534 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: b

[Differential] [Requested Changes To] D4534: [Folder View] Don't show script execution prompt on desktop:/

2017-02-09 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. This reintroduces the security bug that this whole thing was about in the first place. Download some file from the internet, save on desktop, click on it - boom. The Unix pr

[Differential] [Accepted] D4157: [Icon Applet] Use KIO::statJob to work with remote URLs

2017-01-26 Thread David Faure
dfaure accepted this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D4157 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: broulik, #plasma, mart, dfaure Cc: plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, ab

[Differential] [Commented On] D4157: [Icon Applet] Use KIO::statJob to work with remote URLs

2017-01-24 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > iconapplet.cpp:108 > +if (desiredDesktopFileName.isEmpty()) { > +desiredDesktopFileName = > QString::fromLatin1(QCryptographicHash::hash(m_url.toDisplayString().toUtf8(), > QCryptographicHash::Md5).toHex()); > +} Not c

[Differential] [Commented On] D4157: [Icon Applet] Use KIO::statJob to work with remote URLs

2017-01-19 Thread David Faure
dfaure added a comment. What I used to do about all this is to turn every '/' into a unicode fraction slash (so that it still looks like a '/' to the user, but can be used in a filename), and name the link with the full URL like "ftp://www.somehost.com/foo/bar/";. See KIO::encodeFileNam

[Differential] [Commented On] D4157: [Icon Applet] Use KIO::statJob to work with remote URLs

2017-01-19 Thread David Faure
dfaure added a comment. But then every HTTP URL, even with a filename, will the host as filename? Useful if you have only one link to a given website, but what if you have 5 links to different pages on that website? Or maybe I didn't understand the order you wanted to use. REPOSITORY

[Differential] [Commented On] D4157: [Icon Applet] Use KIO::statJob to work with remote URLs

2017-01-19 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > broulik wrote in iconapplet.cpp:159 > There you would usually have a filename that makes sense. No, not necessarily. You can point to the root of a FTP server with ftp://ftp.kde.org/ and you can even point to your home dir on an FTP server with

[Differential] [Commented On] D4157: [Icon Applet] Use KIO::statJob to work with remote URLs

2017-01-19 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > iconapplet.cpp:159 > > -return; > +if (name.isEmpty() && > url.scheme().startsWith(QLatin1String("http"))) { > +name = url.host(); Why only http? This would be useful for FTP, SMB and many other things, no? REPO

[Differential] [Updated] D4157: [Icon Applet] Use KIO::statJob to work with remote URLs

2017-01-16 Thread David Faure
dfaure added a comment. Much nicer indeed. +2 on the KIO part. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D4157 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: broulik, #plasma, mart, dfaure Cc: plasma-devel, lesliez

[Differential] [Commented On] D4157: [Icon Applet] Use KIO::mostLocalUrl instead of just replying on isLocalFile

2017-01-16 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > iconapplet.cpp:159 > > -// TODO use kio stat job which also works for remote stuff > -QMimeDatabase db; > -const QMimeType mimeType = db.mimeTypeForUrl(m_url); > +// TODO use kio stat job which also works for remote stuff > +

[Differential] [Commented On] D4157: [Icon Applet] Use KIO::mostLocalUrl instead of just replying on isLocalFile

2017-01-16 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > iconapplet.cpp:189 > +}); > +statJob->start(); > } You don't need to start() KIO jobs, they start automatically. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D4157 EMAIL PREFERENCES https://phabrica

[Differential] [Accepted] D3906: [Icon Applet] Fully re-populate when user changes Link URL

2017-01-02 Thread dfaure (David Faure)
dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > iconapplet.cpp:369 > +QFile::remove(m_localPath); > +setUrl(QUrl(desktopFile.readUrl())); // calls populate() itself > +} else { It look

Re: Review Request 129732: Fix regression in which the save dialog appears as an Open dialog

2016-12-31 Thread David Faure
/kfiledialog_unittest.cpp (line 109) <https://git.reviewboard.kde.org/r/129732/#comment68103> Will be uninitialized if the lambda isn't called. Better initialize it. - David Faure On Dec. 31, 2016, 9:42 a.m., Albert Astals Cid wrote: > > ---

Re: Review Request 129732: Fix regression in which the save dialog appears as an Open dialog

2016-12-31 Thread David Faure
A better approach would be QTRY_VERIFY(findFileWidget()); This will repeatedly call findFileWidget() until it works or until 5 seconds have passed. Then you don't need timers nor separate methods nor a new member variable. The rest of the test can just follow that line. -

[Differential] [Commented On] D3796: Fix Plasma-QPA filedialog to show wrong directory with QFileDialog::selectUrl()

2016-12-23 Thread dfaure (David Faure)
dfaure added a comment. git log says this is qtbase commit 1a42124, from before v5.8.0-alpha1. So I'll check for 5.8 and use clone, thanks for the information. REPOSITORY R135 Integration for Qt applications in Plasma REVISION DETAIL https://phabricator.kde.org/D3796 EMAIL PREFERENCES

[Differential] [Commented On] D3796: Fix Plasma-QPA filedialog to show wrong directory with QFileDialog::selectUrl()

2016-12-23 Thread dfaure (David Faure)
dfaure added inline comments. INLINE COMMENTS > kdeplatformfiledialoghelper.cpp:368 > +// like file://, so we have to do it ourselves > +QSharedPointer opt(new > QFileDialogOptions(*options())); > +opt->setInitialDirectory(m_dialog->directory()); This does NOT build for me. qplatfo

[Differential] [Accepted] D3707: add simple test rig for service runner

2016-12-22 Thread dfaure (David Faure)
dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > servicerunnertest.cpp:49 > +QDir(appsPath).removeRecursively(); > +Q_ASSERT(QDir().mkpath(appsPath)); > +auto fixtureDir = QDir(QFINDTESTDATA("fixtures"));

[Differential] [Accepted] D3796: Fix Plasma-QPA filedialog to show wrong directory with QFileDialog::selectUrl()

2016-12-22 Thread dfaure (David Faure)
dfaure accepted this revision. dfaure added inline comments. INLINE COMMENTS > kdeplatformfiledialoghelper.cpp:365 > + > +// Qt 5 at least until 5.7.0 does not derive the directory from the > passed url > +// and set the initialDirectory option accordingly, also not for known > schemes

[Differential] [Commented On] D3707: add simple test rig for service runner

2016-12-19 Thread dfaure (David Faure)
dfaure added inline comments. INLINE COMMENTS > servicerunnertest.cpp:59 > + > +KSycoca::self()->ensureCacheValid(); > +} BTW if this is necessary then there's a bug in ksycoca :-) You can leave it (it helps getting the debug output from ksycoca in the right method) but if it fails without

[Differential] [Commented On] D3707: add simple test rig for service runner

2016-12-19 Thread dfaure (David Faure)
dfaure added a comment. I would say yes, it's worth using LGPL v2+ or v2+v3 or v2+v3+e.V., whichever you prefer, for any new code. Otherwise it's even more work the day we want to relicense away from v2 only (I know because I've been trying to do that for some code for a very long time, and

[Differential] [Requested Changes To] D3707: add simple test rig for service runner

2016-12-16 Thread dfaure (David Faure)
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > servicerunnertest.cpp:5 > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU Library General Publ

[Differential] [Commented On] D2687: [Icon Widget] Bring back properties dialog

2016-12-16 Thread dfaure (David Faure)
dfaure added a comment. Shouting or just coping with the file (e.g. displaying raw data, for a text editor) ;) Looks good, thanks. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D2687 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/email

[Differential] [Accepted] D2687: [Icon Widget] Bring back properties dialog

2016-12-16 Thread dfaure (David Faure)
dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > iconapplet.cpp:285 > + > +// otherwise check if the applicaton supports the dropped type > +// TODO should we execute if *any* of the urls are supported,

[Differential] [Requested Changes To] D2687: [Icon Widget] Bring back properties dialog

2016-12-14 Thread dfaure (David Faure)
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > iconapplet.cpp:242 > +{ > +new KRun(QUrl::fromLocalFile(m_localPath), nullptr); > +} Just wondering, why is the parent widget nullptr here, while it's QA

Re: status of kde/plasma kiosk framework in kf5

2016-12-10 Thread David Faure
a terminal in the current folder ^^ > > > > > dolphin shouldn't allow this.. right? > > > > Konsole's desktop file has a key X-KDE-AuthorizeAction=shell_access that > > tells klauncher to refuse to start it when such restriction is in effect. > > > &g

[Differential] [Commented On] D3530: Import plasma-workspace kioslaves

2016-12-04 Thread dfaure (David Faure)
dfaure added a comment. In https://phabricator.kde.org/D3530#65873, @mart wrote: > desktop:/ should be probably conditionally built only on Linux(but in there is quite core), while, may make sense to actually kill applications:/ altogether?? I have no doubt it seems "core on linu

[Differential] [Updated] D3530: Import plasma-workspace kioslaves

2016-11-29 Thread dfaure (David Faure)
dfaure added a comment. remote:/ sounds very workspace-independent indeed, it sounds useful to have in kio. But desktop:/ and applications:/ make no sense in other workspaces (I'm surprised we even still have applications:/, it's kind of a toy, isn't it?). (ok applications:/ might m

Re: Review Request 129315: Don't install plasmoid desktop files as services

2016-11-06 Thread David Faure
> On Nov. 3, 2016, 12:53 p.m., Marco Martin wrote: > > I'm in favor of this, let's wait to 5.29 tough 5.28 RC is tagged, you can now push commits for 5.29. - David --- This is an automatically generated e-mail. To reply, visit: https://

[Differential] [Commented On] D2687: WIP: [Icon Widget] Bring back properties dialog

2016-11-04 Thread dfaure (David Faure)
dfaure added inline comments. INLINE COMMENTS > iconapplet.cpp:57 > +if (destroyed()) { > +QFile(m_localPath).remove(); > +} QFile::remove is enough, no need for a QFile instance. > iconapplet.cpp:78 > +if (!m_url.isValid()) { > +// invalid url, use dummy data ("nk >

Re: Review Request 128767: Fix crash when no QApplication is available.

2016-10-30 Thread David Faure
marked as submitted. Review request for KDE Frameworks and Plasma. Changes --- Submitted with commit a940089a8f2af58b14b4bfa76db23a199b65a04e by David Faure to branch master. Repository: plasma-framework Description --- This happens when QtCreator launches qmlplugindump. Testcase

moving kdepasswd to plasma-desktop/kcms/useraccount?

2016-10-23 Thread David Faure
tead? Not sure who maintains either one, the names I see in there are all from 2004 or before. So I suppose this is nowadays "plasma team collective maintainance", which is why I'm asking on plasma-devel before adding a few more files to plasma-desktop ;-) Please CC me, I'm no

[Differential] [Closed] D2914: folderview: port the context menu away from KonqPopupMenu.

2016-10-13 Thread dfaure (David Faure)
This revision was automatically updated to reflect the committed changes. Closed by commit rPLASMADESKTOP3cf22b335871: folderview: port the context menu away from KonqPopupMenu. (authored by dfaure). REPOSITORY rPLASMADESKTOP Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.o

[Differential] [Commented On] D2914: folderview: port the context menu away from KonqPopupMenu.

2016-10-13 Thread dfaure (David Faure)
dfaure added a comment. Eike: ping? REPOSITORY rPLASMADESKTOP Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D2914 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: dfaure, broulik, hein Cc: mart, plasma-devel, lesliezhai, ali-mohamed, jens

[Differential] [Request, 872 lines] D2914: folderview: port the context menu away from KonqPopupMenu.

2016-10-02 Thread dfaure (David Faure)
dfaure created this revision. dfaure added reviewers: broulik, hein. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY Many actions were coming from this code anyway, all that was missing was: - Copy To / Move To (pro

Re: Review Request 128684: Proofread + update khtml-general kcm docbook

2016-09-18 Thread David Faure
> On Aug. 16, 2016, 7:05 a.m., David Faure wrote: > > doc/kcontrol/khtml-general/index.docbook, line 7 > > <https://git.reviewboard.kde.org/r/128684/diff/1/?file=474210#file474210line7> > > > > Yeah I don't understand what this docbook is doing here :-)

Fwd: Cron ~/bin/update_kf5.sh

2016-09-18 Thread David Faure
. > Unable to perform a git checkout of origin/Plasma/5.8 to a local branch of Plasma/5.8 - -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

[Differential] [Closed] D2654: AppEntry: load icon on demand, to speed up plasma startup

2016-09-04 Thread dfaure (David Faure)
This revision was automatically updated to reflect the committed changes. Closed by commit rPLASMADESKTOPee2577125c5a: AppEntry: load icon on demand, to speed up plasma startup (authored by dfaure). REPOSITORY rPLASMADESKTOP Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.or

[Differential] [Request, 21 lines] D2654: AppEntry: load icon on demand, to speed up plasma startup

2016-09-04 Thread dfaure (David Faure)
dfaure created this revision. dfaure added a reviewer: broulik. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. TEST PLAN Starting plasmashell. It still takes too long, but now it's in QML stuff ;) REPOSITORY rPLASMADESKTOP Plasma Deskt

Re: Review Request 128767: Fix crash when no QApplication is available.

2016-08-26 Thread David Faure
---- On Aug. 26, 2016, 7:05 a.m., David Faure wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128767/ > -

Review Request 128767: Fix crash when no QApplication is available.

2016-08-26 Thread David Faure
s QThread(0x14af560), current thread is QThread(0x14e4240) Could not find any platform plugin Thanks, David Faure

Re: Review Request 128685: Proofread + update performance kcm docbook

2016-08-16 Thread David Faure
other konqueror-related docbook? - David Faure On Aug. 15, 2016, 11:48 a.m., Burkhard Lück wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.

Re: Review Request 128684: Proofread + update khtml-general kcm docbook

2016-08-16 Thread David Faure
They're just not really maintained (but then again that is a problem for konqueror itself as well, especially due to being built on top of deprecated web engines...). (I hate this situation.) - David Faure On Aug. 15, 2016, 11:40 a.m

[Differential] [Commented On] D2365: Add failing test case for selected filter from mimetype

2016-08-15 Thread dfaure (David Faure)
dfaure added a comment. Yes, and yes. REPOSITORY rPLASMAINTEGRATION Integration for Qt applications in Plasma REVISION DETAIL https://phabricator.kde.org/D2365 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: elvisangelaccio, graesslin, dfaure Cc: pla

Branch inconsistencies

2016-08-15 Thread David Faure
to a local branch of Plasma/5.7 ----- -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

[Differential] [Updated] D2365: Add failing test case for selected filter from mimetype

2016-08-15 Thread dfaure (David Faure)
dfaure added a comment. The thing is, mimetype filters should be preferred above name filters. So the bug is in QFileDialog::selectMimeTypeFilter which "falls back" to selectNameFilter. Instead it should call some selectInitiallySelectedMimeTypeFilter in the options, like selectNameFilter d

[plasma-framework] autotests: Skip autotest that is broken with Qt 5.5

2016-08-07 Thread David Faure
Git commit 8abeceaffd0c93028810ea12131b7fa8bcd13b12 by David Faure. Committed on 07/08/2016 at 11:25. Pushed by dfaure into branch 'master'. Skip autotest that is broken with Qt 5.5 Eike Hein said this was fixed by David Rosca in Qt >= 5.6.1. CCMAIL: plasma-devel@kde.org

Failing unittests

2016-08-06 Thread David Faure
gtest fails with Qt 5.5 Guys, get fixing :-) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

Re: Review Request 128519: Fix XDG_DATA_DIRS path in startkde

2016-07-25 Thread David Faure
ex Dieter's opinion in the bug description for https://bugs.kde.org/show_bug.cgi?id=332107 (the bug is fixed, but still, I would argue with not messing with an env var if we don't need to) - David Faure On July 25, 2016, 10:18 a.m., Ni

[Differential] [Accepted] D2258: Do not detect kde4 directory but assume it is the same location, this means people who install as non-root with kde-srcbuild can install it

2016-07-24 Thread dfaure (David Faure)
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks. Installing outside the install prefix is against the law. REPOSITORY rPLASMAPA Plasma Audio Volume Applet BRANCH master REVISION DETAIL https://phabricator.kde.org/D2258

[Differential] [Commented On] D1813: Fix selected name filter with multiple mimetypes

2016-07-18 Thread dfaure (David Faure)
dfaure added inline comments. INLINE COMMENTS > kdeplatformfiledialoghelper.cpp:78 > /* > * Map a KDE filter string into a Qt one. > */ Can you explain and document here what this function does, i.e. input args and return value? It's a bit confusing. "kde" is a mimetype name, e.g. applica

[Differential] [Accepted] D2054: Fix for bug 364530

2016-06-30 Thread dfaure (David Faure)
dfaure accepted this revision. dfaure added a reviewer: dfaure. dfaure added a comment. This revision is now accepted and ready to land. Yes, this is a much better fix, thank you. REPOSITORY rPLASMAWORKSPACE Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D2054 EMAIL PREFERE

[Differential] [Accepted] D2029: [Task Manager Backend] Honor "NotShowIn" and "OnlyShowIn" for Jump List Actions

2016-06-28 Thread dfaure (David Faure)
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks! REPOSITORY rPLASMADESKTOP Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D2029 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To:

[Differential] [Updated] D2029: [Task Manager Backend] Honor "NotShowIn" for Jump List Actions

2016-06-28 Thread dfaure (David Faure)
dfaure added a comment. Oh boy we're heading right into a mess similar to web sites and web browsers, if we continue this way. Next we'll need a "really only show in Unity", then we'll start making special cases for some apps, and then ... the world will implode. I am very much against

[Differential] [Accepted] D1931: Avoid asking model to create string-serialized icon data where possible.

2016-06-17 Thread dfaure (David Faure)
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY rPLASMADESKTOP Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D1931 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: hein, dfaure Cc: p

[Differential] [Updated] D1931: Avoid asking model to create string-serialized icon data where possible.

2016-06-17 Thread dfaure (David Faure)
dfaure added a comment. Maybe it should have been the other way around, LauncherUrlWithIcon where needed ;-) (it seems the icon is only rarely needed) INLINE COMMENTS > ContextMenu.qml:225 > } else { > -tasksModel.requestAddLauncher(visualParent.launcherUrl);

[Differential] [Accepted] D1920: Add a LauncherUrlWithoutIcon data role as a speed optimization.

2016-06-17 Thread dfaure (David Faure)
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY rPLASMAWORKSPACE Plasma Workspace BRANCH blblblbl (branched from master) REVISION DETAIL https://phabricator.kde.org/D1920 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailprefere

[Differential] [Accepted] D1921: Use data role that avoids unnecessary work by the source model.

2016-06-17 Thread dfaure (David Faure)
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY rPLASMADESKTOP Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D1921 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: hein, davidedmunds

[Differential] [Commented On] D1920: Add a LauncherUrlWithoutIcon data role as a speed optimization.

2016-06-17 Thread dfaure (David Faure)
dfaure added inline comments. INLINE COMMENTS > tasksmodel.cpp:357 > || (launcherUrl.isValid() && > launcherUrlsMatch(launcherUrl, > - > launcherIndex.data(AbstractTasksModel::LauncherUrl).toUrl(), > IgnoreQueryItems))) { > +

[Differential] [Accepted] D1898: [Kicker] Use KRun::runService instead of new KRun

2016-06-15 Thread dfaure (David Faure)
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Yes, but please add a comment about porting to KRun::runApplication once https://phabricator.kde.org/D1902 is merged in and you can rely on KF 5.24 :-) REPOSITORY rPLASMADESKTOP Plasma De

[Differential] [Closed] D1865: TasksModel: cache launcherCount().

2016-06-14 Thread dfaure (David Faure)
dfaure closed this revision. REPOSITORY rPLASMAWORKSPACE Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D1865 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: dfaure, hein Cc: broulik, plasma-devel, jensreuterberg, sebas

[Differential] [Updated, 29 lines] D1865: TasksModel: cache launcherCount().

2016-06-14 Thread dfaure (David Faure)
dfaure updated this revision to Diff 4458. dfaure added a comment. Set the new value before emitting the change signal, indeed. REPOSITORY rPLASMAWORKSPACE Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1865?vs=4451&id=4458 BRANCH master REVISION DETAIL http

[Differential] [Request, 30 lines] D1865: TasksModel: cache launcherCount().

2016-06-14 Thread dfaure (David Faure)
dfaure created this revision. dfaure added a reviewer: hein. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY This also allows to only emit launcherCountChanged() when it actually changed. The emit from TasksModel::fil

[Differential] [Accepted] D1859: Prepare KSplash for KDED dropping KSplash support

2016-06-14 Thread dfaure (David Faure)
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks! INLINE COMMENTS > splashapp.cpp:106 > { > +//filter out startup events from KDED as they will be removed in a > future release > +if (stage == QLatin1String("kded") || sta

[Differential] [Closed] D1841: Tasks model: don't load 4 icon sizes to then just use one.

2016-06-13 Thread dfaure (David Faure)
dfaure closed this revision. REPOSITORY rPLASMAWORKSPACE Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D1841 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: dfaure, hein Cc: broulik, plasma-devel, sebas

[Differential] [Updated, 12 lines] D1841: Tasks model: don't load 4 icon sizes to then just use one.

2016-06-13 Thread dfaure (David Faure)
dfaure updated this revision to Diff 4400. dfaure added a comment. Simplify as suggested REPOSITORY rPLASMAWORKSPACE Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1841?vs=4385&id=4400 BRANCH master REVISION DETAIL https://phabricator.kde.org/D1841 AFFECTED

<    1   2   3   4   5   >