D28787: Fix PC3 BusyIndicator binding loop

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Plasma, davidedmundson.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  Without this, the Busy Indicator generates binding loops for me when used in 
the new
  ExpandableListItem component.

TEST PLAN
  No more binding loops in console logging

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  fix-busyindicator-binding-loop (branched from master)

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

AFFECTED FILES
  src/declarativeimports/plasmacomponents3/BusyIndicator.qml

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

D28746: Show previews on encrypted filesystems

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Works and looks good to me modulo one comment, but let's wait for the other 
reviewers too.

REPOSITORY
  R241 KIO

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

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

D28745: Skipping catching of thumbnails on encrytped filesystems

[Nathaniel Graham]

ngraham added reviewers: ivan, broulik, Dolphin.

REPOSITORY
  R320 KIO Extras

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

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

D28746: Show previews on encrypted filesystems

[Nathaniel Graham]

ngraham added reviewers: ivan, Frameworks, dfaure.
ngraham added a comment.


  General idea looks sane to me. Thanks for working on this; it's been one of 
those minor annoyances of mine.

INLINE COMMENTS

> previewjob.cpp:718
>  }
> -
>  if (save) {

unrelated change

REPOSITORY
  R241 KIO

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

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

D28745: Skipping catching of thumbnails on encrypted filesystems

[Nathaniel Graham]

ngraham retitled this revision from "Skipping catching of thumbnails on 
encrytped filesystems" to "Skipping catching of thumbnails on encrypted 
filesystems".
ngraham edited the summary of this revision.

REPOSITORY
  R320 KIO Extras

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

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

D28770: Avoid blocking the UI thread

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> currency.cpp:40
>  #include 
> +#include 
> +#include 

Unused?

> currency.cpp:884
> +
> +#include "currency.moc"

why?

REPOSITORY
  R292 KUnitConversion

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

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

D28769: Add Icelandic Krona

[Nathaniel Graham]

ngraham added a comment.


  Hmm, this patch doesn't apply for me.

REPOSITORY
  R292 KUnitConversion

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

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

Re: Update on Status of Gitlab Migration

[Johan Ouwerkerk]

On Sun, Apr 12, 2020 at 12:49 AM Johan Ouwerkerk  wrote:
>
> >
> > We may need to do on-the-fly conversion of the kde: repo paths if they
> > won't be expressible as 'kde:foo' in the future, but we should have the
> > information needed to do this in kdesrc-build to make this happen
> > on-the-fly.
> >
>
> Yes, this should be fairly straight forward: we could do a `git remote
> set-url` based on what the repo metadata tells us before updating a
> local clone. In fact: we could build this right now and sell it as
> "automagically recover your upstream".  :)
>
> I might try to hack something up tomorrow or monday for that.
>

A basic version of this is now available via:
https://invent.kde.org/kde/kdesrc-build/-/merge_requests/27
With this feature sysadmin should now be free to change the repopath
value in the metadata YAML and kdesrc-build will reconfigure the
remote URL appropriately automatically. This works as long as the same
`kde:$path` expression works for both fetch and push, i.e. the layout
on the anongit.kde.org network must match with the layout of
git.kde.org/invent.kde.org. If necessary a simple path prefix change
could still work with a minor update to the pushInsteadOf mapping,
e.g. when repos on invent are mapped to kde/   instead of
/.

You can also experiment with setting invent.kde.org as the push URL by
setting x-invent-kde-push-urls to true in your rc file. The effect
should be visible through git remote -vv afterwards. (Disable the
setting and re-run again afterwards because this will obviously break
your push URLs as long as the Gitlab migration hasn't completed yet).

Regards,

- Johan

D28782: [Fstab] Do not emit deviceAdded twice on fstab/mtab changes

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> apol wrote in fstabmanager.cpp:122
> The debug seems wrong and probably unnecessary. I'd remove it.

What do you think is wrong with it?

And its the only debug output there is on this level, and always having to add 
debug statements in case of errors and then recompile it to get some traces is 
quite suboptimal. You can't ask a regular user to do it, and its silent by 
default.

REPOSITORY
  R245 Solid

BRANCH
  submit

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

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

D28780: [FstabWatcher] Fix loosing of fstab watcher

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> apol wrote in fstabwatcher.cpp:110
> Use m_isFstabWatched? Or maybe m_isFstabWatched isn't necessary altogether 
> and we can check in other places too.

No. QFileSystemWatcher looses the watch when the file is deleted/renamed, so 
m_isFstabWatched may be (still) true when contains() returns false.

REPOSITORY
  R245 Solid

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

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

D28776: FstabDevice: Avoid recurrent construction of emblems QStringList

[Aleix Pol Gonzalez]

apol requested changes to this revision.
apol added a comment.
This revision now requires changes to proceed.


  Address the scope of the statics as kai mentioned in D28778 
.
  Or use Q_GLOBAL_STATIC.

REPOSITORY
  R245 Solid

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

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

D28778: [FstabWatcher] Use static QStringLiteral instead of macro

[Aleix Pol Gonzalez]

apol added a comment.


  In D28778#646791 , @broulik wrote:
  
  > Don't use global statics in library code
  
  
  in case it gets statically compiled?

REPOSITORY
  R245 Solid

BRANCH
  submit

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

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

D28778: [FstabWatcher] Use static QStringLiteral instead of macro

[Kai Uwe Broulik]

broulik added a comment.


  Don't use global statics in library code

REPOSITORY
  R245 Solid

BRANCH
  submit

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

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

D28776: FstabDevice: Avoid recurrent construction of emblems QStringList

[Aleix Pol Gonzalez]

apol accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R245 Solid

BRANCH
  submit

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

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

D28780: [FstabWatcher] Fix loosing of fstab watcher

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> fstabwatcher.cpp:110
>  emit fstabChanged();
>  if (!m_fileSystemWatcher->files().contains(s_fstabFile)) {
> +m_isFstabWatched = m_fileSystemWatcher->addPath(s_fstabFile);

Use m_isFstabWatched? Or maybe m_isFstabWatched isn't necessary altogether and 
we can check in other places too.

REPOSITORY
  R245 Solid

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

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

D28778: [FstabWatcher] Use static QStringLiteral instead of macro

[Aleix Pol Gonzalez]

apol accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R245 Solid

BRANCH
  submit

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

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

D28782: [Fstab] Do not emit deviceAdded twice on fstab/mtab changes

[Aleix Pol Gonzalez]

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

INLINE COMMENTS

> fstabmanager.cpp:122
> +
> +qCDebug(FSTAB) << oldlist << "->" << newlist;
> +

The debug seems wrong and probably unnecessary. I'd remove it.

REPOSITORY
  R245 Solid

BRANCH
  submit

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

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

D28779: [Fstab] Use new style connect API

[Aleix Pol Gonzalez]

apol accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R245 Solid

BRANCH
  submit

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

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

D8274: Don't create new symlinks when copying symlinks - copy the file's contents - like /usr/bin/cp

[Shlomi Fish]

shlomif abandoned this revision.
shlomif added a comment.


  Closing then. I don't have any immediate plans for reworking this patch.

REPOSITORY
  R241 KIO

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

To: shlomif, dfaure
Cc: kde-frameworks-devel, ngraham, rkflx, dfaure, bcooksley, LeGast00n, cblack, 
michaelh, bruns

D28779: [Fstab] Use new style connect API

[Stefan Brüns]

bruns added a dependent revision: D28782: [Fstab] Do not emit deviceAdded twice 
on fstab/mtab changes.

REPOSITORY
  R245 Solid

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

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

D28782: [Fstab] Do not emit deviceAdded twice on fstab/mtab changes

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Frameworks, afiestas.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  The deviceAdded signal was emitted twice, once with the old device list
  in m_deviceList, once with the new one.
  
  The first one is actually wrong and was fixed in kdelibs4 by moving the
  deviceAdded signal after the m_deviceList update. In KF5, the second
  signal was added but the first one never removed.
  
  Move the m_deviceList update to the top (though for deviceRemoved it
  does not matter much), and remove the second signal.
  
  Depends on D28779 

TEST PLAN
  1. solid-hardware listen
  2. add an entry to the fstab
  
  -> deviceAdded is only emitted once

REPOSITORY
  R245 Solid

BRANCH
  submit

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabmanager.cpp

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

D28779: [Fstab] Use new style connect API

[Stefan Brüns]

bruns added a dependent revision: D28780: [FstabWatcher] Fix loosing of fstab 
watcher.

REPOSITORY
  R245 Solid

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

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

D28780: [FstabWatcher] Fix loosing of fstab watcher

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  In case the fstab is deleted (caused by editing it), the watch will be
  removed. The code already partially dealed with this, i.e. it tried to
  readd the watch, but did not check if the operation succeeded and dropped
  any further changes if not.
  
  Keep a watch on the containing folder and readd the fstab on change
  events. Unfortunately QFilesystemWatcher does not allow fine granular
  change events, though as /etc/ is likely mostly silent this should not
  matter much.
  
  The bug becomes apparent when e.g. editing the fstab with VIM, which
  has a quite broken implementation of creating temporary/backup files
  during save: it moves away the original file and creates the new file
  directly under the original name, instead of using a atomic rename.

TEST PLAN
  1. solid-hardware listen
  2. open fstab with vi, save a few times
  
  Without the change, solid will loose the watch sooner or later.
  
  Depends on D28779 

REPOSITORY
  R245 Solid

BRANCH
  submit

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabwatcher.cpp
  src/solid/devices/backends/fstab/fstabwatcher.h

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

D28779: [Fstab] Use new style connect API

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Depends on D28778 

TEST PLAN
  No functional changes

REPOSITORY
  R245 Solid

BRANCH
  submit

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabdevice.h
  src/solid/devices/backends/fstab/fstabmanager.cpp
  src/solid/devices/backends/fstab/fstabwatcher.cpp

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

D28778: [FstabWatcher] Use static QStringLiteral instead of macro

[Stefan Brüns]

bruns added a dependent revision: D28779: [Fstab] Use new style connect API.

REPOSITORY
  R245 Solid

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

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

D28778: [FstabWatcher] Use static QStringLiteral instead of macro

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  QFilesystemWatcher::addPath takes a QString as argument, so avoid
  construction of tempary QString on each call.
  
  This also avoids a name clash between the Solid::Backends::Fstab::FSTAB
  logging category and the FSTAB macro.

TEST PLAN
  no functional changes

REPOSITORY
  R245 Solid

BRANCH
  submit

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabwatcher.cpp

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

D28776: FstabDevice: Avoid recurrent construction of emblems QStringList

[Stefan Brüns]

bruns added a dependent revision: D28777: FstabDevice: Reevaluate emblems only 
when state changes.

REPOSITORY
  R245 Solid

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

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

D28777: FstabDevice: Reevaluate emblems only when state changes

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Instead of evaluating the mount state on each call just evaluate it
  when it changes, signaled by mtabChanged.
  
  Depends on D28776 

TEST PLAN
  correct emblem still shown and updated

REPOSITORY
  R245 Solid

BRANCH
  submit

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabdevice.h

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

D28776: FstabDevice: Avoid recurrent construction of emblems QStringList

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  emblems can return exactly two different values. Construct the two
  possible values statically, so these can be shared by all FstabDevice
  instances.

TEST PLAN
  correct emblems are still shown

REPOSITORY
  R245 Solid

BRANCH
  submit

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabdevice.cpp

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

D28535: [KIO-MTP] Fix null pointer dereference

[Elvis Angelaccio]

elvisangelaccio added a comment.


  In D28535#642298 , @feverfew wrote:
  
  > In D28535#642289 , 
@elvisangelaccio wrote:
  >
  > > In D28535#640833 , @fvogt 
wrote:
  > >
  > > > I assume there is a reason why `MTPDevice::getDevice()` has code for 
handling this very specific case, so I wouldn't just remove it without figuring 
out why: https://i.redd.it/hfnl7xo8yovy.gif
  > > >
  > > > If not, that would indeed be the best option.
  > >
  > >
  > > Unfortunately git blame doesn't seem to help us here.
  > >
  > > I suggest to push this fix to master only and see what happens.
  >
  >
  > By pushing this to master would we still be able to throw it up to 20.04.* 
if we decide it's stable enough? (also need to know to know what to put down as 
the FIXED-IN in the commit message)?
  
  
  Sure, we can cherry-pick that commit later.

REPOSITORY
  R320 KIO Extras

BRANCH
  fixNullPtr (branched from master)

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

To: feverfew, akrutzler, dfaure, elvisangelaccio
Cc: anthonyfieroni, kde-frameworks-devel, fvogt, kfm-devel, ngraham, azyx, 
nikolaik, pberestov, iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D28755: Breeze Icons cannot be built from read-only source location

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Nice improvement.

REPOSITORY
  R266 Breeze Icons

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

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

D28535: [KIO-MTP] Fix null pointer dereference

[Alexander Saoutkin]

feverfew added a comment.


  In D28535#642298 , @feverfew wrote:
  
  > In D28535#642289 , 
@elvisangelaccio wrote:
  >
  > > In D28535#640833 , @fvogt 
wrote:
  > >
  > > > I assume there is a reason why `MTPDevice::getDevice()` has code for 
handling this very specific case, so I wouldn't just remove it without figuring 
out why: https://i.redd.it/hfnl7xo8yovy.gif
  > > >
  > > > If not, that would indeed be the best option.
  > >
  > >
  > > Unfortunately git blame doesn't seem to help us here.
  > >
  > > I suggest to push this fix to master only and see what happens.
  >
  >
  > By pushing this to master would we still be able to throw it up to 20.04.* 
if we decide it's stable enough? (also need to know to know what to put down as 
the FIXED-IN in the commit message)?
  
  
  @elvisangelaccio ping seeming as 20.04 release is close...

REPOSITORY
  R320 KIO Extras

BRANCH
  fixNullPtr (branched from master)

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

To: feverfew, akrutzler, dfaure, elvisangelaccio
Cc: anthonyfieroni, kde-frameworks-devel, fvogt, kfm-devel, ngraham, azyx, 
nikolaik, pberestov, iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, 
Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, 
andrebarros, bruns, emmanuelp, rdieter, mikesomov

D24773: kio_trash: Add size, modification, access and create date for trash:/

[Méven Car]

meven updated this revision to Diff 79943.
meven added a comment.


  Improve naming of struct CacheData to SizeAndModTime

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24773?vs=79942=79943

BRANCH
  arcpatch-D24773_1

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/kfileitem.h
  src/core/udsentry.h
  src/ioslaves/trash/kio_trash.cpp
  src/ioslaves/trash/trashimpl.cpp
  src/ioslaves/trash/trashimpl.h
  src/ioslaves/trash/trashsizecache.cpp
  src/ioslaves/trash/trashsizecache.h

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

D24773: kio_trash: Add size, modification, access and create date for trash:/

[Méven Car]

meven updated this revision to Diff 79942.
meven added a comment.
This revision is now accepted and ready to land.


  Improve latest modification time of trash implementation, fix case when a 
symlink is in the trash returning incorrect trash size

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24773?vs=79813=79942

BRANCH
  arcpatch-D24773_1

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/kfileitem.h
  src/core/udsentry.h
  src/ioslaves/trash/kio_trash.cpp
  src/ioslaves/trash/trashimpl.cpp
  src/ioslaves/trash/trashimpl.h
  src/ioslaves/trash/trashsizecache.cpp
  src/ioslaves/trash/trashsizecache.h

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

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

[Ahmad Samir]

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

TEST PLAN
  The kfontrequestertest app still works

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  l-kfontreq (branched from master)

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

AFFECTED FILES
  src/kfontrequester.cpp
  src/kfontrequester.h

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

D25267: Improve XCF support

[Albert Astals Cid]

aacid added a comment.


  the files are not uploaded correctly to phabricator so i can't check they 
work.
  
  Given we just released KF 5.69 *yesterday* i'm going to go crazy and say 
commit this and we have a "whole month" to fix regressions if it has any...

REPOSITORY
  R287 KImageFormats

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

To: sandsmark, aacid, cfeck, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

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

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> meven wrote in fstabdevice.cpp:145
> I don't think that it is necessary : m_storageAccess already equals to 
> `FstabHandling::mountPoints(m_device).first()`, looking at 
> `FstabStorageAccess::FstabStorageAccess` and 
> `FstabStorageAccess::onMtabChanged`. (That's why I wrote it like this in the 
> first place).

I think it is. You always create a FstabStorageAccess instance now.

REPOSITORY
  R245 Solid

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

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

D25267: Improve XCF support

[Martin Tobias Holmedahl Sandsmark]

sandsmark updated this revision to Diff 79925.
sandsmark added a comment.


  Fixed the layer composition modes, so now my fruit pilot got colors.
  
  Now I think this is fairly complete, it should support most "normal" XCF 
files (as opposed to before, the current master only supports v0 files).

REPOSITORY
  R287 KImageFormats

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25267?vs=79924=79925

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

AFFECTED FILES
  CMakeLists.txt
  autotests/read/xcf/fruktpilot.png
  autotests/read/xcf/fruktpilot.xcf
  src/imageformats/gimp_p.h
  src/imageformats/xcf.cpp

To: sandsmark, aacid, cfeck, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D25267: Improve XCF support

[Martin Tobias Holmedahl Sandsmark]

sandsmark updated this revision to Diff 79924.
sandsmark added a comment.


  Implemented the last few properties that were missing for my beautiful XCF 
(not actually using them, though, I'm not in the mood to implement blending in 
the LAB color space).

REPOSITORY
  R287 KImageFormats

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25267?vs=79920=79924

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

AFFECTED FILES
  CMakeLists.txt
  autotests/read/xcf/fruktpilot.png
  autotests/read/xcf/fruktpilot.xcf
  src/imageformats/gimp_p.h
  src/imageformats/xcf.cpp

To: sandsmark, aacid, cfeck, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D8274: Don't create new symlinks when copying symlinks - copy the file's contents - like /usr/bin/cp

[David Faure]

dfaure added a comment.
Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks.


  @shlomif any plans on redoing this another way? Otherwise can you close the 
review request, to clean up dashboards?

REPOSITORY
  R241 KIO

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

To: shlomif, dfaure
Cc: kde-frameworks-devel, ngraham, rkflx, dfaure, bcooksley, LeGast00n, cblack, 
michaelh, bruns, #frameworks

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

[Méven Car]

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

INLINE COMMENTS

> bruns wrote in fstabdevice.cpp:145
> use `FstabHandling::mountPoints(m_device).first()` instead of m_storageAccess.

I don't think that it is necessary : m_storageAccess already equals to 
`FstabHandling::mountPoints(m_device).first()`, looking at 
`FstabStorageAccess::FstabStorageAccess` and 
`FstabStorageAccess::onMtabChanged`. (That's why I wrote it like this in the 
first place).

REPOSITORY
  R245 Solid

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

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

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

[Méven Car]

meven updated this revision to Diff 79922.
meven added a comment.


  Fix m_displayName variable naming, move  block to constructor

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28590?vs=79807=79922

BRANCH
  master

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

AFFECTED FILES
  src/solid/devices/backends/fakehw/fakedevice.cpp
  src/solid/devices/backends/fakehw/fakedevice.h
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabdevice.h
  src/solid/devices/backends/fstab/fstabmanager.cpp
  src/solid/devices/backends/shared/rootdevice.cpp
  src/solid/devices/backends/shared/rootdevice.h
  src/solid/devices/backends/udev/udevdevice.cpp
  src/solid/devices/backends/udev/udevdevice.h
  src/solid/devices/backends/udev/udevmanager.cpp
  src/solid/devices/backends/udisks2/udisksdevice.cpp
  src/solid/devices/backends/udisks2/udisksdevice.h
  src/solid/devices/backends/udisks2/udisksmanager.cpp
  src/solid/devices/backends/upower/upowerdevice.cpp
  src/solid/devices/backends/upower/upowerdevice.h
  src/solid/devices/backends/upower/upowermanager.cpp
  src/solid/devices/frontend/device.cpp
  src/solid/devices/frontend/device.h
  src/solid/devices/ifaces/device.h

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

D25267: Improve XCF support

[Martin Tobias Holmedahl Sandsmark]

sandsmark updated this revision to Diff 79920.
sandsmark added a comment.


  Added test files (my own beautiful drawing).

REPOSITORY
  R287 KImageFormats

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25267?vs=69618=79920

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

AFFECTED FILES
  CMakeLists.txt
  autotests/read/xcf/fruktpilot.png
  autotests/read/xcf/fruktpilot.xcf
  src/imageformats/gimp_p.h
  src/imageformats/xcf.cpp

To: sandsmark, aacid, cfeck, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D24367: Some sanity verification

[Martin Tobias Holmedahl Sandsmark]

sandsmark marked 2 inline comments as done.

REPOSITORY
  R287 KImageFormats

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

To: sandsmark, aacid, cfeck
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D24367: Some sanity verification

[Martin Tobias Holmedahl Sandsmark]

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

INLINE COMMENTS

> cfeck wrote in pcx.cpp:312
> If `p` indeed could be zero, the next statements need to be in an `else` 
> block (or a add a `return`).

good catch, thanks

REPOSITORY
  R287 KImageFormats

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

To: sandsmark, aacid, cfeck
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D24367: Some sanity verification

[Martin Tobias Holmedahl Sandsmark]

sandsmark updated this revision to Diff 79917.
sandsmark added a comment.


  Added missing return.

REPOSITORY
  R287 KImageFormats

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24367?vs=67201=79917

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

AFFECTED FILES
  src/imageformats/exr.cpp
  src/imageformats/pcx.cpp
  src/imageformats/pic.cpp
  src/imageformats/psd.cpp
  src/imageformats/rgb.cpp
  src/imageformats/tga.cpp

To: sandsmark, aacid, cfeck
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28208: Move sni icon handling logic from data engine to applet

[Konrad Materka]

kmaterka added a reviewer: Frameworks.

REPOSITORY
  R120 Plasma Workspace

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

To: davidre, kmaterka, broulik, mart, #plasma, #vdg, #frameworks
Cc: bruns, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> fstabdevice.cpp:63
>  }
>  
> +if (m_storageType == StorageType::NetworkShare) {

the `if (m_displayName.isEmpty())` block belongs here

> fstabdevice.cpp:145
> +}
> +const auto filePath = m_storageAccess->filePath();
> +if (!filePath.isEmpty()) {

use `FstabHandling::mountPoints(m_device).first()` instead of m_storageAccess.

> fstabdevice.h:64
>  QString m_vendor;
> +QString m_label;
>  QString m_description;

Wrong variable name

REPOSITORY
  R245 Solid

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

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

D7563: Add privilegeExecution field to file protocol description

[Elvis Angelaccio]

elvisangelaccio added a comment.


  In D7563#645172 , @ngraham wrote:
  
  > The outstanding security issues have been resolved (see T8075 
). We have requested a re-review from the 
SUSE security team, but have not received it yet. Given that the original 
schedule for this feature has already slipped by almost two years, I would like 
to propose that we land this patch and turn it on, and resolve any 
newly-discovered issues in follow-up work.
  >
  > Any objections?
  
  
  +1

BRANCH
  master

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

To: chinmoyr, #frameworks, dfaure
Cc: feverfew, mreeves, mati865, ngraham, elvisangelaccio

D27632: Implement UString operator= to make gcc happy

[Albert Astals Cid]

This revision was automatically updated to reflect the committed changes.
Closed by commit R314:e637e721e40e: Implement UString operator= to make gcc 
happy (authored by aacid).

REPOSITORY
  R314 KJs

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27632?vs=76323=79911

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

AFFECTED FILES
  src/kjs/ustring.h

To: aacid, porten
Cc: porten, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28770: Avoid blocking the UI thread

[Martin Tobias Holmedahl Sandsmark]

sandsmark created this revision.
sandsmark added reviewers: ngraham, Frameworks, broulik.
sandsmark added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
sandsmark requested review of this revision.

REVISION SUMMARY
  Not entirely sure about this approach (and the stuff to get the test 
working), but I use kunitconversion in mangonel and the blocking behavior 
annoyed me a lot.
  
  A simpler approach would be to just try to check and download the cache 
immediately, but still force wait in the foreground, but the code (at least 
behavior) gets a lot more complex, especially with threading.

TEST PLAN
  The tests pass, it gets the currencies fairly quickly but doesn't block the 
UI thread.

REPOSITORY
  R292 KUnitConversion

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/convertertest.cpp
  autotests/convertertest.h
  autotests/valuetest.cpp
  autotests/valuetest.h
  src/currency.cpp
  src/currency_p.h

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

D28769: Add Icelandic Krona

[Martin Tobias Holmedahl Sandsmark]

sandsmark created this revision.
sandsmark added a reviewer: Frameworks.
sandsmark added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
sandsmark requested review of this revision.

REVISION SUMMARY
  Was doing some other stuff in kunitconversion so added some warnings on 
unknown currencies, and noticed that this was in the downloaded XML but not 
defined.

TEST PLAN
  It now knows about it.

REPOSITORY
  R292 KUnitConversion

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

AFFECTED FILES
  src/currency.cpp
  src/unit.h

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

D27152: Introduce FilesystemEntry class

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> hallas wrote in fstabhandling.cpp:374
> 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 

The `id()` method and members are still part of the FstabEntry, where they do 
not belong.

The UDIs go through some further mangling in `FstabHandling::deviceList()`, 
which should be covered by the unit-tests as well.

Conceptually, the 
`_k_deviceNameForMountpoint`/`_k_deviceNameForFilesystemEntry` should be part 
of the m_mtabCache/m_fstabCache class (which currently is just a 
`QMultiHash<>`). This class can then be unit tested easily.

For now, keep the `_k_deviceNameForMountpoint` inside fstabhandling.cpp

REPOSITORY
  R245 Solid

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

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

Re: Update on Status of Gitlab Migration

[Johan Ouwerkerk]

On Sun, Apr 12, 2020 at 1:06 AM Ben Cooksley  wrote:
>
> On Sun, Apr 12, 2020 at 11:04 AM Johan Ouwerkerk  
> wrote:
> >
> > On Sun, Apr 12, 2020 at 12:49 AM Johan Ouwerkerk  
> > wrote:
> > >
> > > Yes the only reason why a cleanup script might be needed is if the
> > > logical path used to express the repo in dependency information
> > > changes at the same time. E.g. suppose a `frameworks/kf5foo` gets
> > > remapped to `frameworks/kf5/foo` or something like that. In that case
> > > unless you use the flat repository layout, kdesrc-build would try to
> > > clone a new `frameworks/kf5/foo` repo, leaving your old
> > > `frameworks/kf5foo` to consume some wasted disk space.
> > >
> >
> > This is obviously a poor example, but the same problem occurs if
> > something moves from playground to extragear. Basically if the
> > `projectpath` YAML key changes.
>
> I had been considering adding the Gitlab Project ID number to the YAML
> metadata files as a way of allowing us to track projects through their
> whole lifetime.
>

That would be very nice, because then `kdesrc-build` could implement
an `arc patch` type feature in a more straightforward fashion. So
people could type things like `kdesrc-build juk!55` as a shorthand for
checkout remote HEAD of whatever branch MR 55 is for `juk`.

Regards,

- Johan

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

[David Faure]

dfaure updated this revision to Diff 79906.
dfaure added a comment.


  Use @warning. Thanks for the tip!

REPOSITORY
  R295 KCMUtils

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28765?vs=79893=79906

BRANCH
  master

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

AFFECTED FILES
  src/kcmoduleinfo.cpp
  src/kcmoduleinfo.h
  src/kcmoduleloader.cpp
  src/kcmultidialog.cpp
  src/kpluginselector.cpp
  src/ksettings/dialog.cpp

To: dfaure, pino, broulik, mart, davidedmundson
Cc: cblack, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D28682: rename and export newBatchTime signal in filecontentindexer

[Alexander Stippich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:19e71b3e4a5f: rename and export newBatchTime signal in 
filecontentindexer (authored by astippich).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28682?vs=79904=79905

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

AFFECTED FILES
  src/file/filecontentindexer.cpp
  src/file/filecontentindexer.h
  src/file/fileindexscheduler.cpp

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

D28682: rename and export newBatchTime signal in filecontentindexer

[Alexander Stippich]

astippich added a comment.


  Thanks

REPOSITORY
  R293 Baloo

BRANCH
  exportSignal

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

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

D28682: rename and export newBatchTime signal in filecontentindexer

[Alexander Stippich]

astippich retitled this revision from "rename and export done signal in 
filecontentindexer" to "rename and export newBatchTime signal in 
filecontentindexer".
astippich edited the summary of this revision.

REPOSITORY
  R293 Baloo

BRANCH
  exportSignal

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

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

D28682: rename and export done signal in filecontentindexer

[Stefan Brüns]

bruns accepted this revision.
bruns added a comment.
This revision is now accepted and ready to land.


  thx

REPOSITORY
  R293 Baloo

BRANCH
  exportSignal

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

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

D28682: rename and export done signal in filecontentindexer

[Alexander Stippich]

astippich marked 2 inline comments as done.

REPOSITORY
  R293 Baloo

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

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

D28682: rename and export done signal in filecontentindexer

[Alexander Stippich]

astippich updated this revision to Diff 79904.
astippich added a comment.


  - implement feedback

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28682?vs=79751=79904

BRANCH
  exportSignal

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

AFFECTED FILES
  src/file/filecontentindexer.cpp
  src/file/filecontentindexer.h
  src/file/fileindexscheduler.cpp

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