T12173: KService: provide solution to migrate away from KServiceTypeTrader/KMimeTypeTrader for loading plugins and parts

2021-01-10 Thread David Faure
dfaure added a comment.


  Thanks Alex, you rock.

TASK DETAIL
  https://phabricator.kde.org/T12173

To: dfaure
Cc: alex, #frameworks, nicolasfella, dfaure, mart, davidre, GB_2, ahmadsamir, 
ngraham, kpiwowarski, usta, asturmlechner, jucato, cfeck, cgiboudeaux, 
cullmann, vkrause, cordlandwehr, knauss


Plasma Framework and Kirigami unittests

2021-01-02 Thread David Faure
Since commit f09b46bec639a97008f3471b4b9bf52648979d12 by Marco Martin
titled "Draft: Replace QString cache IDs with a struct-based version"
the CI says plasma-framework unittests don't pass anymore.


FAIL!  : ThemeTest::loadSvgIcon() 'm_svg->theme()->findInCache(cacheId, result, 
info.lastModified().toSecsSinceEpoch())' returned FALSE. ()
   Loc: [/home/jenkins/workspace/Frameworks/plasma-framework/kf5-qt5 
SUSEQt5.14/autotests/themetest.cpp(81)]

See 
https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.14/job/plasma-framework/job/kf5-qt5%20SUSEQt5.14/40/testReport/projectroot/autotests/plasma_themetest/

Could someone from the plasma team please keep an eye on unittests so I don't 
have to be the unittest police?

Oh, and Kirigami has a broken unittest too:

FAIL!  : Kirigami::AvatarTests::test_bad_names() Uncaught exception: NameUtils 
is not defined
   Loc: [/home/jenkins/workspace/Frameworks/kirigami/kf5-qt5 
SUSEQt5.14/autotests/tst_avatar.qml(38)]

See 
https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.14/job/kirigami/job/kf5-qt5%20SUSEQt5.14/65/testReport/junit/projectroot.home.jenkins.workspace.Frameworks.kirigami.kf5-qt5_SUSEQt514/autotests/tst_avatar_qml/
I think that one is for Carson Black, CC'ed.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D29303: Make KI18N_INSTALL() compatible to KDE_INSTALL_DIRS_NO_DEPRECATED

2020-12-26 Thread David Faure
dfaure added a comment.


  So this is basically the same as https://phabricator.kde.org/D29136 except 
that D29136  gives priority to the 
non-deprecated variable. Any reason against going with D29136 
 after all?

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29303

To: kossebau, ilic, heikobecker
Cc: dfaure, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


Re: KDE review for KWeatherCore

2020-12-25 Thread David Faure
On lundi 21 décembre 2020 07:16:09 CET hanyoung wrote:
> KWeatherCore: https://invent.kde.org/libraries/kweathercore is a library for
> querying weather forecast data. During the development of KWeather, we
> found the need to have a weather library. KWeatherCore is the result of
> extracting weather data fetching code from KWeather. I think having a
> dedicated weather library can serve the following propose: - simplify the
> KWeather code
> - easier to develop a weather daemon
> - potentially less code duplication across KDE

Very interesting. I've been missing such a library 4 years ago when I wrote
a QML app to display the weather on a RPi.
https://github.com/dfaure/qrpiweather

I remember looking at the plasma weather-engine but most of the data sources 
didn't have information about wind, and wind is very very important where I 
live (it's actually blowing up to 105 km/h at the very moment) :)

Feel free to grab any code from qrpiweather that might be useful, BTW.

The backend I ended up using was "Open Weather" (api.openweathermap.org).
But well, that was 4 years ago, things might have changed since :)

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: RFC: Switching to min Qt version 5.14 for KF on December 14th

2020-12-17 Thread David Faure
On jeudi 17 décembre 2020 00:20:41 CET Friedrich W. H. Kossebau wrote:
> Hi,
> 
> Am Samstag, 12. Dezember 2020, 22:25:32 CET schrieb David Faure:
> > Just a data point on this discussion. Every time we raise the min Qt
> > version, we make life easier for KDE developers, and harder for others who
> > might be thinking of integrating a framework into their project.
> > 
> > Just today I tried using a KF5 library to extend a single plugin in an
> > existing webserver (which I don't control, and which is mostly written in
> > python) [1]. That server is entirely set up with a docker environment on
> > top of... debian buster, which has Qt 5.11.3.
> > Fail.
> > I'm going to have to apply a patch to the KF5 library as part of the
> > Dockerfile, to port it back to Qt 5.11. No way I can convince them to
> > change the base distribution, all I'll get as a reply is to port away
> > from QtCore.
> > 
> > Obviously the 5.11 ship has sailed by now, and I know we can't support old
> > versions forever, but this kind of experience makes me very wary of
> > raising
> > requirements too fast.
> 
> I am reading an objection to the proposed bump in these words, am I correct
> in doing that? 

Objection is a strong word. I am not blocking the proposed bump, I am merely
realizing that the balance between contributor convenience and 
user-convenience (where the user is a developer) is difficult to achieve, 
after this (anecdotal indeed) evidence.

(And yes I needed something very recent, but it wasn't actually a framework,
it was another Qt/KDE library, KOpeningHours. I thought it was still 
illustrative of what one might encounter when trying to use a KDE framework 
outside its usual box of "the rest of the KDE software".)

> Though please those who want to support Qt 5.13 for some more time, consider
> adding support for KDE CI then. It leaves a bad feeling in my stomach that
> KF 5.77+ seems effectively for Qt 5.13 with a sticker "Good Luck!" right
> now. I fear that lowers the image with (potential) KF consumers, it does at
> least with me for other projects.
> I (and possibly many other KF contributors) have no way to test against Qt
> 5.13, so might introduce regressions/break things in the future, which feels
> bad :/

Right. That's a reason to fix something indeed, but there are still two ways 
to fix that, if it was the only reason : either raise min req to Qt 5.14, or 
ask for a Qt 5.13 CI.

In general I might have asked for a more conservative approach; but currently
anything we do to help with preparing the Qt 6 migration is a good thing,
and having one less Qt version to support helps with that.

So, in those exceptional circumstances, I'm in favour, go for it.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: Problem with ksycoca not being regenerated under flatpak

2020-12-12 Thread David Faure
Time flies, sorry for the delay.

On samedi 12 septembre 2020 19:44:45 CET Albert Astals Cid wrote:
> flatpak mounts all files as if created on January 1st 1970.

Isn't that a bug in itself? Why doesn't it preserve mtimes?

> This has the unfortunate effect that when we add new plugins to a flatpak
> (i.e. we just added markdown preview support in kate), they are not seen
> because ksycoca doesn't feel the need to regenerate itself (the sycoca file
> for flatpak is written "outside" flatpak container and thus has a newer
> date).
> 
> I remember talking about this probably a year ago at least with Aleix, but i
> guess we did not come up with any solution since it's still broken :D
> 
> I don't know much about sycoca, but can we make it so the cache always get
> regenerated on app launch when run under flatpak?

Is there no way to manually "touch" a directory after adding new files?

Otherwise, the hack you have in mind could be implemented as in the attached 
patch. Untested, except that it compiles.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
diff --git i/src/sycoca/ksycoca.cpp w/src/sycoca/ksycoca.cpp
index 665d8287d4..6f0159ef3f 100644
--- i/src/sycoca/ksycoca.cpp
+++ w/src/sycoca/ksycoca.cpp
@@ -228,6 +228,19 @@ bool KSycocaPrivate::openDatabase()
 
 bool result = true;
 if (!m_databasePath.isEmpty()) {
+// BEGIN flatpak hack
+static bool firstTime = true;
+if (firstTime) {
+firstTime = false;
+if (QFileInfo::exists(QStringLiteral("/.flatpak-info"))) {
+// We're running inside flatpak, which sets all times to 1970
+// So the first very time, don't use an existing database, recreate it
+qCDebug(SYCOCA) << "flatpak detected, ignoring" << m_databasePath;
+return false;
+}
+}
+// END flatpak hack
+
 qCDebug(SYCOCA) << "Opening ksycoca from" << m_databasePath;
 m_dbLastModified = QFileInfo(m_databasePath).lastModified();
 result = checkVersion();


Re: RFC: Switching to min Qt version 5.14 for KF on December 14th

2020-12-12 Thread David Faure
Just a data point on this discussion. Every time we raise the min Qt version, 
we make life easier for KDE developers, and harder for others who might be 
thinking of integrating a framework into their project.

Just today I tried using a KF5 library to extend a single plugin in an 
existing webserver (which I don't control, and which is mostly written in 
python) [1]. That server is entirely set up with a docker environment on top 
of... debian buster, which has Qt 5.11.3.
Fail.
I'm going to have to apply a patch to the KF5 library as part of the 
Dockerfile, to port it back to Qt 5.11. No way I can convince them to change 
the base distribution, all I'll get as a reply is to port away from QtCore.

Obviously the 5.11 ship has sailed by now, and I know we can't support old 
versions forever, but this kind of experience makes me very wary of raising 
requirements too fast.


[1] https://github.com/osm-fr/osmose-backend/issues/555 for the curious


PS: I agree with moving the dates for bumping the min req to just after a KF5 
release, this makes complete sense, feel free to make that adjustment.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: RFC: Switching to min Qt version 5.14 for KF on December 14th

2020-12-06 Thread David Faure
On dimanche 6 décembre 2020 17:39:38 CET Albert Astals Cid wrote:
>  * MidnightBSD
>  * openBSD
> The BSDs are a bit more unfortunate.

Thanks for the information, Albert.

Apparently this means bumping the requirement to Qt 5.14 would break OpenBSD.

How can we reach OpenBSD KDE people?
CC'ing kde-free...@kde.org, I know it's not the same, but maybe you guys know? 
;)

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D22764: Stabilize test KFileWidgetTest::testDropFile

2020-11-30 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> meven wrote in kfilewidgettest.cpp:481
> Let me know if I should relay you.

I thought I did that in commit eb18397fe525d2e4bd9 ?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D22764

To: meven, dfaure, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


Re: Merge tags in master branch?

2020-11-28 Thread David Faure
On lundi 23 novembre 2020 16:11:02 CET Bhushan Shah wrote:
> Hello,
> 
> So I have one question regarding the how we do the framework versioning.
> Namely the tags,
> 
> So currently some packages have a versioned tags on their master branch,
> 
> i.e
> 
> karchive:
> 
> ➜ git describe
> v5.76.0-1-g7304c28
> 
> While in case of some frameworks where translations needs to be taken
> from svn, it is something super weird like,
> 
> kdesu:
> 
> ➜ git describe
> v5.2.0-234-gb7ba89f
> 
> Some packagers who package -git versions in their unstable repos check
> the git describe to figure out what is current revision of the package
> and having "wrong" version there bugs out weirdly.

I know I'm doing something unusual with tags in KDE Frameworks
(tags that are not part of a branch), but I'm surprised anyone would rely on 
`git describe` anyway, I've seen it being a bit unreliable/unexpected in the 
past (in non KDE repositories).

> Do anyone have any opinion on "merging" latest git tag in master branch?
> and potentially doing that for next releases as well?

Merging the tag into master would work, I guess.
One downside for KF5 developers is that the translated docbook files then have 
to be built. That's many screenfuls of things like
[ 21%] Generating po/sr@latin/docs/kioslave5/webdav/index.cache.bz2
which is just "noise" to developers, at least those who read the cmake output 
like I do ;-). And it might slow down compilation, I guess.
You can check out v5.76.0 in kio to see what it looks like.

Pushing translations every day as suggested by Harald creates the risk of a 
bad translation file breaking compilation. I remember catching that quite a 
few times when I started doing KF5 releases. But it hasn't happened for a long 
while so maybe there's now a git hook or something, to prevent that from 
happening?

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D17816: Support for xattrs on kio copy/move

2020-11-18 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> arrowd wrote in ConfigureChecks.cmake:12
> There are other parts of code that are guarded with `HAVE_POSIX_ACL`s, and 
> these aren't enabled ATM for FreeBSD. So, the change is needed and I'm 
> willing to implement it.
> 
> I plan to move `set(HAVE_POSIX_ACL ...)` part into `FindACL.cmake` and use 
> this module everywhere. Does that sound OK?

Yep.

REVISION DETAIL
  https://phabricator.kde.org/D17816

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


XDG_STATE_HOME

2020-11-07 Thread David Faure
... is finally a thing, after I got merge powers in the xdg gitlab :-)
https://gitlab.freedesktop.org/xdg/xdg-specs/-/merge_requests/4

I suggest we start using it in KF6:
https://invent.kde.org/frameworks/kconfig/-/merge_requests/33

What it is:

$XDG_STATE_HOME contains state data that should persist between
(application) restarts, but that is not important or portable enough to the 
user that it should be stored in $XDG_DATA_HOME. It may contain:
 * actions history (logs, history, recently used files, …)
 * current state of the application that can be reused on a restart (view, 
layout, open files, undo history, …)

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D17816: Support for xattrs on kio copy/move

2020-10-30 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> arrowd wrote in ConfigureChecks.cmake:12
> > 1. enable the ACL stuff on systems with extattr, see the little bit of code 
> > in kpropertiesdialog.cpp
> 
> By that you mean that I should edit the CMake module to define 
> `HAVE_POSIX_ACL` when extattr headers are found?
> 
> Or should I change checks in kpropertiesdialog.cpp from `HAVE_POSIX_ACL` to 
> `HAVE_*ATTR_H`?

I'm talking about the implementation of fileSystemSupportsACL in 
kpropertiesdialog.cpp, which uses getxattr.
But now that I take another look at it, I see that it's already implemented on 
FreeBSD, without the use of extattr, it uses statfs instead.
So I think I was wrong, this part is fine.

I guess the only thing that's a bit weird is that kio_file's attr code 
(unrelated to ACLs) relies on FindACL linking to the right lib (on Linux). It 
would be cleaner if the attr stuff was separate. But no big deal I guess.

REVISION DETAIL
  https://phabricator.kde.org/D17816

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-10-29 Thread David Faure
dfaure accepted this revision.
dfaure added inline comments.

INLINE COMMENTS

> arrowd wrote in ConfigureChecks.cmake:12
> > Has this been tested on a system with sys/extattr.h?
> 
> I was working on this revision on such system all the time. FreeBSD contains 
> extattr bits in its `libc`, so no extra libraries are required.
> 
> So, what should be done here, then? Just move `HAVE_SYS_EXTATTR_H` check to 
> `FindACL.cmake` module?

Ah, I see, OK. No extra lib needed makes it simple.

The FindACL.cmake stuff is a bit of a mess now, with the need for extended 
attributes outside the ACL related code.
Maybe this could be split up into "find extended attribute stuff" and "find ACL 
stuff", the latter relying on the former.
But this merge request has been pending for long enough, let's do buildsystem 
refactoring as part of it.

Let's leave this part as is for now.

If you feel like it, I suggest followup commits to 1) enable the ACL stuff on 
systems with extattr, see the little bit of code in kpropertiesdialog.cpp, and 
2) separate the cmake stuff for ACLs from the cmake stuff for extended 
attributes.

REVISION DETAIL
  https://phabricator.kde.org/D17816

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-10-28 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> ConfigureChecks.cmake:12
>  check_include_files(limits.h  HAVE_LIMITS_H)
> +check_include_files(sys/xattr.h   HAVE_SYS_XATTR_H)
> +check_include_files("sys/types.h;sys/extattr.h" HAVE_SYS_EXTATTR_H)

This is already done by `cmake/FindACL.cmake`, why not add the other one 
(extattr.h) to that file as well?

The CMakeLists.txt in this directory calls `find_package(ACL)` so it'll be used.

Even though this isn't technically about ACLs, I'm guessing it all links 
*because* FindACL.cmake links to libattr, right?
And then this might also mean that the condition in FindACL.cmake needs to be 
updated, it currently *requires* HAVE_SYS_XATTR_H instead of allowing both 
variants.

But then I guess this means kpropertiesdialog.cpp needs to be ported to the 
sys/extattr.h API.

Has this been tested on a system with sys/extattr.h? I'm wondering if what will 
happen is: FindACL didn't find sys/attr.h so it doesn't link to libattr, and 
then the new code here fails to link. Or am I missing something?

> file_unix.cpp:535
> +
> +bool FileProtocol::copyXattrs(const int src_fd, const int dest_fd)
> +{

This whole method should be in `#if HAVE_SYS_XATTR_H || HAVE_SYS_EXTATTR_H`.

Otherwise, on systems with neither defined, a lot of compiler warnings will 
happen, like "keyLen isn't initialized" on line 591.

You can leave the method in the .h without #if (those defines aren't known 
there).
Here you can either #if the method itself (declared and not defined and not 
used, is OK, although some might say, a bit unclean)
or #if the whole method *contents*, but then in the #else you need 
`Q_UNUSED(src_fd); Q_UNUSED(dest_fd);` to avoid compiler warnings about unused 
parameters.

REVISION DETAIL
  https://phabricator.kde.org/D17816

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


Re: Dropping the moderation by default flag

2020-10-23 Thread David Faure
On jeudi 15 octobre 2020 16:13:52 CEST Ahmad Samir wrote:
> On 2020-07-21 21:16, Albert Astals Cid wrote:
> > Hi, this list has an unusual setting [for kde mailing lists] inherited
> > from kde-core-devel that is that even subscribed people get moderated,
> > and then the list moderator can decide to clear the moderate flag for
> > each person one by one.> 
> > I'm proposing to change that because:
> >   * it's non consistent with the rest of kde mailing lists
> >   * as moderator of this list i don't think i've seen ever any spam coming
> >   from a subscribed member.> 
> > Opinions?
> > 
> > Cheers,
> > 
> >Albert
> 
> Hello. I asked recently on #kde-devel about the kde-core-devel ML, because I
> sent an email there and it was postponed due to moderation.
> 
> Given that kde-core-devel and kde-frameworks-devel are similar, could you
> please set that same setting for kde-core-devel too?
> 
> Thanks. :)

Done.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: Windows CI Updated to Qt 5.15 - Temporarily KO due to Breeze Icons Breakage

2020-10-18 Thread David Faure
On mardi 6 octobre 2020 11:59:34 CEST Ben Cooksley wrote:
> Hi all,
> 
> This evening i've completed updates to the Windows CI system, bringing it
> from the previous Qt 5.14 setup it was using up to the more recent Qt 5.15.
> As part of this various other libraries will have also been updated.
> 
> This update was prompted by an unannounced dependency change within Breeze
> Icons. As a reminder to all developers, it is imperative that any change to
> your dependencies on a non-KDE project be announced two weeks or more in
> advance.
> 
> Unfortunately due to regressions within Breeze Icons, it is not possible
> for the Dependency Builds to complete at this time, meaning Windows CI
> functionality will be generally unavailable until this is corrected.
> 
> The failure log can be found at
> https://build.kde.org/job/Administration/job/Dependency%20Build%20Extragear%
> 20stable-kf5-qt5%20WindowsMSVCQt5.15/lastFailedBuild/console

That looks like the usual java timeout

I re-ran the exact same job and it passed.

Can you re-enable normal CI service for Windows?
I noticed e.g. that KIO tests were not run anymore...

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D22764: Stabilize test KFileWidgetTest::testDropFile

2020-10-18 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> kfilewidgettest.cpp:481
> +KDirLister *dirLister = fileWidget.dirOperator()->dirLister();
> +QSignalSpy spy(dirLister, SIGNAL(completed(const QUrl &_url)));
> +

For the record, this is broken, it should have been `completed(QUrl)`. 
Specifying argument names is incorrect in SIGNAL() and SLOT() macros.

It makes the wait() below fail every time, but after 5s, so the 100ms became 5s 
:-)

I'm working on fixing this.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D22764

To: meven, dfaure, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


breeze-icons tests broken

2020-10-11 Thread David Faure
This commit:

commit 8fce580335ef86f19df2238f00270820ac74c9f4
Author: Guo Yunhe 
Date:   Mon Oct 5 11:54:29 2020 +0300

Symlink kup.svg to preferences-system-backup.svg

diff --git a/icons-dark/preferences/32/yast-snapper.svg b/icons/apps/32/kup.svg
similarity index 100%
copy from icons-dark/preferences/32/yast-snapper.svg
copy to icons/apps/32/kup.svg

... seems to have triggered this failure :

https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.12/job/breeze-icons/job/kf5-qt5%20SUSEQt5.12/422/testReport/projectroot/autotests/scalable/

Is there a similar symlink missing in the scalable directory maybe?

Please everyone, keep an eye on your own project (either via the mails sent to 
kde-frameworks-devel,
or by visiting build.kde.org), so I don't have to be the unittest police.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D29299: Make KI18N_INSTALL() not rely on only LOCALE_INSTALL_DIR

2020-09-12 Thread David Faure
dfaure added a comment.


  In D29299#676447 , @pino wrote:
  
  > In D29299#676446 , @dfaure wrote:
  >
  > > In D29299#676445 , @pino wrote:
  > >
  > > >
  > >
  > >
  > > One of the primary goals of KF5 is to be useable by other applications 
not written by the KDE community (I actually know quite a few).
  > >  As such, it's not hard to imagine a cmake-based application that uses Qt 
and GNUInstallDirs [with qmake going away this will happen more and more], and 
one day it wants to use one of the frameworks. At that point, it shouldn't be 
forced to switch to ECMInstallDirs. Therefore I definitely see value in keeping 
the two things separate, as long as we keep making things easy for what is the 
most common case for us: using both.
  >
  >
  > Sigh. I know this, I never, ever, ever, and let me say it again, **never**, 
forgot about this.
  
  
  OK sorry for stating the obvious then.
  When you wrote "ki18n_install() is basically used by KF sources that use ECM 
already" it seemed to me that this was looking at KDE community code only; it's 
hard to make such a broad statement if including also external code. If you 
agree that being able to use ki18n without ECM is better, then indeed we all 
agree.
  
  > Sure. But it is not what I referred to when I spoke about "broken code".
  
  I have to apologize again, then, because I don't understand what is the 
"broken code" we're talking about then.
  
  > Yes, sorry, you are not Friedrich. OTOH, it would be nice to not be painted 
as "the one that wants to break things without second thoughts". Can we please 
agree on this?
  
  We do agree on this. I don't think I was painting you that way at all. I'm 
merely trying to understand both sides of the argument and find a solution that 
works for everyone.
  
  >> Also, your patch basically includes D29136 
 in the case of no DESTINATION parameter 
specified, hence my suggestion is:
  >> 
  >> - edit D29136  to do the fallback 
using the same logic introduced here: this way marble is already fixed with no 
other changes, and ki18n_install will work also with 
KDE_INSTALL_DIRS_NO_DEPRECATED (e.g. for release-service packages)
  
  Would this be what is done in D29303 ? (I 
just learned about this third option...)

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29299

To: kossebau, ilic, heikobecker, #frameworks, aacid, ltoscano
Cc: dfaure, pino, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns


D29299: Make KI18N_INSTALL() not rely on only LOCALE_INSTALL_DIR

2020-09-12 Thread David Faure
dfaure added a comment.


  In D29299#676445 , @pino wrote:
  
  > I asked for actual **valid** use cases when using the new variables first 
would break, and I still got none. There is a limit to how much you can keep 
broken code working... assuming such broken code exists. I don't think there is 
any of this such situation, as `ki18n_install()` is basically used by KF 
sources that use ECM already, with marble being the only exception (and even 
that, marble won't break).
  
  
  As you know, there are KF5-based applications outside the realm of what we 
can see in LXR.
  One of the primary goals of KF5 is to be useable by other applications not 
written by the KDE community (I actually know quite a few).
  As such, it's not hard to imagine a cmake-based application that uses Qt and 
GNUInstallDirs [with qmake going away this will happen more and more], and one 
day it wants to use one of the frameworks. At that point, it shouldn't be 
forced to switch to ECMInstallDirs. Therefore I definitely see value in keeping 
the two things separate, as long as we keep making things easy for what is the 
most common case for us: using both.
  
  This is why I'm requesting that the integration with ECM is called 
integration and not "backwards compat fallback".
  
  You say you don't want to support broken code. I agree. Would you agree that 
the situation I'm describing here is NOT broken code?
  
  > Oh, and just to make it clear: none of my comments implies that I don't 
care about ECM, nor about any users of it, nor that I "like" to purposefully 
break cmake scripts.
  
  I didn't say any of this, and you're replying to me here, not to Friedrich. 
I'm trying to bring this whole thing to a solution, so let's move aside all 
such accusations and concentrate on what might be helpful to resolve the 
technical issue.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29299

To: kossebau, ilic, heikobecker, #frameworks, aacid, ltoscano
Cc: dfaure, pino, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns


D29136: Use non-deprecated KDEInstallDir

2020-09-12 Thread David Faure
dfaure added a comment.


  (to remove some confusion: the previous comment had the wrong link and should 
have said "Abandoned in favour of https://phabricator.kde.org/D29299; -- but 
now it's reopened anyway, as an alternative to D29299 
)

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29136

To: heikobecker
Cc: dfaure, kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns


D29299: Make KI18N_INSTALL() not rely on only LOCALE_INSTALL_DIR

2020-09-12 Thread David Faure
dfaure added a comment.


  @pino Other than the fact that you think D29136 
 is "good enough", do you have any concrete 
objection to this version?
  
  My own objection would simply be that "backward-compatible fallback" is a bit 
too strongly worded; it reads like "this smells bad and might go away one day". 
  How about we rather call this ECM integration, to make it clear that as long 
as you're using ECM, we do not expect every caller of ki18n_install to pass the 
destination manually?

INLINE COMMENTS

> KF5I18nMacros.cmake.in:83
>  #
> +# ``DESTINATION`` specifies the installation directory where the files are 
> installed to (since 5.70).
> +# For backward-compatibility, if this argument is not passed, the 
> installation

(to be updated to 5.75 now)

> KF5I18nMacros.cmake.in:97
>  #
> -# KI18N_INSTALL(po) does the following:
> +# KI18N_INSTALL(po DESITNATION ${KDE_INSTALL_LOCALEDIR}) does the following:
>  # - Compiles kfoo.po into kfoo.mo and installs it in

typo: s/DESITNATION/DESTINATION/

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29299

To: kossebau, ilic, heikobecker, #frameworks, aacid, ltoscano
Cc: dfaure, pino, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns


General information for KDE Frameworks developers

2020-09-09 Thread David Faure
Here are some notes from the KF6 BoF at Akademy, as well as general 
information that I realized might not be known to all the relevant developers.

* There's a nice board with many tasks related to preparing KDE Frameworks for 
KF6. https://phabricator.kde.org/project/board/310/
All the tasks in the "Backlog" column, are tasks that can be done TODAY 
already, they don't depend on Qt6 or the possibility to break API/ABI in KDE 
Frameworks. Help is very much needed and welcome!

* We'll have an online meeting at some point to go through the board and add 
tags for priorities and "junior jobs".

* I strongly recommend subscribing to kde-frameworks-devel. If you got scared 
in the past by the amount of "merge requests" emails coming in, this is no 
longer the case with gitlab.

* Instead, I recommend subscribing in gitlab to the projects you care about.
For instance you go to https://invent.kde.org/frameworks/kio and click on the 
bell icon on the right of the large "KIO" header.

* If you want to keep an eye on *all* Frameworks merge requests, as used to 
happen on k-f-d, you can visit 
https://invent.kde.org/groups/frameworks/-/merge_requests (however I don't see 
a way to get notified by email).

* We decided on a two months deadline for the unittests that have always 
failed on FreeBSD and Windows to be whitelisted with QEXPECT_FAIL macros,
and turned into bug reports. This way we can start from a clean state in CI, 
react on regressions more easily, and even set up gitlab to check that MRs 
don't introduce regressions on any platform. It'll also help reducing the 
noise on k-f-d.
If you use either of those platforms, please give a hand with fixing those 
issues for real.
Reminder: to see failing tests, go to
https://build.kde.org/job/Frameworks/view/Everything/
then click on the platform you're interesting in, and then click on the "S" 
column twice. The yellow will propagate to the top.

Happy hacking!

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D28745: Skip caching thumbnails on encrypted filesystems

2020-08-29 Thread David Faure
dfaure accepted this revision.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D28745

To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns, dfaure
Cc: dfaure, thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, 
waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, 
cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov


D28745: Skip caching thumbnails on encrypted filesystems

2020-08-27 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> thumbnail.cpp:741
> +bool allowCache;
> +allowCache = sharesFilesystemWithThumbRoot(filePath);
> +if (!allowCache) {

join with previous line (this isn't C) ;)

> thumbnail.h:95
> +#ifndef Q_OS_WIN
> +static const dev_t deviceIdUnset = std::numeric_limits::max();
> +dev_t m_thumbnailDirDeviceId = deviceIdUnset;

Suggestion: name it s_deviceIdUnset (s for static, to match m for member).

This makes things more readable when looking at the .cpp file (looks like a 
local variable otherwise).

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D28745

To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns, dfaure
Cc: dfaure, thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, 
waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, 
cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov


D28745: Skip caching thumbnails on encrypted filesystems

2020-08-22 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> marcingu wrote in thumbnail.cpp:776
> How about I set it to -1 at start and check for that instead? I checked 
> definitions and it looks like dev_st is unsigned, but it shouldn't be a 
> problem.

-1 isn't exactly a great value for an unsigned int :-)

Or rather, it's not great enough (in the "greater than" sense) :-)

But yeah, 0x is a good "not set yet" value for a dev_t, to be safe 
against the possibility of 0.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D28745

To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns, dfaure
Cc: dfaure, thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, 
waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, 
cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov


Re: kwayland's testRemoteAccess still flaky

2020-08-22 Thread David Faure
On lundi 3 août 2020 11:17:45 CEST David Edmundson wrote:
> Urgh, let me just remove that test.
> 
> No-one will even use that protocol soon anyway.

Ping? It's still there it seems.

https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.15/job/kwayland/job/kf5-qt5%20SUSEQt5.15/14/

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: KDE CI: Frameworks » ktexteditor » kf5-qt5 SUSEQt5.15 - Build # 4 - Still Unstable!

2020-08-21 Thread David Faure
93% (4798/5140)
> > 49% (1812/3680)
> > 
> > autotests.src.vimode
> > 100% (9/9)
> > 100% (9/9)
> > 99% (5528/5570)
> > 58% (984/1708)
> > 
> > src.buffer
> > 88% (15/17)
> > 88% (15/17)
> > 89% (1677/1892)
> > 74% (1078/1464)
> > 
> > src.completion
> > 100% (11/11)
> > 100% (11/11)
> > 57% (1788/3134)
> > 42% (1009/2425)
> > 
> > src.completion.expandingtree
> > 100% (3/3)
> > 100% (3/3)
> > 40% (182/457)
> > 21% (73/340)
> > 
> > src.dialogs
> > 0% (0/4)
> > 0% (0/4)
> > 0% (0/864)
> > 0% (0/184)
> > 
> > src.document
> > 100% (4/4)
> > 100% (4/4)
> > 61% (1936/3179)
> > 48% (1407/2945)
> > 
> > src.export
> > 0% (0/4)
> > 0% (0/4)
> > 0% (0/121)
> > 0% (0/156)
> > 
> > src.include.ktexteditor
> > 78% (14/18)
> > 78% (14/18)
> > 83% (189/227)
> > 55% (125/226)
> > 
> > src.inputmode
> > 100% (8/8)
> > 100% (8/8)
> > 63% (192/304)
> > 51% (39/77)
> > 
> > src.mode
> > 88% (7/8)
> > 88% (7/8)
> > 36% (378/1051)
> > 16% (146/891)
> > 
> > src.part
> > 0% (0/1)
> > 0% (0/1)
> > 0% (0/7)
> > 100% (0/0)
> > 
> > src.printing
> > 0% (0/4)
> > 0% (0/4)
> > 0% (0/862)
> > 0% (0/278)
> > 
> > src.render
> > 100% (8/8)
> > 100% (8/8)
> > 77% (950/1227)
> > 67% (610/914)
> > 
> > src.schema
> > 22% (2/9)
> > 22% (2/9)
> > 1% (19/1472)
> > 1% (6/625)
> > 
> > src.script
> > 94% (16/17)
> > 94% (16/17)
> > 67% (699/1040)
> > 53% (212/403)
> > 
> > src.search
> > 100% (7/7)
> > 100% (7/7)
> > 74% (/1503)
> > 63% (590/932)
> > 
> > src.spellcheck
> > 50% (4/8)
> > 50% (4/8)
> > 7% (82/1207)
> > 3% (19/696)
> > 
> > src.swapfile
> > 50% (1/2)
> > 50% (1/2)
> > 32% (119/374)
> > 32% (64/203)
> > 
> > src.syntax
> > 88% (7/8)
> > 88% (7/8)
> > 64% (488/761)
> > 36% (206/576)
> > 
> > src.undo
> > 100% (5/5)
> > 100% (5/5)
> > 88% (652/741)
> > 73% (276/378)
> > 
> > src.utils
> > 95% (36/38)
> > 95% (36/38)
> > 63% (2406/3814)
> > 45% (1009/2256)
> > 
> > src.variableeditor
> > 0% (0/5)
> > 0% (0/5)
> > 0% (0/579)
> > 0% (0/108)
> > 
> > src.view
> >     88% (15/17)
> > 88% (15/17)
> > 57% (3794/6612)
> > 43% (1663/3884)
> > 
> > src.vimode
> > 100% (30/30)
> > 100% (30/30)
> > 81% (1899/2333)
> > 61% (979/1599)
> > 
> > src.vimode.config
> > 0% (0/1)
> > 0% (0/1)
> > 0% (0/120)
> > 0% (0/76)
> > 
> > src.vimode.emulatedcommandbar
> > 100% (13/13)
> > 100% (13/13)
> > 98% (897/917)
> > 90% (588/652)
> > 
> > src.vimode.modes
> > 100% (10/10)
> > 100% (10/10)
> > 87% (3266/3761)
> > 79% (1884/2374)
> > 
> > Links:
> > --
> > [1]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .15/4/artifact/abi-compatibility-results.yaml [2]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .15/4/artifact/acc/KF5TextEditor-5.72.0.xml [3]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .15/4/artifact/compat_reports/KF5TextEditor_compat_report.html [4]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .15/4/artifact/logs/KF5TextEditor/5.72.0/log.txt


-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D28745: Skip caching thumbnails on encrypted filesystems

2020-08-21 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> marcingu wrote in thumbnail.cpp:776
> If st_dev cannot be 0 on successful call of lstat, I don't think there's need 
> to change anything. m_thumbnailDirDeviceId will remain zero only if lstat 
> returns error and as long it's zero sharesFilesystemWithThumbRoot will return 
> false.

I agree with your second sentence. I never said otherwise. It will work, *if* 
indeed we are both right that st_dev would never be 0 when stat() succeeds.
My suggestion is that we make sure our assumption is correct -- at least, that 
we are told if it ever happens to be incorrect, by adding 
assert(baseStat.st_dev != 0) after the first lstat succeeds, and 
assert(fileStat.st_dev != 0) after the second lstat succeeds.

> marcingu wrote in thumbnail.h:93
> On Windows this check is assumed false. So in case encrypted file shares 
> filesystem with thumbnails directory, it can take longer as those won't be 
> cached, but it will work.

Which check? This is a member variable, and I'm not sure the dev_t type will be 
known at all on Windows.
But if you're not sure either, let's wait until CI tells us.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D28745

To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns, dfaure
Cc: dfaure, thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, 
waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, 
cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov


D28745: Skip caching thumbnails on encrypted filesystems

2020-08-19 Thread David Faure
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> marcingu wrote in thumbnail.cpp:776
> No. The m_thumbnailDirDeviceId is set to 0 and only changed if lstat executes 
> properly. Then it takes value of st_dev. I don't know if st_dev can be 0 
> though and I couldn't find this information in manual either. If it can, we 
> should change it.

I don't see how stat would succeed and st_dev would be 0, but I could be wrong. 
Maybe assert that it's not 0, at least?

> thumbnail.cpp:779
> +if (lstat(QFile::encodeName(m_thumbBasePath).data(), ) == 
> -1) {
> +qCWarning(KIO_THUMBNAIL_LOG) << "Cannot read information about 
> filesystem";
> +return false;

print out the path in the warning (same on line 786)

> thumbnail.cpp:785
> +struct stat fileStat;
> +if(lstat(QFile::encodeName(path).data(), ) == -1) {
> +qCWarning(KIO_THUMBNAIL_LOG) << "Cannot read information about 
> filesystem";

missing space after `if`

s/data/constData/

> thumbnail.h:93
>  qint64 m_maxFileSize;
> +dev_t m_thumbnailDirDeviceId = 0;
>  };

This will break compilation on Windows, I assume?

On Unix, is the corresponding include present in this file? The lack of context 
makes reviewing difficult indeed. Any chance to move this to gitlab? (see 
invent.kde.org)

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D28745

To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns, dfaure
Cc: dfaure, thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, 
waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, 
cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, 
navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov


Re: KIO on Android Failure

2020-08-19 Thread David Faure
On mardi 18 août 2020 13:52:50 CEST Ben Cooksley wrote:
> Hi all,
> 
> At some point recently functionality was added to KIO which broke the
> build on Android.

Is there any reason why Android doesn't appear at
https://build.kde.org/job/Frameworks/job/kio/
?

This is the reason why I didn't notice this problem.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D26448: Add KRecentFilesMenu to replace KRecentFileAction

2020-08-13 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.


  Thanks :-)

BRANCH
  recentfilemenu

REVISION DETAIL
  https://phabricator.kde.org/D26448

To: nicolasfella, #frameworks, dfaure
Cc: broulik, elvisangelaccio, cfeck, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns


D26448: Add KRecentFilesMenu to replace KRecentFileAction

2020-08-13 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> krecentfilesmenu.cpp:74
> +{
> +action = new QAction(titleWithSensibleWidth(qobject_cast *>(menu->parent(;
> +QObject::connect(action, ::triggered, action, [this, menu]() 
> {

QMenu is a QWidget. Why not just pass `menu` as argument?
It would use the menu's font which is more correct than the parent widget's 
font (the menu font could be set by a stylesheet or by 
`QApplication::setFont(font, "QMenu")`.

Also `menu` is never null, which simplifies the code in the method being called.

BRANCH
  recentfilemenu

REVISION DETAIL
  https://phabricator.kde.org/D26448

To: nicolasfella, #frameworks, dfaure
Cc: broulik, elvisangelaccio, cfeck, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns


Re: Dropping the moderation by default flag

2020-08-11 Thread David Faure
On mardi 11 août 2020 01:02:47 CEST Albert Astals Cid wrote:
> El dimecres, 22 de juliol de 2020, a les 17:47:26 CEST, Volker Krause va 
escriure:
> > On Tuesday, 21 July 2020 21:16:00 CEST Albert Astals Cid wrote:
> > > Hi, this list has an unusual setting [for kde mailing lists] inherited
> > > from
> > > kde-core-devel that is that even subscribed people get moderated, and
> > > then
> > > the list moderator can decide to clear the moderate flag for each person
> > > one by one.
> > > 
> > > I'm proposing to change that because:
> > >  * it's non consistent with the rest of kde mailing lists
> > >  * as moderator of this list i don't think i've seen ever any spam
> > >  coming
> > > 
> > > from a subscribed member.
> > > 
> > > Opinions?
> > 
> > +1. That setup on core-devel was already around in the early 2000s, so
> > probably a response to some events from 20 years ago, I doubt we still
> > need
> > that today.
> 
> So i was going to do it, but I've just realized that I can't, I only have
> the moderator password, not the owner password.
> 
> Please owner[s], can you disable this?
> 
> I *think* it's the "By default, should new list member postings be
> moderated?" in "Privacy options..." -> "Sender filters".

Done.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D26448: Add KRecentFilesMenu to replace KRecentFileAction

2020-08-08 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  A unittest would be useful too, especially if we then refactor the loading to 
use KIO jobs.
  
  But after 6 months, let's land this and keep working on it. Are you 
interested in writing the unittest and/or porting to KIO jobs, or do you need 
some help?

INLINE COMMENTS

> krecentfilesmenu.cpp:45
> +}
> +const QFontMetrics fontMetrics = QFontMetrics(QFont());
> +

Can be simplified to

  const QFontMetrics fontMetrics(QFont());

But this uses the application font, not this widget's font. The right thing to 
do would be to pass a QWidget* and use w->fontMetrics() here.

BRANCH
  recentfilemenu

REVISION DETAIL
  https://phabricator.kde.org/D26448

To: nicolasfella, #frameworks, dfaure
Cc: broulik, elvisangelaccio, cfeck, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns


kirigami unittests broken

2020-08-06 Thread David Faure
Hi Arjen,

Nice commit series in kirigami, [1] is an impressive list!

However it seems it broke tst_actiontoolbar, please see:
https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.12/job/kirigami/job/kf5-qt5%20SUSEQt5.12/562/testReport/projectroot.home.jenkins.workspace.Frameworks.kirigami.kf5-qt5_SUSEQt512/autotests/tst_actiontoolbar_qml/

[1] 
https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.12/job/kirigami/job/kf5-qt5%20SUSEQt5.12/557/
-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: KDE CI: Frameworks » kcoreaddons » kf5-qt5 FreeBSDQt5.15 - Build # 33 - Fixed!

2020-08-06 Thread David Faure
On jeudi 6 août 2020 10:48:19 CEST CI System wrote:
>   BUILD SUCCESS
> Build
> URL   https://build.kde.org/job/Frameworks/job/kcoreaddons/job/kf5-qt5%20Free

\o/

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D26448: Add KRecentFilesMenu to replace KRecentFileAction

2020-08-02 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> nicolasfella wrote in krecentfilesmenu.cpp:150
> I'm wondering how to best make this async without selling my soul to the 
> mulitthreading devil. QtConcurrent::filtered looks interesting, but depending 
> on QtConcurrent wouldn't be feasible, would it?

Depending on QtConcurrent is just fine. However QtConcurrent::filtered is for 
CPU-intensive operations, not for I/O operations. 1) you don't want to put 
blocking runnables in the global thread pool [can be solved by assigning to a 
different threadpool], 2) I don't think you really want to parallelize 16 calls 
to QFile::exists, for the case of a local physical harddisk? Not sure it would 
really harm (we're not reading 16 files, at least), but for sure the overhead 
of dispatching runnables to 16 threads would make it slower... One solution 
here might be just a dedicated thread iterating over the list.

However: note that the usual KIO way to do file operations async isn't 
multithreading, it's rather KIO jobs.
This would move the "5 minutes waiting for an NFS server" horror case into a 
kioslave rather than a thread, both achieve the desired outcome which is to not 
block the GUI thread.

Keeping the order of the entries is going to be interesting. I guess we need a 
temporary data structure which stores "exists | does not exist" for each entry, 
and once all jobs are done, we go through that and fill the menu. Note that 
remote URLs should just be assumed to exist, we don't want to actually check 
(slow; might prompt for password; might error on different networks; etc.).

Unlike Kai-Uwe, I think this should be a separate merge request though, it's 
all quite orthogonal to your work. As long as you confirm that filling the menu 
"later" has no negative impact on the API (i.e. the user of the class cannot 
assume that the menu is filled in immediately).

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D26448

To: nicolasfella, #frameworks, dfaure
Cc: broulik, elvisangelaccio, cfeck, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns


kwayland's testRemoteAccess still flaky

2020-08-01 Thread David Faure
https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.12/job/kwayland/job/kf5-qt5%20SUSEQt5.12/149/testReport/junit/projectroot.autotests/client/kwayland_testRemoteAccess/

d_ed: looks like https://phabricator.kde.org/D28892 wasn't enough to make it 
fully stable, can you have another look?

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: xml_mimetypes5 and kcoreaddons

2020-07-19 Thread David Faure
On mercredi 15 juillet 2020 12:24:34 CEST Harald Sitter wrote:
> On Wed, Jul 15, 2020 at 12:39 AM David Faure  wrote:
> > On mardi 14 juillet 2020 19:35:41 CEST Albert Astals Cid wrote:
> > > El dimarts, 14 de juliol de 2020, a les 15:14:38 CEST, Jonathan Riddell
> > > va
> > 
> > escriure:
> > > > We're playing with translations in neon packages and looking at
> > > > kcoreaddons
> > > > the tars have
> > > > xml_mimetypes5
> > > > But we can't see anything in the code which uses this.  Do these
> > > > translations get used?
> > > 
> > > Yes, the translations are used.
> > > 
> > > No, they don't need to be on the tarball.
> > > 
> > > The translations are used at scripty time to to fill
> > > https://invent.kde.org/frameworks/kcoreaddons/-/blob/master/src/mimetype
> > > s/k
> > > de5.xml
> > > 
> > > Luigi recently did a change so the files ending in _xml_mimetypes.po
> > > don't
> > > get added to the release service tarballs.
> > > 
> > > This didn't work here because a) KF5 is using a different branch of
> > > release-tools b) the file doesn't follow the same naming pattern than
> > > the
> > > rest of xml mimetype files we have.
> > > 
> > > If DavidF is fine adapting his scripts to not release files ending in
> > > _xml_mimetypes.po (i.e. him or someone else patches the scripts) i
> > > volunteer to do the small patch for src/mimetypes/XmlMessages.sh to
> > > rename
> > > it.
> > 
> > Fine with me, but don't we have more cases of the same kind, with
> > different
> > names? Any case of translations being "integrated" into some file leads to
> > this. Desktop files, mimetype XML, is there really nothing else?
> 
> It's a bit more "fluid" than one would hope. Looking at
> update_translations the following patterns exist:
> 
> - created by XmlMessages.sh unfortunately arbitrary named files but
> largely using the standard suffix Albert mentioned (the only other
> violation I can find with lxr is kpat)
> - created by StaticMessages.sh also arbitrary, only used for websites
> I think (?)
> - ._desktop_.po automatic - shouldn't be shipped
> - ._json_.po automatic - shouldn't be shipped
> - .appdata.po automatic - shouldn't be shipped
> - .metadata.po automatic - shouldn't be shipped

OK. make_rc_tag.sh already says

find -regextype egrep -regex '.*\.(_desktop_|_json_|appdata|metainfo)\.po' 
-delete

which matches the above, assuming you meant metainfo when writing metadata?

I just changed that regexp to add _xml_mimetypes.po, I hope this solves the 
issue.
https://invent.kde.org/sysadmin/release-tools/commit/ffee07853b586b80236791edd69afc9d9d5dfd8e

Ah, no, Albert still needs to "do the small patch for 
src/mimetypes/XmlMessages.sh to rename it", IIUC.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: Frameworks support for Qt 5.12

2020-07-19 Thread David Faure
On mercredi 15 juillet 2020 18:06:40 CEST Nicolas Fella wrote:
> Hi,
> 
> I received a question on how long KF5 will continue to support Qt 5.12.
> 
> Given that 5.12 is an LTS release according to our policy it would be
> supported "until the next Qt release after the next Qt release", which
> would be 5.16, which will never exist.
> 
> Our policy states "With Qt6 this changes a little bit again. To avoid
> supporting 5.12 LTS and 5.15 LTS for ever, 5.15 LTS will be the minimum
> required version 18 months after its release.". This however does not
> answer what will happen with 5.12.
> 
> If I remember correctly our intention when formalizing this was to
> extrapolate the hypothetical releases and apply the existing rules to
> it. This would mean that by the time Qt 5.16 would be released (6 months
> after Qt 5.15) KF5 would drop support for Qt 5.12 and require 5.13.
> 
> Is my interpretation of this correct? If so the wiki page should be
> updated with this information.

Yes. Done:

* Qt 5.13 will be the minimum required version 6 months after Qt 5.15, i.e. on 
26 Nov 2020
* Qt 5.14 will be the minimum required version 12 months after Qt 5.15, i.e. 
on 26 May 2021
* Qt 5.15 LTS will be the minimum required version 18 months after its 
release, i.e. on 26 Nov 2021

https://community.kde.org/Frameworks/Policies#Frameworks_Qt_requirements 

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: xml_mimetypes5 and kcoreaddons

2020-07-14 Thread David Faure
On mardi 14 juillet 2020 19:35:41 CEST Albert Astals Cid wrote:
> El dimarts, 14 de juliol de 2020, a les 15:14:38 CEST, Jonathan Riddell va 
escriure:
> > We're playing with translations in neon packages and looking at
> > kcoreaddons
> > the tars have
> > xml_mimetypes5
> > But we can't see anything in the code which uses this.  Do these
> > translations get used?
> 
> Yes, the translations are used.
> 
> No, they don't need to be on the tarball.
> 
> The translations are used at scripty time to to fill
> https://invent.kde.org/frameworks/kcoreaddons/-/blob/master/src/mimetypes/k
> de5.xml
> 
> Luigi recently did a change so the files ending in _xml_mimetypes.po don't
> get added to the release service tarballs.
> 
> This didn't work here because a) KF5 is using a different branch of
> release-tools b) the file doesn't follow the same naming pattern than the
> rest of xml mimetype files we have.
> 
> If DavidF is fine adapting his scripts to not release files ending in
> _xml_mimetypes.po (i.e. him or someone else patches the scripts) i
> volunteer to do the small patch for src/mimetypes/XmlMessages.sh to rename
> it.

Fine with me, but don't we have more cases of the same kind, with different 
names? Any case of translations being "integrated" into some file leads to 
this. Desktop files, mimetype XML, is there really nothing else?

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: KDE CI: Frameworks » ktexteditor » kf5-qt5 SUSEQt5.15 - Build # 4 - Still Unstable!

2020-07-07 Thread David Faure
On mardi 7 juillet 2020 17:58:26 CEST Christoph Cullmann wrote:
> On 2020-07-07 12:16, David Faure wrote:
> > Yep :(
> > 
> > You'll tell Simon and/or QTBUG-*?
> 
> I will take a look if I can find some existing bug or open a new.
> 
> Simon left Qt, or?

Yes but I think that was already the case when he fixed the last one :-)

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: KDE CI: Frameworks » ktexteditor » kf5-qt5 SUSEQt5.15 - Build # 4 - Still Unstable!

2020-07-07 Thread David Faure
% (0/184)
> > 
> > src.document
> > 100% (4/4)
> > 100% (4/4)
> > 61% (1936/3179)
> > 48% (1407/2945)
> > 
> > src.export
> > 0% (0/4)
> > 0% (0/4)
> > 0% (0/121)
> > 0% (0/156)
> > 
> > src.include.ktexteditor
> > 78% (14/18)
> > 78% (14/18)
> > 83% (189/227)
> > 55% (125/226)
> > 
> > src.inputmode
> > 100% (8/8)
> > 100% (8/8)
> > 63% (192/304)
> > 51% (39/77)
> > 
> > src.mode
> > 88% (7/8)
> > 88% (7/8)
> > 36% (378/1051)
> > 16% (146/891)
> > 
> > src.part
> > 0% (0/1)
> > 0% (0/1)
> > 0% (0/7)
> > 100% (0/0)
> > 
> > src.printing
> > 0% (0/4)
> > 0% (0/4)
> > 0% (0/862)
> > 0% (0/278)
> > 
> > src.render
> > 100% (8/8)
> > 100% (8/8)
> > 77% (950/1227)
> > 67% (610/914)
> > 
> > src.schema
> > 22% (2/9)
> > 22% (2/9)
> > 1% (19/1472)
> > 1% (6/625)
> > 
> > src.script
> > 94% (16/17)
> > 94% (16/17)
> > 67% (699/1040)
> > 53% (212/403)
> > 
> > src.search
> > 100% (7/7)
> > 100% (7/7)
> > 74% (/1503)
> > 63% (590/932)
> > 
> > src.spellcheck
> > 50% (4/8)
> > 50% (4/8)
> > 7% (82/1207)
> > 3% (19/696)
> > 
> > src.swapfile
> > 50% (1/2)
> > 50% (1/2)
> > 32% (119/374)
> > 32% (64/203)
> > 
> > src.syntax
> > 88% (7/8)
> > 88% (7/8)
> > 64% (488/761)
> > 36% (206/576)
> > 
> > src.undo
> > 100% (5/5)
> > 100% (5/5)
> > 88% (652/741)
> > 73% (276/378)
> > 
> > src.utils
> > 95% (36/38)
> > 95% (36/38)
> > 63% (2406/3814)
> > 45% (1009/2256)
> > 
> > src.variableeditor
> > 0% (0/5)
> > 0% (0/5)
> > 0% (0/579)
> > 0% (0/108)
> > 
> > src.view
> > 88% (15/17)
> >     88% (15/17)
> > 57% (3794/6612)
> > 43% (1663/3884)
> > 
> > src.vimode
> > 100% (30/30)
> > 100% (30/30)
> > 81% (1899/2333)
> > 61% (979/1599)
> > 
> > src.vimode.config
> > 0% (0/1)
> > 0% (0/1)
> > 0% (0/120)
> > 0% (0/76)
> > 
> > src.vimode.emulatedcommandbar
> > 100% (13/13)
> > 100% (13/13)
> > 98% (897/917)
> > 90% (588/652)
> > 
> > src.vimode.modes
> > 100% (10/10)
> > 100% (10/10)
> > 87% (3266/3761)
> > 79% (1884/2374)
> > 
> > Links:
> > --
> > [1]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .15/4/artifact/abi-compatibility-results.yaml [2]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .15/4/artifact/acc/KF5TextEditor-5.72.0.xml [3]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .15/4/artifact/compat_reports/KF5TextEditor_compat_report.html [4]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .15/4/artifact/logs/KF5TextEditor/5.72.0/log.txt


-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D14631: Adds a new RenameDialog to KIO with more options for batch renaming

2020-07-06 Thread David Faure
dfaure added a comment.


  Only if you can find a way to change BatchRenameJob in a binary and behaviour 
compatible way. And then it will be a dual-headed thing with two modes of 
operations, awful. All this sounds to me like much more trouble than writing a 
different job.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D14631

To: emateli, #frameworks, dfaure, mlaurent, meven, #dolphin
Cc: luco, nicopons, meven, chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, 
ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


Re: ThumbSequenceCreator in KIO not working properly

2020-07-04 Thread David Faure
On samedi 4 juillet 2020 12:57:04 CEST David Lerch wrote:
> I have not been able to find out if DelegateAnimationHandler is even being
> used during thumbnail generation. 

It seems to be used by KFileItemDelegate (and nothing else) :
https://lxr.kde.org/ident?_i=DelegateAnimationHandler&_remember=1

KFileItemDelegate itself is use by KDirOperator
https://lxr.kde.org/ident?_i=KFileItemDelegate

Dolphin unfortunately has its own model/view stuff (e.g. for the animations 
where icons move around).

I suggest testing with the file dialog (which uses KDirOperator)
and asking the Dolphin people on kfm-devel about how Dolphin could be changed 
to use DelegateAnimationHandler, possibly?

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





inotify on FreeBSD

2020-07-04 Thread David Faure
Hello Tobias,

your commit 59db8a5ea2241927 in kcoreaddons enabled inotify support on FreeBSD
3 years ago, but I wonder if it's a good idea? According to the FreeBSD CI [1],
inotify support on FreeBSD is either suboptimal or at least different from 
inotify support on Linux:
many tests fail, while they all pass on Linux.

I wonder if the other backends (QFSWatch, Stat) wouldn't be better suited on 
FreeBSD,
since those actually work fine according to the FreeBSD CI.

Unless someone is willing to debug what's happening with inotify on FreeBSD, 
how about we disable the inotify backend on FreeBSD?

[1] 
https://build.kde.org/job/Frameworks/view/Platform%20-%20FreeBSDQt5.15/job/kcoreaddons/job/kf5-qt5%20FreeBSDQt5.15/9/testReport/projectroot/autotests/kdirwatch_inotify_unittest/

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: Shift for parts of the CI system to Qt 5.15

2020-06-21 Thread David Faure
On samedi 20 juin 2020 12:25:42 CEST Ben Cooksley wrote:
> - ktorrent (when linking with taglib)

Looks like the fix I made to fix ktorrent (failing to link with taglib) also 
fixed the FreeBSD issue you mention.

https://build.kde.org/job/Extragear/job/ktorrent/job/kf5-qt5%20FreeBSDQt5.15/

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: Add loop device interface to Solid framework

2020-06-13 Thread David Faure
On dimanche 7 juin 2020 20:10:11 CEST Kwon-Young Choi wrote:
> Hello,
> 
> I have recently made a plugin for dolphin to mount and unmount iso
> files: https://invent.kde.org/sdk/dolphin-plugins/-/merge_requests/1
> 
> This plugin uses DBus calls to directly communicate with UDisks2 to
> attach and detach a loop device backed by an iso file.
> While writing the plugin, I used to Solid framework to do some hardware
> query and realized that there was no concept of loop device in Solid.
> I would like to know if there is interest to add a loop device interface
> to Solid and the ability to attach and detach such device?
> 
> I'm starting to understand the architecture of Solid and I think I would
> be able to add:
> 
> * an abstract Loop DeviceInterface
> * a backend Loop DeviceInterface backed by UDisks2
> * a frontend Loop DeviceInterface
> * the ability for the DeviceManager to create a Loop DeviceInterface
> (I'm not sure this is possible...)
> 
> However, I am not sure if this fit in the score of the Solid framework.

I wish there was a Solid maintainer to answer you...

However for such an architectural/scope question, maybe ervin will be willing 
to provide some input :-)

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D26342: Allow overriding to disable auto language detection

2020-06-13 Thread David Faure
dfaure added a comment.


  In D26342#675180 , @aacid wrote:
  
  > In D26342#675164 , @sdepiets 
wrote:
  >
  > > I don't think that's a regression, in the previous behavior you could try 
to set any language to proofread, it would always auto-detect "Bonjour" as 
French, thus the "Tools / Spelling / change language" had not effect if 
autodetect was enabled at system level (while autodetection should be an 
application or even case by case decision).
  >
  
  
  Autodetection *is* an application setting, it's in Settings / Spellchecker... 
/ [x] Enable autodetection of language
  
  >> It may not seem like an issue for simple cases, but actually for mixed 
contents (i.e. an email that is 50% French, 50% English) that would be detected 
as English, you would have no way at all to check the French text without 
disabling system-wide autodectection.
  
  In my experience a mixed email was perfectly handled, with each paragraph 
having a different language.
  See http://www.davidfaure.fr/2020/Screenshot_20200613_194827.png
  
  But I think you're talking about the spellchecking dialog (which I never use, 
except this month because of the need to find a workaround).
  My concern is for autodetection and background spellchecking. If a fix for 
the spellcheck dialog introduces a change of behaviour for the background 
spellchecking, that's a regression.
  
  >> Calling setAutoDetectLanguageDisabled(false) restores the previous behavior
  
  Changes in KF5 should NOT require applications to adapt in order to restore 
previous *working* behaviour.
  
  > The problem here is that David probably never set any language on purpose.
  
  Right.
  In fact, if I open the settings dialog, it has "français" checked in the top 
listview, and only that one.
  Meanwhile the Tools / Spelling dialog shows "American English (UnitedStates)" 
in the combobox.
  This particular issue happens even with this patch reverted. Something's 
rather broken.
  
  > you could argue that this is a deficiency of the UI, should have a checkbox 
for "enable spellchecking" and another for "enable spellchecking *exactly* in 
this language"
  
  Not sure I understand this.
  
  > And this is were we failed, we changed the default behaviour and that's 
probably not the greatest of the ideas in retrospect. Even if we could not see 
why anyone would set a language and would still want the auto language 
detection to be enabled, well it seems that at least David wanted because it's 
been the behaviour for ages :D
  
  *I* don't really set a language. Maybe KMail does though. @mlaurent might 
know more?

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D26342

To: sdepiets, #frameworks, cullmann, mlaurent, mludwig, aacid
Cc: dfaure, aacid, mludwig, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns


D26342: Allow overriding to disable auto language detection

2020-06-13 Thread David Faure
dfaure added a comment.


  This actually breaks language auto-detection for me in the KMail composer.
  
  Testcase:
  
  - New Mail
  - I type "Bonjour," in the body
  
  Before:
  It's detected as French and not underlined as a typo
  
  After:
  The language remains English, and the word is underlined as a typo
  
  Workaround:
  Tools / Spelling / change language from English to French
  
  Too bad I'm realizing this is the reason for the regression only today (day 
of 5.71 release) by looking at the KF-5.71 changelog :(

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D26342

To: sdepiets, #frameworks, cullmann, mlaurent, mludwig, aacid
Cc: dfaure, aacid, mludwig, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns


Re: Opening files by mimetype instead of by file extension

2020-06-10 Thread David Faure
On mercredi 10 juin 2020 00:52:16 CEST Albert Astals Cid wrote:
> As far as I can see QFileDialog only supports listing by file extension.
> 
> On the other hand it seems in GTK+ land you can get an open file dialog
> listing by mimetype, so I've got one bug about "you're worse than XXX
> because you can't do this"

What does "list by mimetype" mean exactly?
Is this about a mimetype column in the table/tree view? (missing)
Is this about the filter combobox showing mimetypes? (supported)
Or is it something else?

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D29810: Don't use the setenv function after fork

2020-06-07 Thread David Faure
dfaure added a comment.


  This breaks FreeBSD compilation. Please check: 
https://build.kde.org/job/Frameworks/job/kcrash/job/kf5-qt5%20FreeBSDQt5.14/17/

REPOSITORY
  R285 KCrash

REVISION DETAIL
  https://phabricator.kde.org/D29810

To: jpalecek, #frameworks, dfaure
Cc: bruns, apol, anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham


D29810: Don't use the setenv function after fork

2020-06-07 Thread David Faure
dfaure closed this revision.

REPOSITORY
  R285 KCrash

REVISION DETAIL
  https://phabricator.kde.org/D29810

To: jpalecek, #frameworks, dfaure
Cc: bruns, apol, anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham


D29814: Fix segfault on no restart args

2020-06-07 Thread David Faure
dfaure closed this revision.

REPOSITORY
  R285 KCrash

REVISION DETAIL
  https://phabricator.kde.org/D29814

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29814: Fix segfault on no restart args

2020-06-07 Thread David Faure
dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R285 KCrash

BRANCH
  for-upstream

REVISION DETAIL
  https://phabricator.kde.org/D29814

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


KRunProxy

2020-06-06 Thread David Faure
In order to switch kdeclarative to 
-DKF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x054700
we need to port KRunProxy away from KRun, or to deprecate KRunProxy.

AFAICS it exposes an object named KRun to QML files, but searching for
qml files with KRun in them leads to no results: 
https://lxr.kde.org/search?_filestring=*.qml&_string=KRun

Am I doing this wrong? Are there other file extensions to take into 
consideration?

If not, can I deprecate KRunProxy?

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D29809: Don't invoke qstring localized stuff in critical section

2020-06-06 Thread David Faure
dfaure added a comment.


  Done in 
https://invent.kde.org/frameworks/kcrash/commit/c315685f0e0920e8847afe12c97fbbe4653eb351

REPOSITORY
  R285 KCrash

REVISION DETAIL
  https://phabricator.kde.org/D29809

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29809: Don't invoke qstring localized stuff in critical section

2020-06-06 Thread David Faure
This revision was automatically updated to reflect the committed changes.
Closed by commit R285:c315685f0e09: Dont invoke qstring localized stuff 
in critical section (authored by dfaure).

REPOSITORY
  R285 KCrash

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29809?vs=83231=83233

REVISION DETAIL
  https://phabricator.kde.org/D29809

AFFECTED FILES
  src/kcrash.cpp

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29814: Fix segfault on no restart args

2020-06-06 Thread David Faure
dfaure added a comment.


  This question is still without an answer: "Can you explain how to trigger 
this crash in the first place? Which application triggers it?"

REPOSITORY
  R285 KCrash

REVISION DETAIL
  https://phabricator.kde.org/D29814

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29809: Don't invoke qstring localized stuff in critical section

2020-06-06 Thread David Faure
dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R285 KCrash

BRANCH
  for-upstream

REVISION DETAIL
  https://phabricator.kde.org/D29809

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29814: Fix segfault on no restart args

2020-06-06 Thread David Faure
dfaure added a comment.


  This commit seems to include the changes requested in the other review...

REPOSITORY
  R285 KCrash

REVISION DETAIL
  https://phabricator.kde.org/D29814

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29809: Don't invoke qstring localized stuff in critical section

2020-06-06 Thread David Faure
dfaure added a comment.


  The changes you mention don't appear in phabricator.
  
  I suggest amending your commit locally (so there's only one) and then
  
arc diff HEAD~

REPOSITORY
  R285 KCrash

REVISION DETAIL
  https://phabricator.kde.org/D29809

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D18915: Fix batchrename changing extension to lower case

2020-06-02 Thread David Faure
dfaure added a comment.


  The fix will be in 5.15.1.
  Can you add Qt version checks if you intend to keep the KIO workaround?
  Or we just say it's fixed in Qt and we discard this. Your choice. But the 
unittest would make sense to keep (with Qt version checks, if there's no 
workaround).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18915

To: cfoster, #dolphin, #frameworks, abalaji, dfaure
Cc: cfeck, bruns, ngraham, elvisangelaccio, chinmoyr, kde-frameworks-devel, 
LeGast00n, cblack, michaelh


D18915: Fix batchrename changing extension to lower case

2020-06-01 Thread David Faure
dfaure added a comment.


  The fix landed in dev.
  
  I now uploaded a backport to 5.15 for review.
  https://codereview.qt-project.org/c/qt/qtbase/+/302532

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18915

To: cfoster, #dolphin, #frameworks, abalaji, dfaure
Cc: cfeck, bruns, ngraham, elvisangelaccio, chinmoyr, kde-frameworks-devel, 
LeGast00n, cblack, michaelh


D26113: Places: Use Solid::Device::DisplayName for DisplayRole

2020-05-30 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Thanks!

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D26113

REVISION DETAIL
  https://phabricator.kde.org/D26113

To: meven, #frameworks, ngraham, dfaure
Cc: dfaure, feverfew, bruns, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham


D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

2020-05-29 Thread David Faure
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  I have modified DesktopExecParser in kio commit 
d1e9325fba37eddb9cb44173edfc1fee122a0c57 
 to 
return an error string so that its users don't need to do the work again of 
figuring out what went wrong (see DesktopExecParser::errorMessage()).
  I just realized that this would be very useful here as well. AFAICS the 
current patch only works if the user types a full path, not with a relative 
path.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29445

To: broulik, #frameworks, #vdg, dfaure
Cc: ahmadsamir, dfaure, apol, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns


D26113: Places: Use Solid::Device::DisplayName for DisplayRole

2020-05-29 Thread David Faure
dfaure added a comment.


  Please check that kfileplacesmodeltest and kfileplacesviewtest still pass.
  
  (they fail here with baloosearch: stuff for some reason, I didn't 
investigate; but it passes on CI)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D26113

To: meven, #frameworks, ngraham
Cc: dfaure, feverfew, bruns, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham


D29814: Fix segfault on no restart args

2020-05-29 Thread David Faure
dfaure added a comment.


  Can you explain how to trigger this crash in the first place? Which 
application triggers it?

INLINE COMMENTS

> jpalecek wrote in kcrash.cpp:272
> Well I was thinking about `resize()`, then found there wasn't any. However, 
> if we want to be clear, maybe `args = { QString() }` would be the best?
> 
> (In reality, the effect should be `args = { filePath }`, but that comes with 
> the next line.)

I like your suggestion.

REPOSITORY
  R285 KCrash

REVISION DETAIL
  https://phabricator.kde.org/D29814

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29814: Fix segfault on no restart args

2020-05-27 Thread David Faure
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  I'm a bit confused by the bug this is fixing. Surely this doesn't happen to 
all cases of crashes without autorestart enabled??
  
  Also, it sounds like a null check might be enough.

INLINE COMMENTS

> kcrash.cpp:107
>  static char *s_appPath = nullptr;
>  static int s_autoRestartArgc = 0;
> +static char **s_autoRestartCommandLine = new char*[1]{ nullptr };

Should this become 1 then? Otherwise the `for` loop won't delete that new `new 
char*[1]`

> kcrash.cpp:272
> +if (args.isEmpty()) // edge case: tst_QX11Info::startupId does 
> QApplication app(argc, nullptr)...
> +args.insert(0, QString());
> +

insert(0) reads like prepend, but args is empty anyway, why not just use append?

> kcrash.cpp:275
> +args[0] = filePath; // replace argv[0] with full path above
> +for (int arg = 0; arg < s_autoRestartArgc; arg++)
> +delete [] s_autoRestartCommandLine[arg];

We use { ... } braces also around single-line statements in KF5.

Can you do the same for the `if` above too?

REPOSITORY
  R285 KCrash

REVISION DETAIL
  https://phabricator.kde.org/D29814

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29810: Don't use the setenv function after fork

2020-05-27 Thread David Faure
dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R285 KCrash

BRANCH
  for-upstream

REVISION DETAIL
  https://phabricator.kde.org/D29810

To: jpalecek, #frameworks, dfaure
Cc: bruns, apol, anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham


D29809: Don't invoke qstring localized stuff in critical section

2020-05-27 Thread David Faure
dfaure added a comment.


  Makes sense; just two minor things.

INLINE COMMENTS

> kcrash.cpp:95
> +#ifdef Q_OS_LINUX
> +QByteArray socketpath;
> +#endif

prepend `static`, it's only used in this file.

> kcrash.cpp:662
>  
> -// Create socket path to transfer ptrace scope and open connection
> -const QByteArray socketpath = QFile::encodeName(
> -
> QStringLiteral("%1/kcrash_%2").arg(QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation))
> -  .arg(getpid()));
> +using KCrash::socketpath;
>  int sockfd = openDrKonqiSocket(socketpath);

It would be more consistent with the other global vars to name it `s_socketpath`
(and outside the KCrash namespace)

REPOSITORY
  R285 KCrash

REVISION DETAIL
  https://phabricator.kde.org/D29809

To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29668: Do not reject icon theme dir with invalid context/type.

2020-05-26 Thread David Faure
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kicontheme.cpp:761
>  } else if (tmp.isEmpty()) {
>  // do nothing. key not required
>  } else {

(OK, if this key is not required then surely unknown values are fine too..)

> kicontheme.cpp:774
>  qWarning() << "Invalid Type=" << tmp << "line for icon theme: " << 
> constructFileName(QString());
> -return;
> +mType = KIconLoader::Threshold;
>  }

So, if index.theme says `Type=Fixed`, why did you change this line? It should 
work without this second change, only the first one seems useful.

REPOSITORY
  R302 KIconThemes

REVISION DETAIL
  https://phabricator.kde.org/D29668

To: xuetianweng, #frameworks, dfaure, apol
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns


Re: Recent breakage in kwallet

2020-05-26 Thread David Faure
Thanks for the pointer.

Pretty sure it's wrong, but I'll reply there.

On mardi 26 mai 2020 09:31:26 CEST Marco Martin wrote:
> would the recent patch
> https://invent.kde.org/frameworks/kwallet/-/merge_requests/1
> fix anything?
> 
> On Sat, May 16, 2020 at 10:40 AM David Faure  wrote:
> > On vendredi 15 mai 2020 12:43:37 CEST Marco Martin wrote:
> > > Hi all,
> > > Recently, the package of KWallet framework for neon (devel unstable,
> > > package built out of current master state) seems to be broken: kwallet
> > > now can't open anymore wallets previously created (it works if the
> > > local kwallet data is deleted and recreated)
> > > the weird thing is that it seems that with a manual build it works
> > > instead.
> > > The only kinda suspicious commit is
> > > 850219f83e7d746 fix compilation with -Werror=undef
> > > which seems to fix.. big endian for everyone?
> > 
> > It was supposed to only fix compiler warnings without changing behaviour
> > at
> > all.
> > 
> > #if Q_BYTE_ORDER == Q_BIG_ENDIAN
> > when neither is defined, evaluates to #if 0 == 0  (and warns)
> > 
> > I defined both to 1, for the same result, but so that -Wundef doesn't
> > warn.
> > 
> > > still no idea why is broken only the distro-built package
> > 
> > Any neon-specific patches to kwallet?
> > 
> > --
> > David Faure, fa...@kde.org, http://www.davidfaure.fr
> > Working on KDE Frameworks 5


-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: KDE CI: Frameworks » ktexteditor » kf5-qt5 SUSEQt5.14 - Build # 19 - Still Unstable!

2020-05-25 Thread David Faure
 src.export
> > 0% (0/4)
> > 0% (0/4)
> > 0% (0/121)
> > 0% (0/156)
> > 
> > src.include.ktexteditor
> > 93% (14/15)
> > 93% (14/15)
> > 84% (187/222)
> > 55% (125/226)
> > 
> > src.inputmode
> > 100% (8/8)
> > 100% (8/8)
> > 63% (192/304)
> > 51% (39/77)
> > 
> > src.mode
> > 88% (7/8)
> > 88% (7/8)
> > 36% (378/1050)
> > 16% (146/897)
> > 
> > src.part
> > 0% (0/1)
> > 0% (0/1)
> > 0% (0/7)
> > 100% (0/0)
> > 
> > src.printing
> > 0% (0/4)
> > 0% (0/4)
> > 0% (0/862)
> > 0% (0/278)
> > 
> > src.render
> > 100% (7/7)
> > 100% (7/7)
> > 77% (949/1226)
> > 67% (611/916)
> > 
> > src.schema
> > 29% (2/7)
> > 29% (2/7)
> > 1% (19/1468)
> > 1% (6/625)
> > 
> > src.script
> > 100% (16/16)
> > 100% (16/16)
> > 67% (698/1038)
> > 53% (212/403)
> > 
> > src.search
> > 100% (7/7)
> > 100% (7/7)
> > 74% (/1503)
> > 63% (590/932)
> > 
> > src.spellcheck
> > 57% (4/7)
> > 57% (4/7)
> > 7% (82/1205)
> > 3% (19/696)
> > 
> > src.swapfile
> > 50% (1/2)
> > 50% (1/2)
> > 32% (119/374)
> > 32% (64/203)
> > 
> > src.syntax
> > 88% (7/8)
> > 88% (7/8)
> > 64% (488/761)
> > 36% (206/576)
> > 
> > src.undo
> > 100% (5/5)
> > 100% (5/5)
> > 88% (652/741)
> > 73% (276/378)
> > 
> > src.utils
> > 95% (36/38)
> > 95% (36/38)
> > 63% (2388/3796)
> > 45% (1009/2262)
> > 
> > src.variableeditor
> > 0% (0/5)
> > 0% (0/5)
> > 0% (0/580)
> > 0% (0/108)
> > 
> > src.view
> > 88% (15/17)
> >     88% (15/17)
> > 57% (3782/6599)
> > 43% (1661/3880)
> > 
> > src.vimode
> > 100% (30/30)
> > 100% (30/30)
> > 81% (1898/2332)
> > 61% (979/1599)
> > 
> > src.vimode.config
> > 0% (0/1)
> > 0% (0/1)
> > 0% (0/120)
> > 0% (0/76)
> > 
> > src.vimode.emulatedcommandbar
> > 100% (12/12)
> > 100% (12/12)
> > 98% (896/916)
> > 90% (588/652)
> > 
> > src.vimode.modes
> > 100% (10/10)
> > 100% (10/10)
> > 87% (3266/3761)
> > 79% (1884/2374)
> > 
> > Links:
> > --
> > [1]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .14/19/artifact/abi-compatibility-results.yaml [2]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .14/19/artifact/acc/KF5TextEditor-5.71.0.xml [3]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .14/19/artifact/compat_reports/KF5TextEditor_compat_report.html [4]
> > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5
> > .14/19/artifact/logs/KF5TextEditor/5.71.0/log.txt


-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

2020-05-23 Thread David Faure
dfaure added a comment.


  @meven you're confusing me with my clone @ervin.

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D28590

To: meven, #frameworks, bruns, sitter, dfaure
Cc: dfaure, broulik, ervin, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns


D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

2020-05-23 Thread David Faure
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> fstabdevice.cpp:73
> +if (m_displayName.isEmpty()) {
> +QStringList currentMountPoints = 
> FstabHandling::currentMountPoints(m_device);
> +if (currentMountPoints.isEmpty()) {

`const`

> fstabdevice.cpp:75
> +if (currentMountPoints.isEmpty()) {
> +QStringList mountPoints = FstabHandling::mountPoints(m_device);
> +m_displayName = mountPoints.isEmpty() ? m_description : 
> mountPoints.first();

`const` so that first() doesn't detach.

> device.h:93
> +/**
> +  * Retrieves the display Name to use for this device.
> +  * Same as description when not defined.

Why "Name" with an uppercase letter?

> device.h:96
> +  *
> +  * @return the display Name
> +  * @since 5.71

(same)

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D28590

To: meven, #frameworks, bruns, sitter, dfaure
Cc: dfaure, broulik, ervin, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns


D28765: KSettings::Dialog: add support for KPluginInfos without a KService

2020-05-20 Thread David Faure
dfaure added a comment.


  https://invent.kde.org/frameworks/kcmutils/-/merge_requests/2

REPOSITORY
  R295 KCMUtils

REVISION DETAIL
  https://phabricator.kde.org/D28765

To: dfaure, pino, broulik, mart, davidedmundson, ngraham, svuorela
Cc: ahmadsamir, rikmills, wbauer, kossebau, svuorela, cblack, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D29738: Fix service file specifying 'Run in terminal' giving an error code 100

2020-05-17 Thread David Faure
dfaure added a comment.


  (Intrusive) fix is up at 
https://invent.kde.org/frameworks/kio/-/merge_requests/3

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29738

To: marten, #frameworks, dfaure
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29738: Fix service file specifying 'Run in terminal' giving an error code 100

2020-05-17 Thread David Faure
dfaure added a comment.


  Well spotted. Indeed, when using kioexec (because of the DeleteTemporaryFiles 
option) we no longer detect non-existing executables. I'll look into fixing 
this.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29738

To: marten, #frameworks, dfaure
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-17 Thread David Faure
dfaure added a comment.


  Yeah, the full story is that I pushed to git.kde.org, reverted because I 
changed my mind, then the sysadmins told me I wasn't supposed to push to 
git.kde.org at all (it was blocked for everyone, but I have sysadmin privileges 
for historical and bus-factor reasons). We realized there was nothing to sync 
to gitlab anyway, since I had simply done push+push+revert+revert.
  
  I think I want to add ApplicationLauncherJob(/*no service arg*/) and make it 
pop up the open with dialog and launch the selected application.

REPOSITORY
  R241 KIO

BRANCH
  2020_05_displayOpenWithDialog

REVISION DETAIL
  https://phabricator.kde.org/D29558

To: dfaure, ahmadsamir, broulik, svuorela
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-17 Thread David Faure
dfaure reopened this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Not committed after all.

REPOSITORY
  R241 KIO

BRANCH
  2020_05_displayOpenWithDialog

REVISION DETAIL
  https://phabricator.kde.org/D29558

To: dfaure, ahmadsamir, broulik, svuorela
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29800: Fix URL being passed as argument when launching a .desktop file

2020-05-16 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Thanks for the unittest and fix!
  
  Oversight on my side indeed.

INLINE COMMENTS

> openurljobtest.h:58
>  void takeOverAfterMimeTypeFound();
> +void runDeskopFileDirectly();
>  

Typo: Desktop

REPOSITORY
  R241 KIO

BRANCH
  execfi

REVISION DETAIL
  https://phabricator.kde.org/D29800

To: fvogt, dfaure, marten
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29782: [StatJob] Make mostLocalUrl ignore remote (ftp, http...etc) URLs

2020-05-16 Thread David Faure
dfaure added a comment.


  OK, I picked testtrash because kio_trash *is* a :local protocol. If it 
doesn't use UDS_LOCAL_PATH, at least it means the job will actually go through 
(no early filtering out). That's at least interesting to test too.
  
  Good that http will give no error. That's something else the test can check 
:-)
  
  kio_remote is part of kio and sets UDS_LOCAL_PATH, maybe it can be used for 
testing the case where we'll actually find a local path. I'm afraid there isn't 
much testing of that kioslave though, so this sounds like a bigger effort.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29782

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-16 Thread David Faure
dfaure added a comment.


  OK I'm having second thoughts about this. Because of Windows, and because of 
the case of multiple URLs.
  
  There's "displaying an open with dialog because we couldn't find any app, 
after a left-click on a file"
  and there's "displaying an open with dialog because the user explicitly 
clicked on Open With..."
  
  On Windows, it's OK if the first one falls back to QDesktopServices::openUrl 
(i.e. actually launch an app, no open with dialog)
  while the second case must show a dialog (KRun::displayOpenWithDialog had 
code for the Windows native open-with dialog).
  
  Also OpenUrlJob can only ever support a single URL while 
displayOpenWithDialog supports multiple URLs.
  
  Maybe I should move all this over to ApplicationLauncherJob, which supports 
multiple URLs (adding a ctor without a KService). Or to yet another job class...
  We lose mimetype determination... but that's what we didn't have in 
KRun::displayOpenWithDialog either.
  
  Input welcome...

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29558

To: dfaure, ahmadsamir, broulik, svuorela
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-16 Thread David Faure
dfaure closed this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29558

To: dfaure, ahmadsamir, broulik, svuorela
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-16 Thread David Faure
dfaure updated this revision to Diff 82992.
dfaure marked 2 inline comments as done.
dfaure added a comment.


  API docs: remove trailing dot, mention 2 default values

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29558?vs=82358=82992

BRANCH
  2020_05_displayOpenWithDialog

REVISION DETAIL
  https://phabricator.kde.org/D29558

AFFECTED FILES
  autotests/openurljobtest.cpp
  autotests/openurljobtest.h
  src/gui/openurljob.cpp
  src/gui/openurljob.h

To: dfaure, ahmadsamir, broulik, svuorela
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29782: [StatJob] Make mostLocalUrl ignore remote (ftp, http...etc) URLs

2020-05-16 Thread David Faure
dfaure added a comment.


  Can you add a unittest for a KIO::mostLocalUrl() in testtrash.cpp (which is 
:local, so it should work)
  and another one in jobtest.cpp for a http URL (e.g. to test which error code 
we're getting, if any?)
  
  + call mostLocalUrl() on a "normal" StatJob in testtrash.cpp
  
  Thanks.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29782

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


Re: Request for ktexteditor patch release

2020-05-16 Thread David Faure
On vendredi 15 mai 2020 15:37:00 CEST Nate Graham wrote:
> If we're adding stuff to a 5.70 bugfix point release,
> https://cgit.kde.org/kio.git/commit/?id=8769b6360d87c1b688eac4d0ce97594351ba
> d13c is another good candidate, which fixes a recent regression
> (https://bugs.kde.org/show_bug.cgi?id=421213).

Indeed. Sigh. 5.70.0 was really a bad release...

kio v5.70.1
9f64e5213460c258515705f07b97877fd34fe52a
b440f1a9408cd90ecc8a6bbeecbc4bde12365547bd8ca55c69619a2e659363b8  
sources/kio-5.70.1.tar.xz

+ info added to https://kde.org/info/kde-frameworks-5.70.0.php

Packagers, sorry for all this. Here's a changelog for the three 5.70.1 packages

### Plasma Framework

Fix crash due to disconnect of all signals in IconItem (bug 421170)

### KIO

CopyJob: Fix copying/moving files into a symlink to a directory (bug 421213)

### KTextEditor

KTextEditor global view setting changes ignored after session reopening (Kate, 
KDevelop) (bug 421375)


-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: Recent breakage in kwallet

2020-05-16 Thread David Faure
On vendredi 15 mai 2020 12:43:37 CEST Marco Martin wrote:
> Hi all,
> Recently, the package of KWallet framework for neon (devel unstable,
> package built out of current master state) seems to be broken: kwallet
> now can't open anymore wallets previously created (it works if the
> local kwallet data is deleted and recreated)
> the weird thing is that it seems that with a manual build it works instead.
> The only kinda suspicious commit is
> 850219f83e7d746 fix compilation with -Werror=undef
> which seems to fix.. big endian for everyone?

It was supposed to only fix compiler warnings without changing behaviour at 
all.

#if Q_BYTE_ORDER == Q_BIG_ENDIAN
when neither is defined, evaluates to #if 0 == 0  (and warns)

I defined both to 1, for the same result, but so that -Wundef doesn't warn.

> still no idea why is broken only the distro-built package

Any neon-specific patches to kwallet?

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D29610: [kio_file] Handle renaming file 'A' to 'a' on FAT32 filesystems

2020-05-16 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  OK, at least we'll have moved the code to the right place :-)

INLINE COMMENTS

> ahmadsamir wrote in file_unix.cpp:1052
>   const QByteArray dest1 = "/mnt/fat32/A";
>   const QByteArray dest2 = "/mnt/fat32/a";
>   QT_STATBUF buff_dest;
>   qDebug() << QT_LSTAT(dest1, _dest);
>   qDebug() << QT_LSTAT(dest2, _dest);
> 
> IIUC, from FAT32 POV, 'A' exists if 'a' exists and vice versa.

You're right.

REPOSITORY
  R241 KIO

BRANCH
  l-qfile-rename (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29610

To: ahmadsamir, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29787: Fix krununittest

2020-05-16 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Let's hope the CI has xterm installed I doubt it. Maybe we'll need to 
pick "ls" instead, even if that will look weird.

REPOSITORY
  R241 KIO

BRANCH
  l-terminal (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29787

To: ahmadsamir, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-15 Thread David Faure
dfaure added a comment.


  That's the point, a NFS mount *is* a local URL, so we do use QFile for it. 
And then it takes forever because the kernel has to talk over a socket to 
answer us.
  Yes this is horrendous. I hate network mounts :-)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29385

To: dfaure, ahmadsamir, broulik, meven, kossebau, davidedmundson, nicolasfella, 
svuorela
Cc: feverfew, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-15 Thread David Faure
dfaure added a comment.


  It's 3 times faster on my local SSD.
  
  Now think of a NFS mount on a server from another country

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29385

To: dfaure, ahmadsamir, broulik, meven, kossebau, davidedmundson, nicolasfella, 
svuorela
Cc: feverfew, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


Re: Request for ktexteditor patch release

2020-05-15 Thread David Faure
On vendredi 15 mai 2020 11:01:04 CEST Friedrich W. H. Kossebau wrote:
> Hi,
> 
> I would like to ask for a 5.70 patch release for ktexteditor, with
> 972da14f486a83556e192d09bb18a2500728895a cherry-picked.
> 
> Not a crasher, but preventing the pickup of any global view setting changes
> after a kate/kdevelop session close & open cycle for existing document
> views, which has been hit and reported by a few people already the last
> days.

Thanks for the notification. Done:

ktexteditor v5.70.1
5e6ea19f95a36e21473c00a8d30cbea0f150a13f
c7b568e75c147161992f8875fe36fb46885bccddb63c22edaf81071583f4204c  
sources/ktexteditor-5.70.1.tar.xz

Please add a description of the bug in www/info/kde-frameworks-5.70.0.php (or 
give me a patch if you can't push).

Also, please make sure to write a unittest for this regression so we catch it 
next time.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





D29767: CopyJob: Check if destination dir is a symlink

2020-05-15 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Thanks for the quick fix and for the test! Just minor issues in comments, 
feel free to push after fixing.

INLINE COMMENTS

> jobtest.cpp:626
> +// just the same as copyDirectoryToSamePartition, but this time dest is 
> a symlink.
> +// So we get a file in the symblink dir, 
> "dirFromHome_symlink/dirFromHome" and
> +// "dirFromHome_symOrigin/dirFromHome"

Typo: symblink

> jobtest.cpp:633
> +createTestDirectory(origSymlink);
> +//createTestSymlink(origSymlink, targetSymlink.toLatin1());
> +

remove

> jobtest.cpp:648
> +
> +// file is visible in both symlinks ends
> +QVERIFY(QFileInfo(origSymlink + "/dirFromHome").isDir());;

there is only one symlink ;-)

Maybe "The file is visible in both places, due to the symlink" ?

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D29767

To: meven, #frameworks, dfaure, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29738: Fix service file specifying 'Run in terminal' giving an error code 100

2020-05-14 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Nice! In addition to the bugfix, calling resultingArguments() only once is a 
definite improvement, given everything that happens in there. I didn't realize 
we were calling it twice.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29738

To: marten, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


Re: Request for plasma framework patch release

2020-05-14 Thread David Faure
On jeudi 14 mai 2020 15:24:57 CEST David Faure wrote:
> On jeudi 14 mai 2020 13:56:50 CEST David Edmundson wrote:
> > We seem to have a common crasher newly introduced in plasma-framework. A
> > dozen reports in a few days.
> > 
> > Can I ask for a plasma-framework 5.70.1. with
> > c215c54eced5bd0b195c208dd72bb580e65f8fe4 cherry-picked.
> > 
> > Sorry.
> 
> Here you go.
> 
> plasma-framework v5.70.1
> bfea43379bc364bc1bb0719d9a1a73a57284214d
> e28884da2e3389513d96794f71fdfa22a8b4ba62396b7d061b4f18d9b5549d19 
> sources/plasma-framework-5.70.1.tar.xz

+ mention added to https://kde.org/info/kde-frameworks-5.70.0.php

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





Re: Request for plasma framework patch release

2020-05-14 Thread David Faure
On jeudi 14 mai 2020 13:56:50 CEST David Edmundson wrote:
> We seem to have a common crasher newly introduced in plasma-framework. A
> dozen reports in a few days.
> 
> Can I ask for a plasma-framework 5.70.1. with
> c215c54eced5bd0b195c208dd72bb580e65f8fe4 cherry-picked.
>
> Sorry.

Here you go.

plasma-framework v5.70.1
bfea43379bc364bc1bb0719d9a1a73a57284214d
e28884da2e3389513d96794f71fdfa22a8b4ba62396b7d061b4f18d9b5549d19  
sources/plasma-framework-5.70.1.tar.xz

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5





  1   2   3   4   5   6   7   8   9   10   >