[Heads-up] ECM 5.96 KDECompilerSettings will add -Werror=undef to GCC/Clang build flags

[Ahmad Samir]

This[1] should catch undefined preprocessor expressions, i.e. instead of
showing a warning if FOO_BAR_H is undefined:
 #if FOO_BAR_H

make the build fail with an error instead.

This combined with using #cmakedefine01 (instead of #cmakedefine), could
catch things like:
https://invent.kde.org/pim/kdepim-runtime/-/commit/05a0e5bbfbb06ed6aa88640f25f563cf7e5b1330


[1]https://invent.kde.org/frameworks/extra-cmake-modules/-/commit/25bbb2a4776d36b542857709bb0448a7ea030b59

Regards
Ahmad Samir

Re: KF 5.95-rc1 delayed

[Ahmad Samir]

On 5/6/22 21:26, Ahmad Samir wrote:
[...]


Could be an issue due to change in 
https://invent.kde.org/frameworks/knewstuff/-/merge_requests/178
KNewStuff header install locations:
https://invent.kde.org/utilities/yakuake/-/merge_requests/61
https://invent.kde.org/games/ksirk/-/merge_requests/9




KnewStuff issue was solved by backporting the fixes to yakuake and ksirk in 
release/22.04.

--
Ahmad Samir

Re: KF 5.95-rc1 delayed

[Ahmad Samir]

On 5/6/22 20:57, Nicolas Fella wrote:

On 6/5/22 20:53, David Faure wrote:

Hi everyone,

I'm delaying the tagging of KF 5.95 rc1 until:

- the regression in kirigami is fixed
https://invent.kde.org/frameworks/kirigami/-/merge_requests/548#note_462793

- the regression in kio (trash) is fixed
https://invent.kde.org/frameworks/kio/-/merge_requests/854

Please let me know if there are other blocker issues.


Hi,

while we're at it we might also add
https://bugs.kde.org/show_bug.cgi?id=454635 to the list of blockers.

The relevant change that caused it is from plasma-framework:
https://invent.kde.org/frameworks/plasma-framework/-/merge_requests/500

Cheers

Nico



Could be an issue due to change in 
https://invent.kde.org/frameworks/knewstuff/-/merge_requests/178
KNewStuff header install locations:
https://invent.kde.org/utilities/yakuake/-/merge_requests/61
https://invent.kde.org/games/ksirk/-/merge_requests/9


--
Ahmad Samir

5.94 respin request: Commit fixing bug in KFileWidget

[Ahmad Samir]

Due to a (my) previous refactoring in KFileWidget the "show full path" action in KUrlNavigator 
didn't work any more.


https://invent.kde.org/frameworks/kio/-/commit/f2ef14ad1be7e9c87c3a21569ff94a99cd5d4c93

Thanks,

Ahmad Samir

Re: RFC: Frameworks headers install location

[Ahmad Samir]

On 12/4/22 02:17, Aleix Pol wrote:

Back when we did KF5, what we tried to do was mainly to make sure kde4
code still compiled after the big split.

I think that stanradrising how they're placed within submodules of
include/KF6 is a good idea.

The fact that headers from some repositories can be included using the
project name is more an artifact of time than a feature. Those that
are meant to be used that way (e.g. Plasma or Purpose), are already
namespaced within "include/KF*/".

Ensuring that no KF6 project brings "include/KF6" as its include
directories will be a useful cleanup.
Aleix



Thanks for the feedback.

We discussed that again in the previous KF6 meeting[1], and since there are no objections so far, 
we'll go ahead and put that into motion, starting with[2].


[1] 
https://invent.kde.org/teams/frameworks-devs/kf6-workboard/-/issues/3#note_431116
[2] https://invent.kde.org/frameworks/syntax-highlighting/-/merge_requests/305

Thanks,
Ahmad Samir


On Tue, Apr 5, 2022 at 4:19 PM Ahmad Samir  wrote:


Hello.

- In KF5, ECM, magically, added /usr/include/KF5/ to the CMake targets 
interface include directories
(IIUC for reasons of backwards compatibility, which was necessary in the 4-5 
transition era, I could
be wrong because I wasn't around at that time :))

- While building KF against Qt6, this suddenly broke building in some modules 
due to #include
directives not finding the headers, as /usr/include/KF6/ didn't have the same 
magic-injection
treatment as KF5

- To fix the issue, proper paths had to be added to targets interface include 
directories so that
module A linking against module B will have the proper include paths to search 
for headers

- The typical include dir layout for a KF module is:
/usr/include/KF*/ModuleName/{ForwardingHeaderA,headera.h}

e.g. KCMutils, #include , and /usr/include/KF5/KCMUtils added to 
KCMutils target
include dirs.

- If there is a namespace, the original plan was to guard the include paths, by 
making them match
the C++ namespaces, e.g.:
/usr/include/KF*/ModuleName/NameSpace/ForwardingHeaderA
/usr/include/KF*/ModuleName/namespace/headera.h

e.g. KSynatxHighlighting:
/usr/include/KF*/KSyntaxHighlighting/
/usr/include/KF*/KSyntaxHighlighting/KSyntaxHighlighting/Repository
/usr/include/KF*/KSyntaxHighlighting/ksyntaxhighlighting/repository.h


There are some issues with the namespace use-case:
- On case-insensitive and/or case-preserving filesystems (which still exist in 
2022...) extra care
has to be taken so that installation actually works, as you can't have two dirs 
in the same path
with the same name but different cases
- Compiler warnings when using e.g. #include  
if the dir that was
installed first was ksyntaxhighlighting, and all files ended up there (and 
vice-versa, e.g. if the
CamelCase dir got installed first #include  
would give a warning)
- A more complicated layout

The proposal is to change the layout when there is a namespace:
- Only have one dir, /usr/include/KF*/ModuleName/NameSpace/, where all headers 
(ForwadingHeaders and
lowercase.h ones) are installed


Pros:
- less complicated layout/setup (simpler CMake code)
- all KDE namespaces are CamelCase, we don't have any that are lower case
- we promote/encourage using FowardingHeaders everywhere; that's what we use in 
our own code and
what the API docs advise to use

Cons:
- The case not matching when using , but as I 
said above the
argument for this is mitigated by the fact that all our namespaces are CamelCase

If we agree with that change this will be for KF6 so as not to break source 
compatibility for KF5.

Best regards,
Ahmad Samir

RFC: Frameworks headers install location

[Ahmad Samir]

Hello.

- In KF5, ECM, magically, added /usr/include/KF5/ to the CMake targets interface include directories 
(IIUC for reasons of backwards compatibility, which was necessary in the 4-5 transition era, I could 
be wrong because I wasn't around at that time :))


- While building KF against Qt6, this suddenly broke building in some modules due to #include 
directives not finding the headers, as /usr/include/KF6/ didn't have the same magic-injection 
treatment as KF5


- To fix the issue, proper paths had to be added to targets interface include directories so that 
module A linking against module B will have the proper include paths to search for headers


- The typical include dir layout for a KF module is:
/usr/include/KF*/ModuleName/{ForwardingHeaderA,headera.h}

e.g. KCMutils, #include , and /usr/include/KF5/KCMUtils added to KCMutils target 
include dirs.


- If there is a namespace, the original plan was to guard the include paths, by making them match 
the C++ namespaces, e.g.:

/usr/include/KF*/ModuleName/NameSpace/ForwardingHeaderA
/usr/include/KF*/ModuleName/namespace/headera.h

e.g. KSynatxHighlighting:
/usr/include/KF*/KSyntaxHighlighting/
/usr/include/KF*/KSyntaxHighlighting/KSyntaxHighlighting/Repository
/usr/include/KF*/KSyntaxHighlighting/ksyntaxhighlighting/repository.h


There are some issues with the namespace use-case:
- On case-insensitive and/or case-preserving filesystems (which still exist in 2022...) extra care 
has to be taken so that installation actually works, as you can't have two dirs in the same path 
with the same name but different cases
- Compiler warnings when using e.g. #include  if the dir that was 
installed first was ksyntaxhighlighting, and all files ended up there (and vice-versa, e.g. if the 
CamelCase dir got installed first #include  would give a warning)

- A more complicated layout

The proposal is to change the layout when there is a namespace:
- Only have one dir, /usr/include/KF*/ModuleName/NameSpace/, where all headers (ForwadingHeaders and 
lowercase.h ones) are installed



Pros:
- less complicated layout/setup (simpler CMake code)
- all KDE namespaces are CamelCase, we don't have any that are lower case
- we promote/encourage using FowardingHeaders everywhere; that's what we use in our own code and 
what the API docs advise to use


Cons:
- The case not matching when using , but as I said above the 
argument for this is mitigated by the fact that all our namespaces are CamelCase


If we agree with that change this will be for KF6 so as not to break source 
compatibility for KF5.

Best regards,
Ahmad Samir

KIdleTime commit fixing crash

[Ahmad Samir]

Hello.

Please include:

https://invent.kde.org/frameworks/kidletime/-/commit/cd5040684723b87c7ba5b7cc1b1a63402902a641

in 5.93.

Thanks,
Ahmad Samir

Re: KF6 meeting notes 2022-01-04

[Ahmad Samir]

On 4/1/22 19:07, Volker Krause wrote:

https://phabricator.kde.org/T15127 (Qt6 build status update):
- 60+ modules build
- Kate (without plugins) with Breeze style and platform file dialogs mostly
working
- Kirigami might not be doable without branching (Button.icon,
QtGraphicalEffects)
- CI setup in progress
- in some places includes with the framework prefix don't seem to work anymore
(ie.  vs ), any idea where those extra include
directories came from/got lost?

https://invent.kde.org/frameworks/kwallet/-/merge_requests/22
- was launching kwalletmanager via deprecated KService API
- KIO::ApplicationLaunchJob becomes possible after a QtKeyChain migration of
KIO > - QProcess as temporary porting scaffolding is an option, but how to 
ensure
this doesn't stay around for KF6?
- make this conditional on KF version < 6.0? needs to fail hard when disabled,
not silently drop code for the release



Actually on second thought, David F's plan is to move (at least some of) the classes from kiogui to 
KService, that includes ApplicationLauncherJob and CommandLauncherJob, so it can be fixed by using 
those from KService in KF6.



https://invent.kde.org/frameworks/kio/-/merge_requests/677
- same solution might be applicable here

Timeline
- KF6 branching is not too far away given the current speed
- so we'll need to discuss whether that already makes sense, or whether there
are hard KF blockers to sort out first, and whether that's too fast for apps/
Plasma and we need to help those catch up first
- full branching vs. only branch some modules

https://phabricator.kde.org/T14316 - QtKeyChain porting
- David E found a lot of problems for porting
- QtKeyChain doesn't support KWallet's concept of two folders - might be a
problem limited to Plasma/KF users, applications use their own top-level entry
- use a separator char in the key name for implementing backward compatible
two-level lookup in KWallet

https://phabricator.kde.org/T15140
- "using namespace" in a public header isn't a good idea
- check if typedef aliases are an option for this instead

https://invent.kde.org/frameworks/extra-cmake-modules/-/merge_requests/214
- ready to be merged

KDeclarative
- its KCM module contains generic code used outside of KCM context as well,
like the GridView
- Because we want to split up KDeclarative, it is probably better placed in
kirigami-addons, but we finally need to get that into KF and released.
- KCM specific QML files should be moved to KCMUtils

https://phabricator.kde.org/T13970
- can be closed

Topics to discuss in a slightly larger scope:
- solution for the QTextCodec problem
- determine hard KF6 blocker tasks



--
Ahmad Samir

KF6 meeting notes

[Ahmad Samir]

Due to only a few making it to the meeting, we didn't discuss any major tasks.

An open question, are we going to have a meeting next week? What with Christmas 
and all?

Regards,
Ahmad Samir

Reminder: KF6 weekly meeting

[Ahmad Samir]

Hello.

As was said in the last meeting's notes, the time of the meeting has been changed to 17:00 CET (due 
to DST change in continental Europe), so that's 16:00 UTC.


Have a good day.
--
Ahmad Samir

Correction to git script to create a work branch

[Ahmad Samir]

This concerns this section:
https://community.kde.org/Infrastructure/GitLab#Using_work_branches_instead_of_forks

The first version of the script (written by me) had a stupid flaw, it didn't guard against being 
called with an empty arg; usually it should be used like this:

$ git work branch-name

if you end up using it without an arg, just `git work`, the setup of your git repo could get screwed 
up, for example, trying to create a branch afterwards, you could end up with:

warning: refname 'origin/master' is ambiguous.
warning: refname 'origin/master' is ambiguous.
fatal: Ambiguous object name: 'origin/master'.


Fortunately, the fix seems to be simple:
$ git show-ref master
a6b947711efdf83718ad4d5ffb384a2bd36e559d refs/heads/master
3ae9d7d897d248024d5bc060fe89f7c30154d4c6 refs/heads/origin/master
a6b947711efdf83718ad4d5ffb384a2bd36e559d refs/remotes/origin/master

$ git update-ref -d refs/heads/origin/master

see this excellent https://stackoverflow.com/a/26046933 post for explanation (I am not a git expert, 
but that worked, AFAICS).


Anyway, the script has been fixed to bail out early if it's called without an 
arg.

Sorry for the inconvenience. :)

--
Ahmad Samir

Debugging io-slaves

[Ahmad Samir]

The wiki has been updated recently 
https://community.kde.org/Guidelines_and_HOWTOs/Debugging/Debugging_IOSlaves#If_KDE_FORK_SLAVES_is_set


With KIO master, now we should be able to catch a kio-slave in gdb when slaves are forked (the 
current default), just like we were able to do with kdeinit (when io-slaves aren't forked).


Regards,
Ahmad Samir

Re: 5.87 respin request

[Ahmad Samir]

On 10/4/21 14:01, David Faure wrote:

On lundi 4 octobre 2021 13:22:22 CEST Ahmad Samir wrote:

On 10/3/21 08:29, Ahmad Samir wrote:

Please include
https://invent.kde.org/frameworks/kwidgetsaddons/-/commit/99d8dca607327ebf
0b6d2f3cff089207c61d7276

Fixes https://bugs.kde.org/show_bug.cgi?id=442332

Regards,
Ahmad Samir


And
https://invent.kde.org/frameworks/kwidgetsaddons/-/commit/3687f696a48b5917e
3edb5d1513543c0d9939de1

(Which replaces 99d8dca607327e with a supposedly better fix).


Yes this second commit makes more sense, the comment was wrong in the old
commit. But for users it should be all the same, so isn't 99d8dca607327e good
enough for 5.87 ?



Thinking about it, yes, disconnecting all signals before making our one, should 
have the same effect.

--
Ahmad Samir

Re: 5.87 respin request

[Ahmad Samir]

On 10/3/21 08:29, Ahmad Samir wrote:

Please include
https://invent.kde.org/frameworks/kwidgetsaddons/-/commit/99d8dca607327ebf0b6d2f3cff089207c61d7276

Fixes https://bugs.kde.org/show_bug.cgi?id=442332

Regards,
Ahmad Samir



And 
https://invent.kde.org/frameworks/kwidgetsaddons/-/commit/3687f696a48b5917e3edb5d1513543c0d9939de1

(Which replaces 99d8dca607327e with a supposedly better fix).

--
Ahmad Samir

5.87 respin request

[Ahmad Samir]

Please include 
https://invent.kde.org/frameworks/kwidgetsaddons/-/commit/99d8dca607327ebf0b6d2f3cff089207c61d7276


Fixes https://bugs.kde.org/show_bug.cgi?id=442332

Regards,
Ahmad Samir

Re: KF API Documentation proposed, small, addition

[Ahmad Samir]

On 09/09/2021 23:34, Friedrich W. H. Kossebau wrote:

Am Donnerstag, 9. September 2021, 23:23:29 CEST schrieb Ahmad Samir:

On 09/09/2021 22:47, Friedrich W. H. Kossebau wrote:

Am Donnerstag, 9. September 2021, 22:35:05 CEST schrieb Ahmad Samir:

On 09/09/2021 22:24, Friedrich W. H. Kossebau wrote:

Am Donnerstag, 9. September 2021, 21:45:33 CEST schrieb Ahmad Samir:

On 30/08/2021 16:35, Friedrich W. H. Kossebau wrote:

Thanks for pushing this here.

Am Montag, 30. August 2021, 14:17:42 CEST schrieb Ahmad Samir:
Open question:
in which places is it a good idea to use "code"-style with class names
and
method names? So when does readability suffer by too many changes in
the
formatting in a text body?

Looking e.g. at the Qt docs for a reference, they do not use
"code"-style
when referencing classes or methods from text, as well as in the
listing
of classes and methods. So I wonder if this is by design or just
historic?


They use QDoc, IIUC; and it looks like they recommend using \c
https://doc.qt.io/qt-5/04-qdoc-commands-textmarkup.html

at least that page suggests so.


Then our question was not "if" they have a command to markup things to
be
printed code-like, but "when" it should be used. And the examples they
give
(not sure though if exclusive or inclusive) are

"
variable names, user-defined class names, and C++ keywords (for example,
int and for)
".

So no mention of names of the methods, members, properties and classes
the
documentation is about (note the "user-defined"). And looking at the
existing Qt API documentation, I would guess the given list is rather
exclusive then, and \c with Qt is not to be used when referencing the
elements of the documented API itself (at least in flow text).

Myself I meanwhile rather think that this might be a good choice.
Imagine
how e.g. https://doc.qt.io/qt-6/qstring.html would look like if all text
elements referencing Qt classes or method names would be in code-style.
I
guess the reading flow in the flow text blocks would suffer a lot.


So one vote for and one vote against, we need a tie-breaker.


What I would like to see are some argument for why you want this change?
What is broken now, what does it improve?
Can you give an example where your proposal is applied and what the result
is, before and after?


I think it's useful to markup the method names, that makes reading the API
docs in text format in a header file easier, the same way marking up
true/false is useful, the same way syntax highlighting in most text editors
is useful.


I see, that is where you are coming from, it makes some sense there. But...

... it only makes sense for people looking at headers in a plain text editor
which also is capable of parsing doxygen comments and adding respective
highlighting.

.. for every other plain text viewers/editors, including on the gitlab file
viewer, it makes things harder to read.



I don't see how it's harder to read the API docs than '@c true' or '@param'
or any other doxygen command when viewed in an editor that doesn't highlight doxygen syntax 
properly; those same editors highlight C++ code properly?



.. it very possibly lowers the quality of the generated API docs, which should
be the main purpose of writing those API documentation snippets, no?

So im summary not convinced this would be a change for the better.

Cheers
Friedrich




Since no one else seems to care either way, I'll drop the matter.

Regards,
Ahmad Samir

Re: KF API Documentation proposed, small, addition

[Ahmad Samir]

On 09/09/2021 22:47, Friedrich W. H. Kossebau wrote:

Am Donnerstag, 9. September 2021, 22:35:05 CEST schrieb Ahmad Samir:

On 09/09/2021 22:24, Friedrich W. H. Kossebau wrote:

Am Donnerstag, 9. September 2021, 21:45:33 CEST schrieb Ahmad Samir:

On 30/08/2021 16:35, Friedrich W. H. Kossebau wrote:

Thanks for pushing this here.

Am Montag, 30. August 2021, 14:17:42 CEST schrieb Ahmad Samir:
Open question:
in which places is it a good idea to use "code"-style with class names
and
method names? So when does readability suffer by too many changes in the
formatting in a text body?

Looking e.g. at the Qt docs for a reference, they do not use
"code"-style
when referencing classes or methods from text, as well as in the listing
of classes and methods. So I wonder if this is by design or just
historic?


They use QDoc, IIUC; and it looks like they recommend using \c
https://doc.qt.io/qt-5/04-qdoc-commands-textmarkup.html

at least that page suggests so.


Then our question was not "if" they have a command to markup things to be
printed code-like, but "when" it should be used. And the examples they
give
(not sure though if exclusive or inclusive) are

"
variable names, user-defined class names, and C++ keywords (for example,
int and for)
".

So no mention of names of the methods, members, properties and classes the
documentation is about (note the "user-defined"). And looking at the
existing Qt API documentation, I would guess the given list is rather
exclusive then, and \c with Qt is not to be used when referencing the
elements of the documented API itself (at least in flow text).

Myself I meanwhile rather think that this might be a good choice. Imagine
how e.g. https://doc.qt.io/qt-6/qstring.html would look like if all text
elements referencing Qt classes or method names would be in code-style. I
guess the reading flow in the flow text blocks would suffer a lot.



So one vote for and one vote against, we need a tie-breaker.


What I would like to see are some argument for why you want this change?
What is broken now, what does it improve?
Can you give an example where your proposal is applied and what the result is,
before and after?



I think it's useful to markup the method names, that makes reading the API docs in text format in a 
header file easier, the same way marking up true/false is useful, the same way syntax highlighting 
in most text editors is useful.


For examples, open any header file in a KDE repo and add @c before a method name in the comment 
above any method. :)



Cheers
Friedrich





Regards,
Ahmad Samir

Re: KF API Documentation proposed, small, addition

[Ahmad Samir]

On 09/09/2021 22:24, Friedrich W. H. Kossebau wrote:

Am Donnerstag, 9. September 2021, 21:45:33 CEST schrieb Ahmad Samir:

On 30/08/2021 16:35, Friedrich W. H. Kossebau wrote:

Thanks for pushing this here.

Am Montag, 30. August 2021, 14:17:42 CEST schrieb Ahmad Samir:
Open question:
in which places is it a good idea to use "code"-style with class names and
method names? So when does readability suffer by too many changes in the
formatting in a text body?

Looking e.g. at the Qt docs for a reference, they do not use "code"-style
when referencing classes or methods from text, as well as in the listing
of classes and methods. So I wonder if this is by design or just
historic?


They use QDoc, IIUC; and it looks like they recommend using \c
https://doc.qt.io/qt-5/04-qdoc-commands-textmarkup.html

at least that page suggests so.


Then our question was not "if" they have a command to markup things to be
printed code-like, but "when" it should be used. And the examples they give
(not sure though if exclusive or inclusive) are

"
variable names, user-defined class names, and C++ keywords (for example, int
and for)
".

So no mention of names of the methods, members, properties and classes the
documentation is about (note the "user-defined"). And looking at the existing
Qt API documentation, I would guess the given list is rather exclusive then,
and \c with Qt is not to be used when referencing the elements of the
documented API itself (at least in flow text).

Myself I meanwhile rather think that this might be a good choice. Imagine how
e.g. https://doc.qt.io/qt-6/qstring.html would look like if all text elements
referencing Qt classes or method names would be in code-style. I guess the
reading flow in the flow text blocks would suffer a lot.

Cheers
Friedrich




So one vote for and one vote against, we need a tie-breaker.

Regards,
Ahmad Samir


--
Ahmad Samir

Re: KF API Documentation proposed, small, addition

[Ahmad Samir]

On 30/08/2021 16:35, Friedrich W. H. Kossebau wrote:

Thanks for pushing this here.

Am Montag, 30. August 2021, 14:17:42 CEST schrieb Ahmad Samir:

I would like to add the following to:
https://community.kde.org/Frameworks/Frameworks_Documentation_Policy#Documen
t_the_Classes


One can use [https://www.doxygen.nl/manual/commands.html Doxygen commands]
to improve readability, for example
* @c which will make the word after it use a monospace/typewriter font face,
typically e.g. @c true, @c false, @c setSomeValue(); for multiple words
you'll have to use: multiple words
* @copydoc to copy the docs of a different method, e.g. if one method
overloads another


I propose to turn "can use" into "should use" and define where which command
should be used where (and which variant in case of aliases), for a consistent
result.
Doxygen commands are already used, so I cannot see how the current proposal
for an addition makes a difference about helping when to do what?

So in the given case, the motivation of the proposal is to improve the
resulting documentation to have all in-text (C++) code expressions to be
formatted in code-typical ways (i.e. monospace fonts). So far this was mainly
done for literal code expressions like "nullptr", "true" and "false", whereas
method names or class names were not, until recently at least.



I agree about "should use".

For how to use a command, one should read the Doxygen docs, which are linked in 
the text.


Open question:
in which places is it a good idea to use "code"-style with class names and
method names? So when does readability suffer by too many changes in the
formatting in a text body?

Looking e.g. at the Qt docs for a reference, they do not use "code"-style when
referencing classes or methods from text, as well as in the listing of classes
and methods. So I wonder if this is by design or just historic?



They use QDoc, IIUC; and it looks like they recommend using \c
https://doc.qt.io/qt-5/04-qdoc-commands-textmarkup.html

at least that page suggests so.


Leaving @copydoc for a separate discussion.


There are 123 instances of @copydoc in kdelibs, with the the first appearance being [1] from back in 
2003, I am mainly trying to bring it back into use, by pointing it out in the docs.


Incidentally, I think whenever we update one of the core guide pages in the wiki, we should also 
post a link to the new addition in this ML, otherwise people who already know the guidelines 
inside-out won't read the whole thing again, so they miss the new bit (or miss disagreeing with it 
and starting a discussion).


[1] 
https://invent.kde.org/unmaintained/kdelibs/-/commit/cd9de18af7a5b8fc752346596d1ddde512203537



Cheers
Friedrich




Regards,
--
Ahmad Samir

Please include this in 5.86

[Ahmad Samir]

Fixes KMountPoint::findByPath():
https://invent.kde.org/frameworks/kio/-/commit/147193f5661173d24d75f6558e5607d3418f6c71

Thanks,
Ahmad Samir

KF API Documentation proposed, small, addition

[Ahmad Samir]

I would like to add the following to:
https://community.kde.org/Frameworks/Frameworks_Documentation_Policy#Document_the_Classes


One can use [https://www.doxygen.nl/manual/commands.html Doxygen commands] to improve readability, 
for example
* @c which will make the word after it use a monospace/typewriter font face, typically e.g. @c true, 
@c false, @c setSomeValue(); for multiple words you'll have to use:

multiple words
* @copydoc to copy the docs of a different method, e.g. if one method overloads 
another



--
Ahmad Samir

Heads-up: kioslaves .so libs have been renamed

[Ahmad Samir]

Due to a change in kcoreaddons_add_plugin() macro, kioslaves (e.g. kio_file), the name of the actual 
.so has changed from /bin/kf5/kio/file.so to /bin/kf5/kio/kio_file.so, the same 
goes for when you install with `make install`, (the change makes sense as now the file name matches 
the name of the kio slaves, and the plugin unique id...etc).


This isn't an issue for distro-provided packages as e.g. RPM packages remove the file from the old 
package and replace it with the new ones, but it is an issue if you build from source and test right 
from the build-dir (without cleaning the build-dir, as most of us do) or from your kdesrc-build 
installation; so you may want to clear up that situation.


That leads to interesting issue, as kioslave5 binary could pick either kf5/kio/file.so or 
kf5/kio/kio_file.so and you get weird results.


Have a good day.

--
Ahmad Samir

Re: Porting notes / deprecation docs

[Ahmad Samir]

On 10/07/2021 18:00, Frederik Schwarzer wrote:

Hi,

as mentioned earlier, I would like to document classes/methods/etc that
are going to be deprecated during KF6 development.

For that I scraped up all the deprecation macros from the frameworks and
edited them to be more unified.



Good work, that must have been a huge task! (82 frameworks ... :)).


Now I need some opinions.

For once, there is still some stuff in deprecation from KDE4 times.
E.g. void setDoScanFile(bool scanFile); from kiowidgets.

I looked up a few of them in
https://community.kde.org/Frameworks/Porting_Notes but they are not
mentioned there.

Do you think these need to be mentioned in current porting notes as well
or have they been deprecated for long enough to consider them "over with"?



I would agree, these ones were probably just forgotten after the kdelibs split, and then couldn't be 
removed after the first KF5 release so as not to break BIC... etc. (Others who have been around 
longer will know for sure).




As for the location, I would probably start putting content in the
community wiki to a place like
https://community.kde.org/Frameworks/KF6_Porting_Notes. Does anyone see
a problem with that? Might is be better to write such docs in markdown
or restructured text for being better suited for a more modern location?



A wiki page is not most friendly way of editing huge technical documents. Personally, I think a 
markdown file in a git repo would be great, and then it can be "published" to a wiki page or a 
static web page on one of KDE's web sites. Or, we start with an markdown text file, then after it's 
fleshed out / polished, put it in the wiki, editing/adding a small section here or there would be 
easier. (But I do prefer text files, much easier to edit in my usual editor of choice).


FWIW, there is supposed to be a KF6 meeting soon[1]. Not sure if we'll start this week or the next 
one though.


[1] https://mail.kde.org/pipermail/kde-frameworks-devel/2021-July/118028.html



Thanks!

Cheers,
Frederik




--
Ahmad Samir

Re: Weekly KF6 meeting poll

[Ahmad Samir]

On 09/07/2021 19:08, Ahmad Samir wrote:

On 29/06/2021 16:48, Ahmad Samir wrote:

Hello, as was discussed during the KF6 BoF at Akademy, David Edmundson (aka "David 
2", not to be
confused with "David 1" aka "4" or David 3, aka "the David who still has all 
the hair on his
head"... sorry, I digress :)).

David has created a poll as a way to find a more suitable time for the weekly 
KF6 meeting (that we
used to have on Saturdays 13:00 UTC), apparently the link might have gone too 
far under the radar in
the KF6 meeting notes that were posted to this ML by Volker. So here it is, 
please vote:
https://doodle.com/poll/94646r75keyxgqkx

Have a good day.



It's been 18 days since the poll was created, I'd say that's long enough. If no 
one disagrees, I'd
say the deadline is today 00:00 UTC.

As it stands now, it looks like most of the participants find the 03:00 - 04:00 
PM slot most
convenient, and it's a tie between Monday, Tuesday and Friday (each with 
exactly 11 votes).

Best regards.



OK, poll is done now (since no one disagreed).

And the result is that it looks like we'll have the meeting on Mondays at 015:00-016:00 UTC. The 
BigBlueButton room link is at https://community.kde.org/Frameworks#Meetings (I'll update the wiki 
page with the new meeting time shortly).


Have a good day.

--
Ahmad Samir

Re: Weekly KF6 meeting poll

[Ahmad Samir]

On 29/06/2021 16:48, Ahmad Samir wrote:

Hello, as was discussed during the KF6 BoF at Akademy, David Edmundson (aka "David 
2", not to be
confused with "David 1" aka "4" or David 3, aka "the David who still has all 
the hair on his
head"... sorry, I digress :)).

David has created a poll as a way to find a more suitable time for the weekly 
KF6 meeting (that we
used to have on Saturdays 13:00 UTC), apparently the link might have gone too 
far under the radar in
the KF6 meeting notes that were posted to this ML by Volker. So here it is, 
please vote:
https://doodle.com/poll/94646r75keyxgqkx

Have a good day.



It's been 18 days since the poll was created, I'd say that's long enough. If no one disagrees, I'd 
say the deadline is today 00:00 UTC.


As it stands now, it looks like most of the participants find the 03:00 - 04:00 PM slot most 
convenient, and it's a tie between Monday, Tuesday and Friday (each with exactly 11 votes).


Best regards.

--
Ahmad Samir

Weekly KF6 meeting poll

[Ahmad Samir]

Hello, as was discussed during the KF6 BoF at Akademy, David Edmundson (aka "David 2", not to be 
confused with "David 1" aka "4" or David 3, aka "the David who still has all the hair on his 
head"... sorry, I digress :)).


David has created a poll as a way to find a more suitable time for the weekly KF6 meeting (that we 
used to have on Saturdays 13:00 UTC), apparently the link might have gone too far under the radar in 
the KF6 meeting notes that were posted to this ML by Volker. So here it is, please vote:

https://doodle.com/poll/94646r75keyxgqkx

Have a good day.

--
Ahmad Samir

Re: KF5 modules development branches now build in C++17 mode (but keep public headers compatible)

[Ahmad Samir]

On 20/06/2021 12:36, Albert Astals Cid wrote:
[...]


Semi-related question, does anyone know why we're not using
   set(CMAKE_CXX_EXTENSIONS OFF)
?



Good question. Given there _are_ differences between libstdc++ (gcc) and libc++ (clang) and 
sometimes we have issues due to those differences, it does make sense to disable compiler specific 
extensions.



Cheers,
   Albert

>


--
Ahmad Samir

Pushing and creating a merge request from the command line

[Ahmad Samir]

Hello. Remember when we used to use Phabricator and just 'arc diff' and it would push and create a 
diff review in one go? There is a way to do that with Gitlab using Git push options, see:

https://community.kde.org/Infrastructure/GitLab#Creating_a_merge_request_using_the_command_line

Have a good day.

--
Ahmad Samir

Weekly meeting reminder

[Ahmad Samir]

Hello; today is Saturday, we're supposed to have an hour-long weekly
meeting, 13 UTC (that's 15:00 CEST), the usual room on KDE's BigBlueButton:
https://meet.kde.org/b/ada-mi8-aem

See: https://community.kde.org/Frameworks#Meetings for more details

The KF6 board https://phabricator.kde.org/project/board/310/, we're supposed to 
whittle it
down if we want to finally, branch for KF6, no rush, but it should hopefully 
happen at some
point, preferably during our lifetimes. :)

All are welcome to join, even if you just want to listen/see how things are 
done in KDE.

Have a good day.

--
Ahmad Samir

KF6 weekly meeting

[Ahmad Samir]

Hello; it's been a week already, and today Saturday, we're supposed to have an hour-long weekly 
meeting, 13 UTC (that's 15:00 CEST), the usual room on KDE's BigBlueButton: 
https://meet.kde.org/b/ada-mi8-aem


See: https://community.kde.org/Frameworks#Meetings for more details

Remeber the KF6 board https://phabricator.kde.org/project/board/310/ ? we're supposed to whittle it 
down if we want to actully, finally, branch for KF6, no rush, but it should hopefully happen at some 
point, preferably during our lifetimes. :)


All are welcome to join, even if you just want to listen/see how things are 
done in KDE.

Have a good day.

--
Ahmad Samir

KF6 weekly meeting

[Ahmad Samir]

Hello; it's Saturday - you know the drill - weekly meeting, 13 UTC (that's 15:00 CEST), the usual 
room on KDE's BigBlueButton: https://meet.kde.org/b/ada-mi8-aem


See: https://community.kde.org/Frameworks#Meetings for more details

We spend about an hour discussing the various tasks that still aren't finished on the huge KF6 
board, https://phabricator.kde.org/project/board/310/, according to those who can actually keep 
track of all the discussions going on in those meetings, it seems to actually be useful, and indeed 
some tasks do get finished.


You can join just to listen, see how stuff are discussed, maybe there is a task there that has your 
name on it, something that intrigues you and you'd like to work on ...etc.


Have a good day.

--
Ahmad Samir

KF6 weekly meeting

[Ahmad Samir]

Hello :)

Just like last Saturday, meeting usually starts at 13 UTC (that's 15:00 CEST), the usual room on 
KDE's BigBlueButton: https://meet.kde.org/b/ada-mi8-aem


See: https://community.kde.org/Frameworks#Meetings for more details

We typically spend about an hour trying to bring the number tasks of the ever growing KF6 board, 
https://phabricator.kde.org/project/board/310/, down; by discussing any stuck/needs-input tasks 
(which sometimes results in more tasks being created, so, sometimes the number goes up instead of 
down... at least it's not stuck any more, right?).



(I would like to take this chance to thank Luigi, who somehow can keep track of all the tasks being 
discussed and posts the meeting notes on this ML every week o/).


Have a good day.

--
Ahmad Samir

T14471: Make Qt 5.15.0 to 5.15.2 for frameworks

[Ahmad Samir]

ahmadsamir added a comment.


  Anything other than the methods that were added for "portability at the 
expense of performance", should be OK, famous last words™. :)
  
  I've been bitten twice by the QStringView API in Qt5, it has too many 
surprises, basically because upstream decided to make "code" portable any which 
way... but I digress.
  
  Since QStringRef is in a Qt6 compat module, I think QStringView should wait 
until we branch KF6; we also get QStrinTokenizer Qt6 (yes, it means we might 
scramble a bit between when we branch KF6 and the first release to fix affected 
public API's, but I think that's safer).

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

To: ahmadsamir
Cc: ahmadsamir, nicolasfella, kde-frameworks-devel, usta, LeGast00n, cblack, 
michaelh, ngraham, bruns

KF6 weekly meeting reminder

[Ahmad Samir]

Hello :)

This is a reminder that since today is Saturday we are supposed to have our KF6 weekly meeting, 
which usually starts at 13 UTC (that's 15:00 CEST), the usual room on BBB 
https://meet.kde.org/b/ada-mi8-aem


https://community.kde.org/Frameworks#Meetings

We typically spend an hour trying to chisel away at the ever growing KF6 tasks board 
https://phabricator.kde.org/project/board/310/


Have a good day.

--
Ahmad Samir

Re: Including instead of , does it upset POSIX?

[Ahmad Samir]

On 21/04/2021 15:34, Ahmad Samir wrote:

On 21/04/2021 15:20, Harald Sitter wrote:

To conclude this our verdict is to always use the modern headers?



I am re-checking the rest of the frameworks that failed the first time around; Albert already 
reported that it built for him locally with '#include '; I'll test again, push the changes 
and if the CI doesn't barf on any of it, then my conclusion is it's OK so far.




I've now pushed all those changes; only remains:

Deprecated:
kdelibs4support
kjs

kdesu/src/kdesu_stub.c: this stays since this is C code

these two files are generated by some tool, I left them be:
kholidays/src/parsers/plan2/holidayscannerplan.cpp
kholidays/src/parsers/plan2/holidayscannerplan.lpp


kinit/src/kdeinit/kinit_mac.mm: no idea about mac, can't test, so didn't change 
it
kinit/src/start_kdeinit/start_kdeinit.c: C code


They seemed to build fine on the CI.

--
Ahmad Samir

Re: Including instead of , does it upset POSIX?

[Ahmad Samir]

On 21/04/2021 15:20, Harald Sitter wrote:

To conclude this our verdict is to always use the modern headers?



I am re-checking the rest of the frameworks that failed the first time around; Albert already 
reported that it built for him locally with '#include '; I'll test again, push the changes 
and if the CI doesn't barf on any of it, then my conclusion is it's OK so far.


--
Ahmad Samir

Re: Including instead of , does it upset POSIX?

[Ahmad Samir]

On 15/04/2021 00:16, Albert Astals Cid wrote:

[...]


karchive/autotests/karchivetest.cpp:28:#include 
karchive/src/karchive.cpp:23:#include 


I changed those and it compiles just fine here.



kdesu/src/kdesu_stub.c:31:#include  (this is C code (guessing since I 
don't know C), so it
makes sense to leave that one in)


Yes, this is not C++


kholidays/src/parsers/plan2/holidayscannerplan.cpp:1313:# include 
kholidays/src/parsers/plan2/holidayscannerplan.lpp:17:# include 


This works too


kinit/src/kdeinit/kinit.cpp:24:#include 
kinit/src/kdeinit/kinit_mac.mm:25:#include 
kinit/src/kdeinit/kinit_win.cpp:15:#include 
kinit/src/klauncher/klauncher.cpp:21:#include 
kinit/src/start_kdeinit/start_kdeinit.c:12:#include 
kinit/src/wrapper.cpp:23:#include 


Same (obviously excluding start_kdeinit.c)



ktexteditor/src/buffer/katesecuretextbuffer.cpp:12:#include 
ktexteditor/src/buffer/katetextbuffer.cpp:20:#include 


Same.

Maybe your problem was that you tried to change C files?



I'll try again, maybe there is/was something in my build environment... no idea 
what though. :)

There was only a couple of .c files (I remember kdesu); but ktexteditor doesn't 
have an .c files.

Thanks for re-checking.


Cheers,
   Albert

  


--
Ahmad Samir

Re: Including instead of , does it upset POSIX?

[Ahmad Samir]

On 14/04/2021 22:20, Albert Astals Cid wrote:

El dimecres, 14 d’abril de 2021, a les 15:13:09 (CEST), Ahmad Samir va escriure:

Hello :)

A week or so ago I created an MR to include  instead of  in 
KIO[1].

  From /usr/include/c++/10/cerrno:
/** @file cerrno
   *  This is a Standard C++ Library file.  You should @c \#include this file
   *  in your programs, rather than any of the @a *.h implementation files.
   *
   *  This is the C++ version of the Standard C Library header @c errno.h,
   *  and its contents are (mostly) the same as that header, but are all
   *  contained in the namespace @c std (except for names which are defined
   *  as macros in C).
   */


And then I made similar commits to a lot of the other Frameworks (not all, 
since the build failed
for some of them, so I left them alone).


I honestly didn't think this would be a problem, but you say the build of some 
of them failed, so i guess it is :D

Could you link to the particular MR's that failed?



I created one MR for KIO, once that was reviewed, I then applied similar patches to the other 
Frameworks directly.


Fortunately, it's easy to find which ones weren't changed, grep/rg; (I excluded frameworks that are 
going to be deprecated, kdelibs4support and kjs):


karchive/autotests/karchivetest.cpp:28:#include 
karchive/src/karchive.cpp:23:#include 

kdesu/src/kdesu_stub.c:31:#include  (this is C code (guessing since I don't know C), so it 
makes sense to leave that one in)


kholidays/src/parsers/plan2/holidayscannerplan.cpp:1313:# include 
kholidays/src/parsers/plan2/holidayscannerplan.lpp:17:# include 

kinit/src/kdeinit/kinit.cpp:24:#include 
kinit/src/kdeinit/kinit_mac.mm:25:#include 
kinit/src/kdeinit/kinit_win.cpp:15:#include 
kinit/src/klauncher/klauncher.cpp:21:#include 
kinit/src/start_kdeinit/start_kdeinit.c:12:#include 
kinit/src/wrapper.cpp:23:#include 

ktexteditor/src/buffer/katesecuretextbuffer.cpp:12:#include 
ktexteditor/src/buffer/katetextbuffer.cpp:20:#include 



Cheers,
   Albert




--
Ahmad Samir

Including instead of , does it upset POSIX?

[Ahmad Samir]

Hello :)

A week or so ago I created an MR to include  instead of  in 
KIO[1].

From /usr/include/c++/10/cerrno:
/** @file cerrno
 *  This is a Standard C++ Library file.  You should @c \#include this file
 *  in your programs, rather than any of the @a *.h implementation files.
 *
 *  This is the C++ version of the Standard C Library header @c errno.h,
 *  and its contents are (mostly) the same as that header, but are all
 *  contained in the namespace @c std (except for names which are defined
 *  as macros in C).
 */


And then I made similar commits to a lot of the other Frameworks (not all, since the build failed 
for some of them, so I left them alone).


Harald Sitter asked me to raise the point here, basically he's wondering whether this change might 
cause issues, not being 100% POSIX compliant...etc; I'll quote him because he explained it much 
better than I would:


 ahmadsamir: 
https://invent.kde.org/frameworks/kcrash/-/commit/7a20755723dc1527edb7ed5c3fdcccdbcf7fa768 was this 
ever discussed anywhere? cause there are others like this and my thinking up until now was that 
using the C .h is more appropriate since we use them for POSIX APIs and from that POV the POSIX 
specified header is errno.h


[...]

 might be worth raising for discussion on the mailing list. with c++>=14 there's a whole 
mountain of "deprecated" headers OTOH I also recall reading that the c++ standard doesn't guarantee 
compatibility
 which may or may not be a concern when we use stuff specifically with the expectation that 
they will behave as documented from a POSIX or linux POV (such as signal safety in kcrash)



[1] https://invent.kde.org/frameworks/kio/-/merge_requests/397
--
Ahmad Samir

Saturday, 2021-04-11 KF6 meeting notes

[Ahmad Samir]

Hello, we held the second ever KF6 weekly meeting yesterday, and here are the meeting notes, taken 
by Luigi Toscano (lightly proof-read by myself).



Investigate replacing KSelectAction with QActionGroup 
(<https://phabricator.kde.org/T12097> )

Moved back to backlog, because it (most likely) doesn't require any input. Nicolas Fella will check 
whether any work is really needed.




KGlobalAccel cleanup and improvement for desktop files 
(<https://phabricator.kde.org/T12063>)

A bit of recap:

* the original API has some issues (see the ticket)
* some time ago it was changed to accepte a desktop file with shortcut definitions, but the code is 
not trivial


The question is: should we support only the desktop file as a way to define the 
global shortcuts?

Probably fine for applications, it may not work for plasma or kwin, which have a lot of actions. A 
huge file with all actions would be needed. But in fact anything that can be installed can install a 
desktop file.


The discussion moved to some cross -framework dependencies (powerdevil uses kxmlgui to access 
shortcut stuff for example), and to the question on whether KGlobalAccel should be part of 
Frameworks or Plasma.


In short, KGlobalAccel is made of an API and a daemon.

The deamon is really Plasma-specific, but there are really no drawbacks of keeping that in 
Frameworks, as long as the code is fixed to never trigger it outside Plasma (not even through 
dbus-activation). Right now it can happen and it leads to bad interference with other DE's shortcuts 
API (e.g. GNOME).


The API can probably stay as it is. While the only implementation is on Plasma, it may stay with the 
current Windows/macOS code to allow people to easily compile it on those platforms without #ifdefs, 
and smart enough to disable for example the global shortcut creation integration in KXMLGui when 
running on other platforms.


Future contributors may decide whether a) revamp the platform-specific backends or b) create a new, 
really multiplatform API which would use KGlobalAccel as Plasma backend.




Create replacement for KPluginInfo::kcmServices 
(<https://phabricator.kde.org/T13555>)

The work is mostly done: a refactor of the way plugins and their KCMs are associated, with a direct 
link in the plugin definition instead of the complicated code which runs right now (and which relies 
on a deprecated classes).


The pending question is whether to allow each plugin to list multiple KCMs (which would cover some 
use cases, like Kontact plugins, even though those don't use this mechanism right now). After some 
discussion, it seems there is really no drawback on using a list of KCMs instead of a single KCM.



What we forgot to do is pick the tasks for discussion for the next meeting, so if you have 
suggestions please post them somewhere (on this ML would work best).


--
Ahmad Samir

Request for wide spread testing

[Ahmad Samir]

Hello.

Just to give this more visibility, there is a plan being contemplated to set evn var 
KDE_FORK_SLAVES=1 by default, see[1] for details.


So, if you can, please set KDE_FORK_SLAVES=1 in your env, and report any 
breakage.


[1] https://phabricator.kde.org/T12140


Thanks and have a good day.

--
Ahmad Samir

clang-format section added to the coding style guide

[Ahmad Samir]

Thanks to Alexander Lohnau, there is now a section in the KF coding style that explains about the 
usage of clang-format in the Frameworks:

https://community.kde.org/Policies/Frameworks_Coding_Style#Clang-format_automatic_code_formatting

including tips and tricks (mainly to trick clang-format so that it doesn't 
mangle your code)


Have a good day. :)

--
Ahmad Samir

D27224: add Kongress icon

[Ahmad Samir]

ahmadsamir added a comment.


  Hello. FWIW, I would also close/abandon this diff :) (nice icon BTW).

REPOSITORY
  R266 Breeze Icons

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

To: mbruchert, dkardarakos, #vdg, ndavis
Cc: ahmadsamir, ndavis, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

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

[Ahmad Samir]

On 17/12/2020 22:06, David Faure wrote:
[...]


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.



Ben said on IRC:
"I used 5.14 as a practical matter as 5.13 is essentially unavailable"

That's why team jumped to 5.14 directly, don't know the details however.

[...]

--
Ahmad Samir

T8349: Improve Places panel usability and presentation

[Ahmad Samir]

ahmadsamir added subscribers: sitter, ahmadsamir.
ahmadsamir added a comment.


  ATM, the behaviour is that if you go to /foo/bar/baz/ and "bar" is present in 
the places panel, then while you're at baz or any of its sub-folders then "bar" 
is highlighted in the places panel, i.e the "closest" bookmark is highlighted. 
There is a bug report about changing the behaviour to only highlight the 
"current" folder if it's in the panel.
  
  Posting here to start discussing the issue.
  
  https://bugs.kde.org/show_bug.cgi?id=156678

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

To: ngraham, ahmadsamir
Cc: ahmadsamir, sitter, #frameworks, tomsk, bruns, michaelh, acrouthamel, 
sharvey, mmustac, jtamate, rkflx, #dolphin, ngraham, fabiogomes, waitquietly, 
azyx, dmenig, nikolaik, pberestov, manueljlin, iasensio, Orage, aprcela, 
fprice, cblack, konkinartem, ian, jguidon, Ghost6, jraleigh, fbampaloukas, 
squeakypancakes, alexde, IohannesPetros, GB_2, Codezela, feverfew, 
trickyricky26, meven, crozbo, spoorun, ndavis, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, rdieter, mikesomov, aaronhoneycutt, mbohlender

Re: Dropping the moderation by default flag

[Ahmad Samir]

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. :)

--
Ahmad Samir

"Approve"ing an MR

[Ahmad Samir]

Hello; this is just a reminder that at the moment the "Approve" button in an MR doesn't send an 
email/notification, so please if you approve an MR also add a comment (otherwise it could be some 
while before it actually gets merged by the author of the MR).


This has been reported by bshah at 
https://gitlab.com/gitlab-org/gitlab/-/issues/231351

Have a good day :)

--
Ahmad Samir

Re: KIO on Android Failure

[Ahmad Samir]

On 18/08/2020 13:52, Ben Cooksley wrote:

Hi all,

At some point recently functionality was added to KIO which broke the
build on Android.

I'm not sure why we're building KIO on Android, but it appears that
some applications may be using it - and this in turn causes the
Dependency Build jobs to fail.

Could someone take a look please?
https://build.kde.org/job/Administration/job/Docker%20Generate%20AndroidQt5.14%20KF5-SDK/lastFailedBuild/console

Thanks,
Ben



It looks like it's caused by systemd integration in kprocessrunner; also it seems Q_OS_UNIX includes 
Android systems, so we need to guard it with !defind(Q_OS_ANDROID). Should hopefully be fixed by 
https://invent.kde.org/frameworks/kio/-/merge_requests/107 .


--
Ahmad Samir

D28460: Add KCModuleData as base class for plugin

[Ahmad Samir]

ahmadsamir closed this revision.
ahmadsamir added a comment.


  Has been moved to 
https://invent.kde.org/frameworks/kcmutils/-/merge_requests/10

REPOSITORY
  R295 KCMUtils

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

To: bport, #plasma, ervin
Cc: ahmadsamir, meven, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D28460: Add KCModuleData as base class for plugin

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> kcmoduleloader.cpp:164
> +
> +QVariantList args2(args.cbegin(), args.cend());
> +

Hello. This breaks the build for Qt 5.12 on the CI, (the min. required version 
in KF is still 5.12):
https://build.kde.org/job/Frameworks/job/kcmutils/job/kf5-qt5%20SUSEQt5.12/155/console

REPOSITORY
  R295 KCMUtils

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

To: bport, #plasma, ervin
Cc: ahmadsamir, meven, broulik, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

[Ahmad Samir]

ahmadsamir added a comment.


  In D28774#676000 , @volkov wrote:
  
  > Why not add support for `QPlatformTheme::FontDialog` in 
`KdePlatformTheme::createPlatformDialogHelper()`?
  
  
  Sorry about the late reply; I had looked into the platformtheme integration 
stuff, but at the time I didn't think it was a viable solution (nor was it easy 
for me to port); see https://phabricator.kde.org/D27808 for more details.

REPOSITORY
  R236 KWidgetsAddons

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

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

D28880: [KWallet] Port last usage of QRegExp to QRegularExpression

[Ahmad Samir]

ahmadsamir added a comment.


  @blaze, (I couldn't find your user name at invent.kde.org); FYI:
  https://invent.kde.org/network/falkon/-/merge_requests/7

REPOSITORY
  R311 KWallet

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

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

D29017: WIP: Introduce three new methods that return all "entries" in a folder

[Ahmad Samir]

ahmadsamir abandoned this revision.

REPOSITORY
  R311 KWallet

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

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

D29017: WIP: Introduce three new methods that return all "entries" in a folder

[Ahmad Samir]

ahmadsamir reopened this revision.
ahmadsamir marked an inline comment as done.
ahmadsamir added a comment.


  Move to gitlab https://invent.kde.org/frameworks/kwallet/-/merge_requests/2

REPOSITORY
  R311 KWallet

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

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

D29810: Don't use the setenv function after fork

[Ahmad Samir]

ahmadsamir added a comment.


  In D29810#674824 , @jpalecek wrote:
  
  > In D29810#674815 , @dfaure wrote:
  >
  > > This breaks FreeBSD compilation. Please check: 
https://build.kde.org/job/Frameworks/job/kcrash/job/kf5-qt5%20FreeBSDQt5.14/17/
  >
  >
  > I see. It needs the declaration of `environ`, which is only provided on 
GNU. Should I amend it here?
  
  
  This has been committed already, create a new review request. (Note that KDE 
has moved to Gitlab at https://invent.kde.org, best if you create a merge 
request there; Phabricator still works, but it's planned to become read-only). 
:)

REPOSITORY
  R285 KCrash

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

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

D29017: WIP: Introduce three new methods that return all "entries" in a folder

[Ahmad Samir]

This revision was not accepted when it landed; it landed in state "Needs 
Revision".
This revision was automatically updated to reflect the committed changes.
Closed by commit R311:1a19afb6a06d: WIP: Introduce three new methods that 
return all "entries" in a folder (authored by ahmadsamir).

REPOSITORY
  R311 KWallet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29017?vs=80670&id=83224

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

AFFECTED FILES
  CMakeLists.txt
  src/api/KWallet/CMakeLists.txt
  src/api/KWallet/kwallet.cpp
  src/api/KWallet/kwallet.h
  src/api/KWallet/org.kde.KWallet.xml
  src/runtime/kwalletd/backend/kwalletbackend.cc
  src/runtime/kwalletd/backend/kwalletbackend.h
  src/runtime/kwalletd/kwalletd.cpp
  src/runtime/kwalletd/kwalletd.h

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

D29017: WIP: Introduce three new methods that return all "entries" in a folder

[Ahmad Samir]

ahmadsamir added a comment.


  In D29017#664279 , @blaze wrote:
  
  > Check this commit message 
https://phabricator.kde.org/R32:f56cdda54b7f88b119f2c7cfb2f534b4fe74478f
  
  
  Sorry for the delay. We applied a workable hack in 
https://phabricator.kde.org/D28880 to get rid of the '[^/]' in the string that 
wildcardToRegularExpression returns, and it seemed to work when I tested falkon.
  
  I guess I'll have migrate this to invent.kde and try addressing the review 
comments.

REPOSITORY
  R311 KWallet

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

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

D29362: [KCharSelect] Initially give focus to the search lineedit

[Ahmad Samir]

ahmadsamir added a comment.


  Moved to https://invent.kde.org/frameworks/kwidgetsaddons/-/merge_requests/2

REPOSITORY
  R236 KWidgetsAddons

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

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

D29362: [KCharSelect] Initially give focus to the search lineedit

[Ahmad Samir]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R236:c0c93cc0fcb5: [KCharSelect] Initially give focus to the 
search lineedit (authored by ahmadsamir).

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29362?vs=81741&id=83183

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

AFFECTED FILES
  src/kcharselect.cpp

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

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

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> kopenwithdialog.cpp:1009
> +if (QDir::isAbsolutePath(binaryName)) {
> +QFileInfo fi(binaryName);
> +if (fi.exists() && !fi.isExecutable()) {

It's created on the next line. Anyway my initial comment was a nit-pick... it's 
a micro-optimisation for when the code goes through the if block, technically 
DesktopExecParser should get the absolute path in most cases.

REPOSITORY
  R241 KIO

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

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

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

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> broulik wrote in kopenwithdialog.cpp:1008
> Actually, that doesn't exist

https://doc.qt.io/qt-5.15/qfileinfo.html#isAbsolute ?

REPOSITORY
  R241 KIO

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

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

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

[Ahmad Samir]

ahmadsamir added a comment.


  This wasn't landed on master, but rather phabricator is confused by me 
pushing to work/ branch on gitlab :)

REPOSITORY
  R241 KIO

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

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

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

[Ahmad Samir]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:f2984ff28e72: [StatJob] Make mostLocalUrl ignore remote 
(ftp, http...etc) URLs (authored by ahmadsamir).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D29782?vs=82974&id=83130#toc

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29782?vs=82974&id=83130

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/core/statjob.cpp
  src/core/statjob.h
  src/ioslaves/trash/tests/testtrash.cpp
  src/ioslaves/trash/tests/testtrash.h

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

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

[Ahmad Samir]

ahmadsamir added a comment.


  I guess a proper fix will need to revert part of this to have the KModuleInfo 
ctor that took a KService not use KPluginInfo internally; better still, of 
course, is having the KPluginInfo ctor work for the case of 
X-KDE-ServiceTypes=SystemSettingsExternalApp, (I couldn't grok KPluginInfo 
stuff fully yet).

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

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

[Ahmad Samir]

ahmadsamir added a comment.


  systemsettings uses KCModuleInfo::service() to check whether the moduleinfo 
is valid[1], the problem is creating a KPluginInfo from the KService based on a 
.desktop file with X-KDE-ServiceTypes=SystemSettingsExternalApp fails because 
it doesn't seem to have valid metadata. Since the KPluginInfo ctor fails, 
accessing d->pluginInfo.service() causes a crash, I've submitted a proposed fix 
at [2]. This doesn't fix the bug, but merely prevents the crash.
  
  [1] 
https://invent.kde.org/plasma/systemsettings/-/blob/master/core/ModuleView.cpp#L229
  [2] https://invent.kde.org/frameworks/kcmutils/-/merge_requests/1

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

D28673: [PackageUrlInterceptor] Make QRegularExpression static

[Ahmad Samir]

ahmadsamir commandeered this revision.
ahmadsamir added a reviewer: broulik.
ahmadsamir added a comment.


  Should be handled by 
https://invent.kde.org/frameworks/plasma-framework/-/merge_requests/2

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ahmadsamir, #plasma, broulik
Cc: ahmadsamir, bruns, pino, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham

D28673: [PackageUrlInterceptor] Make QRegularExpression static

[Ahmad Samir]

ahmadsamir abandoned this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ahmadsamir, #plasma, broulik
Cc: ahmadsamir, bruns, pino, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham

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

[Ahmad Samir]

ahmadsamir added a comment.


  In D29800#672399 , @fvogt wrote:
  
  > Landed to invent - hopefully correctly: 
https://invent.kde.org/frameworks/kio/commit/84e9372f4fa2636f57dc456ac2fa2be271d6a7ec
  
  
  LGTM.

REPOSITORY
  R241 KIO

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

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

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

[Ahmad Samir]

ahmadsamir added a comment.


  In D29558#672395 , @dfaure wrote:
  
  > Not committed after all.
  
  
  It was committed, as I had it when I did git pull, but you lucked out, it 
seemed to have been eaten by the migration to gitlab somehow; I had to 'git 
reset --hard' to be in sync with invent.kde.org :)

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

D29787: Fix krununittest

[Ahmad Samir]

ahmadsamir added a comment.


  In D29787#672139 , @dfaure wrote:
  
  > 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.
  
  
  The CI seems to have xterm available as the unit test passed (and then after 
the switch to gitlab, a new CI setup will be available, let's hope it has xterm 
too :)).

REPOSITORY
  R241 KIO

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

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

D29787: Fix krununittest

[Ahmad Samir]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:ef7708d1f5ec: Fix krununittest (authored by ahmadsamir).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29787?vs=82975&id=83011

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

AFFECTED FILES
  autotests/krununittest.cpp

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

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

[Ahmad Samir]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:bed09e39fe24: [kio_file] Handle renaming file 
'A' to 'a' on FAT32 filesystems (authored by ahmadsamir).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29610?vs=82750&id=83010

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

AFFECTED FILES
  src/core/copyjob.cpp
  src/ioslaves/file/file_unix.cpp

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

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

[Ahmad Samir]

ahmadsamir added a comment.


  I've just realised, we won't get an error with an http url; in that case we 
return the url the statjob was called on as-is and cancel the job.

REPOSITORY
  R241 KIO

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

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

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

[Ahmad Samir]

ahmadsamir added a comment.


  In D29782#672157 , @dfaure wrote:
  
  > Can you add a unittest for a KIO::mostLocalUrl() in testtrash.cpp (which is 
:local, so it should work)
  
  
  I don't see where the trash ioslave sets UDS_LOCAL_PATH; I think it doesn't 
set it, I could be wrong, though.
  
  > and another one in jobtest.cpp for a http URL (e.g. to test which error 
code we're getting, if any?)
  
  I'll look into that next.
  
  > + call mostLocalUrl() on a "normal" StatJob in testtrash.cpp
  
  Same as the first point :)
  
  [...]

REPOSITORY
  R241 KIO

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

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

D29787: Fix krununittest

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, dfaure.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  After 6452a34cf01d03d316 
, 
DesktopExecParser::resultingArguments() needs
  to find the terminal app binary, otherwise it returns an empty QStringList.

TEST PLAN
  krununittest passes again

REPOSITORY
  R241 KIO

BRANCH
  l-terminal (branched from master)

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

AFFECTED FILES
  autotests/krununittest.cpp

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

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

[Ahmad Samir]

ahmadsamir updated this revision to Diff 82974.
ahmadsamir retitled this revision from "[StatJob] Change mostLocalUrl to only 
handle protocols with Class=:local" to "[StatJob] Make mostLocalUrl ignore 
remote (ftp, http...etc) URLs".
ahmadsamir edited the summary of this revision.
ahmadsamir edited the test plan for this revision.
ahmadsamir added a comment.


  Improve

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29782?vs=82959&id=82974

BRANCH
  l-most-local-url-local (branched from master)

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

AFFECTED FILES
  src/core/statjob.cpp
  src/core/statjob.h

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

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

[Ahmad Samir]

ahmadsamir added a comment.


  In D29385#671987 , @dfaure wrote:
  
  > 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 :-)
  
  
  I don't have *any* network mounts, never have, so I wasn't aware of that. 
Yep, that is horrendous.

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

[Ahmad Samir]

ahmadsamir added a comment.


  In D29385#671980 , @dfaure wrote:
  
  > It's 3 times faster on my local SSD.
  >
  > Now think of a NFS mount on a server from another country
  
  
  I was thinking mostly of QFile when url.isLocalFile() is true, but yeah I see 
your point :)

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

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in openurljob.cpp:261
> LOL we're like an old couple, the explicit discussion doesn't actually need 
> to happen anymore ;)
> 
> OK for everyone else, we're debating whether it's ok to use blocking 
> local-file I/O like QFile or QMimeDatabase which reads from the file.
> 
> Of course less code paths is a good thing for maintenance, but it seems *so* 
> overkill to make two calls to a kioslave just to find out the mimetype of a 
> file My main counter argument is performance.
> 
> For the whole OpenUrlJob until the mimetype is found:
> 
> With KIO
> 
>   RESULT : OpenUrlJobTest::takeOverAfterMimeTypeFound():
>0.29 msecs per iteration (total: 75, iterations: 256)
> 
> With the local-file optimization
> 
>   RESULT : OpenUrlJobTest::takeOverAfterMimeTypeFound():
>0.0986 msecs per iteration (total: 101, iterations: 1024)
> 
> That's 3 times faster. Admittedly this is not the kind of things people do in 
> a loop.
> 
> Well, OK, if nobody objects I can remove the local-files fast paths.
> 0.2ms is nothing when lharming 2018 PG Demi Lovato Ashley Tisdale Avril 
> Lavigne-02052020.mpgaunching an app, or even when opening a URL in a browser.
> 
> [More context: QMimeDatabase *might* determine the mimetype from just the 
> extension, in which case no I/O happens and we could do that here, or it 
> might need to look into the contents of the file. We can ask it to not do 
> that but then the mimetype determination will be less good, for some 
> mimetypes; and we can't ask it if we would get better information by looking 
> at content, so there's no way to split up the work between here and the 
> kioslave. It's "quick search" vs "full search", not phase 1, phase 2.]

> OK for everyone else, we're debating whether it's ok to use blocking 
> local-file I/O like QFile or QMimeDatabase which reads from the file.

I know this ship has sailed (well, sunk in this case :)), but if it's 3 times 
faster to use QFile, then is it really a "blocking I/O" operation? it's too 
fast to be "blocking"...

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

D29782: [StatJob] Change mostLocalUrl to only handle protocols with Class=:local

[Ahmad Samir]

ahmadsamir planned changes to this revision.

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

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> kossebau wrote in openurljob.h:120
> You meant, according to 
> https://community.kde.org/Frameworks/Frameworks_Documentation_Policy#Document_Public_and_Protected_Members
>  :)

I got the info from a commit in kwidgetsaddons where you "fixed" a previous 
commit of mine :)

Thanks for the link though.

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

[Ahmad Samir]

ahmadsamir accepted this revision.
ahmadsamir added a subscriber: kossebau.
ahmadsamir added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> openurljob.h:120
> + *
> + * @param b whether to only show the "open with" dialog.
> + */

It seems that we shouldn't end @param with a ".", according to @kossebau 
anyway...

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

D29782: [StatJob] Change mostLocalUrl to only handle protocols with Class=:local

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, dfaure, sitter.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  Previously mostLocalUrl would check that !url.isLocalFile(), that meant
  a statjob would be fired for remote urls, such urls will never have a
  mostLocalUrl. Instead check for protocols with Class=:local.
  
  For a list of such protocols: `grep :local /usr/share/kservices5/*.protocol`
  
  Thanks to sitter for figuring it out in the bug report.
  
  BUG: 420985
  FIXED-IN: 5.71

TEST PLAN
  make && ctest

REPOSITORY
  R241 KIO

BRANCH
  l-most-local-url-local (branched from master)

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

AFFECTED FILES
  src/core/statjob.cpp
  src/core/statjob.h

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

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

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in file_unix.cpp:1052
> I'm confused. We want to solve renaming 'a' to 'A' when 'A' does *not* exist.
> 
> QFile::rename will not overwrite an existing file, so it will do nothing if 
> dest exists.

const QByteArray dest1 = "/mnt/fat32/A";
  const QByteArray dest2 = "/mnt/fat32/a";
  QT_STATBUF buff_dest;
  qDebug() << QT_LSTAT(dest1, &buff_dest);
  qDebug() << QT_LSTAT(dest2, &buff_dest);

IIUC, from FAT32 POV, 'A' exists if 'a' exists and vice versa.

> dfaure wrote in file_unix.cpp:1074
> Wouldn't it be enough to just call QFile::rename here?
> 
> The whole idea is: if QFile::rename is able to rename a file in all cases, 
> including the a->A special case on FAT, then let's just delegate the renaming 
> to QFile.
> 
> Then we don't need to have any special case in our code.
> 
> QFile::rename does not overwrite, though, so if the dest exists and the 
> Overwrite flag is set, we might have to either delete the dest first (race 
> condition, not sure it matters here), or in *that* case use ::rename() since 
> we know it can't be a FAT32-case-change (FAT32 can't have both a and A).

Too many quirks if we don't use ::rename(). I tested with e.g. in file_unix.cpp:

  bool renamed =  QFile::rename(src, dest);
  if (!renamed) {
  if ((_flags & KIO::Overwrite) || isSrcSymLink) {
  renamed = ::rename(_src.data(), _dest.data()) == 0;
  }
  }

- QFile::rename doesn't overwrite as you said
- renaming symlinks depends on ::rename
- some unit tests are failing due to permissions

  FAIL!  : JobTest::statDetailsBasic() Compared values are not the same
 Actual   (kioItem.permissions()): 420
 Expected (438)  : 438
 Loc: [/home/ahmad/dev/kio/autotests/jobtest.cpp(1464)]
  INFO   : JobTest::statDetailsBasicSetDetails() entering
  FAIL!  : JobTest::statDetailsBasicSetDetails() Compared values are not the 
same
 Actual   (kioItem.permissions()): 420
 Expected (438)  : 438
 Loc: [/home/ahmad/dev/kio/autotests/jobtest.cpp(1503)]

That and ::rename() is used all over the place in file_unix.cpp, I am not that 
comfortable using QFile::rename except for the freaky FAT32 case :)

REPOSITORY
  R241 KIO

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

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

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

[Ahmad Samir]

ahmadsamir added a comment.


  In D29610#667858 , @dfaure wrote:
  
  > OK for now, to fix the unittests. The *real* fix however is to use 
QFile::rename in kio_file so that this failure to rename doesn't even happen in 
the first place.
  >
  > In this commit can you at least leave a TODO to that effect?
  
  
  Took me a long while "to do" it.
  
  So many quirks, bin/jobtest shows output (qDebug() << 'blah bleh' in e.g. 
file_unix.cpp) that 'ctest -V -R kiocore-jobtest' for some reason doesn't show. 
And of course kdeinit5.
  
  So it's CopyJob -> SimpleJob -> SlaveBase -> FileProtocol, so many 
redirections and pinging from one place in the code to the other, fun stuff... 
:)

INLINE COMMENTS

> dfaure wrote in copyjob.cpp:1965
> Why this check? The next line compares absolute paths -- including the parent 
> dir.
> 
> Hmm OK one could manufacture a special case with /dir/file and /DIR/FILE 
> where the compare() passes but the parent dirs differ. The orig code had that 
> bug... OK :)

Good point, I missed that the original code was checking the file names and the 
paths with the same compare() call.

About /dir/file and /DIR/FILE, the parent dirs is actually one dir since "dir" 
and "DIR" are the same from FAT32 old POV?

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

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

[Ahmad Samir]

ahmadsamir updated this revision to Diff 82750.
ahmadsamir retitled this revision from "[CopyJob] Use stricter conditions when 
using QFile::rename in slotResultRenaming" to "[kio_file] Handle renaming file 
'A' to 'a' on FAT32 filesystems".
ahmadsamir edited the summary of this revision.
ahmadsamir edited the test plan for this revision.
ahmadsamir added a comment.


  Move the code to kio_file

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29610?vs=82464&id=82750

BRANCH
  l-qfile-rename (branched from master)

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

AFFECTED FILES
  src/core/copyjob.cpp
  src/ioslaves/file/file_unix.cpp

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

D29610: [CopyJob] Use stricter conditions when using QFile::rename in slotResultRenaming

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, dfaure.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  The code now uses QFile::rename() only if direct renaming fails and we're
  moving a file/dir e.g. 'A' to 'a' on a case-insensitive filesystem such
  as FAT3. This matches the behaviour from before commit cb89bab36a5aa4e78c 
,
  and also fixes the jobtest moveFileDestAlreadyExists unit test.

TEST PLAN
  jobtest unit test passes again
  Moving a dir 'A' to 'a' on a FAT32 partition still works

REPOSITORY
  R241 KIO

BRANCH
  l-qfile-rename (branched from master)

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

AFFECTED FILES
  src/core/copyjob.cpp

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

D29547: KRun: deprecate all static 'run*' methods, with full porting instructions

[Ahmad Samir]

ahmadsamir accepted this revision.
ahmadsamir added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> dfaure wrote in krun.h:216
> I did, but Friedrich had a less confusing suggestion:
> 
> @deprecated since 5.6. Since 5.71 use ApplicationLauncherJob, otherwise 
> runApplication instead.

Yeah, makes sense; the point is keeping the users of the class informed/aware 
anyway.

REPOSITORY
  R241 KIO

BRANCH
  2020_05_deprecate_KRun_run_methods

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

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

D29547: KRun: deprecate all static 'run*' methods, with full porting instructions

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> krun.h:216
> + * @deprecated since 5.6, use runApplication instead.
> + * @deprecated since 5.71, use ApplicationLauncherJob instead.
> + * @code

I don't think you want both @deprecated?

> krun.h:229
>   */
> -KIOWIDGETS_DEPRECATED_VERSION(5, 6, "Use KRun::runApplication(const 
> KService &, const QList &, QWidget *, RunFlags, const QString &, const 
> QByteArray &")
> +KIOWIDGETS_DEPRECATED_VERSION(5, 6, "Use KIO::ApplicationLauncherJob, 
> see API docs for a code sample")
>  static bool run(const KService &service, const QList &urls, 
> QWidget *window,

You meant "5, 71"

> krun.h:369
>   * @return @c true on success, @c false on error
> - * @deprecated since 5.31, use runUrl() with RunFlags instead.
> + * @deprecated since 5.31, use OpenUrlJob instead.
> + * @code

But OpenUrlJob is in 5.71 not 5.31? people will be reading that on api.kde.org 
not only in 5.71 header files, right?
maybe:
@deprecated since 5.31
Since 5.71, use OpenUrlJob instead.

> krun.h:380
>   */
> -KIOWIDGETS_DEPRECATED_VERSION(5, 31, "Use KRun::const QUrl &, const 
> QString &, QWidget *, RunFlags, const QString &, const QByteArray &")
> +KIOWIDGETS_DEPRECATED_VERSION(5, 31, "Use KIO::OpenUrlJob, see API docs 
> for a code sample")
>  static bool runUrl(const QUrl &url, const QString &mimetype, QWidget 
> *window,

The same issue 5.31 vs 5.71.

> krun.h:452
>   * @param asn Application startup notification id, if any (otherwise "").
>   * @return @c true on success, @c false on error
> + * @code

Missing @deprecated.

> krun.h:599
>   */
> -void slotTimeout(); // KDE5: rename to slotNextStep() or something like 
> that
> +void slotTimeout();
>  

So, you're keeping the name slotTimout? I guess after 5-10 years, no point 
changing it :)

REPOSITORY
  R241 KIO

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

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

D29599: [CopyJob] Try to fix windows build

[Ahmad Samir]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:d026227574b8: [CopyJob] Try to fix windows build 
(authored by ahmadsamir).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29599?vs=82444&id=82447

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

AFFECTED FILES
  src/core/copyjob.cpp

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

D29599: [CopyJob] Try to fix windows build

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, dfaure.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  The windows build is failing on the CI because of S_IWUSR; include
  kioglobal_p.h to try and fix that.

REPOSITORY
  R241 KIO

BRANCH
  l-fix-win-build (branched from master)

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

AFFECTED FILES
  src/core/copyjob.cpp

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

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in copyjob.cpp:444
> I'm a bit confused. My explanation here points to kio_desktop and kio_remote 
> (and was apparently clear), and the API docs for UDS_LOCAL_PATH say
> 
>   /// A local file path if the ioslave display files sitting
>   /// on the local filesystem (but in another hierarchy, e.g. settings:/ or 
> remote:/)
> 
> which is basically the same information? What is unclear?

> to map URLs from kioslaves-that-wrap-the-local-file-system back to local 
> paths.

That made much more sense to me, reading the docs again now, I know what it 
meant, but I did not before.

REPOSITORY
  R241 KIO

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

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

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

[Ahmad Samir]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:1cac602d9966: [CopyJob] Check free space for remote urls 
before copying and other improvements (authored by ahmadsamir).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29485?vs=82334&id=82356

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

AFFECTED FILES
  src/core/copyjob.cpp

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

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

[Ahmad Samir]

ahmadsamir added a comment.


  @dfaure please don't forget to flesh out the UDS_LOCAL_PATH docs 
https://phabricator.kde.org/D29485#inline-169199

REPOSITORY
  R241 KIO

BRANCH
  l-freespace-remote-2 (branched from master)

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

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

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in copyjob.cpp:477
> I'm assuming the TODO was about "What if I'm using a NFS mount and the 
> connection breaks at the time of KDiskFreeSpaceInfo, i.e. what should we do 
> about error handling".
> 
> But I think the current code -- which ignores errors and moves on, both for 
> local and now for remote files, actually makes most sense. This is after all 
> just a preliminary check. The worst that will happen is that there will 
> indeed not be enough room and the copy will fail. But that's better than not 
> trying at all, possibly due to a bug in one of those two classes, or possibly 
> because of intermittent network failures.

That makes sense :)

REPOSITORY
  R241 KIO

BRANCH
  l-freespace-remote-2 (branched from master)

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

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

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

[Ahmad Samir]

ahmadsamir updated this revision to Diff 82334.
ahmadsamir added a comment.


  Remove comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29485?vs=82305&id=82334

BRANCH
  l-freespace-remote-2 (branched from master)

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

AFFECTED FILES
  src/core/copyjob.cpp

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

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in copyjob.cpp:477
> This comment is now completely out of context. It used to be about NFS/SMB, 
> now this information has gone and one is left wondering why kind of 
> connections we're talking about (connection to the kioslave???)

I meant connection to the remote ulr/host; however e.g Dolphin already reports 
those connection errors in the status bar when it can't connect, so the comment 
is redundant now... (I tried to keep the original code/comments in tact, 
apparently that backfired spectacularly).

REPOSITORY
  R241 KIO

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

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

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

[Ahmad Samir]

ahmadsamir updated this revision to Diff 82305.
ahmadsamir marked an inline comment as done.
ahmadsamir added a comment.


  "existingDest" is a better name for the var relating to m_asMethod

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29485?vs=82285&id=82305

BRANCH
  l-freespace-remote-2 (branched from master)

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

AFFECTED FILES
  src/core/copyjob.cpp

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

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

[Ahmad Samir]

ahmadsamir marked an inline comment as done.
ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in copyjob.cpp:430
> This is the same as "actualDest" too, so its definition could be moved 
> further up and shared with this too.
> 
> (Not that the name is perfect --- when copying ~/file.txt to ~/file2.txt, the 
> actual destination is ~/file2.txt.
> Or copying ~/dir1 as ~/dir2, then the actual destination is ~/dir2. So even a 
> name like destDir isn't perfect...)
> 
> existingDest maybe. ~ exists. ~/dir2 not yet.

> This is the same as "actualDest" too, so its definition could be moved 
> further up and shared with this too.

The code handling UDS_LOCAL_PATH is in between them, it may change m_dest.

And "existingDest" it is.

REPOSITORY
  R241 KIO

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

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

D29528: [OpenUrlJob] Improve comments/docs

[Ahmad Samir]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:1b9b239eb262: [OpenUrlJob] Improve comments/docs 
(authored by ahmadsamir).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29528?vs=82286&id=82304

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

AFFECTED FILES
  src/gui/openurljob.cpp
  src/gui/openurljob.h
  src/gui/openurljobhandlerinterface.h
  src/widgets/kdesktopfileactions.cpp
  src/widgets/kopenwithdialog.cpp
  src/widgets/krun.cpp
  src/widgets/widgetsopenurljobhandler.h

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