D27152: Introduce FilesystemEntry class

2020-03-09 Thread David Hallas
hallas marked 4 inline comments as done.
hallas added inline comments.

INLINE COMMENTS

> bruns wrote in fstabhandling.cpp:374
> For now just keep it here, I can see now benefit of making it public (even 
> unexported).
> 
> And thanks for baring with my nitpicking ;-), this now is pretty much what I 
> hoped for.

No problem, I really appreciate the thorough review feedback you provide. I 
definitely prefer to take more time and get things right, then to rush things 
and potentially get them wrong.

So, have you had a chance to test out the patch? Do you think it is ready to 
land?

I will also start to prepare the next patch as part of splitting up the 
mega-refactor patch from D26600 

REPOSITORY
  R245 Solid

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

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


D26983: Enable adding place as first child

2020-03-09 Thread George Vogiatzis
gvgeo edited the summary of this revision.

REPOSITORY
  R241 KIO

BRANCH
  after (branched from master)

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

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


D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

2020-03-09 Thread Tranter Madi
trmdi updated this revision to Diff 77329.
trmdi added a comment.


  - Check possible actions in determineDropAction

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77298=77329

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

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


D27961: GIT_SILENT Upgrade ECM and KF5 version requirements for 5.68.0 release.

2020-03-09 Thread Tranter Madi
trmdi created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
trmdi requested review of this revision.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  src/widgets/dropjob.cpp

To: trmdi
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27961: GIT_SILENT Upgrade ECM and KF5 version requirements for 5.68.0 release.

2020-03-09 Thread Tranter Madi
trmdi abandoned this revision.

REPOSITORY
  R241 KIO

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

To: trmdi
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D25302: Remove stale symlink

2020-03-09 Thread David Faure
dfaure requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R243 KArchive

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

To: sandsmark, dfaure
Cc: dfaure, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D25302: Remove stale symlink

2020-03-09 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> karchive.cpp:986
> +// link target or be non-relative
> +if (QFile::exists(linkName) && linkName != 
> curEntry->symLinkTarget()) {
> +QFile::remove(linkName);

This is comparing the name of the symlink with the new target. Was this 
supposed to be looking at the target of the old symlink instead?

REPOSITORY
  R243 KArchive

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

To: sandsmark, dfaure
Cc: dfaure, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D27913: Icons for emoji categories

2020-03-09 Thread Aleix Pol Gonzalez
apol added a comment.


  In D27913#624990 , @guoyunhe wrote:
  
  > D27915  is for the change. The ibus 
dict comes with untranslated categories name. The category names may change. 
But it seems the dict hasn't been updated that often.
  
  
  Ah, you are right. Well then +1 from my end overall.
  
  I'd say it brings control over the UX so it makes sense.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze
Cc: apol, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns


D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

2020-03-09 Thread Tranter Madi
trmdi planned changes to this revision.

REPOSITORY
  R241 KIO

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

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


D23926: Move "Details" tab to second place in Properties dialog

2020-03-09 Thread Méven Car
meven added a comment.


  In D23926#624753 , @mthw wrote:
  
  > @meven So, the first version of this diff is good enough?
  
  
  Well no I missed @ngraham points that still stands.
  And your current code result with the same effect anyway as long as we miss 
the condition "only if details plugin"
  
  We don't have a "clean" way to do this.
  One way to do the trick would be to look at the `KService::Ptr ` `icon()` 
which will return "baloo" or the `storageId()` that should give 
"baloofilepropertiesplugin.desktop" for the baloo details plugin.
  The storageId method should be preferred as less likely to change.
  
if (ptr->serviceId() == QStringLitteral("baloofilepropertiesplugin")) {
q->insertPluginAt(plugin,1);
} else {
q->insertPlugin(plugin);
}
  
  Should be close enough to a working somewhat satisfying solution
  
  I hope this helps.

REPOSITORY
  R241 KIO

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

To: mthw, #dolphin, #frameworks, #vdg, ngraham, cfeck, pino
Cc: meven, ngraham, GB_2, #vdg, #dolphin, #frameworks, kde-frameworks-devel, 
pberestov, manueljlin, iasensio, Orage, fprice, LeGast00n, cblack, konkinartem, 
ian, jguidon, Ghost6, jraleigh, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, Codezela, feverfew, trickyricky26, michaelh, crozbo, spoorun, 
ndavis, navarromorales, firef, andrebarros, bruns, skadinna, emmanuelp, 
mikesomov, aaronhoneycutt, mbohlender


D27928: [VIM Mode] Add g g commands

2020-03-09 Thread Mikhail Zolotukhin
gikari added a comment.


  I am not sure where I should add it. Is it supposed to be a new pair of 
`bug418486` `.h/.cpp` files? Or should the test reside inside `keys.cpp` in big 
intimidating 500 line wide `MappingTest` function?

REPOSITORY
  R39 KTextEditor

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

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


D27913: Icons for emoji categories

2020-03-09 Thread Yunhe Guo
guoyunhe added a comment.


  D27915  is for the change. The ibus dict 
comes with untranslated categories name. The category names may change. But it 
seems the dict hasn't been updated that often.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze
Cc: apol, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns


D27916: Add Overpass QL highlighting

2020-03-09 Thread Dominik Haumann
dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  Almost good, please fix + ship.

INLINE COMMENTS

> overpassql.xml:7
> +-->
> + extensions="*.overpassql" author="Volker Krause (vkra...@kde.org)" 
> license="MIT">
> +  

For dsPreprocessor I think you need kateversion= 5.0.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

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


D27929: CMake syntax fixes

2020-03-09 Thread Dominik Haumann
dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  Thanks!

REPOSITORY
  R216 Syntax Highlighting

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

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


D27928: [VIM Mode] Add g g commands

2020-03-09 Thread Dominik Haumann
dhaumann added a comment.


  I like the patch, but please add a unit test before we commit this. See 
https://github.com/KDE/ktexteditor/tree/master/autotests/src/vimode
  Could you add one? :)

REPOSITORY
  R39 KTextEditor

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

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


D27844: Store and fetch complete view config in and from session config

2020-03-09 Thread Dominik Haumann
dhaumann added a comment.


  I guess we can give this a try. As I understand, though, with this patch you 
will never be able to use e.g. a global zoom once you change the zoom of a 
view. This was different before this patch.
  
  Which brings me to another point: e.g. zoom should simply zoom all documents, 
so zoom should not be done per view imho. In particular, with this patch you 
can end up with a different font size for each view, which is certainly not 
what a user wants.

REPOSITORY
  R39 KTextEditor

BRANCH
  fullviewconfigsessionsupport

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

To: kossebau, #kate, loh.tar, cullmann, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann


D26983: Enable adding place as first child

2020-03-09 Thread George Vogiatzis
gvgeo added inline comments.

INLINE COMMENTS

> kfileplacesmodel.cpp:1059
>  {
> -KBookmark bookmark = KFilePlacesItem::createBookmark(d->bookmarkManager,
> - text, url, iconName);
> +KBookmark bookmark = KFilePlacesItem::createBookmark(d->bookmarkManager, 
> text, url, iconName);
>  

It is a style change, but it is hardly unrelated, I copy this line above. I 
need to mirror the style for ease of reading. And, I'm not going to copy a 
"bad"(in lack of a better word) style.

REPOSITORY
  R241 KIO

BRANCH
  after (branched from master)

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

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


D27913: Icons for emoji categories

2020-03-09 Thread Aleix Pol Gonzalez
apol added a comment.


  In D27913#624850 , @ngraham wrote:
  
  > It seems like there are two goals here:
  >
  > - Use monochrome icons
  > - Be able to choose which icon is shown in the sidebar.
  >
  >   I don't have strong opinions about #1, but I agree that #2 is useful. 
However do we have to use breeze icons for this? Couldn't we adjust the code to 
choose a specific emoji rather than always using the first one in the grid view?
  
  
  I don't have strong opinions about #1. regarding #2 I'd say that if we want 
to decide what to show there, we can just as well show icons from the icon 
theme.
  
  Also I wonder if it will be any easy to implement since categories are coming 
from ibus and the category name comes translated.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze
Cc: apol, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
bruns


D27923: [FileIndexScheduler] Cleanup firstRun handling

2020-03-09 Thread Stefan Brüns
bruns marked 2 inline comments as done.

REPOSITORY
  R293 Baloo

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

To: bruns, #baloo, ngraham
Cc: broulik, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams


D27923: [FileIndexScheduler] Cleanup firstRun handling

2020-03-09 Thread Stefan Brüns
bruns updated this revision to Diff 77309.
bruns added a comment.


  Mark as @deprecated since 5.69
  whitespace

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27923?vs=77190=77309

BRANCH
  submit

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

AFFECTED FILES
  autotests/unit/file/filewatchtest.cpp
  src/file/fileindexerconfig.cpp
  src/file/fileindexerconfig.h
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/firstrunindexer.cpp
  src/file/indexerstate.h
  src/file/main.cpp
  src/file/mainhub.cpp
  src/file/mainhub.h
  src/file/migrator.cpp
  src/lib/indexerconfig.cpp
  src/lib/indexerconfig.h

To: bruns, #baloo, ngraham
Cc: broulik, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams


D26969: fix type namespace requirement

2020-03-09 Thread David Edmundson
davidedmundson accepted this revision.

REPOSITORY
  R283 KAuth

BRANCH
  master

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

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


D26983: Enable adding place as first child

2020-03-09 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added inline comments.

INLINE COMMENTS

> kfileplacesmodel.cpp:1059
>  {
> -KBookmark bookmark = KFilePlacesItem::createBookmark(d->bookmarkManager,
> - text, url, iconName);
> +KBookmark bookmark = KFilePlacesItem::createBookmark(d->bookmarkManager, 
> text, url, iconName);
>  

unrelated change

REPOSITORY
  R241 KIO

BRANCH
  after (branched from master)

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

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


D26555: Rename 'nepomuk' Plasma Theme icons to 'search'

2020-03-09 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.


  You're right, it was a local cache.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: The-Feren-OS-Dev, #plasma, #vdg, ndavis, bruns, ngraham
Cc: ngraham, bruns, ndavis, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh


D27953: Properly read string lists from desktop files when parsing the files

2020-03-09 Thread Aleix Pol Gonzalez
apol added a dependent revision: D27954: appstreamrunner: Port to 
KApplicationTrader.

REPOSITORY
  R309 KService

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

To: apol
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27953: Properly read string lists from desktop files when parsing the files

2020-03-09 Thread Aleix Pol Gonzalez
apol created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
apol requested review of this revision.

REVISION SUMMARY
  If we know that a field is a StringList, read it with the XDG string list 
specific codepath.
  Otherwise we get something such as QStringList("potato;potatoe") which is not 
really helpful.

TEST PLAN
  At the moment we don't get anymore X-Flatpak-RenamedFrom as
  QVariant(QStringList, ("vlc.desktop;"))

REPOSITORY
  R309 KService

BRANCH
  master

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

AFFECTED FILES
  src/services/kservice.cpp

To: apol
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27444: Added top area

2020-03-09 Thread Nathaniel Graham
ngraham added a comment.


  The actual component is called `PlasmoidHeading`, but this file is 
`PlasmoisHeader`. Maybe they should match?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg, ngraham, ndavis
Cc: mart, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D27695: WIP: Create a new TopArea element using widgets/toparea svg

2020-03-09 Thread Nathaniel Graham
ngraham added a comment.


  Watch your whitespace. This new file is full of cases where there are spaces 
on on blank lines or trailing spaces. `git show HEAD` will show you for the 
latest commit:
  
  F8167726: Screenshot_20200309_114735.png 

  
  Also it's a good habit to run `git diff` one last time before committing or 
submitting the patch to make sure little things like these are resolved easly.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

To: niccolove, mart
Cc: ngraham, davidedmundson, ahiemstra, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, bruns


KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.13 - Build # 165 - Still Unstable!

2020-03-09 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.13/165/
 Project:
kf5-qt5 SUSEQt5.13
 Date of build:
Mon, 09 Mar 2020 17:38:38 +
 Build duration:
4 min 41 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

KDE CI: Frameworks » breeze-icons » kf5-qt5 FreeBSDQt5.13 - Build # 158 - Still Unstable!

2020-03-09 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20FreeBSDQt5.13/158/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Mon, 09 Mar 2020 17:38:38 +
 Build duration:
3 min 23 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink

KDE CI: Frameworks » breeze-icons » kf5-qt5 SUSEQt5.12 - Build # 272 - Still Unstable!

2020-03-09 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/breeze-icons/job/kf5-qt5%20SUSEQt5.12/272/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Mon, 09 Mar 2020 17:38:38 +
 Build duration:
3 min 52 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 1 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: projectroot.autotests.symlink
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)100%
(6/6)100%
(6/6)80%
(239/300)62%
(112/182)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault100%
(1/1)100%
(1/1)73%
(40/55)56%
(10/18)autotests100%
(5/5)100%
(5/5)81%
(199/245)62%
(102/164)

D27952: Simplify minimum size handling

2020-03-09 Thread Volker Krause
vkrause added a comment.


  The deprecation version is off by one, but that will be fixed when rebasing 
the entire patch set on 5.69 once 5.68 is released.

REPOSITORY
  R280 Prison

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

To: vkrause, svuorela
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27952: Simplify minimum size handling

2020-03-09 Thread Volker Krause
vkrause created this revision.
vkrause added a reviewer: svuorela.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
vkrause requested review of this revision.

REVISION SUMMARY
  We don't need to track the minimum size separately anymore, so avoid this
  getting out of sync by removing it.

REPOSITORY
  R280 Prison

BRANCH
  pending

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

AFFECTED FILES
  src/lib/abstractbarcode.cpp
  src/lib/abstractbarcode.h
  src/lib/aztecbarcode.cpp
  src/lib/code128barcode.cpp
  src/lib/code39barcode.cpp
  src/lib/code93barcode.cpp
  src/lib/datamatrixbarcode.cpp
  src/lib/qrcodebarcode.cpp

To: vkrause, svuorela
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27926: Add flameshot tray icons

2020-03-09 Thread Nathaniel Graham
This revision was automatically updated to reflect the committed changes.
Closed by commit R266:8c73f3f445ee: Add flameshot tray icons (authored by 
axionl, committed by ngraham).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27926?vs=77223=77301

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

AFFECTED FILES
  icons-dark/status/22/flameshot-tray.svg
  icons/status/22/flameshot-tray.svg

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


D27913: Icons for emoji categories

2020-03-09 Thread Nathaniel Graham
ngraham added a comment.


  It seems like there are two goals here:
  
  - Use monochrome icons
  - Be able to choose which icon is shown in the sidebar.
  
  I don't have strong opinions about #1, but I agree that #2 is useful. However 
do we have to use breeze icons for this? Couldn't we adjust the code to choose 
a specific emoji rather than always using the first one in the grid view?

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, #breeze
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns


D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

2020-03-09 Thread Méven Car
meven added a comment.


  And BTW I reported a QT bug relating to the keyboard modifiers not working as 
expected a while back, making this feature more appealing : 
https://bugreports.qt.io/browse/QTBUG-79919

REPOSITORY
  R241 KIO

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

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


D27926: Add flameshot tray icons

2020-03-09 Thread Nathaniel Graham
ngraham retitled this revision from "[breeze-icons] Add flameshot tray icons" 
to "Add flameshot tray icons".

REPOSITORY
  R266 Breeze Icons

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

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


D27926: [breeze-icons] Add flameshot tray icons

2020-03-09 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Derp, silly me.

REPOSITORY
  R266 Breeze Icons

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

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


D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

2020-03-09 Thread Méven Car
meven added a comment.


  I added the bug wish https://bugs.kde.org/show_bug.cgi?id=224257 althoug it 
will require a KCM setting to be feature complete.

REPOSITORY
  R241 KIO

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

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


D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

2020-03-09 Thread Méven Car
meven edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

2020-03-09 Thread Nathaniel Graham
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  You need to only apply this logic for files. Try dragging a URL from a web 
browser to the desktop; now that fails because it can't delete the URL (the 
fact that "Move" is even an option for URL drop operations is another bug of 
course, but unrelated to this patch). Also try dragging an icon from Kickoff to 
the desktop. You should still see the drop menu by default in those cases.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27871: sftp: fix seekPos + file resuming when part file is of size 11

2020-03-09 Thread Stefan Brüns
bruns added inline comments.

INLINE COMMENTS

> sitter wrote in kio_sftp.cpp:1687
> Oh, I'm sorry. My reading comprehension isn't so good. I am not sure what you 
> are asking of me though. errno access in qdebug streaming is used all over 
> our software and this line here isn't even new. If the errno access wasn't a 
> problem before, why is it now? And what does it have to do with the fix this 
> diff has? Why don't you just do something about the errno problems?

Essentially, apply the change from the man page example to the code here:

  if (pos != sbPart->size) {
  int errsv = errno;
  qCDebug(KIO_SFTP_LOG) << "Failed to seek to" << sbPart->size << "bytes in 
source file. Reason given:" << strerror(errsv);
  }

As the errno value is now held in a local variable (and thus the 
global/thread-local errno does not matter), no function called from qCDebug() 
is able to mess with the value. qCDebug() *may* call all kind of functions 
(remember, it may log to the terminal, to the system journal, ...) which may 
fail in a non-fatal way ("if this does not work, try that"), overwriting errno 
in the course.

Making assumptions about errno can lead to some surprises, and while it may 
work most of the time it is better to be save than sorry.

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, feverfew
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov


D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

2020-03-09 Thread Tranter Madi
trmdi edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

2020-03-09 Thread Tranter Madi
trmdi added reviewers: ngraham, dfaure.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

2020-03-09 Thread Tranter Madi
trmdi created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
trmdi requested review of this revision.

REVISION SUMMARY
  Lots of users request to be able to change the default behavior of the 
drag action to MoveAction.

TEST PLAN
  With dndTomove=true in kdeglobals, drag files will move them without 
showing the menu when no key modifier is pressed.

REPOSITORY
  R241 KIO

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


Re: KIO: try to assign an icon to action submenus

2020-03-09 Thread Aleix Pol
Hi Chloe,
I just submitted the patch on your behalf:
https://phabricator.kde.org/D27950

Built it and seems to work.

Hope this helps.
Aleix

On Sat, Mar 7, 2020 at 12:27 AM Chloe Kudryavtsev  wrote:
>
> Currently, action submenus (X-KDE-Submenu) have no icons.
> This patch makes it inherit the icon of the first action.
> There will always be at least one action, since empty submenus are
> eliminated earlier in the function.
> If the icon of the first action is empty, it will gracefully revert to
> old behavior (not having an icon).
>
> Please find the single-line patch attached (I have little desire to make
> yet another account in phabricator, especially for such a small patch,
> and was encouraged to send the patch via email on irc).


D27950: Assign an icon to action submenus

2020-03-09 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 77295.
apol added a comment.


  oops

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27950?vs=77294=77295

BRANCH
  master

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

AFFECTED FILES
  src/widgets/kfileitemactions.cpp

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


D27950: Assign an icon to action submenus

2020-03-09 Thread Aleix Pol Gonzalez
apol created this revision.
apol added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
apol requested review of this revision.

REVISION SUMMARY
  Currently, action submenus (X-KDE-Submenu) have no icons.
  This patch makes it inherit the icon of the first action.
  There will always be at least one action, since empty submenus are
  eliminated earlier in the function.
  If the icon of the first action is empty, it will gracefully revert to
  old behavior (not having an icon).
  
  Patch by Chloe Kudryavtsev 

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/widgets/kfileitemactions.cpp

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


D23926: Move "Details" tab to second place in Properties dialog

2020-03-09 Thread Matej Mrenica
mthw added a comment.


  @meven So, the first version of this diff is good enough?

REPOSITORY
  R241 KIO

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

To: mthw, #dolphin, #frameworks, #vdg, ngraham, cfeck, pino
Cc: meven, ngraham, GB_2, #vdg, #dolphin, #frameworks, kde-frameworks-devel, 
pberestov, manueljlin, iasensio, Orage, fprice, LeGast00n, cblack, konkinartem, 
ian, jguidon, Ghost6, jraleigh, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, Codezela, feverfew, trickyricky26, michaelh, crozbo, spoorun, 
ndavis, navarromorales, firef, andrebarros, bruns, skadinna, emmanuelp, 
mikesomov, aaronhoneycutt, mbohlender


D27947: Port away from deprecated QList::toSet()

2020-03-09 Thread Ahmad Samir
ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, dfaure, meven.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

TEST PLAN
  make && ctest

REPOSITORY
  R241 KIO

BRANCH
  l-kcoredirlister-toset (branched from master)

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

AFFECTED FILES
  src/core/kcoredirlister.cpp
  src/filewidgets/kfilepreviewgenerator.cpp
  src/urifilters/ikws/searchproviderdlg.cpp

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


D27871: sftp: fix seekPos + file resuming when part file is of size 11

2020-03-09 Thread Harald Sitter
sitter added inline comments.

INLINE COMMENTS

> bruns wrote in kio_sftp.cpp:1687
> Which part of
> 
> > A common mistake is to do
> 
> did you miss?

Oh, I'm sorry. My reading comprehension isn't so good. I am not sure what you 
are asking of me though. errno access in qdebug streaming is used all over our 
software and this line here isn't even new. If the errno access wasn't a 
problem before, why is it now? And what does it have to do with the fix this 
diff has? Why don't you just do something about the errno problems?

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, feverfew
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov


D27635: Restore kio-webdav plugin

2020-03-09 Thread Nicolas Fella
nicolasfella added inline comments.

INLINE COMMENTS

> leinir wrote in kaccountsdplugin.h:57
> This still introduces a API break, though... Which seems especially silly 
> given the arguments aren't actually used. Perhaps add a new ctor, and have 
> the one without the arguments call the new one, but also make a qwarning (or 
> something similar) note that it's using an old ctor and really out to upgrade?

This shouldn't be in this diff at all

REPOSITORY
  R155 KAccounts Integration

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

To: nicolasfella, bshah, leinir, #frameworks


D27635: Restore kio-webdav plugin

2020-03-09 Thread Nicolas Fella
nicolasfella updated this revision to Diff 77280.
nicolasfella added a comment.


  - Remove whitespace change

REPOSITORY
  R155 KAccounts Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27635?vs=77279=77280

BRANCH
  arcpatch-D27635

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

AFFECTED FILES
  CMakeLists.txt
  src/kded/kio-webdav/CMakeLists.txt
  src/kded/kio-webdav/createkioservice.cpp
  src/kded/kio-webdav/createkioservice.h
  src/kded/kio-webdav/createnetattachjob.cpp
  src/kded/kio-webdav/createnetattachjob.h
  src/kded/kio-webdav/kioservices.cpp
  src/kded/kio-webdav/kioservices.h
  src/kded/kio-webdav/removekioservice.cpp
  src/kded/kio-webdav/removekioservice.h
  src/kded/kio-webdav/removenetattachjob.cpp
  src/kded/kio-webdav/removenetattachjob.h
  src/kded/kio-webdav/tests/CMakeLists.txt
  src/kded/kio-webdav/tests/testnetattachjobs.cpp
  src/plugins/CMakeLists.txt
  src/plugins/kio-webdav/CMakeLists.txt
  src/plugins/kio-webdav/createkioservice.cpp
  src/plugins/kio-webdav/createkioservice.h
  src/plugins/kio-webdav/createnetattachjob.cpp
  src/plugins/kio-webdav/createnetattachjob.h
  src/plugins/kio-webdav/kio-webdav.json
  src/plugins/kio-webdav/kioservices.cpp
  src/plugins/kio-webdav/kioservices.h
  src/plugins/kio-webdav/removekioservice.cpp
  src/plugins/kio-webdav/removekioservice.h
  src/plugins/kio-webdav/removenetattachjob.cpp
  src/plugins/kio-webdav/removenetattachjob.h
  src/plugins/kio-webdav/tests/CMakeLists.txt
  src/plugins/kio-webdav/tests/testnetattachjobs.cpp

To: nicolasfella, bshah, leinir, #frameworks


D27635: Restore kio-webdav plugin

2020-03-09 Thread Nicolas Fella
nicolasfella updated this revision to Diff 77279.
nicolasfella marked an inline comment as done.
nicolasfella added a comment.


  Rebase

REPOSITORY
  R155 KAccounts Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27635?vs=76416=77279

BRANCH
  arcpatch-D27635

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

AFFECTED FILES
  CMakeLists.txt
  src/kded/kded_accounts.cpp
  src/kded/kio-webdav/CMakeLists.txt
  src/kded/kio-webdav/createkioservice.cpp
  src/kded/kio-webdav/createkioservice.h
  src/kded/kio-webdav/createnetattachjob.cpp
  src/kded/kio-webdav/createnetattachjob.h
  src/kded/kio-webdav/kioservices.cpp
  src/kded/kio-webdav/kioservices.h
  src/kded/kio-webdav/removekioservice.cpp
  src/kded/kio-webdav/removekioservice.h
  src/kded/kio-webdav/removenetattachjob.cpp
  src/kded/kio-webdav/removenetattachjob.h
  src/kded/kio-webdav/tests/CMakeLists.txt
  src/kded/kio-webdav/tests/testnetattachjobs.cpp
  src/plugins/CMakeLists.txt
  src/plugins/kio-webdav/CMakeLists.txt
  src/plugins/kio-webdav/createkioservice.cpp
  src/plugins/kio-webdav/createkioservice.h
  src/plugins/kio-webdav/createnetattachjob.cpp
  src/plugins/kio-webdav/createnetattachjob.h
  src/plugins/kio-webdav/kio-webdav.json
  src/plugins/kio-webdav/kioservices.cpp
  src/plugins/kio-webdav/kioservices.h
  src/plugins/kio-webdav/removekioservice.cpp
  src/plugins/kio-webdav/removekioservice.h
  src/plugins/kio-webdav/removenetattachjob.cpp
  src/plugins/kio-webdav/removenetattachjob.h
  src/plugins/kio-webdav/tests/CMakeLists.txt
  src/plugins/kio-webdav/tests/testnetattachjobs.cpp

To: nicolasfella, bshah, leinir, #frameworks


D27704: Drop FindAccountsFileDir.cmake

2020-03-09 Thread Nicolas Fella
This revision was automatically updated to reflect the committed changes.
Closed by commit R155:17ff746ba128: Drop FindAccountsFileDir.cmake (authored by 
nicolasfella).

REPOSITORY
  R155 KAccounts Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27704?vs=76561=77278

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

AFFECTED FILES
  src/lib/CMakeLists.txt
  src/lib/cmake/CMakeLists.txt
  src/lib/cmake/FindAccountsFileDir.cmake

To: nicolasfella, #frameworks, bshah, leinir


D27708: Cleanup accounts kded module

2020-03-09 Thread Nicolas Fella
This revision was automatically updated to reflect the committed changes.
Closed by commit R155:4d36ced449f2: Cleanup accounts kded module (authored by 
nicolasfella).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D27708?vs=77182=77277#toc

REPOSITORY
  R155 KAccounts Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27708?vs=77182=77277

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

AFFECTED FILES
  src/CMakeLists.txt
  src/daemon/CMakeLists.txt
  src/daemon/daemon.cpp
  src/daemon/daemon.h
  src/daemon/kded_accounts.json
  src/daemon/kio-webdav/CMakeLists.txt
  src/daemon/kio-webdav/createkioservice.cpp
  src/daemon/kio-webdav/createkioservice.h
  src/daemon/kio-webdav/createnetattachjob.cpp
  src/daemon/kio-webdav/createnetattachjob.h
  src/daemon/kio-webdav/kioservices.cpp
  src/daemon/kio-webdav/kioservices.h
  src/daemon/kio-webdav/removekioservice.cpp
  src/daemon/kio-webdav/removekioservice.h
  src/daemon/kio-webdav/removenetattachjob.cpp
  src/daemon/kio-webdav/removenetattachjob.h
  src/daemon/kio-webdav/tests/CMakeLists.txt
  src/daemon/kio-webdav/tests/testnetattachjobs.cpp
  src/kded/CMakeLists.txt
  src/kded/kded_accounts.cpp
  src/kded/kded_accounts.h
  src/kded/kded_accounts.json
  src/kded/kio-webdav/CMakeLists.txt
  src/kded/kio-webdav/createkioservice.cpp
  src/kded/kio-webdav/createkioservice.h
  src/kded/kio-webdav/createnetattachjob.cpp
  src/kded/kio-webdav/createnetattachjob.h
  src/kded/kio-webdav/kioservices.cpp
  src/kded/kio-webdav/kioservices.h
  src/kded/kio-webdav/removekioservice.cpp
  src/kded/kio-webdav/removekioservice.h
  src/kded/kio-webdav/removenetattachjob.cpp
  src/kded/kio-webdav/removenetattachjob.h
  src/kded/kio-webdav/tests/CMakeLists.txt
  src/kded/kio-webdav/tests/testnetattachjobs.cpp

To: nicolasfella, #frameworks, bshah, leinir


D27871: sftp: fix seekPos + file resuming when part file is of size 11

2020-03-09 Thread Stefan Brüns
bruns added inline comments.

INLINE COMMENTS

> sitter wrote in kio_sftp.cpp:1687
> We do that all over the place.

Which part of

> A common mistake is to do

did you miss?

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, feverfew
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov


D27839: Properly name the content of the kcmcontrols project

2020-03-09 Thread Méven Car
meven added a comment.


  LGTM

REPOSITORY
  R296 KDeclarative

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

To: ervin, crossi, hchain, meven, bport, davidedmundson, mart, ngraham, 
#frameworks, #plasma
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 323 - Still Unstable!

2020-03-09 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/323/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Mon, 09 Mar 2020 12:33:17 +
 Build duration:
7 min 9 sec and counting
   JUnit Tests
  Name: projectroot Failed: 4 test(s), Passed: 48 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_jobtestFailed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

D27941: [protocoltojson] Replace foreach with range-for

2020-03-09 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:283b8fce056e: [protocoltojson] Replace foreach with 
range-for (authored by ahmadsamir).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27941?vs=77270=77273

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

AFFECTED FILES
  src/protocoltojson/main.cpp

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


D27444: Added top area

2020-03-09 Thread Niccolò Venerandi
niccolove updated this revision to Diff 77272.
niccolove added a comment.


  Renamed file as plasmoidheader and used scour

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27444?vs=76562=77272

BRANCH
  top_area (branched from master)

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

AFFECTED FILES
  src/desktoptheme/breeze/widgets/plasmoidheader.svg

To: niccolove, #vdg, ngraham, ndavis
Cc: mart, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D27941: [protocoltojson] Replace foreach with range-for

2020-03-09 Thread Méven Car
meven accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  l-protocoltojson (branched from master)

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

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


D27695: WIP: Create a new TopArea element using widgets/toparea svg

2020-03-09 Thread Marco Martin
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

To: niccolove, mart
Cc: davidedmundson, ahiemstra, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns


D25454: Prevent a crash when deleting a device Interface

2020-03-09 Thread Méven Car
meven abandoned this revision.
meven added a comment.


  Was superseeded by D26117 

REPOSITORY
  R245 Solid

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

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


D27695: WIP: Create a new TopArea element using widgets/toparea svg

2020-03-09 Thread Niccolò Venerandi
niccolove updated this revision to Diff 77271.
niccolove added a comment.


  documentation

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27695?vs=77266=77271

BRANCH
  master

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

AFFECTED FILES
  src/declarativeimports/plasmaextracomponents/qml/PlasmoidHeading.qml

To: niccolove, mart
Cc: davidedmundson, ahiemstra, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns


D27941: [protocoltojson] Replace foreach with range-for

2020-03-09 Thread Ahmad Samir
ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, dfaure, meven.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

TEST PLAN
  make && ctest

REPOSITORY
  R241 KIO

BRANCH
  l-protocoltojson (branched from master)

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

AFFECTED FILES
  src/protocoltojson/main.cpp

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


D27502: Create ConfigView an unmanaged ConfigWidget

2020-03-09 Thread Benjamin Port
bport updated this revision to Diff 77269.
bport added a comment.


  rename check to skip and add missing negation for default

REPOSITORY
  R246 Sonnet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27502?vs=76517=77269

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

AFFECTED FILES
  autotests/test_settings.cpp
  src/core/CMakeLists.txt
  src/core/languagefilter.cpp
  src/core/loader.cpp
  src/core/loader_p.h
  src/core/settings.cpp
  src/core/settings.h
  src/core/settings_p.h
  src/core/settingsimpl.cpp
  src/core/settingsimpl_p.h
  src/core/speller.cpp
  src/ui/CMakeLists.txt
  src/ui/configui.ui
  src/ui/configview.cpp
  src/ui/configview.h
  src/ui/configwidget.cpp
  src/ui/dialog.cpp
  src/ui/highlighter.cpp

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


D27695: WIP: Create a new TopArea element using widgets/toparea svg

2020-03-09 Thread Niccolò Venerandi
niccolove updated this revision to Diff 77266.
niccolove added a comment.


  using =|

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27695?vs=77263=77266

BRANCH
  master

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

AFFECTED FILES
  src/declarativeimports/plasmaextracomponents/qml/PlasmoidHeading.qml

To: niccolove, mart
Cc: davidedmundson, ahiemstra, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns


D27695: WIP: Create a new TopArea element using widgets/toparea svg

2020-03-09 Thread Niccolò Venerandi
niccolove marked an inline comment as done.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, mart
Cc: davidedmundson, ahiemstra, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns


D27923: [FileIndexScheduler] Cleanup firstRun handling

2020-03-09 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> indexerstate.h:41
>  Unavailable,
> + Startup,
>  };

Indentation

> indexerconfig.cpp:112
>  
> +// KF6 TODO: remove
>  bool IndexerConfig::firstRun() const

Please mark them as deprecated in the header file

REPOSITORY
  R293 Baloo

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

To: bruns, #baloo, ngraham
Cc: broulik, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams


D27695: WIP: Create a new TopArea element using widgets/toparea svg

2020-03-09 Thread Marco Martin
mart added inline comments.

INLINE COMMENTS

> PlasmoidHeading.qml:52
> +enabledBorders: {
> +var borders = new Array()
> +borders.push('LeftBorder')

this can be an int
and instead of borders.push('LeftBorder')
borders |= PlasmaCore.FrameSvg.LeftBorder

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, mart
Cc: davidedmundson, ahiemstra, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns


D27695: WIP: Create a new TopArea element using widgets/toparea svg

2020-03-09 Thread Niccolò Venerandi
niccolove updated this revision to Diff 77263.
niccolove added a comment.


  Correct padding and borders

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27695?vs=76947=77263

BRANCH
  master

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

AFFECTED FILES
  src/declarativeimports/plasmaextracomponents/qml/PlasmoidHeading.qml

To: niccolove, mart
Cc: davidedmundson, ahiemstra, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns


D26560: Fix selection printing

2020-03-09 Thread Robert Hoffmann
hoffmannrobert added a comment.


  In case my last comment was mistakable, I fixed this:
  
  > 1. Create a document with just one long line that wraps over two printed 
pages. I this case, I am not able to print only the selected text properly.
  
  but I couldn't fix this:
  
  > 2. Say you have a line that wraps over e.g. 5 visual lines. And you just 
want to print the visually wrapped line 4 and 5. Right now, the visual lines 
1-3 are just empty, but still take place. Maybe it makes sense to omit fully 
empty lines completely?

REPOSITORY
  R39 KTextEditor

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

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


D26969: fix type namespace requirement

2020-03-09 Thread Harald Sitter
sitter updated this revision to Diff 77262.
sitter added a comment.


  switch to indexOfMethod, also construct a signature from the slotname. 
indexOf requires a normalized signature (e.g. `save(QVariantMap)`)

REPOSITORY
  R283 KAuth

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26969?vs=74514=77262

BRANCH
  master

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

AFFECTED FILES
  src/backends/dbus/DBusHelperProxy.cpp
  src/kauthactionreply.h

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


D27871: sftp: fix seekPos + file resuming when part file is of size 11

2020-03-09 Thread Harald Sitter
sitter added a comment.


  In D27871#623593 , @bruns wrote:
  
  > In D27871#623542 , @sitter wrote:
  >
  > > In D27871#622933 , @bruns 
wrote:
  > >
  > > > Can you also mention why errno == EGAIN does not have to be handled 
("EAGAIN could only happen iff the file where opened with O_NONBLOCK. All other 
seek errors are fatal.").
  > >
  > >
  > > I do not understand what that means.
  >
  >
  > Previously, EAGAIN was handled explicitly (although the implementation was 
wrong). Why is it fine to **not** handle it? This belongs in the commit message.
  
  
  But lseek does not use EAGAIN, does it?
  In any case, can't you edit the message to make it say what you think it 
should say? (if not I am sure sysadmins can give you magic powers)

INLINE COMMENTS

> bruns wrote in kio_sftp.cpp:1687
> qCDebug(), operator<<

We do that all over the place.

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, feverfew
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov


D27873: sftp: do not show creation time when we do not know it

2020-03-09 Thread Harald Sitter
sitter added a comment.


  In D27873#623582 , @bruns wrote:
  
  > Please correct the comment in the code - it depends on the server SFTP 
implementation.
  
  
  Birth time does not depend on the file system?

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, bruns
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov


D27872: sftp: fix partial transfer resuming when copying to local

2020-03-09 Thread Harald Sitter
sitter added inline comments.

INLINE COMMENTS

> bruns wrote in kio_sftp.cpp:1935
> Dependent on the file system, this will no longer catch `partFile.isDir()`. 
> stat.st_size is only defined for regular files and symlinks, for all other 
> types it is implementation defined. E.g. XFS and Btrfs return 0 for empty 
> directories.

Your comment is dazzling. What you meant to say is that the size check ought to 
be after the dir check?

> bruns wrote in kio_sftp.cpp:1936
> should be `if (!partFile.isFile())` - we cant resume a pipe or socket ...

Please make a diff.

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, feverfew, bruns
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov


D26555: Rename 'nepomuk' Plasma Theme icons to 'search'

2020-03-09 Thread Stefan Brüns
bruns added a comment.


  In D26555#591657 , @ngraham wrote:
  
  > Looks like this needs a bit of CMake work to stop installing the old icons:
  >
  >   # from directory: /home/nate/kde/build/plasma-framework
  >   gmake[2]: *** No rule to make target 
'/home/nate/kde/src/plasma-framework/src/desktoptheme/air/icons/nepomuk.svg', 
needed by 'src/desktoptheme/oxygen/oxygen.gzipped/icons/nepomuk.svgz'.  Stop.
  >   gmake[1]: *** [CMakeFiles/Makefile2:880: 
src/desktoptheme/oxygen/CMakeFiles/oxygen_desktoptheme_graphics_icons.dir/all] 
Error 2
  >
  >
  > Probably need to add the new ones too.
  
  
  @ngraham - the icons are picked up by a glob, 
https://phabricator.kde.org/source/plasma-framework/browse/master/src/desktoptheme/air/CMakeLists.txt$30,
 probably just some stale CMake cache on your side? Can you recheck?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: The-Feren-OS-Dev, #plasma, #vdg, ndavis, bruns, ngraham
Cc: ngraham, bruns, ndavis, davidre, kde-frameworks-devel, LeGast00n, cblack, 
GB_2, michaelh


D27708: Cleanup accounts kded module

2020-03-09 Thread Bhushan Shah
bshah accepted this revision.

REPOSITORY
  R155 KAccounts Integration

BRANCH
  arcpatch-D27708

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

To: nicolasfella, #frameworks, bshah, leinir


D27695: WIP: Create a new TopArea element using widgets/toparea svg

2020-03-09 Thread Marco Martin
mart added a comment.


  In D27695#621985 , @niccolove 
wrote:
  
  > - Is code on line 51 fine? Lines are a bit long there
  
  
  yeah, is fine..
  unfortunately sunch superlong lines end up being very common in QML

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, mart
Cc: davidedmundson, ahiemstra, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns


D27152: Introduce FilesystemEntry class

2020-03-09 Thread Stefan Brüns
bruns added inline comments.

INLINE COMMENTS

> hallas wrote in fstabhandling.cpp:374
> I guess the main reason is that the id is a function of a `FilesystemEntry`, 
> meaning that the id is computed from the contents of it. You are absolutely 
> right that it is not picked up directly from the fstab/mtab file, but it is 
> computed from the contents. I think it should either be a member function of 
> the `FilesystemEntry` class or a static member function of some other class 
> (taking the `FilesystemEntry` as an input paramter) - it at least has to be 
> in a place where it is possible to write a unit test for it's functionality. 
> especially since the id() is used as part of the public API. Therefore I 
> don't think we should make this an internal function in fstabhandling. 
> Another reason is that, we might need the id in other places then the 
> fstabhandling, at least we need it for the rest of the fstab handling 
> refactor I posted in the other review.
> 
> So long story short :)
> I think we should either keep it here or make it a public (static) method 
> somewhere else - @bruns what do you think?

For now just keep it here, I can see now benefit of making it public (even 
unexported).

And thanks for baring with my nitpicking ;-), this now is pretty much what I 
hoped for.

REPOSITORY
  R245 Solid

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

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


D27635: Restore kio-webdav plugin

2020-03-09 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  Minus that little API funniness, looks pretty good from here :)

INLINE COMMENTS

> CMakeLists.txt:9
>  
> -add_library(kaccounts_kio_webdav_plugin MODULE ${kio-webdav_SRCS})
> +
> +kcoreaddons_add_plugin(kaccounts_kio_webdav_plugin

wouldn't mention it, but as there's other stuff as well, looks like a stray 
newline :)

> kaccountsdplugin.h:57
>  public:
> -KAccountsDPlugin(QObject *parent = nullptr);
> +KAccountsDPlugin(QObject *parent, const QVariantList& args);
>  virtual ~KAccountsDPlugin();

This still introduces a API break, though... Which seems especially silly given 
the arguments aren't actually used. Perhaps add a new ctor, and have the one 
without the arguments call the new one, but also make a qwarning (or something 
similar) note that it's using an old ctor and really out to upgrade?

REPOSITORY
  R155 KAccounts Integration

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

To: nicolasfella, bshah, leinir, #frameworks


D27704: Drop FindAccountsFileDir.cmake

2020-03-09 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  Removing unused stuff is always nice, well spotted :)

REPOSITORY
  R155 KAccounts Integration

BRANCH
  cma

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

To: nicolasfella, #frameworks, bshah, leinir


D27708: Cleanup accounts kded module

2020-03-09 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  With the caveat that i'm not a big pile of brains about the plugin system, 
this looks like pretty sensible to me :)

REPOSITORY
  R155 KAccounts Integration

BRANCH
  arcpatch-D27708

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

To: nicolasfella, #frameworks, bshah, leinir


D27940: Add a KCapacityBar::State enum and attribute to allow display different status

2020-03-09 Thread David Redondo
davidre added a comment.


  Yes that's a problem that KColorScheme currently lives in KConfigWidgets. 
Maybe similiar to  D27038  and D27039 
?  Hopefully we can achieve T12218 


REPOSITORY
  R236 KWidgetsAddons

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

To: meven, cfeck, #frameworks
Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D27940: Add a KCapacityBar::State enum and attribute to allow display different status

2020-03-09 Thread Méven Car
meven updated this revision to Diff 77257.
meven added a comment.


  Add @since 5.69

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27940?vs=77256=77257

BRANCH
  master

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

AFFECTED FILES
  src/kcapacitybar.cpp
  src/kcapacitybar.h
  src/kmessagewidget.cpp

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


D27940: Add a KCapacityBar::State enum and attribute to allow display different status

2020-03-09 Thread Méven Car
meven updated this revision to Diff 77256.
meven added a comment.


  Remove unnecessary changes

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27940?vs=77255=77256

BRANCH
  master

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

AFFECTED FILES
  src/kcapacitybar.cpp
  src/kcapacitybar.h
  src/kmessagewidget.cpp

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


D27940: Add a KCapacityBar::State enum and attribute to allow display different status

2020-03-09 Thread Méven Car
meven created this revision.
meven added reviewers: cfeck, Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
meven requested review of this revision.

REVISION SUMMARY
  Question:
  Is there a way to get theme colors for kconfigwidgets rather than a harcoded 
color ?
  
  For use in D16353 
  Will need a patch to breeze as well.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  master

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

AFFECTED FILES
  src/kcapacitybar.cpp
  src/kcapacitybar.h
  src/kmessagewidget.cpp

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


D26983: Enable adding place as first child

2020-03-09 Thread George Vogiatzis
gvgeo added a comment.


  ping #frameworks 

REPOSITORY
  R241 KIO

BRANCH
  after (branched from master)

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

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


D26983: Enable adding place as first child

2020-03-09 Thread George Vogiatzis
gvgeo edited the test plan for this revision.

REPOSITORY
  R241 KIO

BRANCH
  after (branched from master)

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

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


D27279: Port away from QWidget

2020-03-09 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R274:5b6d26cd758c: Port away from QWidget (authored by 
broulik).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D27279?vs=75345=77252#toc

REPOSITORY
  R274 KIdleTime

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27279?vs=75345=77252

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

AFFECTED FILES
  src/widgetbasedpoller.cpp
  src/widgetbasedpoller.h

To: broulik, #frameworks, davidedmundson
Cc: alnikiforov, davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns


D27357: Fix infinite recursion in xscreensaver plugin

2020-03-09 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R274:3dae871e6b89: Fix infinite recursion in xscreensaver 
plugin (authored by alnikiforov, committed by broulik).

REPOSITORY
  R274 KIdleTime

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27357?vs=75583=77251

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

AFFECTED FILES
  src/plugins/xscreensaver/xscreensaverbasedpoller.cpp

To: alnikiforov, davidedmundson, broulik, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27932: NetworkManager-Qt: Convert license headers to SPDX expressions

2020-03-09 Thread Jan Grulich
jgrulich accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R282 NetworkManagerQt

BRANCH
  spdx

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

To: cordlandwehr, jgrulich
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D23926: Move "Details" tab to second place in Properties dialog

2020-03-09 Thread Méven Car
meven added a comment.


  In D23926#538778 , @mthw wrote:
  
  > > 3. Instead of unconditionally doing `q->insertPluginAt(plugin, 1);`, 
you'd want to do something more like this:
  > > 
  > > 
  > > 
  > >   if (plugin == "details"  /* this is pseudocode */) {
  > >   q->insertPluginAt(plugin, 1);
  > >   } else {
  > >   q->insertPlugin(plugin);
  > >   }
  > > 
  > > 
  > > I'm actually not sure how to check for what the plugin is though. Maybe 
someone from #frameworks  can help.
  >
  > This is mostly done but we still need to do this the check, but neither me 
nor @ngraham know how to do it.
  
  
  An alternative would be to move the whole block inserting 
KPropertiesDialog/Plugin before "KChecksumsPlugin".
  AFAIK there are only two KPropertiesDialog/Plugin, so this would move details 
and baloo both of before the rest, which is fine IMO.
  You would not need an integer parameter to `insertPluginAt` anymore.

INLINE COMMENTS

> kpropertiesdialog.cpp:586
>  
> +
>  if (KFilePermissionsPropsPlugin::supports(m_items)) {

Not needed, if you could remove

REPOSITORY
  R241 KIO

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

To: mthw, #dolphin, #frameworks, #vdg, ngraham, cfeck, pino
Cc: meven, ngraham, GB_2, #vdg, #dolphin, #frameworks, kde-frameworks-devel, 
pberestov, manueljlin, iasensio, Orage, fprice, LeGast00n, cblack, konkinartem, 
ian, jguidon, Ghost6, jraleigh, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, Codezela, feverfew, trickyricky26, michaelh, crozbo, spoorun, 
ndavis, navarromorales, firef, andrebarros, bruns, skadinna, emmanuelp, 
mikesomov, aaronhoneycutt, mbohlender


D27152: Introduce FilesystemEntry class

2020-03-09 Thread David Hallas
hallas added inline comments.

INLINE COMMENTS

> bruns wrote in fstabhandling.cpp:374
> Why is the id() a property of the entry now?
> 
> - it does not correspond to anything from the fstab/mtab
> - it is only used here to generate the key for the cache
> 
> Please move the id generation back here, eventually changing it from 
> `_k_deviceNameForMountpoint(const QString , const QString 
> ,const QString )` to `_k_deviceIdForFsentry(const 
> FilesystemEntry&)`

I guess the main reason is that the id is a function of a `FilesystemEntry`, 
meaning that the id is computed from the contents of it. You are absolutely 
right that it is not picked up directly from the fstab/mtab file, but it is 
computed from the contents. I think it should either be a member function of 
the `FilesystemEntry` class or a static member function of some other class 
(taking the `FilesystemEntry` as an input paramter) - it at least has to be in 
a place where it is possible to write a unit test for it's functionality. 
especially since the id() is used as part of the public API. Therefore I don't 
think we should make this an internal function in fstabhandling. Another reason 
is that, we might need the id in other places then the fstabhandling, at least 
we need it for the rest of the fstab handling refactor I posted in the other 
review.

So long story short :)
I think we should either keep it here or make it a public (static) method 
somewhere else - @bruns what do you think?

REPOSITORY
  R245 Solid

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

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