D29787: Fix krununittest

2020-05-16 Thread 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=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

2020-05-16 Thread 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=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


D29789: Make text always align with font base line

2020-05-16 Thread Dominik Haumann
dhaumann added a comment.


  I like this patch. In fact, I observed over the past years again and again 
that sometimes, especially if chinese or similar letters are included, the 
baseline is wrong in Kate, leading to much more overpainting that needed.
  If this patch fixes this, then I'm all for it. Or let's put it like this: The 
current implementation is wrong, this patch looks less wrong than the current 
state :-)
  
  +1

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, rjvbb, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann


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

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


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

INLINE COMMENTS

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

Typo: Desktop

REPOSITORY
  R241 KIO

BRANCH
  execfi

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

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


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

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


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

REPOSITORY
  R241 KIO

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

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


D29802: Require out-of-source builds

2020-05-16 Thread David Redondo
davidre accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R266 Breeze Icons

BRANCH
  require-in-source-build (branched from master)

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

To: ngraham, #frameworks, #vdg, ognarb, davidre
Cc: ltoscano, davidre, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns


D29802: Require out-of-source builds

2020-05-16 Thread Nathaniel Graham
ngraham retitled this revision from "Require in-source build" to "Require 
out-of-source builds".

REPOSITORY
  R266 Breeze Icons

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

To: ngraham, #frameworks, #vdg, ognarb, davidre
Cc: ltoscano, davidre, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns


D29802: Require in-source build

2020-05-16 Thread Luigi Toscano
ltoscano added a comment.


  I believe a few sentences in the the commit message contradicts each other.

REPOSITORY
  R266 Breeze Icons

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

To: ngraham, #frameworks, #vdg, ognarb, davidre
Cc: ltoscano, davidre, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns


D29802: Require in-source build

2020-05-16 Thread David Redondo
davidre requested changes to this revision.
davidre added a comment.
This revision now requires changes to proceed.


  I don't think we want to require in source builds

REPOSITORY
  R266 Breeze Icons

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

To: ngraham, #frameworks, #vdg, ognarb, davidre
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29797: [RAW PATCH] Unbreak generation with dep diagrams with Python 3 (& break P2 :) )

2020-05-16 Thread Friedrich W. H. Kossebau
kossebau added a comment.


  I just applied the diff as-is to the working checkout of kapidox on the 
server, to see if the patch also works for the setup of the nightly run, other 
than my separate testing which might have missed any special environment setup. 
So tomorrow before noon CEST we should have proof this patch catches all issues.
  So no immediate pressure to get this patch out ASAP to also get api.kde.org 
running again :)
  
  Thanks for the additional replies, will look with attention tomorrow, afk 
this evening now.

REPOSITORY
  R264 KApiDox

BRANCH
  makedepworkwithpython3

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

To: kossebau, #frameworks, ochurlaud, ognarb, cblack
Cc: kde-frameworks-devel, kde-doc-english, LeGast00n, cblack, gennad, 
fbampaloukas, michaelh, ngraham, bruns, skadinna


D29802: Require in-source build

2020-05-16 Thread Carl Schwan
ognarb accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R266 Breeze Icons

BRANCH
  require-in-source-build (branched from master)

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

To: ngraham, #frameworks, #vdg, ognarb
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29802: Require in-source build

2020-05-16 Thread Nathaniel Graham
ngraham created this revision.
ngraham added reviewers: Frameworks, VDG.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  In addition to just being a good idea in general, the dynamic icon
  generation bit doesn't work properly with in-source builds. Let's
  formally require out-of-source builds.
  
  BUG: 421637
  FIXED-IN: 5.71

TEST PLAN
  Try out of source build, get yelled at

REPOSITORY
  R266 Breeze Icons

BRANCH
  require-in-source-build (branched from master)

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

AFFECTED FILES
  CMakeLists.txt

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


D29797: [RAW PATCH] Unbreak generation with dep diagrams with Python 3 (& break P2 :) )

2020-05-16 Thread Carl Schwan
ognarb added a comment.


  In D29797#672263 , @kossebau wrote:
  
  > Thanks for the (first) review :)
  >
  > Open questions I have are these:
  >  a) how to properly check for the presence of the yaml.safe_load() method? 
and whether to support a fallback to load() otherwise? It was only introduced 
at a certain version of pyyaml
  
  
  Something like this should work
  
try:
result = yaml.safe_load()
except AttributeError:
result = yaml.load()
  
  But I think you should require having a recent enough version of pyyaml for 
safe_load() to work. load() is not great in term of security :(
  
  > b) by supposedly breaking support for Python 2, how to properly catch any 
usage of python2 now?
  
  Already answered by @cblack

REPOSITORY
  R264 KApiDox

BRANCH
  makedepworkwithpython3

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

To: kossebau, #frameworks, ochurlaud, ognarb, cblack
Cc: kde-frameworks-devel, kde-doc-english, LeGast00n, cblack, gennad, 
fbampaloukas, michaelh, ngraham, bruns, skadinna


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

2020-05-16 Thread Fabian Vogt
fvogt created this revision.
fvogt added reviewers: dfaure, marten.
Herald added a project: Frameworks.
fvogt requested review of this revision.

REVISION SUMMARY
  When a .desktop file is executed directly, it doesn't receive a parameter.
  BUG: 421364

TEST PLAN
  Test passes, applications open normally again.

REPOSITORY
  R241 KIO

BRANCH
  execfi

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

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

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


D29797: [RAW PATCH] Unbreak generation with dep diagrams with Python 3 (& break P2 :) )

2020-05-16 Thread Carson Black
cblack added a comment.


  In D29797#672263 , @kossebau wrote:
  
  > Thanks for the (first) review :)
  >
  > Open questions I have are these:
  >  a) how to properly check for the presence of the yaml.safe_load() method? 
and whether to support a fallback to load() otherwise? It was only introduced 
at a certain version of pyyaml
  >  b) by supposedly breaking support for Python 2, how to properly catch any 
usage of python2 now?
  
  
  Best thing to do would be a follow-up patch that uses python3 shebangs. But 
frankly, I wouldn't care about Python 2. It's been EOL for half a year now and 
people have had more than enough time to migrate to Python 3.

REPOSITORY
  R264 KApiDox

BRANCH
  makedepworkwithpython3

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

To: kossebau, #frameworks, ochurlaud, ognarb, cblack
Cc: kde-frameworks-devel, kde-doc-english, LeGast00n, cblack, gennad, 
fbampaloukas, michaelh, ngraham, bruns, skadinna


D29797: [RAW PATCH] Unbreak generation with dep diagrams with Python 3 (& break P2 :) )

2020-05-16 Thread Friedrich W. H. Kossebau
kossebau added a comment.


  Thanks for the (first) review :)
  
  Open questions I have are these:
  a) how to properly check for the presence of the yaml.safe_load() method? and 
whether to support a fallback to load() otherwise? It was only introduced at a 
certain version of pyyaml
  b) by supposedly breaking support for Python 2, how to properly catch any 
usage of python2 now?

REPOSITORY
  R264 KApiDox

BRANCH
  makedepworkwithpython3

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

To: kossebau, #frameworks, ochurlaud, ognarb, cblack
Cc: kde-frameworks-devel, kde-doc-english, LeGast00n, cblack, gennad, 
fbampaloukas, michaelh, ngraham, bruns, skadinna


D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-16 Thread Alexander Lohnau
alex added inline comments.

INLINE COMMENTS

> meven wrote in kpluginselector.cpp:855
> moduleInfo is part of the ctor here, so the fileName is already available 
> indirectly.
> A good thing would be to avoid having an option for such a niche usage 
> although legitimate.

> moduleInfo is part of the ctor here, so the fileName is already available 
> indirectly.

Yes, but the KCModuleProxy is just a wrapper class for the KCModule.
The actual KCModule gets created in kcmoduleloader.cpp line 93+.

PS: The moduleInfo is also available there, but I don't see any elegant way to 
make it available to the KCModule other than a property or appending it to the 
list of arguments.

>   A good thing would be to avoid having an option for such a niche usage 
> although legitimate.

Always open to suggestions 

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns


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

2020-05-16 Thread 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


D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on slow fs

2020-05-16 Thread Méven Car
meven added inline comments.

INLINE COMMENTS

> kmountpoint.cpp:444
> +// for /dir/link/dir/test will return result for 
> /destLink/dir/test
> +return findByPath(fileinfo.symLinkTarget() + 
> path.mid(cursor));
> +}

Need to pass the flag here.

REPOSITORY
  R241 KIO

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

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


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

2020-05-16 Thread 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


D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-16 Thread Méven Car
meven added inline comments.

INLINE COMMENTS

> alex wrote in kpluginselector.cpp:855
> You could add a field to the KCModule class (thats where you ultimately want 
> to access the fileName).
> But then the downside is that you don't have them as a constructor argument.

moduleInfo is part of the ctor here, so the fileName is already available 
indirectly.
A good thing would be to avoid having an option for such a niche usage although 
legitimate.

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns


D29797: [RAW PATCH] Unbreak generation with dep diagrams with Python 3 (& break P2 :) )

2020-05-16 Thread Carson Black
cblack accepted this revision.

REPOSITORY
  R264 KApiDox

BRANCH
  makedepworkwithpython3

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

To: kossebau, #frameworks, ochurlaud, ognarb, cblack
Cc: kde-frameworks-devel, kde-doc-english, LeGast00n, cblack, gennad, 
fbampaloukas, michaelh, ngraham, bruns, skadinna


D29797: [RAW PATCH] Unbreak generation with dep diagrams with Python 3 (& break P2 :) )

2020-05-16 Thread Carl Schwan
ognarb accepted this revision.
ognarb added a comment.
This revision is now accepted and ready to land.


  +1 and also +1 for removing python2 support

REPOSITORY
  R264 KApiDox

BRANCH
  makedepworkwithpython3

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

To: kossebau, #frameworks, ochurlaud, ognarb
Cc: kde-frameworks-devel, kde-doc-english, LeGast00n, cblack, gennad, 
fbampaloukas, michaelh, ngraham, bruns, skadinna


D29797: [RAW PATCH] Unbreak generation with dep diagrams with Python 3 (& break P2 :) )

2020-05-16 Thread Friedrich W. H. Kossebau
kossebau created this revision.
kossebau added reviewers: Frameworks, ochurlaud, ognarb.
Herald added projects: Frameworks, Documentation.
Herald added subscribers: kde-doc-english, kde-frameworks-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  This patch makies kapidox work for me when also generating dependency
  diagrams, both locally with what openSUSE TW provides as well as what there
  is now on the updated zivo (api.kde.org).
  
  Uploaded for first feedback, surely needs more refinement.
  
  Not being a real Python developer myself, this is just naive sample-based 
hackery and was
  not tested against Python2 (possibly breaking things due to the string
  stuff). Though I guess everyone agrees we should simply drop now the Python2
  support from kapidix, with api.kde.org server no longer needing it.

REPOSITORY
  R264 KApiDox

BRANCH
  makedepworkwithpython3

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

AFFECTED FILES
  src/kapidox/depdiagram/framework.py
  src/kapidox/depdiagram/frameworkdb.py
  src/kapidox/depdiagram/generate.py
  src/kapidox/depdiagram/gvutils.py
  src/kapidox/preprocessing.py

To: kossebau, #frameworks, ochurlaud, ognarb
Cc: kde-frameworks-devel, kde-doc-english, LeGast00n, cblack, gennad, 
fbampaloukas, michaelh, ngraham, bruns, skadinna


D29502: kwidgetsaddons: Add a named colors support in KColorCombo.

2020-05-16 Thread Friedrich W. H. Kossebau
kossebau added inline comments.

INLINE COMMENTS

> kcolorcombo.cpp:222
> +namedColors.reserve(colors.size());
> +for (auto color : colors) {
> +namedColors.append({QString(), color});

`auto& ` as we just want a reference, avoids a copy constructor call each time.

> kcolorcombo.cpp:253
> +list.reserve(d->colorList.size());
> +for (auto color : d->colorList) {
> +list.append({color.second});

`auto& ` as we just want a reference, avoids a copy constructor call each time.

> kcolorcombo.cpp:264
> +ColorList list;
> +for (int index = 0; index < STANDARD_PALETTE_SIZE; ++index) {
> +list += {QString(), standardColor(index)};

list.reserve(STANDARD_PALETTE_SIZE);

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  named_color_support

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

To: araujoluis, tcanabrava, patrickelectric, hindenburg, ngraham, #vdg
Cc: kossebau, cblack, broulik, cfeck, kde-frameworks-devel, LeGast00n, 
michaelh, ngraham, bruns


D29502: kwidgetsaddons: Add a named colors support in KColorCombo.

2020-05-16 Thread Friedrich W. H. Kossebau
kossebau added a comment.


  Isn't the recommendation to rather avoid using things like QPair, and instead 
used properly defined structs? And ideally non-nested ones, to help with cases 
of forward declarations? 
  Even QPair's API dox says so: 
  "The advent of C++11 automatic variable type deduction (auto) shifts the 
emphasis from the type name to the name of functions and members. Thus, QPair, 
like std::pair and std::tuple, is mostly useful in generic (template) code, 
where defining a dedicated type is not possible."
  
  Code which uses ".first" and ".second" is harder to understand. And any users 
of the new API who want to pass in named colors but who cannot make use of 
auto-derived type name or init-list constructors, so have to explicitly write 
the name would also prefer some named type over QPair. So 
please reconsider using some non-nested struct, perhaps named e.g. 
"KNamedColor".
  The alias "ColorList" might be also confusing, as it misses to point out this 
is a list of named colors, not just a list of colors (one might naively think 
of QList). "NamedColorList" would be less ambiguous.
  
  And as long as we are Qt5 at least., QVector would also be favourable here 
over QList given the size of the list item type and given that insertions are 
not expected to be typically done on this type.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  named_color_support

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

To: araujoluis, tcanabrava, patrickelectric, hindenburg, ngraham, #vdg
Cc: kossebau, cblack, broulik, cfeck, kde-frameworks-devel, LeGast00n, 
michaelh, ngraham, bruns


D29795: Add collaboration guide

2020-05-16 Thread Nibaldo González
nibags created this revision.
nibags added reviewers: Framework: Syntax Highlighting, dhaumann, cullmann.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
nibags requested review of this revision.

REVISION SUMMARY
  Add a file with instructions and basic information on how to collaborate in 
the repository.
  
  The idea is to provide information for first-time contributors or who are 
only going to improve or fix some XML file without anything else. This file 
collects the doubts and errors that I have seen that some new contributors have 
had.
  
  Any correction or improvement in the text is appreciated (especially, I would 
appreciate if you could check the wording).

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  add-collaboration-guide

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

AFFECTED FILES
  COLLABORATE.md

To: nibags, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann


Re: Request for ktexteditor patch release

2020-05-16 Thread Nate Graham
If we're adding stuff to a 5.70 bugfix point release, 
https://cgit.kde.org/kio.git/commit/?id=8769b6360d87c1b688eac4d0ce97594351bad13c 
is another good candidate, which fixes a recent regression 
(https://bugs.kde.org/show_bug.cgi?id=421213).


Nate



On 5/15/20 4:42 AM, Friedrich W. H. Kossebau wrote:

Am Freitag, 15. Mai 2020, 12:03:08 CEST schrieb David Faure:

On vendredi 15 mai 2020 11:01:04 CEST Friedrich W. H. Kossebau wrote:

Hi,

I would like to ask for a 5.70 patch release for ktexteditor, with
972da14f486a83556e192d09bb18a2500728895a cherry-picked.

Not a crasher, but preventing the pickup of any global view setting
changes
after a kate/kdevelop session close & open cycle for existing document
views, which has been hit and reported by a few people already the last
days.


Thanks for the notification. Done:

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


Merci.


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


No checkout of the respective repo, so possibly fastest if I give you the raw
data here:
* KTextEditor global view setting changes ignored after session reopening
(Kate, KDevelop)
https://bugs.kde.org/show_bug.cgi?id=421375


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


Yup.

Cheers
Friedrich




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

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


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

REPOSITORY
  R241 KIO

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

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


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

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

REPOSITORY
  R241 KIO

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

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


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

2020-05-16 Thread Méven Car
meven added a comment.


  It seems it is causing two failures in kiogui_applicationlauncherjobtest :
  
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.14/105/testReport/junit/projectroot/autotests/kiogui_applicationlauncherjobtest/

REPOSITORY
  R241 KIO

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

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


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

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


  API docs: remove trailing dot, mention 2 default values

REPOSITORY
  R241 KIO

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

BRANCH
  2020_05_displayOpenWithDialog

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

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

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


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

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


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

REPOSITORY
  R241 KIO

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

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


D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-16 Thread Méven Car
meven added a reviewer: ngraham.

REPOSITORY
  R241 KIO

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

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


Re: Request for ktexteditor patch release

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

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

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

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

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

### Plasma Framework

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

### KIO

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

### KTextEditor

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


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





Re: Recent breakage in kwallet

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

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

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

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

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

Any neon-specific patches to kwallet?

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





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

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


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

INLINE COMMENTS

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

You're right.

REPOSITORY
  R241 KIO

BRANCH
  l-qfile-rename (branched from master)

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

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


D29787: Fix krununittest

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


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

REPOSITORY
  R241 KIO

BRANCH
  l-terminal (branched from master)

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

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


D28874: Taiwanese holidays

2020-05-16 Thread Ricky Lindén
shrapnel added a comment.


  I just wanted to say thank you to everybody for taking the time. Taiwan is 
available in the clock/calendar settings now, really cool!

REPOSITORY
  R175 KHolidays

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

To: shrapnel, #vdg, Zren, winterz
Cc: nhiga, ngraham, winterz, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
bruns


D29258: Don't use notifybysnore.h on MSYS2

2020-05-16 Thread Łukasz Wojniłowicz
wojnilowicz added a comment.


  Could anyone review my patch again?

REPOSITORY
  R289 KNotifications

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

To: wojnilowicz, broulik, brute4s99, vonreth
Cc: vonreth, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-16 Thread Méven Car
meven added a comment.


  ping reviewers

INLINE COMMENTS

> previewjob.cpp:754
>  thumb.setText(QStringLiteral("Thumb::Mimetype"), 
> currentItem.item.mimetype());
> +thumb.setText(QStringLiteral("Thumb::DevicePixelRatio"), 
> QString::number(thumb.devicePixelRatio()));
>  QString thumbnailerVersion = 
> currentItem.plugin->property(QStringLiteral("ThumbnailerVersion"), 
> QVariant::String).toString();

Maybe add it optionally, when different from 1

REPOSITORY
  R241 KIO

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

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