D27152: Introduce FilesystemEntry class

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> hallas wrote in filesystem_entry.cpp:47
> Yeah I agree ;) But I would really like for this class to be used more 
> generically so having a list of filesystems here would probably make it hard 
> for that. We could also have two methods, one similar to this one given the 
> "raw" filesystem type, and one that would give an enum entry from a so called 
> "known" list, but would return "unknown" for a filesystem type not in the 
> list, then you would have access to both? What do you think?

This sound sane to me.
Ideally we would store only the string in case of an unknown fs or the enum but 
not both for a same entry, the choice could be done in ctor.

> hallas wrote in filesystem_entry.h:88
> No, I only put them here for completeness sake. Basically I sat down with the 
> fstab man page and the header file documenting the mntent structure and based 
> this class around that, but if this code should be exported an made generally 
> available then we should probably add these, what do you think? Should I 
> remove them now, and then we can always add them if needed?

Just leave a leave a comment as to why we want to keep those.

> fstabhandling.cpp:388
> +QStringList mountpoints;
> +for (const auto& dev : globalFstabCache->localData().m_mtabCache) {
> +if (!dev.mountPath().isEmpty()) {

Detaches m_mtabCache, just qAsConst or temporary const variable

REPOSITORY
  R245 Solid

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

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

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/289/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Wed, 05 Feb 2020 04:46:20 +
 Build duration:
6 min 52 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)

D25698: New query mechanism for applications: KApplicationTrader

[David Faure]

dfaure closed this revision.

REPOSITORY
  R309 KService

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

To: dfaure, broulik, mart, vkrause, nicolasfella, aacid, davidedmundson, apol
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27158: Fix some compiler warnings

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> KConfigCodeGeneratorBase.cpp:45
>  ParseResult )
> -: m_inputFile(inputFile), m_baseDir(baseDir), m_fileName(fileName), 
> m_cfg(parameters), parseResult(parseResult)
> +: parseResult(parseResult), m_inputFile(inputFile),m_baseDir(baseDir), 
> m_fileName(fileName), m_cfg(parameters)
>  {

missing space before m_baseDir

REPOSITORY
  R237 KConfig

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

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

D27148: update d->m_file in ReadOnlyPart::setUrl()

[David Faure]

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


  statJob->exec() creates a nested event loop, which processes timers and 
socket events etc. -- this can create unexpected re-entrancy and is a nasty 
source of bugs.
  Therefore it should be avoided as much as possible, *especially* in libraries 
which don't have the full picture about what happens in the application.
  
  The only safe way to do this is with an async job, i.e. signals and slots. 
This is tricky in general (when the caller expects things to happen 
immediately), but by luck this isn't the case here. Apps expect KParts to 
download remote files. So what could be done, is a statjob before the download, 
and if stat says there's a local path KParts can *then* set m_file instead of 
downloading.
  
  But wait KParts::ReadOnlyPart::openUrl already does *exactly* that.
  See the code under
  
// Maybe we can use a "local path", to avoid a temp copy?
  
  which uses KIO::mostLocalUrl() the right way, async.
  
  So I have to ask... what is this fixing, exactly? Why isn't openUrl called, 
or why doesn't it go into that code path, in your use case?

REPOSITORY
  R306 KParts

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

To: pdabrowski, elvisangelaccio, ngraham, #frameworks, dfaure
Cc: marten, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27017: [KColorUtils] Add hue(), chroma(), getHcyColor() and update documentation

[Noah Davis]

ndavis marked 3 inline comments as done.

REPOSITORY
  R273 KGuiAddons

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

To: ndavis, #frameworks, dfaure
Cc: mwoehlke, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27017: [KColorUtils] Add hue(), chroma(), getHcyColor() and update documentation

[Noah Davis]

ndavis updated this revision to Diff 75022.
ndavis marked an inline comment as done.
ndavis added a comment.


  - More edits based on comments

REPOSITORY
  R273 KGuiAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27017?vs=75021=75022

BRANCH
  hue-chroma (branched from master)

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

AFFECTED FILES
  src/colors/kcolorspaces.cpp
  src/colors/kcolorspaces_p.h
  src/colors/kcolorutils.cpp
  src/colors/kcolorutils.h

To: ndavis, #frameworks, dfaure
Cc: mwoehlke, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27017: [KColorUtils] Add hue(), chroma(), getHcyColor() and update documentation

[Noah Davis]

ndavis marked 5 inline comments as done.
ndavis added inline comments.

INLINE COMMENTS

> mwoehlke wrote in kcolorutils.h:36
> "0.999..." is a little odd, and I wonder if it is accurate for e.g. 
> `QColor::fromRgba64(0x, 0, 1}`? Maybe something like "just under 1.0 
> (also red)" would be more useful? (If you want to get technical, "to 
> (asymptotically) 1.0" would also work. Maybe "0.0 //approaching// 1.0"?)
> 
> The range is of course [0, 1), but I don't know how to say that in 
> straight-forward English .
> 
> OTOH, I wonder if it's worth bothering, vs. just saying "0.0 to 1.0" and 
> ignoring that it will never be //exactly// 1.0.

> from 0.0 (red) to almost 1.0 (slightly blue-ish red)

This sounds good to me. I think it's worth saying that it won't be 1.0 so that 
people don't assume the number is a rounding error or something.

REPOSITORY
  R273 KGuiAddons

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

To: ndavis, #frameworks, dfaure
Cc: mwoehlke, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27017: [KColorUtils] Add hue(), chroma(), getHcyColor() and update documentation

[Noah Davis]

ndavis updated this revision to Diff 75021.
ndavis added a comment.


  - Address comments

REPOSITORY
  R273 KGuiAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27017?vs=74964=75021

BRANCH
  hue-chroma (branched from master)

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

AFFECTED FILES
  src/colors/kcolorspaces.cpp
  src/colors/kcolorspaces_p.h
  src/colors/kcolorutils.cpp
  src/colors/kcolorutils.h

To: ndavis, #frameworks, dfaure
Cc: mwoehlke, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27152: Introduce FilesystemEntry class

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> CMakeLists.txt:3
> +filesystem_entry.cpp
> +filesystem_entry.h
>  fstabmanager.cpp

remove

> filesystem_entry.cpp:59
> +{
> +if (m_type == QLatin1Literal("fuse.encfs")) {
> +return m_device + QLatin1Char('@') + m_mountPath;

store this in the entry, otherwise you pay the cost on every access

> filesystem_entry.h:34
> +/*
> + * Class that represents a filesystem. The filesystem is either mounted by 
> the system or can be mounted by the system.
> + * The information in the class is modelled around the fstab/mtab type files.

wrap long lines, also below.

> filesystem_entry.h:48
> + */
> +QString mountPath() const;
> +/*

Whats wrong with mountPoint? Its used everywhere else here, and is a well known 
term.

> fstabhandling.cpp:168
>  const QString device = _k_deviceNameForMountpoint(fsname, 
> fstype, mountpoint);
>  QStringList options = 
> QFile::decodeName(fe->mnt_opts).split(QLatin1Char(','));
>  

should be const now

> fstabhandling.cpp:209
>  const QString mountpoint = items.at(1);
>  
> +globalFstabCache->localData().m_fstabCache.insert(device, 
> FilesystemEntry(device, mountpoint, items.at(2), QStringList()));

add temporary for fstype

> fstabhandling.cpp:259
> +QStringList mountpoints;
> +for (const auto& dev : globalFstabCache->localData().m_fstabCache) {
> +if (!dev.mountPath().isEmpty()) {

detaches m_fstabCache, also below.

> fstabhandling.cpp:275
>  {
>  _k_updateFstabMountPointsCache();
>  

missing `_k_updateMtabMountPointsCache();`

> fstabhandling.cpp:290
>  {
>  _k_updateFstabMountPointsCache();
>  

dito

REPOSITORY
  R245 Solid

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

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

D27017: [KColorUtils] Add hue(), chroma(), getHcyColor() and update documentation

[Matthew Woehlke]

mwoehlke added inline comments.

INLINE COMMENTS

> kcolorutils.cpp:60
>  *c = khcy.c;
>  *h = khcy.h;
>  *y = khcy.y;

I guess it will be less confusing if this is also normalized?

  *h = khcy.h + (khcy.h < 0.0 ? 1.0 : 0.0);

> kcolorutils.h:36
> +/**
> + * Calculate the hue of a color. The range is 0.0 (red) to 0.99 
> (slightly blue red).
> + * This is based on linear RGB.

"0.999..." is a little odd, and I wonder if it is accurate for e.g. 
`QColor::fromRgba64(0x, 0, 1}`? Maybe something like "just under 1.0 (also 
red)" would be more useful? (If you want to get technical, "to (asymptotically) 
1.0" would also work. Maybe "0.0 //approaching// 1.0"?)

The range is of course [0, 1), but I don't know how to say that in 
straight-forward English .

OTOH, I wonder if it's worth bothering, vs. just saying "0.0 to 1.0" and 
ignoring that it will never be //exactly// 1.0.

> kcolorutils.h:37
> + * Calculate the hue of a color. The range is 0.0 (red) to 0.99 
> (slightly blue red).
> + * This is based on linear RGB.
> + * 

Suggestion: "The result is computed in linear (not sRGB) color space and may 
differ slightly from QColor::hue()."

For the others, drop "and may differ...".

> kcolorutils.h:69
> + * 
> + * The range of hue is cyclical. Examples: 0.0 and 1.0 are both red, 
> -0.17 and 0.83 are both magenta.
> + * The range of chroma is from 0.0 (none) to 1.0 (full).

I guess here should use the same text as `hue`?

> kcolorutils.h:82
> + * 
> + * The range of hue is cyclical. Examples: 0.0 and 1.0 are both red, 
> -0.17 and 0.83 are both magenta.
> + * The range of chroma is from 0.0 (none) to 1.0 (full).

This reads a little odd. I think "For example, 0.0 and ..." would be better.

> kcolorutils.h:83
> + * The range of hue is cyclical. Examples: 0.0 and 1.0 are both red, 
> -0.17 and 0.83 are both magenta.
> + * The range of chroma is from 0.0 (none) to 1.0 (full).
> + * The range of luma is from 0.0 (black) to 1.0 (white).

It might be useful to add (to luma also) "Out of range values will be clamped.".

> kcolorutils.h:90
> + */
> +KGUIADDONS_EXPORT QColor getHcyColor(qreal hue, qreal chroma, qreal luma, 
> qreal alpha = 1.0);
> +

This name feels a little odd, but I'd appreciate some broader input. Maybe 
`hcyColor`?

REPOSITORY
  R273 KGuiAddons

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

To: ndavis, #frameworks, dfaure
Cc: mwoehlke, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove added a comment.


  I'm unable to build anything right now unluckly so I tried to use code from 
the breeze folder but I'm not good at cmake. I will try to make cmake for me 
tomorrow

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis, ngraham
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Nathaniel Graham]

ngraham added a comment.


  Still not installing the file for me. Is this working for you?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis, ngraham
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove updated this revision to Diff 75019.
niccolove added a comment.


  Replace dialogs with widgets as that's the actual one

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27122?vs=75018=75019

BRANCH
  contrast_effect_background_color (branched from master)

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

AFFECTED FILES
  src/desktoptheme/breeze-dark/CMakeLists.txt
  src/desktoptheme/breeze-dark/metadata.desktop.cmake
  src/desktoptheme/breeze-dark/translucent/widgets/panel-background.svg
  src/desktoptheme/breeze-light/metadata.desktop.cmake
  src/desktoptheme/breeze/metadata.desktop.cmake
  src/desktoptheme/breeze/translucent/dialogs/background.svg
  src/desktoptheme/breeze/translucent/widgets/panel-background.svg
  src/desktoptheme/breeze/translucent/widgets/tooltip.svg

To: niccolove, ndavis, ngraham
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove updated this revision to Diff 75018.
niccolove added a comment.


  Fix cmake

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27122?vs=75017=75018

BRANCH
  contrast_effect_background_color (branched from master)

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

AFFECTED FILES
  src/desktoptheme/breeze-dark/CMakeLists.txt
  src/desktoptheme/breeze-dark/metadata.desktop.cmake
  src/desktoptheme/breeze-dark/translucent/widgets/panel-background.svg
  src/desktoptheme/breeze-light/metadata.desktop.cmake
  src/desktoptheme/breeze/metadata.desktop.cmake
  src/desktoptheme/breeze/translucent/dialogs/background.svg
  src/desktoptheme/breeze/translucent/widgets/panel-background.svg
  src/desktoptheme/breeze/translucent/widgets/tooltip.svg

To: niccolove, ndavis, ngraham
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> CMakeLists.txt:12
>  
> +FILE(GLOB translucent_dialogs translucent/dialogs/*.svg)
> +plasma_install_desktoptheme_svgs(default SUBPATH translucent/dialogs FILES 
> ${translucent_dialogs})

Using GLOB in CMake is considered a code smell; just list the individual files 
explicitly.

Also this doesn't work; the files don't get installed.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis, ngraham
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove updated this revision to Diff 75017.
niccolove added a comment.


  Updated cmake

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27122?vs=75010=75017

BRANCH
  contrast_effect_background_color (branched from master)

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

AFFECTED FILES
  src/desktoptheme/breeze-dark/CMakeLists.txt
  src/desktoptheme/breeze-dark/metadata.desktop.cmake
  src/desktoptheme/breeze-dark/translucent/widgets/panel-background.svg
  src/desktoptheme/breeze-light/metadata.desktop.cmake
  src/desktoptheme/breeze/metadata.desktop.cmake
  src/desktoptheme/breeze/translucent/dialogs/background.svg
  src/desktoptheme/breeze/translucent/widgets/panel-background.svg
  src/desktoptheme/breeze/translucent/widgets/tooltip.svg

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27152: Introduce FilesystemEntry class

[Stefan Brüns]

bruns requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R245 Solid

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

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

D27148: update d->m_file in ReadOnlyPart::setUrl()

[Piotr Dabrowski]

pdabrowski marked an inline comment as done.

REPOSITORY
  R306 KParts

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

To: pdabrowski, elvisangelaccio, ngraham, #frameworks, dfaure
Cc: marten, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27148: update d->m_file in ReadOnlyPart::setUrl()

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 75015.

REPOSITORY
  R306 KParts

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27148?vs=74970=75015

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

AFFECTED FILES
  src/readonlypart.cpp
  src/readonlypart.h

To: pdabrowski, elvisangelaccio, ngraham, #frameworks, dfaure
Cc: marten, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Nathaniel Graham]

ngraham added a comment.


  Light version still looks good to me. For the dark version, I still see a 
difference between the background opacity of the panel and the pop-up:
  
  F8085688: Screenshot_20200204_132412.png 


REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27148: update d->m_file in ReadOnlyPart::setUrl()

[Nathaniel Graham]

ngraham added reviewers: Frameworks, dfaure.

REPOSITORY
  R306 KParts

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

To: pdabrowski, elvisangelaccio, ngraham, #frameworks, dfaure
Cc: marten, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27148: update d->m_file in ReadOnlyPart::setUrl()

[Piotr Dabrowski]

pdabrowski added inline comments.

INLINE COMMENTS

> readonlypart.cpp:80
>  
> +QUrl ReadOnlyPart::mostLocalUrl(QUrl url) const {
> +KIO::StatJob* statJob = KIO::mostLocalUrl(url);

TODO: mostLocalUrl(const QUrl )

REPOSITORY
  R306 KParts

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

To: pdabrowski, elvisangelaccio, ngraham
Cc: marten, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove added a comment.


  I've done the dark theme again, should be nice now

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove edited the test plan for this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove updated this revision to Diff 75010.
niccolove added a comment.


  Tweaked contrast effect for dark and light versions, added new panel file for 
dark

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27122?vs=74911=75010

BRANCH
  contrast_effect_background_color (branched from master)

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

AFFECTED FILES
  src/desktoptheme/breeze-dark/metadata.desktop.cmake
  src/desktoptheme/breeze-dark/translucent/widgets/panel-background.svg
  src/desktoptheme/breeze-light/metadata.desktop.cmake
  src/desktoptheme/breeze/metadata.desktop.cmake
  src/desktoptheme/breeze/translucent/dialogs/background.svg
  src/desktoptheme/breeze/translucent/widgets/panel-background.svg
  src/desktoptheme/breeze/translucent/widgets/tooltip.svg

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27148: update d->m_file in ReadOnlyPart::setUrl()

[Piotr Dabrowski]

pdabrowski added a comment.


  In D27148#605830 , @marten wrote:
  
  > Yes, I'd concluded that the real place to fix the problem was at the KParts 
level, but not being a KParts expert wanted to leave that decision to its 
maintainers.  +1 for the elegant fix.
  
  
  I'm in no case KParts expert. That's why I would like a maintainer to review 
this change.
  Especially I'm not sure if it is ok to use KIO::StatJob here.
  
  > One coding style change? - mostLocalUrl(QUrl url) -> mostLocalUrl(const 
QUrl )
  
  Good catch. I will fix it.

REPOSITORY
  R306 KParts

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

To: pdabrowski, elvisangelaccio, ngraham
Cc: marten, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25698: New query mechanism for applications: KApplicationTrader

[Aleix Pol Gonzalez]

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


  +1 Makes sense.

REPOSITORY
  R309 KService

BRANCH
  kapplicationtrader

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

To: dfaure, broulik, mart, vkrause, nicolasfella, aacid, davidedmundson, apol
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25698: New query mechanism for applications: KApplicationTrader

[Albert Astals Cid]

aacid added a comment.


  For me it can go in, but i'm not really really netiher a Frameworks developer 
nor potentially a user of this class, so i'd feel more confortable if someone 
else also +1'ed
  
  On the other hand you're the mega-manintainer of everything, so i guess it 
can go in :)

REPOSITORY
  R309 KService

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

To: dfaure, broulik, mart, vkrause, nicolasfella, aacid, davidedmundson
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27002: Fix memory leak in KUrlNavigatorPlacesSelector::updateMenu

[Fabian Vogt]

fvogt added a comment.


  In D27002#605999 , @ngraham wrote:
  
  > Could this be the fix for https://bugs.kde.org/show_bug.cgi?id=398908, or 
part of it?
  
  
  This leak presented itself by steadily growing memory use while something 
still unknown triggered solid's `onMtabChanged` excessively.
  If that's also the case for the reporter of that bug, this should've made a 
big difference, otherwise only a small one.
  
  The output of heaptrack would tell for sure.

REPOSITORY
  R241 KIO

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

To: fvogt, #frameworks, davidedmundson, meven
Cc: ngraham, anthonyfieroni, meven, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D27158: Fix some compiler warnings

[Ahmad Samir]

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

REVISION SUMMARY
  - Fix order of member initialisation (compiling with -Wreorder)
  - Use Q_UNUSED where needed
  - QAtomicInteger::loadRelaxed() has been available since Qt 5.14

TEST PLAN
  make && ctest

REPOSITORY
  R237 KConfig

BRANCH
  l-compiler-warnings (branched from master)

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

AFFECTED FILES
  src/core/kconfig.cpp
  src/kconfig_compiler/KConfigCodeGeneratorBase.cpp
  src/kconfig_compiler/KConfigHeaderGenerator.cpp
  src/kconfig_compiler/KConfigXmlParser.cpp
  src/kconfig_compiler/kconfig_compiler.cpp

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

D27002: Fix memory leak in KUrlNavigatorPlacesSelector::updateMenu

[Nathaniel Graham]

ngraham added a comment.


  Could this be the fix for https://bugs.kde.org/show_bug.cgi?id=398908, or 
part of it?

REPOSITORY
  R241 KIO

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

To: fvogt, #frameworks, davidedmundson, meven
Cc: ngraham, anthonyfieroni, meven, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove added a comment.


  In D27122#605987 , @ndavis wrote:
  
  > Don't make a color file for the breeze desktop theme. It doesn't have one 
so that it can use any system color scheme.
  
  
  Uhm; but then, there's no way for the default theme to not look gray-ish 
without changing the globar colorscheme? (Also - since this is approaching a 
discussion that would work better in chat, can we talk in the VDG chat 
directly?)

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27157: [KConfigLoader] Code cleanup

[Ahmad Samir]

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

REVISION SUMMARY
  - Drop redundant args to startElement()/endEelement()
  - Use range-for
  - Drop ConfigLoaderHandler methods that weren't used anywhere AFAICS and 
ConfigLoaderHandler is private API

TEST PLAN
  make && ctest

REPOSITORY
  R237 KConfig

BRANCH
  l-configloader (branched from master)

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

AFFECTED FILES
  src/gui/kconfigloader.cpp
  src/gui/kconfigloaderhandler_p.h

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

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Noah Davis]

ndavis added a comment.


  In D27122#605982 , @niccolove 
wrote:
  
  > In D27122#605981 , @ndavis wrote:
  >
  > > In D27122#605976 , @niccolove 
wrote:
  > >
  > > > In D27122#605975 , @ndavis 
wrote:
  > > >
  > > > > Also, you can set contrast effects per desktop theme. If you do that, 
you may be able to improve the look of breeze light without hurting breeze 
dark. Let the Breeze desktop theme be a fairly neutral one that works well with 
most colorschemes.
  > > >
  > > >
  > > > Yes, I was just doing that as well. One question: if I change the 
colorscheme for background of default Breeze, it will become the same as Breeze 
Light, won't it? Should I remove that one? It would be probably better than 
having two different ones but the default one (Breeze) is a bit more gray-ish 
than the other (Breeze Light)
  > >
  > >
  > > Keep in mind there's the Breeze Light desktop theme (uses the Breeze 
color scheme) and the Breeze Light color scheme (not that great). What are you 
referring to?
  >
  >
  > I'm referring the the Breeze Light desktop theme, which I guess uses the 
"color" file to look, well, lighter, but it won't be different if I make the 
color file for Breeze as well
  
  
  Don't make a color file for the breeze desktop theme. It doesn't have one so 
that it can use any system color scheme.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove added a comment.


  In D27122#605981 , @ndavis wrote:
  
  > In D27122#605976 , @niccolove 
wrote:
  >
  > > In D27122#605975 , @ndavis 
wrote:
  > >
  > > > Also, you can set contrast effects per desktop theme. If you do that, 
you may be able to improve the look of breeze light without hurting breeze 
dark. Let the Breeze desktop theme be a fairly neutral one that works well with 
most colorschemes.
  > >
  > >
  > > Yes, I was just doing that as well. One question: if I change the 
colorscheme for background of default Breeze, it will become the same as Breeze 
Light, won't it? Should I remove that one? It would be probably better than 
having two different ones but the default one (Breeze) is a bit more gray-ish 
than the other (Breeze Light)
  >
  >
  > Keep in mind there's the Breeze Light desktop theme (uses the Breeze color 
scheme) and the Breeze Light color scheme (not that great). What are you 
referring to?
  
  
  I'm referring the the Breeze Light desktop theme, which I guess uses the 
"color" file to look, well, lighter, but it won't be different if I make the 
color file for Breeze as well

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Noah Davis]

ndavis added a comment.


  In D27122#605976 , @niccolove 
wrote:
  
  > In D27122#605975 , @ndavis wrote:
  >
  > > Also, you can set contrast effects per desktop theme. If you do that, you 
may be able to improve the look of breeze light without hurting breeze dark. 
Let the Breeze desktop theme be a fairly neutral one that works well with most 
colorschemes.
  >
  >
  > Yes, I was just doing that as well. One question: if I change the 
colorscheme for background of default Breeze, it will become the same as Breeze 
Light, won't it? Should I remove that one? It would be probably better than 
having two different ones but the default one (Breeze) is a bit more gray-ish 
than the other (Breeze Light)
  
  
  Keep in mind there's the Breeze Light desktop theme (uses the Breeze color 
scheme) and the Breeze Light color scheme (not that great). What are you 
referring to?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D26245: Set SYSCONFDIR to /etc when CMAKE_INSTALL_SYSCONFDIR is etc relative to /usr

[Piotr Wójcik]

pwojcik added a comment.


  I want to remind about topic.

REPOSITORY
  R240 Extra CMake Modules

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

To: pwojcik, kossebau, alexmerry
Cc: apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D27082: [Tests]Make radiobutton3.qml use PC3

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:dccf6c4dd629: [Tests]Make radiobutton3.qml use PC3 
(authored by gvgeo, committed by ngraham).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27082?vs=74826=75006

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

AFFECTED FILES
  tests/components/radiobutton3.qml

To: gvgeo, #plasma, davidedmundson
Cc: ngraham, davidedmundson, ndavis, kde-frameworks-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
michaelh, ZrenBot, bruns, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27082: [Tests]Make radiobutton3.qml use PC3

[Nathaniel Graham]

ngraham added a comment.


  All right, your choice. I'll land this for you now. Great work!

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  radiobutton (branched from master)

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

To: gvgeo, #plasma, davidedmundson
Cc: ngraham, davidedmundson, ndavis, kde-frameworks-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
michaelh, ZrenBot, bruns, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove added a comment.


  In D27122#605975 , @ndavis wrote:
  
  > Also, you can set contrast effects per desktop theme. If you do that, you 
may be able to improve the look of breeze light without hurting breeze dark. 
Let the Breeze desktop theme be a fairly neutral one that works well with most 
colorschemes.
  
  
  Yes, I was just doing that as well. One question: if I change the colorscheme 
for background of default Breeze, it will become the same as Breeze Light, 
won't it? Should I remove that one? It would be probably better than having two 
different ones but the default one (Breeze) is a bit more gray-ish than the 
other (Breeze Light)

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Noah Davis]

ndavis added a comment.


  Also, you can set contrast effects per desktop theme. If you do that, you may 
be able to improve the look of breeze light without hurting breeze dark. Let 
the Breeze desktop theme be a fairly neutral one that works well with most 
colorschemes.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Noah Davis]

ndavis added a comment.


  In D27122#605969 , @niccolove 
wrote:
  
  > In D27122#605804 , @ndavis wrote:
  >
  > > Using View Background is semantically incorrect. If you want to change 
the color, you should patch the color scheme.
  >
  >
  > But in order to change ColorScheme-Background I should change Window 
Background in the colorscheme, which would make all KDE apps lighter, right? I 
don't think that it makes sense to use the same color of windows for dialogs, 
since they're not windows, especially since they are also transparent and that 
requires adjusting the color.
  
  
  If you need to adjust the color specifically for the desktop theme, you 
should adjust the color schemes of the Plasma Light/Dark desktop themes.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove added a comment.


  In D27122#605973 , @ndavis wrote:
  
  > In D27122#605969 , @niccolove 
wrote:
  >
  > > In D27122#605804 , @ndavis 
wrote:
  > >
  > > > Using View Background is semantically incorrect. If you want to change 
the color, you should patch the color scheme.
  > >
  > >
  > > But in order to change ColorScheme-Background I should change Window 
Background in the colorscheme, which would make all KDE apps lighter, right? I 
don't think that it makes sense to use the same color of windows for dialogs, 
since they're not windows, especially since they are also transparent and that 
requires adjusting the color.
  >
  >
  > If you need to adjust the color specifically for the desktop theme, you 
should adjust the color schemes of the Plasma Light/Dark desktop themes.
  
  
  Gotcha.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Niccolò Venerandi]

niccolove added a comment.


  In D27122#605804 , @ndavis wrote:
  
  > Using View Background is semantically incorrect. If you want to change the 
color, you should patch the color scheme.
  
  
  But in order to change ColorScheme-Background I should change Window 
Background in the colorscheme, which would make all KDE apps lighter, right? I 
don't think that it makes sense to use the same color of windows for dialogs, 
since they're not windows, especially since they are also transparent and that 
requires adjusting the color.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27017: [KColorUtils] Add hue(), chroma(), getHcyColor() and update documentation

[Noah Davis]

ndavis added a comment.


  Anyone want to accept this?

REPOSITORY
  R273 KGuiAddons

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

To: ndavis, #frameworks, dfaure
Cc: mwoehlke, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26368: Add an isImmutable to know if a property is immutable

[Méven Car]

This revision was automatically updated to reflect the committed changes.
Closed by commit R237:d46739294d04: Add an isPropertyNameImmutable to 
know if a property is immutable (authored by meven).

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26368?vs=74589=75001

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

AFFECTED FILES
  autotests/kconfig_compiler/test1.h.ref
  autotests/kconfig_compiler/test10.h.ref
  autotests/kconfig_compiler/test11.h.ref
  autotests/kconfig_compiler/test11a.h.ref
  autotests/kconfig_compiler/test12.h.ref
  autotests/kconfig_compiler/test13.h.ref
  autotests/kconfig_compiler/test2.h.ref
  autotests/kconfig_compiler/test3.h.ref
  autotests/kconfig_compiler/test3a.h.ref
  autotests/kconfig_compiler/test4.h.ref
  autotests/kconfig_compiler/test5.h.ref
  autotests/kconfig_compiler/test6.h.ref
  autotests/kconfig_compiler/test7.h.ref
  autotests/kconfig_compiler/test8a.h.ref
  autotests/kconfig_compiler/test8b.h.ref
  autotests/kconfig_compiler/test8c.h.ref
  autotests/kconfig_compiler/test9.h.ref
  autotests/kconfig_compiler/test_dpointer.cpp.ref
  autotests/kconfig_compiler/test_dpointer.h.ref
  autotests/kconfig_compiler/test_notifiers.h.ref
  autotests/kconfig_compiler/test_qdebugcategory.h.ref
  autotests/kconfig_compiler/test_signal.h.ref
  autotests/kconfig_compiler/test_translation_kde.h.ref
  autotests/kconfig_compiler/test_translation_kde_domain.h.ref
  autotests/kconfig_compiler/test_translation_qt.h.ref
  src/kconfig_compiler/KConfigCodeGeneratorBase.cpp
  src/kconfig_compiler/KConfigCodeGeneratorBase.h
  src/kconfig_compiler/KConfigCommonStructs.h
  src/kconfig_compiler/KConfigHeaderGenerator.cpp
  src/kconfig_compiler/KConfigHeaderGenerator.h
  src/kconfig_compiler/KConfigSourceGenerator.cpp
  src/kconfig_compiler/KConfigSourceGenerator.h
  src/kconfig_compiler/kconfig_compiler.cpp

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

D26749: WIP: Support NDK r20 and Qt 5.14

[Francis Herne]

flherne added a comment.


  I got my Kirigami app to load by patching kirigami.{cpp,h}:
  
  Altered `resolveFilePath()` to `return 
QStringLiteral("qrc:/android_rcc_bundle/qml/org/kde/kirigami.2/") + path;` and 
similarly for `resolveFileUrl()`.
  
  Added 
`QResource::registerResource(QStringLiteral("assets:/android_rcc_bundle.rcc"));`
 in `registerTypes()` [Qt is supposed to do this, but perhaps not early enough?]
  
  https://codereview.qt-project.org/c/qt/qtbase/+/270573/24 is the relevant Qt 
change.
  
  Icons are still broken.
  There are probably better ways to solve this.

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: flherne, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D27152: Introduce FilesystemEntry class

[David Hallas]

hallas added inline comments.

INLINE COMMENTS

> meven wrote in filesystem_entry.cpp:47
> It would be great to expose an enum (KIO's KFileSystemType does this a 
> little).
> But given the number of filesystems this days, it may be overcomplicated.

Yeah I agree ;) But I would really like for this class to be used more 
generically so having a list of filesystems here would probably make it hard 
for that. We could also have two methods, one similar to this one given the 
"raw" filesystem type, and one that would give an enum entry from a so called 
"known" list, but would return "unknown" for a filesystem type not in the list, 
then you would have access to both? What do you think?

> meven wrote in filesystem_entry.h:88
> Is it necessary to keep if it is  commented out ?

No, I only put them here for completeness sake. Basically I sat down with the 
fstab man page and the header file documenting the mntent structure and based 
this class around that, but if this code should be exported an made generally 
available then we should probably add these, what do you think? Should I remove 
them now, and then we can always add them if needed?

REPOSITORY
  R245 Solid

BRANCH
  introduce_filesystem_entry

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

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

D25326: Move Plasma's SortFilterProxyModel into KItemModel's QML plugin

[Kai Uwe Broulik]

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

REPOSITORY
  R275 KItemModels

BRANCH
  arcpatch-D25326

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

To: davidedmundson, broulik
Cc: kmaterka, iasensio, ahmadsamir, broulik, ahiemstra, mart, 
kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25117: file ioslave: stop copying as soon as the ioslave is killed

[Méven Car]

meven added a task: T11627: Improve KIO asynchronicity.

REPOSITORY
  R241 KIO

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

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

T11627: Improve KIO asynchronicity

[Méven Car]

meven added a revision: D25117: file ioslave: stop copying as soon as the 
ioslave is killed.

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

To: meven
Cc: ahartmetz, broulik, ognarb, #dolphin, #frameworks, meven, dfaure, 
pberestov, iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, 
Codezela, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov

D26342: Allow overriding to disable auto language detection

[Simon Depiets]

sdepiets added reviewers: cullmann, mlaurent.

REPOSITORY
  R246 Sonnet

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

To: sdepiets, #frameworks, cullmann, mlaurent
Cc: mludwig, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27153: port sftp to Result system to force serialization of error/finish condition

[Harald Sitter]

sitter created this revision.
sitter added reviewers: dfaure, feverfew.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  the Result system was originally introduced to the FTP slave and now also
  makes an appearance in the SFTP slave. the system introduces a separation
  between logic and fronting API class to more tightly control when state
  changing calls (finished()/error()) are made. since these calls may only
  be made once during a given command multiple calls are at the very least
  indicative of bad code and at worst cause severe state confusion for the
  slavebase that it won't be able to recover from, rendering the slave
  instance broken.
  
  in the internal class Results are returned whenever an error can appear and
  these Results must be handled in some form. the only way to effectively
  produce user visible errors is to forward results up the call chain.

TEST PLAN
  - connect
  - copy remotely
  - overwrite remotely
  - copy to local
  - overwrite to local
  - copy from local to remote
  - copy form local to remote and overwrite

REPOSITORY
  R320 KIO Extras

BRANCH
  sftp-errors

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

AFFECTED FILES
  sftp/kio_sftp.cpp
  sftp/kio_sftp.h

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

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/288/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Tue, 04 Feb 2020 10:46:12 +
 Build duration:
8 min 5 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)

D26858: Provide an implementation for the tablet interface

[David Edmundson]

davidedmundson added a comment.


  Feedback from some testing:
  
  - we're adding the same tool a bunch of times when a new client is created
  - sometimes we fail to enter the surface leaving the cursor "stuck" (with 
gtk3-demo)
  
  I'll see if I can spot why.
  
  FWIW, Qt have a tablet test at qtbase/examples/widgets/widgets/tablet though 
that means compiling 5.15 to get the  qtwayland tablet  integration

REPOSITORY
  R127 KWayland

BRANCH
  apol/tablet-unstable-v2-1

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

To: apol, #kwin, #frameworks, zzag
Cc: davidedmundson, zzag, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27002: Fix memory leak in KUrlNavigatorPlacesSelector::updateMenu

[Méven Car]

meven added a comment.


  In D27002#605864 , @fvogt wrote:
  
  > In D27002#605860 , @meven wrote:
  >
  > > User feedback: "so far so good, 160 MB Memory usage"
  > >  Does not sound reassuring, I guess the user meant 160 MB compared to 
200MB or similar prior to patch.
  >
  >
  > Updated - should be clearer now :-)
  
  
  Great to see the impact, this was a serious leak.

REPOSITORY
  R241 KIO

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

To: fvogt, #frameworks, davidedmundson, meven
Cc: anthonyfieroni, meven, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27002: Fix memory leak in KUrlNavigatorPlacesSelector::updateMenu

[Fabian Vogt]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:213ed50634c0: Fix memory leak in 
KUrlNavigatorPlacesSelector::updateMenu (authored by fvogt).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27002?vs=74645=74989

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

AFFECTED FILES
  src/filewidgets/kurlnavigatorplacesselector.cpp

To: fvogt, #frameworks, davidedmundson, meven
Cc: anthonyfieroni, meven, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27002: Fix memory leak in KUrlNavigatorPlacesSelector::updateMenu

[Fabian Vogt]

fvogt added a comment.


  In D27002#605860 , @meven wrote:
  
  > User feedback: "so far so good, 160 MB Memory usage"
  >  Does not sound reassuring, I guess the user meant 160 MB compared to 200MB 
or similar prior to patch.
  
  
  Updated - should be clearer now :-)

REPOSITORY
  R241 KIO

BRANCH
  noleak

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

To: fvogt, #frameworks, davidedmundson, meven
Cc: anthonyfieroni, meven, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27002: Fix memory leak in KUrlNavigatorPlacesSelector::updateMenu

[Fabian Vogt]

fvogt edited the test plan for this revision.

REPOSITORY
  R241 KIO

BRANCH
  noleak

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

To: fvogt, #frameworks, davidedmundson, meven
Cc: anthonyfieroni, meven, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27123: Add a default provider (as per documentation)

[Dan Leinir Turthra Jensen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R235:5f799b92559f: Add a default provider (as per 
documentation) (authored by leinir).

REPOSITORY
  R235 Attica

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27123?vs=74917=74988

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

AFFECTED FILES
  src/qtplatformdependent.cpp

To: leinir, kossebau, #frameworks, #knewstuff, apol
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27002: Fix memory leak in KUrlNavigatorPlacesSelector::updateMenu

[Méven Car]

meven accepted this revision.
meven added a comment.


  User feedback: "so far so good, 160 MB Memory usage"
  Does not sound reassuring, I guess the use meant 160 MB compared to 200MB 
prior to patch.

REPOSITORY
  R241 KIO

BRANCH
  noleak

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

To: fvogt, #frameworks, davidedmundson, meven
Cc: anthonyfieroni, meven, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27002: Fix memory leak in KUrlNavigatorPlacesSelector::updateMenu

[David Edmundson]

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

REPOSITORY
  R241 KIO

BRANCH
  noleak

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

To: fvogt, #frameworks, davidedmundson
Cc: anthonyfieroni, meven, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D27002: Fix memory leak in KUrlNavigatorPlacesSelector::updateMenu

[Fabian Vogt]

fvogt added a comment.


  I'll land tomorrow if no objections.

REPOSITORY
  R241 KIO

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

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

Re: WARNING: was bad recommendation (Re: Recommendation: drop ProvidersUrl entry to rely on default value)

[Dan Leinir Turthra Jensen]

On Friday, 31 January 2020 01:08:35 GMT Friedrich W. H. Kossebau wrote:
> Hi,
> 
> TLDR: removing the ProvidersUrl entry actually breaks things in non-Plasma
> installations, so for now has to be hardcoded, using the non-deprecated
> 
> ProvidersUrl=https://autoconfig.kde.org/ocs/providers.xml
> 
> See below for investigation results:
> 
> Am Freitag, 31. Januar 2020, 00:14:19 CET schrieb Christoph Feck:
> > On 01/30/20 13:53, Friedrich W. H. Kossebau wrote:
> > > as found out by discussion on irc, a good solution for everyone relying
> > > on
> > > the default GHNS storage as provided by KDE is to just not hard-code any
> > > value for ProvidersUrl, but leave it out and let KNewStuff default to
> > > what is built into the KNewStuff library as current value.
> > 
> > Does it work with all KNewStuff 5.x versions? Otherwise, the required
> > KF5 version would need to be bumped where this change was made.

  We'll need to adjust the documentation to ensure the below holds true

 
> So seems there is some flaw in the design of Attica when it comes to the
> concept of platforms: having a different provider for a non-Plasma platform
> does not make sense to me.
> This needs to be rethought, also when it comes to defaults.

  Entirely correct, that is definitely an ommission. While Attica's 
qtplatformdependent does lack functionality like the persistence layer for 
adding, removing, and disabling/disabling providers, it should most definitely 
be returning the default provider as the documentation says it can be expected 
to. So, just posted a patch[1] which makes it do so (instead of a simple, 
empty list).

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

> Too sleepy to think further now. And this best is sorted out by KNewStuff
> developers :)
> 
> Cheers
> Friedrich


-- 
..dan / leinir..
http://leinir.dk/

Re: Banning QNetworkAccessManager

[Johan Ouwerkerk]

On Mon, Feb 3, 2020 at 11:27 AM laurent Montel  wrote:
>
> Le lundi 3 février 2020, 10:49:10 CET David Edmundson a écrit :
> > I updated:
> >
> > https://community.kde.org/Policies/API_to_Avoid
> >
> > Which had no mention of this.
> >
> > David
>
> I think that you made an error
>
> "networkAccessManger->setAttribute(QNetworkRequest::FollowRedirectsAttribute,
> true); "
> Doesn't exist it's a enum from QnetworkRequest::RedirectPolicy
> And  FollowRedirectsAttribute is old value
> It seems that we need to use QnetworkRequest::NoLessSafeRedirectPolicy
> directly no ?
>

Yes, the example code is definitely wrong: in the real world redirects
are an attack vector. A few cases to consider:

 * Loops of redirects (could happen if the site is broken)
 * Leaking sensitive information via e.g. the Referrer header

Regards,

 - Johan

T12641: Refactor KFileProtocol::copy

[Méven Car]

meven created this task.
meven triaged this task as "Normal" priority.
meven added projects: Frameworks, Dolphin.

TASK DESCRIPTION
  Our man file copy function has some glaring issues that I'd like to tackle in 
a few steps:
  
  - Split the sendfile and read/write code path to function, to make evolution 
easier
  - Then Make those function io calls  (read, write, sendfile) O_NONBLOCK and 
use select/poll freeing user space CPU time to for instance notify the user of 
progressand  and reducing chances of blocking in both ways (file io blocking or 
socket/ksmserver blocking)
  - Then make the sendFile/write operation O_DIRECT and O_SYNC 
(https://linux.die.net/man/2/open), perhaps by default, or option or depending 
on the fs destination. Fixing https://bugs.kde.org/show_bug.cgi?id=281270 
`Inconsistent notifications during/after file operations (copying, moving, 
deleting, compressing, extracting) depending on amount of data/items involved` 
(another to fix this is to mount usb sticks with the sync mount option)
  
  Then if needed :
  
  - Tune the parameters of the sendFile code path to copy data by chunks bigger 
than 32kB, I am thinking 128/512 to keep granular progress reporting,
  - Tune the read/write (it reads and write by chunks of 32kB at the moment
  
  Bonus steps
  
  - Add a io_uring (https://lwn.net/Articles/778411/) code path for recent 
linux kernels
  
  All of those steps should make the code more async, maintainable and 
performant, and fix a few longstanding bugs.
  
  It should help on 
   *`Ridiculously slow file copy (multiple small files)` 
https://bugs.kde.org/show_bug.cgi?id=342056

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

To: meven
Cc: apol, dfaure, #frameworks, #dolphin, ognarb, broulik, meven, pberestov, 
iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
bruns, emmanuelp, mikesomov

Re: Banning QNetworkAccessManager

[Johnny Jazeix]

[...]

> All of the places where we have actively hit this issue have already
> been fixed (Marble and Attica come immediately to mind, not sure if
> GCompris fixed their code).
>

I took a quick look and we use some code to handle redirection:
https://github.com/gcompris/GCompris-qt/blob/master/src/core/DownloadManager.cpp#L502
It's not the same code as mentioned by David in
https://community.kde.org/Policies/API_to_Avoid#QNetworkAccessManager,
I'll update the code tonight.

Johnny

> The rest continue to be sleeping timebombs which we will only discover
> when we change something on the server side and put in place a
> redirect. They may never be triggered, or they could be triggered next
> week. Even if we fix the code now, due to LTS distributions and people
> using software for far longer than they should, it will take years for
> the fixes to fully reach user systems.
>
> To illustrate how long this takes, Marble moved from using
> http://files.kde.org/marble/maps/ to https://maps.kde.org/ back in
> January 2016. Four years later, we still get 13,000 hits to paths
> under the old URL every day. The version numbers sent by Marble as
> part of these requests indicate that some of them are from the version
> released with KDE 4.14 which was released back in August 2014
> (fortunately this code path was fixed to follow redirects prior to
> that release)
>
> >
> > Regards,
> > Volker
>
> Cheers,
> Ben

D26600: Refactor fstab handling

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> system_filesystem_table_test.cpp:55
> +"devices /sys/fs/cgroup/devices cgroup 
> rw,nosuid,nodev,noexec,relatime,devices 0 0\n"
> +"freezer /sys/fs/cgroup/freezer cgroup 
> rw,nosuid,nodev,noexec,relatime,freezer 0 0\n"
> +"net_cls /sys/fs/cgroup/net_cls cgroup 
> rw,nosuid,nodev,noexec,relatime,net_cls 0 0\n"

Perhaps add a squashfs mount "/dev/loop15 /snap/core/8268 squashfs 
ro,nodev,relatime 0 0"

> system_filesystem_table.cpp:61
> +{
> +connect(_fstabWatcher, SIGNAL(tableChanged()), this, 
> SLOT(fstabChanged()));
> +connect(_mtabWatcher, SIGNAL(tableChanged()), this, 
> SLOT(mtabChanged()));

Can't you use the new syntax ?

REPOSITORY
  R245 Solid

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

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

D27152: Introduce FilesystemEntry class

[Méven Car]

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


  Nice one. Good step splitting out this.

INLINE COMMENTS

> filesystem_entry.cpp:47
> +
> +QString FilesystemEntry::type() const
> +{

It would be great to expose an enum (KIO's KFileSystemType does this a little).
But given the number of filesystems this days, it may be overcomplicated.

> filesystem_entry.h:88
> + */
> +//int m_dumpFrequency;
> +/*

Is it necessary to keep if it is  commented out ?

> filesystem_entry.h:94
> + */
> +//int m_passNo;
> +};

Same

REPOSITORY
  R245 Solid

BRANCH
  introduce_filesystem_entry

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

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

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kdeclarative/job/kf5-qt5%20FreeBSDQt5.13/74/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Tue, 04 Feb 2020 08:44:49 +
 Build duration:
13 min and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 2 test(s)Failed: projectroot.autotests.quickviewsharedengine

D27148: update d->m_file in ReadOnlyPart::setUrl()

[Jonathan Marten]

marten added a comment.


  Yes, I'd concluded that the real place to fix the problem was at the KParts 
level, but not being a KParts expert wanted to leave that decision to its 
maintainers.  +1 for the elegant fix.

REPOSITORY
  R306 KParts

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

To: pdabrowski, elvisangelaccio, ngraham
Cc: marten, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D27132: Allow to use KPropertySkeletonItem in ManagedConfigModule

[Benjamin Port]

This revision was automatically updated to reflect the committed changes.
Closed by commit R296:cacff0a30bc8: Allow to use KPropertySkeletonItem in 
ManagedConfigModule (authored by bport).

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27132?vs=74980=74983

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

AFFECTED FILES
  src/quickaddons/managedconfigmodule.cpp

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

D27132: Allow to use KPropertySkeletonItem in ManagedConfigModule

[Kevin Ottens]

ervin accepted this revision.

REPOSITORY
  R296 KDeclarative

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

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

D27132: Allow to use KPropertySkeletonItem in ManagedConfigModule

[Benjamin Port]

bport updated this revision to Diff 74980.
bport added a comment.


  cleanup

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27132?vs=74979=74980

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

AFFECTED FILES
  src/quickaddons/managedconfigmodule.cpp

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

D27132: Allow to use KPropertySkeletonItem in ManagedConfigModule

[Benjamin Port]

bport updated this revision to Diff 74979.
bport added a comment.


  Don't cast twice if not needed

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27132?vs=74935=74979

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

AFFECTED FILES
  src/quickaddons/managedconfigmodule.cpp

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

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi added a comment.


  In D26691#605802 , @jjazeix wrote:
  
  > Compilation fails: 
https://build.kde.org/view/Failing/job/Plasma/job/plasma-framework/job/kf5-qt5%20SUSEQt5.12/288/console
  >  Missing the new file in the CMakeLists.txt 
(https://cgit.kde.org/plasma-framework.git/tree/src/scriptengines/qml/CMakeLists.txt#n9)?
  >
  > Fixed in 
https://cgit.kde.org/plasma-framework.git/commit/?id=2ea27fb06887237d49c63f6b7e74702bffef57ea
  
  
  Thanks. :)

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: jjazeix, davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, 
LeGast00n, GB_2, michaelh, ngraham, bruns

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Noah Davis]

ndavis requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Noah Davis]

ndavis added a comment.


  Why View Background? That's semantically incorrect. If you want to change the 
color, you should patch the color scheme.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove
Cc: ndavis, filipf, ngraham, davidedmundson, kde-frameworks-devel, manueljlin, 
Orage, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, GB_2, trickyricky26, ragreen, michaelh, crozbo, ZrenBot, firef, 
bruns, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart