D15155: [KMountPoint] Fix typo in probablySlow()

[Nathaniel Graham]

ngraham accepted this revision.

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, dhaumann, aacid, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14807: [AppStream Runner] Also search when there were errors during Pool::load

[Stefan Brüns]

bruns added a comment.


  Anyone at home? Not answering a simple question is rude, at best.

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #plasma, apol, #frameworks
Cc: ngraham, apol, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, mart

D15158: [KMountPoint] Remove Windows CE support

[Stefan Brüns]

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

REVISION SUMMARY
  The Windows CE line ended in 2012, with Windows Mobile 8. Remove supporting
  code.

REPOSITORY
  R241 KIO

BRANCH
  remove_wince

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

AFFECTED FILES
  src/core/kmountpoint.cpp

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

D15157: [KMountPoint] Remove AIX support

[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
  AIX has not seen a release for 3 years and can be considered legacy
  and completely untested. Remove supporting code.

REPOSITORY
  R241 KIO

BRANCH
  remove_aix

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

AFFECTED FILES
  src/core/kmountpoint.cpp

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

D15156: [KMountPoint] Remove traces of supermount

[Stefan Brüns]

bruns updated this revision to Diff 40675.
bruns added a comment.


  remove stray comments

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15156?vs=40674=40675

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

AFFECTED FILES
  src/core/kcoredirlister.cpp
  src/core/kmountpoint.cpp

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

D15156: [KMountPoint] Remove traces of supermount

[Stefan Brüns]

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

REVISION SUMMARY
  supermount was a out-of-tree linux kernel patch for linux 2.6.x, for
  automatic mounting of removable media (see e.g.
  https://wiki.debian.org/Supermount ). It is obsolete and dead for a long
  time.

TEST PLAN
  make

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/core/kcoredirlister.cpp
  src/core/kmountpoint.cpp

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

D15141: [KSambaShareData] Accept spaces in ACL host name

[Stefan Brüns]

bruns added a comment.


  In D15141#317370 , @broulik wrote:
  
  > > Any chance you can create a unit test?
  >
  > Will try. Should it just check a couple of strings for validity? And also 
check that the ACL it read are valid since they must be as they came from `net 
usershare`?
  
  
  For the beginning, a couple of strings which previously failed, and some 
which are ok before and after, are a good start.
  
  Some input which should be rejected would cover the majority of this 
functions scope.

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, fvogt
Cc: ngraham, bruns, kde-frameworks-devel, michaelh

D15155: [KMountPoint] Fix typo in probablySlow()

[Albert Astals Cid]

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

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, dhaumann, aacid
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15155: [KMountPoint] Fix typo in probablySlow()

[Kai Uwe Broulik]

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

REVISION SUMMARY
  It probably should have been `smbfs` as everywhere else.

TEST PLAN
  Cannot find any traces of `subfs` in the code. There's a FUSE for Subsonic 
media server called that way but I doubt it's this one given how old that piece 
of code is :)

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/core/kmountpoint.cpp

To: broulik, dfaure, dhaumann
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15154: Also check smb-share for whether it's an SMB mount

[Kai Uwe Broulik]

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

REVISION SUMMARY
  This is the "type" exposed by gvfsd
  
  CCBUG: 344146

TEST PLAN
  Depends on D15153 
  
  I no longer get a "failed to change permission error" when copying a file 
onto a SMB share mounted by gvfs

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/core/kmountpoint.cpp

To: broulik, dfaure, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15153: RFC: Resolve gvfsd mounts

[Kai Uwe Broulik]

broulik added a dependent revision: D15154: Also check smb-share for whether 
it's an SMB mount.

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15153: RFC: Resolve gvfsd mounts

[Kai Uwe Broulik]

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

REVISION SUMMARY
  We only see the toplevel `/run/user/foo/gvfs` folder as a mount, not the 
virtual mounts inside.
  This will eventually allow us to tell what mount type a location mounted 
through gvfs-fuse is
  
  CCBUG: 344146

TEST PLAN
  `gvfs-mount smb://foo/bar` and got a new mount of type `smb-share`

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/core/kmountpoint.cpp

To: broulik, dfaure, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14529: Android: Make sure Qm translations get loaded

[Aleix Pol Gonzalez]

apol added a comment.


  It is where Qt installs things indeed. I've done a couple of failed attempts 
at finding other ways to get this different. In the best scenario anyway we'll 
have to add an `if(ANDROID)` elsewhere if it's not here.
  
  I'd say we can iterate it in the future but the patch as is shouldn't be too 
problematic and it actually maps how our Android builds work at the moment.
  
  For completion, it's QtLoader.java that puts everything in 
`qt-reserved-files`. I'll ask why's the current status on the qt-android 
mailing list, but I wouldn't want this to stop our Android builds to have 
translations.

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks
Cc: svuorela, aacid, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, 
bruns

Re: KDE apps have missing icons when not on Breeze

[Luigi Toscano]

Albert Astals Cid ha scritto:

El dimecres, 29 d’agost de 2018, a les 21:13:03 CEST, Albert Vaca va escriure:

Do you think it's a good idea to ask packagers to make KIconThemes depend
on Breeze icons?


I think depend is probably "too strong", but recommend for sure.



Did you also consider to find a way to add icons to the Adwaita theme? That 
would cover most of the use cases. Not sure how much their developer would 
like to add more icons, but still...


Ciao
--
Luigi

Re: KDE apps have missing icons when not on Breeze

[Albert Astals Cid]

El dimecres, 29 d’agost de 2018, a les 21:13:03 CEST, Albert Vaca va escriure:
> Do you think it's a good idea to ask packagers to make KIconThemes depend
> on Breeze icons?

I think depend is probably "too strong", but recommend for sure.

Cheers,
  Albert

> 
> On Wed, Aug 29, 2018, 19:29 Albert Astals Cid  wrote:
> 
> > El dimecres, 29 d’agost de 2018, a les 12:56:53 CEST, Aleix Pol va
> > escriure:
> > > On Tue, Aug 28, 2018 at 10:45 PM Albert Astals Cid 
> > wrote:
> > > >
> > > > El dijous, 16 d’agost de 2018, a les 15:19:36 CEST, Albert Astals Cid
> > va escriure:
> > > > > Missatge de Albert Vaca  del dia dj., 16
> > d’ag. 2018 a
> > > > > les 13:57:
> > > > >
> > > > > > Hi everyone,
> > > > > >
> > > > > > If we want to reach more potential users, we have to make sure KDE
> > > > > > apps look properly in every desktop. Take a look at how KDE apps
> > look
> > > > > > in Gnome, you will see there are lots of missing icons.
> > > > > >
> > > > > > https://imgur.com/a/nkeiryb
> > > > > >
> > > > > > It think it is important to get this fixed, as it can make people
> > not
> > > > > > use our apps.
> > > > > >
> > > > > > Note that, on the other hand, Gnome apps look good in Plasma even
> > when
> > > > > > Breeze doesn't have the icons they need. This is because they fall
> > > > > > back to their own theme when an icon is not found in the system
> > theme:
> > > > > > I just tested it setting the icon theme to Breeze on Gnome, and
> > > > > > deleting the "actions" directory from Breeze, they show the Gnome
> > > > > > icons instead.
> > > > > >
> > > > > > Anyone has a suggestion on how to fix this?
> > > > > >
> > > > >
> > > > > If we can get this accepted into Qt
> > > > > https://codereview.qt-project.org/#/c/237025/
> > > > > Which i think we could since it mimics nicely the setThemeName
> > > > > functionality.
> > > > >
> > > > > We can then just add a Q_COREAPP_STARTUP_FUNCTION in KIconTheme that
> > does
> > > > >   QIcon::setFallbackThemeName("breeze");
> > > > >
> > > > > And that'd be a one place fix all solution.
> > > >
> > > > FWIW this just landed
> > > > https://phabricator.kde.org/D14983
> > > >
> > > > Cheers,
> > > >   Albert
> > > >
> > > > >
> > > > > Cheers,
> > > > >   Albert
> > > > >
> > > > >
> > > > > >
> > > > > > (Please CC me as I'm not subscribed to the list.)
> > > > > >
> > > > > > Albert
> > > > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > > This will still not fix it entirely, as not every application will be
> > > linking against KIconThemes, especially on Gnome.
> >
> > KXMLGui links against KIconThemes.
> >
> > Any application that pretends to look like a desktop application should be
> > using KXmlGui.
> >
> > Applications that are too fancy for their own good and don't want to use
> > KXmlGui can call that one liner by themselves if they care about their look
> > and feel on Gnome desktops.
> >
> > Cheers,
> >   Albert
> >
> > >
> > > Aleix
> >
> >
> >
> >
> >

Re: KDE apps have missing icons when not on Breeze

[Albert Vaca]

Do you think it's a good idea to ask packagers to make KIconThemes depend
on Breeze icons?

On Wed, Aug 29, 2018, 19:29 Albert Astals Cid  wrote:

> El dimecres, 29 d’agost de 2018, a les 12:56:53 CEST, Aleix Pol va
> escriure:
> > On Tue, Aug 28, 2018 at 10:45 PM Albert Astals Cid 
> wrote:
> > >
> > > El dijous, 16 d’agost de 2018, a les 15:19:36 CEST, Albert Astals Cid
> va escriure:
> > > > Missatge de Albert Vaca  del dia dj., 16
> d’ag. 2018 a
> > > > les 13:57:
> > > >
> > > > > Hi everyone,
> > > > >
> > > > > If we want to reach more potential users, we have to make sure KDE
> > > > > apps look properly in every desktop. Take a look at how KDE apps
> look
> > > > > in Gnome, you will see there are lots of missing icons.
> > > > >
> > > > > https://imgur.com/a/nkeiryb
> > > > >
> > > > > It think it is important to get this fixed, as it can make people
> not
> > > > > use our apps.
> > > > >
> > > > > Note that, on the other hand, Gnome apps look good in Plasma even
> when
> > > > > Breeze doesn't have the icons they need. This is because they fall
> > > > > back to their own theme when an icon is not found in the system
> theme:
> > > > > I just tested it setting the icon theme to Breeze on Gnome, and
> > > > > deleting the "actions" directory from Breeze, they show the Gnome
> > > > > icons instead.
> > > > >
> > > > > Anyone has a suggestion on how to fix this?
> > > > >
> > > >
> > > > If we can get this accepted into Qt
> > > > https://codereview.qt-project.org/#/c/237025/
> > > > Which i think we could since it mimics nicely the setThemeName
> > > > functionality.
> > > >
> > > > We can then just add a Q_COREAPP_STARTUP_FUNCTION in KIconTheme that
> does
> > > >   QIcon::setFallbackThemeName("breeze");
> > > >
> > > > And that'd be a one place fix all solution.
> > >
> > > FWIW this just landed
> > > https://phabricator.kde.org/D14983
> > >
> > > Cheers,
> > >   Albert
> > >
> > > >
> > > > Cheers,
> > > >   Albert
> > > >
> > > >
> > > > >
> > > > > (Please CC me as I'm not subscribed to the list.)
> > > > >
> > > > > Albert
> > > > >
> > >
> > >
> > >
> > >
> >
> > This will still not fix it entirely, as not every application will be
> > linking against KIconThemes, especially on Gnome.
>
> KXMLGui links against KIconThemes.
>
> Any application that pretends to look like a desktop application should be
> using KXmlGui.
>
> Applications that are too fancy for their own good and don't want to use
> KXmlGui can call that one liner by themselves if they care about their look
> and feel on Gnome desktops.
>
> Cheers,
>   Albert
>
> >
> > Aleix
>
>
>
>
>

D15141: [KSambaShareData] Accept spaces in ACL host name

[Kai Uwe Broulik]

broulik added a comment.


  > Any chance you can create a unit test?
  
  Will try. Should it just check a couple of strings for validity? And also 
check that the ACL it read are valid since they must be as they came from `net 
usershare`?

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, fvogt
Cc: ngraham, bruns, kde-frameworks-devel, michaelh

D11353: manager: add support to R/W the GlobalDnsConfiguration property

[Jan Grulich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R282:6b2c3e8fce2f: manager: add support to R/W the 
GlobalDnsConfiguration property (authored by aleksanderm, committed by 
jgrulich).
Herald added a subscriber: kde-frameworks-devel.

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D11353?vs=29593=40662#toc

REPOSITORY
  R282 NetworkManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11353?vs=29593=40662

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

AFFECTED FILES
  src/CMakeLists.txt
  src/dnsconfiguration.cpp
  src/dnsconfiguration.h
  src/dnsdomain.cpp
  src/dnsdomain.h
  src/manager.cpp
  src/manager.h
  src/manager_p.h

To: aleksanderm, #frameworks, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

Re: KDE apps have missing icons when not on Breeze

[Albert Astals Cid]

El dimecres, 29 d’agost de 2018, a les 12:56:53 CEST, Aleix Pol va escriure:
> On Tue, Aug 28, 2018 at 10:45 PM Albert Astals Cid  wrote:
> >
> > El dijous, 16 d’agost de 2018, a les 15:19:36 CEST, Albert Astals Cid va 
> > escriure:
> > > Missatge de Albert Vaca  del dia dj., 16 d’ag. 2018 
> > > a
> > > les 13:57:
> > >
> > > > Hi everyone,
> > > >
> > > > If we want to reach more potential users, we have to make sure KDE
> > > > apps look properly in every desktop. Take a look at how KDE apps look
> > > > in Gnome, you will see there are lots of missing icons.
> > > >
> > > > https://imgur.com/a/nkeiryb
> > > >
> > > > It think it is important to get this fixed, as it can make people not
> > > > use our apps.
> > > >
> > > > Note that, on the other hand, Gnome apps look good in Plasma even when
> > > > Breeze doesn't have the icons they need. This is because they fall
> > > > back to their own theme when an icon is not found in the system theme:
> > > > I just tested it setting the icon theme to Breeze on Gnome, and
> > > > deleting the "actions" directory from Breeze, they show the Gnome
> > > > icons instead.
> > > >
> > > > Anyone has a suggestion on how to fix this?
> > > >
> > >
> > > If we can get this accepted into Qt
> > > https://codereview.qt-project.org/#/c/237025/
> > > Which i think we could since it mimics nicely the setThemeName
> > > functionality.
> > >
> > > We can then just add a Q_COREAPP_STARTUP_FUNCTION in KIconTheme that does
> > >   QIcon::setFallbackThemeName("breeze");
> > >
> > > And that'd be a one place fix all solution.
> >
> > FWIW this just landed
> > https://phabricator.kde.org/D14983
> >
> > Cheers,
> >   Albert
> >
> > >
> > > Cheers,
> > >   Albert
> > >
> > >
> > > >
> > > > (Please CC me as I'm not subscribed to the list.)
> > > >
> > > > Albert
> > > >
> >
> >
> >
> >
> 
> This will still not fix it entirely, as not every application will be
> linking against KIconThemes, especially on Gnome.

KXMLGui links against KIconThemes.

Any application that pretends to look like a desktop application should be 
using KXmlGui.

Applications that are too fancy for their own good and don't want to use 
KXmlGui can call that one liner by themselves if they care about their look and 
feel on Gnome desktops.

Cheers,
  Albert

> 
> Aleix

Re: KDE apps have missing icons when not on Breeze

[Albert Vaca]

For icons to work on Windows, we rely on apps linking against
KIconThemes (see [1]).

I would add a similar hack for Gnome: I think it is a safe-enough
assumption that apps will link against KIconThemes.

I will submit a patch proposal soon-ish.

[1] https://github.com/KDE/kiconthemes/blob/master/src/kicontheme.cpp#L70
On Wed, Aug 29, 2018 at 12:58 PM Aleix Pol  wrote:
>
> On Tue, Aug 28, 2018 at 10:45 PM Albert Astals Cid  wrote:
> >
> > El dijous, 16 d’agost de 2018, a les 15:19:36 CEST, Albert Astals Cid va 
> > escriure:
> > > Missatge de Albert Vaca  del dia dj., 16 d’ag. 2018 
> > > a
> > > les 13:57:
> > >
> > > > Hi everyone,
> > > >
> > > > If we want to reach more potential users, we have to make sure KDE
> > > > apps look properly in every desktop. Take a look at how KDE apps look
> > > > in Gnome, you will see there are lots of missing icons.
> > > >
> > > > https://imgur.com/a/nkeiryb
> > > >
> > > > It think it is important to get this fixed, as it can make people not
> > > > use our apps.
> > > >
> > > > Note that, on the other hand, Gnome apps look good in Plasma even when
> > > > Breeze doesn't have the icons they need. This is because they fall
> > > > back to their own theme when an icon is not found in the system theme:
> > > > I just tested it setting the icon theme to Breeze on Gnome, and
> > > > deleting the "actions" directory from Breeze, they show the Gnome
> > > > icons instead.
> > > >
> > > > Anyone has a suggestion on how to fix this?
> > > >
> > >
> > > If we can get this accepted into Qt
> > > https://codereview.qt-project.org/#/c/237025/
> > > Which i think we could since it mimics nicely the setThemeName
> > > functionality.
> > >
> > > We can then just add a Q_COREAPP_STARTUP_FUNCTION in KIconTheme that does
> > >   QIcon::setFallbackThemeName("breeze");
> > >
> > > And that'd be a one place fix all solution.
> >
> > FWIW this just landed
> > https://phabricator.kde.org/D14983
> >
> > Cheers,
> >   Albert
> >
> > >
> > > Cheers,
> > >   Albert
> > >
> > >
> > > >
> > > > (Please CC me as I'm not subscribed to the list.)
> > > >
> > > > Albert
> > > >
> >
> >
> >
> >
>
> This will still not fix it entirely, as not every application will be
> linking against KIconThemes, especially on Gnome.
>
> Aleix

KDE CI: Frameworks karchive kf5-qt5 SUSEQt5.9 - Build # 31 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20karchive%20kf5-qt5%20SUSEQt5.9/31/
 Project:
Frameworks karchive kf5-qt5 SUSEQt5.9
 Date of build:
Wed, 29 Aug 2018 17:12:38 +
 Build duration:
6 min 12 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report67%
(2/3)77%
(27/35)77%
(27/35)78%
(4251/5462)54%
(1826/3408)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(6/6)100%
(6/6)99%
(1178/1185)53%
(540/1027)src84%
(21/25)84%
(21/25)77%
(3073/4010)58%
(1286/2205)tests0%
(0/4)0%
(0/4)0%
(0/267)0%
(0/176)

KDE CI: Frameworks karchive kf5-qt5 SUSEQt5.10 - Build # 38 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20karchive%20kf5-qt5%20SUSEQt5.10/38/
 Project:
Frameworks karchive kf5-qt5 SUSEQt5.10
 Date of build:
Wed, 29 Aug 2018 17:12:38 +
 Build duration:
1 min 4 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report67%
(2/3)77%
(27/35)77%
(27/35)78%
(4251/5462)54%
(1826/3408)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(6/6)100%
(6/6)99%
(1178/1185)53%
(540/1027)src84%
(21/25)84%
(21/25)77%
(3073/4010)58%
(1286/2205)tests0%
(0/4)0%
(0/4)0%
(0/267)0%
(0/176)

D14580: support for multi pages kcms

[Andres Betts]

abetts added a comment.


  In D14580#317289 , @mart wrote:
  
  > F6223394: Spectacle.k3.png 
  >
  > in case the kcm wants to go multi columns (default off):
  >  F6223399: Spectacle.l3.png 
  >  that secon page with the giant pop button is just a placeholder for testing
  
  
  Very cool! Thanks for the pics Marco!

REPOSITORY
  R295 KCMUtils

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

To: mart, #plasma, #frameworks
Cc: abetts, kde-frameworks-devel, michaelh, ngraham, bruns

D14580: support for multi pages kcms

[Marco Martin]

mart added a comment.


  F6223394: Spectacle.k3.png 
  
  in case the kcm wants to go multi columns (default off):
  F6223399: Spectacle.l3.png 

REPOSITORY
  R295 KCMUtils

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

To: mart, #plasma, #frameworks
Cc: abetts, kde-frameworks-devel, michaelh, ngraham, bruns

D14674: handle non-ASCII encodings of file names in tar archives

[Rinat Ibragimov]

ibragimovrinat added a comment.


  @aacid, I've just sent file to the e-mail. I also attached the same file to 
this message: F6223361: tar_non_ascii_file_name.tar.gz 


REPOSITORY
  R243 KArchive

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

To: ibragimovrinat, dfaure, kossebau
Cc: aacid, xyquadrat, broulik, cfeck, ibragimovrinat, kde-frameworks-devel, 
michaelh, ngraham, bruns

D14580: support for multi pages kcms

[Andres Betts]

abetts added a comment.


  What does this look like now Marco?

REPOSITORY
  R295 KCMUtils

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

To: mart, #plasma, #frameworks
Cc: abetts, kde-frameworks-devel, michaelh, ngraham, bruns

D15141: [KSambaShareData] Accept spaces in ACL host name

[Stefan Brüns]

bruns added a comment.


  Any chance you can create a unit test?

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, fvogt
Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D15146: RFC: Don't consider KDiskFreeSpaceInfo valid if all relevant statvfs fields are zero

[Kai Uwe Broulik]

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

REVISION SUMMARY
  BUG: 344146

TEST PLAN
  Accessed a SMB share from Nautilus, chose "Open in other app", opened it with 
Dolphin, it would mount it to `/run/user/foo/gvfs/bar`.
  I can now copy files over without getting a "disk full" error. However, after 
copy completes, I get a "cannot change permissions" error. It's a start, 
though... however, all those fields being zero could be valid? Imho the bug is 
in the GIO slave.

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/core/kdiskfreespaceinfo.cpp

To: broulik, dfaure, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15146: RFC: Don't consider KDiskFreeSpaceInfo valid if all relevant statvfs fields are zero

[Kai Uwe Broulik]

broulik added a reviewer: fvogt.

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, ngraham, fvogt
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D6512: Add support for proposed tags addition in OCS 1.7

[Dan Leinir Turthra Jensen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R235:76f66bab8841: Add support for proposed tags addition in 
OCS 1.7 (authored by leinir).

REPOSITORY
  R235 Attica

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6512?vs=39175=40648

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

AFFECTED FILES
  src/content.cpp
  src/content.h
  src/contentparser.cpp
  src/downloaddescription.cpp
  src/downloaddescription.h
  src/parser.cpp

To: leinir, #knewstuff, apol, whiting, #kde_store, ahiemstra, ngraham
Cc: cfeck, ahiemstra, ngraham, kde-frameworks-devel, #kde_store, michaelh, 
ZrenBot, bruns, akiraohgaki, alexanderschmidt, siyuandong, ronaldv, mikesomov, 
starbuck, sebas

D6512: Add support for proposed tags addition in OCS 1.7

[Nathaniel Graham]

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


  +1, shipit!

REPOSITORY
  R235 Attica

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

To: leinir, #knewstuff, apol, whiting, #kde_store, ahiemstra, ngraham
Cc: cfeck, ahiemstra, ngraham, kde-frameworks-devel, #kde_store, michaelh, 
ZrenBot, bruns, akiraohgaki, alexanderschmidt, siyuandong, ronaldv, mikesomov, 
starbuck, sebas

KDE CI: Frameworks syntax-highlighting kf5-qt5 WindowsMSVCQt5.11 - Build # 19 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20syntax-highlighting%20kf5-qt5%20WindowsMSVCQt5.11/19/
 Project:
Frameworks syntax-highlighting kf5-qt5 WindowsMSVCQt5.11
 Date of build:
Wed, 29 Aug 2018 12:38:21 +
 Build duration:
5 min 36 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 8 test(s)

D6513: Add support for Attica tags support

[Dan Leinir Turthra Jensen]

leinir added a comment.


  Welcome back from Akademy - sorry for being a pest, but it'd be really nifty 
if we could get this and it's dependent in D6512 
 through before 5.50... :)

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, apol, #kde_store, whiting, ahiemstra, mlaurent, dfaure
Cc: dfaure, cfeck, mlaurent, ngraham, ahiemstra, kde-frameworks-devel, 
#knewstuff, michaelh, ZrenBot, bruns

D15136: Fixed a typeo

[Laurent Montel]

mlaurent added a comment.


  done.

REPOSITORY
  R293 Baloo

BRANCH
  bug397843 (branched from master)

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

To: ssloan, mlaurent
Cc: mlaurent, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D15136: Fixed a typeo

[Laurent Montel]

mlaurent closed this revision.

REPOSITORY
  R293 Baloo

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

To: ssloan, mlaurent
Cc: mlaurent, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D15141: [KSambaShareData] Accept spaces in ACL host name

[Kai Uwe Broulik]

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

REVISION SUMMARY
  Otherwise it would reject `Unix User\foo`

TEST PLAN
  (or whatever the part before the backslash is called)
  
  Thanks Fabian for help with regular expressions :)
  
  - I can now enable and disable guest mode again in kdenetwork-filesharing 
Share dialog and can actually access my shares now (somewhat)

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/core/ksambashare.cpp

To: broulik, dfaure, fvogt
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15136: Fixed a typeo

[Scott Sloan]

ssloan added a comment.


  Thanks
  
  scottsl...@gmail.com

REPOSITORY
  R293 Baloo

BRANCH
  bug397843 (branched from master)

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

To: ssloan, mlaurent
Cc: mlaurent, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

Re: KDE apps have missing icons when not on Breeze

[Aleix Pol]

On Tue, Aug 28, 2018 at 10:45 PM Albert Astals Cid  wrote:
>
> El dijous, 16 d’agost de 2018, a les 15:19:36 CEST, Albert Astals Cid va 
> escriure:
> > Missatge de Albert Vaca  del dia dj., 16 d’ag. 2018 a
> > les 13:57:
> >
> > > Hi everyone,
> > >
> > > If we want to reach more potential users, we have to make sure KDE
> > > apps look properly in every desktop. Take a look at how KDE apps look
> > > in Gnome, you will see there are lots of missing icons.
> > >
> > > https://imgur.com/a/nkeiryb
> > >
> > > It think it is important to get this fixed, as it can make people not
> > > use our apps.
> > >
> > > Note that, on the other hand, Gnome apps look good in Plasma even when
> > > Breeze doesn't have the icons they need. This is because they fall
> > > back to their own theme when an icon is not found in the system theme:
> > > I just tested it setting the icon theme to Breeze on Gnome, and
> > > deleting the "actions" directory from Breeze, they show the Gnome
> > > icons instead.
> > >
> > > Anyone has a suggestion on how to fix this?
> > >
> >
> > If we can get this accepted into Qt
> > https://codereview.qt-project.org/#/c/237025/
> > Which i think we could since it mimics nicely the setThemeName
> > functionality.
> >
> > We can then just add a Q_COREAPP_STARTUP_FUNCTION in KIconTheme that does
> >   QIcon::setFallbackThemeName("breeze");
> >
> > And that'd be a one place fix all solution.
>
> FWIW this just landed
> https://phabricator.kde.org/D14983
>
> Cheers,
>   Albert
>
> >
> > Cheers,
> >   Albert
> >
> >
> > >
> > > (Please CC me as I'm not subscribed to the list.)
> > >
> > > Albert
> > >
>
>
>
>

This will still not fix it entirely, as not every application will be
linking against KIconThemes, especially on Gnome.

Aleix

D14580: support for multi pages kcms

[Marco Martin]

mart updated this revision to Diff 40629.
mart added a comment.


  - cosmetic spacing

REPOSITORY
  R295 KCMUtils

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14580?vs=40625=40629

BRANCH
  mart/multipageKCM

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kcmoduleqml.cpp
  src/kcmoduleqml_p.h
  src/kcmultidialog.cpp

To: mart, #plasma, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15134: decrease StateData space by more than 50% and half the number of needed mallocs

[Christoph Cullmann]

cullmann added a comment.


  For the std::vector or other things, I will create new requests.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D15134: decrease StateData space by more than 50% and half the number of needed mallocs

[Christoph Cullmann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:4d3ed1fc7297: replace raw pointer with weak_ptr (authored 
by cullmann).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D15134?vs=40621=40626#toc

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15134?vs=40621=40626

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

AFFECTED FILES
  src/lib/abstracthighlighter.cpp
  src/lib/definition.cpp
  src/lib/definitionref_p.h
  src/lib/state.cpp
  src/lib/state_p.h

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D15134: decrease StateData space by more than 50% and half the number of needed mallocs

[Volker Krause]

vkrause added a comment.


  This seems reasonable, the raw pointer use here probably predates 
DefinitionRef.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D14580: support for multi pages kcms

[Marco Martin]

mart updated this revision to Diff 40625.
mart added a comment.


  hide title of qml modules from KCMultidialog

REPOSITORY
  R295 KCMUtils

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14580?vs=40500=40625

BRANCH
  mart/multipageKCM

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kcmoduleqml.cpp
  src/kcmoduleqml_p.h
  src/kcmultidialog.cpp

To: mart, #plasma, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15139: hide title of qml modules from kcmultidialog

[Marco Martin]

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

REPOSITORY
  R295 KCMUtils

BRANCH
  mart/multipageKCM

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

AFFECTED FILES
  src/kcmultidialog.cpp

To: mart
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15139: hide title of qml modules from kcmultidialog

[Marco Martin]

mart abandoned this revision.

REPOSITORY
  R295 KCMUtils

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

To: mart
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15134: decrease StateData space by more than 50% and half the number of needed mallocs

[Christoph Cullmann]

cullmann updated this revision to Diff 40621.
cullmann added a comment.


  Proposed solution that is "safe": We use a weak reference to check for state 
validity.
  This is bit more expensive than the raw-pointer but even save if e.g. by 
accident a new definition gets allocated to the same address as an old one.
  I added shortcut to check for pointer equality for shared states.

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15134?vs=40587=40621

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

AFFECTED FILES
  src/lib/abstracthighlighter.cpp
  src/lib/definition.cpp
  src/lib/definitionref_p.h
  src/lib/state.cpp
  src/lib/state_p.h

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D15134: decrease StateData space by more than 50% and half the number of needed mallocs

[Christoph Cullmann]

cullmann added a comment.


  Regarding implicit sharing, is this actually shared between two (subsequent) 
lines with the same state (think of e.g. a multi-line license header block)? 
Then it might be interesting to keep it, otherwise I agree, the increased 
control of a std::vector might be preferable here indeed.
  
  At the moment, KTextEditor compares the returned states and reuses the input 
state.
  I would do the same in highlightLine, then you get that for free.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D15134: decrease StateData space by more than 50% and half the number of needed mallocs

[Volker Krause]

vkrause added a comment.


  Regarding implicit sharing, is this actually shared between two (subsequent) 
lines with the same state (think of e.g. a multi-line license header block)? 
Then it might be interesting to keep it, otherwise I agree, the increased  
control of a std::vector might be preferable here indeed.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D15134: decrease StateData space by more than 50% and half the number of needed mallocs

[Christoph Cullmann]

cullmann added a comment.


  For: I can't answer the m_defData question without digging deeper into this 
unfortunately. The rest looks good to me though.
  
  Ok. I think in 99% of the cases it is save enough, thought some 100% save 
thing would be nicer. Actually one could just add a shared::ptr to keep the 
definition alive to the state.
  
  For the indentationBasedFoldingEnabled() detach: Actually, do we want to have 
the implicit stuff at all there?
  I would tend to just use a std::vector as stack as we do the sharing ourself 
(and ensure we return pointer equal states in highlightLine)

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D15134: decrease StateData space by more than 50% and half the number of needed mallocs

[Volker Krause]

vkrause added a comment.


  I can't answer the m_defData question without digging deeper into this 
unfortunately. The rest looks good to me though.

INLINE COMMENTS

> state.cpp:117
>  return false;
> -return d->m_contextStack.top()->indentationBasedFoldingEnabled();
> +return d->m_contextStack.last().first->indentationBasedFoldingEnabled();
>  }

last() has a non-const (detaching) overload, is there a risk we are hitting 
this here due to the d-> indirection? All other occurrences seem to be clearly 
on const members.

REPOSITORY
  R216 Syntax Highlighting

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

To: cullmann, vkrause, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D13700: implement reading of the replaygain tags

[Matthieu Gallien]

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


  Some inline comments

INLINE COMMENTS

> taglibextractor.cpp:254-263
> +if(!strcmp( userTextFrame->description().toCString( true ), 
> "replaygain_track_gain" ) ) {
> +data.replayGainTrackGain = 
> convertWCharsToQString(userTextFrame->fieldList().back().toCString(true));
> +}
> +if(!strcmp( userTextFrame->description().toCString( true ), 
> "replaygain_track_peak" ) ) {
> +data.replayGainTrackPeak = 
> convertWCharsToQString(userTextFrame->fieldList().back().toCString(true));
> +}
> +if(!strcmp( userTextFrame->description().toCString( true ), 
> "replaygain_album_gain" ) ) {

I am not a fan of the strcmp usage here. Is it not possible to do without them ?

> taglibextractor.cpp:468
>  
> -// Rating.
>  itMPC = lstMusepack.find("RATING");

This change is unrelated to the patch. Please remove it.

> taglibextractor.cpp:662
>  
> -// Rating.
>  itOgg = lstOgg.find("RATING");

same comment

> taglibextractor.cpp:934
> +}
> +qDebug() << data.replayGainAlbumGain;
> +result->add(Property::ReplayGainAlbumGain, 
> data.replayGainAlbumGain.toDouble());

Can you remove it ?

> taglibextractor.cpp:944
> +/* remove " dB" suffix */
> +if (data.replayGainTrackGain.endsWith(QStringLiteral(" 
> dB"),Qt::CaseInsensitive))
> +{

Could you check if it is not faster to use QLatin1String here ?

> properties.h:327-334
> +/**
> +  * Contains ReplayGain information for audio files
> +  * The album/track gain is given in "dB"
> +  */
> +ReplayGainAlbumPeak,
> +ReplayGainAlbumGain,
> +ReplayGainTrackPeak,

Please document each entry separately otherwise only the first one will have 
documentation (as seen on api.kde.org).

REPOSITORY
  R286 KFileMetaData

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

To: astippich, mgallien, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15136: Fixed a typeo

[Laurent Montel]

mlaurent added a comment.


  I will commit for you. what is your email address ?

REPOSITORY
  R293 Baloo

BRANCH
  bug397843 (branched from master)

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

To: ssloan, mlaurent
Cc: mlaurent, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams