D13013: Small fixes for execute_kwallet

[Anthony Fieroni]

anthonyfieroni added a comment.


  It's ok to me.

REPOSITORY
  R107 KWallet PAM Integration

BRANCH
  master

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

To: aacid, fvogt
Cc: anthonyfieroni, mgerstner, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D13529: [minimizeAll] Do not try to restore window state on model change

[Anthony Fieroni]

anthonyfieroni created this revision.
anthonyfieroni added reviewers: davidedmundson, Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
anthonyfieroni requested review of this revision.

REVISION SUMMARY
  onDataChanged: some cases deactivate can not be called or persistent model 
index is invalidated (this should not happen in this case or it's a bug) which 
result in effect to stay active

TEST PLAN
  1. Click on applet
  2. Open new windows that rearrange task manager
  3. Try to unminimize window will result in unminimizing other

REPOSITORY
  R114 Plasma Addons

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

AFFECTED FILES
  applets/minimizeall/package/contents/ui/main.qml

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13529: [minimizeAll] Do not try to restore window state on model change

[Anthony Fieroni]

anthonyfieroni added a comment.


  On previous version onStackingOrderChanged will not restore windows state, it 
just stops the effect. So onActiveTaskChanged to replace onDataChanged?

REPOSITORY
  R114 Plasma Addons

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

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13529: [minimizeAll] Do not try to restore window state on model change

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 36270.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13529?vs=36160=36270

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

AFFECTED FILES
  applets/minimizeall/package/contents/ui/main.qml

To: anthonyfieroni, davidedmundson, #plasma
Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13529: [minimizeAll] Do not try to restore window state on model change

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 36160.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13529?vs=36142=36160

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

AFFECTED FILES
  applets/minimizeall/package/contents/ui/main.qml

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13529: [minimizeAll] Do not try to restore window state on model change

[Anthony Fieroni]

anthonyfieroni added a comment.


  onVirtualDesktopChanged and onActivityChanged it looks good to deactivate 
effect with restoring windows state. Because you can miss or forgive that you 
activate it, we definitely NOT want to deactivate effect when you click desktop 
:) I think now it's looking good.

REPOSITORY
  R114 Plasma Addons

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

To: anthonyfieroni, davidedmundson, #plasma
Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13752: Kill solidautoeject

[Anthony Fieroni]

anthonyfieroni added a comment.


  I have looking on https://bugs.kde.org/show_bug.cgi?id=394348 that's using 
same interface when ISO is mounting?

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, #frameworks, adridg, davidedmundson, dfaure, fvogt, ervin
Cc: anthonyfieroni, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D10342: From 100% plasma cpu usage to normal when using vivaldi

[Anthony Fieroni]

anthonyfieroni added a comment.


  In D10342#286360 , @jtamate wrote:
  
  > Or perhaps is it better to try to create that cache in KServiceTypeTrader?
  
  
  That will be the right place, reading from disk is always costly, it will be 
better to make service parsing on file changes otherwise to read from cache.

REPOSITORY
  R120 Plasma Workspace

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

To: jtamate, #plasma_workspaces, hein
Cc: anthonyfieroni, mart, mwolff, broulik, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12641: Workaround Device Notifier showing an error despite volume being mounted correctly

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> deviceserviceaction.cpp:91
> +} else {
> +connect(access, ::StorageAccess::accessibilityChanged,
> +this, ::_k_accessibilityChanged);

Why not use it in all cases?

REPOSITORY
  R120 Plasma Workspace

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

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

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Anthony Fieroni]

anthonyfieroni added a reviewer: dfaure.
anthonyfieroni added a comment.


  I'm not sure it's Qt bug, it change behavior and now it doing in *right* way. 
So i'm patch to be merged +1.

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9506: milti-screen sleep fix

[Anthony Fieroni]

anthonyfieroni added reviewers: sebas, drosca.

REPOSITORY
  R110 KScreen Library

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

To: McPain, sebas, drosca
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9738: Support font/ttf and font/otf mimetypes in kfontinst

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> Misc.cpp:293
>  {
> -return "application/x-font-type1"==mime || 
> "application/x-font-ttf"==mime ||
> +return "font/otf"==mime || "font/ttf"==mime ||
> +   "application/x-font-type1"==mime || 
> "application/x-font-ttf"==mime ||

So why not

  return CFontList::fontMimeTypes.contains(mime);

In other places too.

REPOSITORY
  R119 Plasma Desktop

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

To: fvogt, #plasma
Cc: anthonyfieroni, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D9506: milti-screen sleep fix

[Anthony Fieroni]

anthonyfieroni added a comment.


  @McPain can you provide some debug info from qCDebug(KSCREEN_XRANDR), i don't 
have multi screen setup but we can make better fix.

REPOSITORY
  R110 KScreen Library

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

To: McPain, sebas, drosca
Cc: anthonyfieroni, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9998: Actually quit threads nicely

[Anthony Fieroni]

anthonyfieroni added a comment.


  If it's not running event loop why run is called?

REPOSITORY
  R161 KActivity Manager Service

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

To: fvogt, #plasma, ivan, anthonyfieroni
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D10118: Disable keyboard geometry preview code

[Anthony Fieroni]

anthonyfieroni added a comment.


  I believe that port away from boost will fix the problem

REPOSITORY
  R119 Plasma Desktop

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

To: bshah, #plasma
Cc: anthonyfieroni, davidedmundson, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

[Anthony Fieroni]

anthonyfieroni added inline comments.
Restricted Application edited projects, added KWin; removed Plasma.

INLINE COMMENTS

> blur.cpp:94
>  delete shader;
> -delete target;
> +shader = NULL;
> +

m_simpleTarget should be deleted too

> blur.cpp:96-104
> +for (int i = 0; i < renderTargets.size(); i++) {
> +delete renderTargets[i];
> +renderTargets[i] = NULL;
> +
> +renderTextures[i].discard();
> +}
> +

Make a function that clears them.

> blur.cpp:117
>  
> -void BlurEffect::updateTexture() {
> -delete target;
> -// Offscreen texture that's used as the target for the horizontal blur 
> pass
> -// and the source for the vertical pass.
> -tex = GLTexture(GL_RGBA8, effects->virtualScreenSize());
> -tex.setFilter(GL_LINEAR);
> -tex.setWrapMode(GL_CLAMP_TO_EDGE);
> +bool BlurEffect::renderTargetsValid()
> +{

Make it const.

> blur.cpp:119-129
> +if (renderTargets.isEmpty()) {
> +return false;
> +}
> +
> +for (int i = 0; i < renderTargets.size(); i++) {
> +if (!renderTargets[i]->valid()) {
> +return false;

You can minimize it

  return !renderTargets.isEmpty() 
&& std::find_if(renderTargets.cbegin(), renderTargets.cend(), [](const 
GLRenderTarget *target) {
 return !target->valid(); }) == 
renderTarget.cend();

> blur.cpp:180-188
> +for (int i = 0; i < renderTargets.size(); i++) {
> +delete renderTargets[i];
> +renderTargets[i] = NULL;
> +
> +renderTextures[i].discard();
> +}
> +

Use same function from above.

> blur.cpp:670-724
> +shader->bind(BlurShader::copySampleType);
> +
> +shader->setModelViewProjectionMatrix(screenProjection);
> +shader->setTextureSize(screen.size());
> +shader->setBlurRect(shape.boundingRect().adjusted(BORDER_SIZE, 
> BORDER_SIZE, -BORDER_SIZE, -BORDER_SIZE), screen.size());
> +
> +GLRenderTarget::pushRenderTarget(renderTargets[0]);

Make a 3 function copySampleType, downSampleType, upSampleType with needed 
parameters. Call them here and in doCachedBlur

> blur.h:83
>  QRegion blurRegion(const EffectWindow *w) const;
> -bool shouldBlur(const EffectWindow *w, int mask, const WindowPaintData 
> ) const;
> +bool shouldBlur(const EffectWindow *w, int mask, const WindowPaintData 
> );
>  void updateBlurRegion(EffectWindow *w) const;

Still can be const.

> blurshader.cpp:232-239
>  if (gles) {
>  if (core) {
> -stream << "#version 300 es\n\n";
> +streamVert << "#version 300 es\n\n";
>  }
> -stream << "precision highp float;\n";
> +
> +streamVert << "precision highp float;\n";
>  } else if (glsl_140) {

QString glHeaderString;
  if (gles) {
  if (core) {
  glHeaderString += "#version 300 es\n\n";
  }
  glHeaderString += "precision highp float;\n";
  } else if (glsl_140) {
  glHeaderString += "#version 140\n\n";
  }
  streamVert << glHeaderString;

> blurshader.cpp:256-264
> +if (gles) {
> +if (core) {
> +streamFragDown << "#version 300 es\n\n";
> +}
> +
> +streamFragDown << "precision highp float;\n";
> +} else if (glsl_140) {

streamFragDown << glHeaderString;

> blurshader.cpp:266-271
> +streamFragDown << "uniform sampler2D texUnit;\n";
> +streamFragDown << "uniform float offset;\n";
> +streamFragDown << "uniform vec2 textureSize;\n";
> +
> +if (core)
> +streamFragDown << "out vec4 fragColor;\n\n";

QString glUniformString = "uniform sampler2D texUnit;\n"
"uniform float offset;\n"
"uniform vec2 textureSize;\n";
  if (core) {
  glUniformString += "out vec4 fragColor;\n\n";
  }
  streamFragDown << glUniformString;

> blurshader.cpp:293-308
>  if (gles) {
>  if (core) {
> -stream2 << "#version 300 es\n\n";
> +streamFragUp << "#version 300 es\n\n";
>  }
> -stream2 << "precision highp float;\n";
> +
> +streamFragUp << "precision highp float;\n";
>  } else if (glsl_140) {

streamFragUp << glHeaderString << glUniformString;

> blurshader.cpp:333-348
> +if (gles) {
> +if (core) {
> +streamFragCopy << "#version 300 es\n\n";
> +}
> +
> +streamFragCopy << "precision highp float;\n";
> +} else if (glsl_140) {

streamFragCopy << glHeaderString << glUniformString;

REPOSITORY
  R108 KWin

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

To: anemeth, #plasma, #kwin
Cc: anthonyfieroni, mart, davidedmundson, fredrik, ngraham, plasma-devel, kwin, 
#kwin, iodelay, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, 
jensreuterberg, abetts, sebas, apol

D10304: [BUG 358930] Moving the mouse over a different application icon immediately changes the window list

[Anthony Fieroni]

anthonyfieroni added a comment.


  You should increase minimal required plasma-framework version to 5.44 but 
when you do this this patch goes to master i.e. 5.13.

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

To: lshoravi, #plasma, #plasma_accessibility
Cc: anthonyfieroni, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10297: Add new "Tools" button above System Monitor's process list

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> ksysguardprocesslist.cpp:199
>  KSysGuardProcessListPrivate(KSysGuardProcessList* q, const QString 
> )
> -: mModel(q, hostName), mFilterModel(q), mUi(new 
> Ui::ProcessWidget()), mProcessContextMenu(nullptr), mUpdateTimer(nullptr)
> +: mModel(q, hostName), mFilterModel(q), mUi(new 
> Ui::ProcessWidget()), mProcessContextMenu(nullptr), mUpdateTimer(nullptr), 
> mToolsMenu(new QMenu())
>  {

new QMenu(q) or delete in destructor.

REPOSITORY
  R111 KSysguard Library

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

To: gregormi, #plasma
Cc: anthonyfieroni, andreaska, rkflx, ngraham, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10429: Disable the title bar separator by default

[Anthony Fieroni]

anthonyfieroni added a comment.


  -1

REPOSITORY
  R31 Breeze

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

To: fvogt, #vdg, #plasma, ngraham
Cc: anthonyfieroni, januz, rikmills, anemeth, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D8267: Correcting symlinks between col- and row-resize and split_v and _hBug 384864

[Anthony Fieroni]

anthonyfieroni added a reviewer: apol.

REPOSITORY
  R113 Oxygen Theme

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

To: ostroffjh, #plasma, apol
Cc: apol, asturmlechner, cfeck, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, mart

D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> main.qml:42
>  property bool vertical: plasmoid.formFactor == PlasmaCore.Types.Vertical
> -property int itemSize: units.roundToIconSize(Math.min(Math.min(width, 
> height), units.iconSizes[iconSizes[plasmoid.configuration.iconSize]]))
>  property int hiddenItemSize: units.iconSizes.smallMedium

I see it has configuration for icon size ? What about to use it?

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces, davidedmundson
Cc: anthonyfieroni, ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10610: Fix favicons in firefox bookmarks runner

[Anthony Fieroni]

anthonyfieroni added reviewers: Plasma, broulik.

REPOSITORY
  R120 Plasma Workspace

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

To: z3ntu, #plasma, broulik
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D10512: try to preload certain applets in a smart way

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> mart wrote in appletquickitem.cpp:57-68
> > So Adaptive can a default even environment variable is not setted?
> 
> yes, adaptive is the default
> 
> > Furthermore you can make a static function to access value
> 
> it's done anyways only once as preloadPolicy is already a static?

But if PLASMA_PRELOAD_POLICY is not setted s_preloadPolicy stays Uninitialized, 
no? But since you check against >= Adaptive it's still ok :)

REPOSITORY
  R242 Plasma Framework (Library)

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

To: mart, #plasma
Cc: anthonyfieroni, davidedmundson, broulik, apol, ngraham, plasma-devel, 
#frameworks, michaelh, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, mart

D10019: Port minimize all applet from it's own plugin to org.kde.TaskManager

[Anthony Fieroni]

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


  It looks good to me.

REPOSITORY
  R114 Plasma Addons

BRANCH
  master

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

To: davidedmundson, #plasma, anthonyfieroni
Cc: ngraham, anthonyfieroni, sebas, broulik, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart

D10750: wayland: Add support for zwp_linux_dmabuf

[Anthony Fieroni]

anthonyfieroni added inline comments.
Restricted Application edited projects, added Plasma; removed KWin.

INLINE COMMENTS

> abstract_egl_backend.cpp:117
> +for (auto *dmabuf : qAsConst(m_dmabufBuffers)) {
> +dmabuf->destroyImage();
> +}

Call delete dmabuf will have same effect.

> abstract_egl_backend.cpp:738
> +
> +void EglDmabufBuffer::destroyImage()
> +{

This function is useless.

REPOSITORY
  R108 KWin

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

To: fredrik, #kwin, #plasma, davidedmundson, mart, graesslin
Cc: anthonyfieroni, plasma-devel, kwin, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart, iodelay, bwowk, hardening

D10512: try to preload certain applets in a smart way

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> appletquickitem.cpp:57-68
> +if (s_preloadPolicy == Uninitialized) {
> +if (qEnvironmentVariableIsSet("PLASMA_PRELOAD_POLICY")) {
> +const QString policy = 
> QString::fromUtf8(qgetenv("PLASMA_PRELOAD_POLICY")).toLower();
> +if (policy == QStringLiteral("aggressive")) {
> +s_preloadPolicy = Aggressive;
> +} else if (policy == QStringLiteral("none")) {
> +s_preloadPolicy = None;

So Adaptive can a default even environment variable is not setted?
Furthermore you can make a static function to access value

  AppletQuickItemPrivate::PreloadPolicy AppletQuickItemPrivate::appletPolicy()
  {
  static const PreloadPolicy preloadPolicy = []() -> PreloadPolicy {
  if (qEnvironmentVariableIsSet("PLASMA_PRELOAD_POLICY")) {
  const QString policy = 
QString::fromUtf8(qgetenv("PLASMA_PRELOAD_POLICY")).toLower();
  if (policy == QStringLiteral("aggressive")) {
  return Aggressive;
  } else if (policy == QStringLiteral("none")) {
  return None;
  }
  }
  return Adaptive;
  }();
  return preloadPolicy;
  }

REPOSITORY
  R242 Plasma Framework (Library)

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

To: mart, #plasma
Cc: anthonyfieroni, davidedmundson, broulik, apol, ngraham, plasma-devel, 
#frameworks, michaelh, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, mart

D10835: [kicker] Never execute recent documents

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> recentusagemodel.cpp:286
>  if (!resource.startsWith(QLatin1String("applications:"))) {
> -new KRun(docData(resource, Kicker::UrlRole).toUrl(), 0);
> +QUrl resourceUrl = docData(resource, Kicker::UrlRole).toUrl();
> +

const

> recentusagemodel.cpp:288
> +
> +QList urlsList;
> +urlsList.append(resourceUrl);

const QList urlsList{ resourceUrl };

> recentusagemodel.cpp:294
> +KService::Ptr service = 
> KMimeTypeTrader::self()->preferredService(mime.name());
> +if (service != nullptr) {
> +KRun::runApplication(*service, urlsList, nullptr);

if (service) {

REPOSITORY
  R119 Plasma Desktop

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

To: aleksejshilin, #plasma
Cc: anthonyfieroni, broulik, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D9473: Revert "[widgetexplorer] Make sure KNS download dialog is destroyed"

[Anthony Fieroni]

anthonyfieroni added a comment.


  Deletion is needed
  
https://phabricator.kde.org/source/knewstuff/browse/master/src/downloaddialog.cpp;6de0754aa9258558a7203afc366b49984f0192eb$112
  
https://phabricator.kde.org/source/knewstuff/browse/master/src/downloaddialog.cpp;6de0754aa9258558a7203afc366b49984f0192eb$50
  
https://phabricator.kde.org/source/knewstuff/browse/master/src/downloadwidget.cpp;6de0754aa9258558a7203afc366b49984f0192eb$69
  
https://phabricator.kde.org/source/knewstuff/browse/master/src/downloadwidget.cpp;6de0754aa9258558a7203afc366b49984f0192eb$121
  
https://phabricator.kde.org/source/knewstuff/browse/master/src/core/engine.cpp;6de0754aa9258558a7203afc366b49984f0192eb$107
  Save this cache to disk, otherwise not saving anything since dialog just leak.

REPOSITORY
  R120 Plasma Workspace

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

To: fvogt, #plasma, davidedmundson, anthonyfieroni
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9473: Revert "[widgetexplorer] Make sure KNS download dialog is destroyed"

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in widgetexplorer.cpp:448
> Such a slot not existing at all.

Even more WidgetExplorer is closed upon DownloadDialog is shown, which makes 
such slot useless. But now it can be opened more than one DownloadDialogs and 
it looks strange.

REPOSITORY
  R120 Plasma Workspace

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

To: fvogt, #plasma, davidedmundson, anthonyfieroni
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9338: [comic] Fix comic updater time interval

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 24295.
anthonyfieroni marked 2 inline comments as done.
anthonyfieroni edited the test plan for this revision.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9338?vs=23936=24295

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

AFFECTED FILES
  applets/comic/comicupdater.cpp

To: anthonyfieroni, davidedmundson, broulik, #plasma, mart
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9338: [comic] Fix comic updater time interval

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> davidedmundson wrote in comicupdater.cpp:86
> I assume this line is the important change?
> 
> +1 to this bit.

Yes.

REPOSITORY
  R114 Plasma Addons

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

To: anthonyfieroni, davidedmundson, broulik, #plasma, mart
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9479: [widgetexplorer] Fix trying to show deleted dialog

[Anthony Fieroni]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:acd7034952bc: [widgetexplorer] Fix trying to show deleted 
dialog (authored by anthonyfieroni).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9479?vs=24297=24298

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

AFFECTED FILES
  components/shellprivate/widgetexplorer/widgetexplorer.cpp

To: anthonyfieroni, fvogt, davidedmundson, #plasma
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9473: Revert "[widgetexplorer] Make sure KNS download dialog is destroyed"

[Anthony Fieroni]

anthonyfieroni added a comment.


  Yes it is. Reverting it you will not fix anything. Please explain 
step-by-step what not working.

REPOSITORY
  R120 Plasma Workspace

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

To: fvogt, #plasma, davidedmundson, anthonyfieroni
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9473: Revert "[widgetexplorer] Make sure KNS download dialog is destroyed"

[Anthony Fieroni]

anthonyfieroni added a comment.


  In https://phabricator.kde.org/D9473#182455, @fvogt wrote:
  
  > In https://phabricator.kde.org/D9473#182449, @anthonyfieroni wrote:
  >
  > > Yes it is. Reverting it you will not fix anything. Please explain 
step-by-step what not working.
  >
  >
  > "Download new widgets" -> Either plasmashell crashes instantly or the 
dialog does not show up and plasmashell crashes on the next click.
  
  
  I'll investigate.

REPOSITORY
  R120 Plasma Workspace

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

To: fvogt, #plasma, davidedmundson, anthonyfieroni
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9479: [widgetexplorer] Fix trying to show deleted dialog

[Anthony Fieroni]

anthonyfieroni created this revision.
anthonyfieroni added reviewers: fvogt, davidedmundson, Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  WidgetExplorer is destroyed on close which deletes KNS dialog.

TEST PLAN
  Manually test, verify it.

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  components/shellprivate/widgetexplorer/widgetexplorer.cpp

To: anthonyfieroni, fvogt, davidedmundson, #plasma
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9338: [comic] Fix comic updater time interval

[Anthony Fieroni]

This revision was automatically updated to reflect the committed changes.
Closed by commit R114:095196ca6438: [comic] Fix update interval (authored by 
anthonyfieroni).

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9338?vs=24295=24303

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

AFFECTED FILES
  applets/comic/comicupdater.cpp

To: anthonyfieroni, davidedmundson, broulik, #plasma, mart
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9479: [widgetexplorer] Fix trying to show deleted dialog

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 24297.
anthonyfieroni added a comment.


  Remove entire destructor.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9479?vs=24296=24297

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

AFFECTED FILES
  components/shellprivate/widgetexplorer/widgetexplorer.cpp

To: anthonyfieroni, fvogt, davidedmundson, #plasma
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9473: Revert "[widgetexplorer] Make sure KNS download dialog is destroyed"

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> davidedmundson wrote in widgetexplorer.cpp:448
> what's the situation with this line?

Such a slot not existing at all.

REPOSITORY
  R120 Plasma Workspace

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

To: fvogt, #plasma, davidedmundson, anthonyfieroni
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D13529: [minimizeAll] Do not try to restore window state on model change

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 36300.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13529?vs=36270=36300

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

AFFECTED FILES
  applets/minimizeall/package/contents/ui/main.qml

To: anthonyfieroni, davidedmundson, #plasma
Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D4667: [applet] Let specify a version for applets private plugins

[Anthony Fieroni]

anthonyfieroni abandoned this revision.
anthonyfieroni added a comment.


  Let's abandon it, i'll return to it, if still needed.

REPOSITORY
  R119 Plasma Desktop

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

To: anthonyfieroni, #plasma, davidedmundson, sitter, dfaure
Cc: hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D14546: [Service Runner] Look up relative entryPaths

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> servicerunner.cpp:474
> +
> +if (path.isEmpty()) {
> +return nullptr;

Does it better to check that file exists?

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, hein, dfaure
Cc: anthonyfieroni, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14576: [Desktop Scripting ConfigGroup] Add more nullptr checks

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> configgroup.cpp:67
>  d->synchTimer->stop();
>  d->configGroup->sync();
>  }

Here as well should be sync(), if it destroys object before timer is elapsed 
e.g. before 1.5 sec it will crash.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: anthonyfieroni, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

[Anthony Fieroni]

anthonyfieroni added a comment.


  +1 from me.

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

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik

D14437: Fix QFileDialog not remembering the last visited directory.

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> apol wrote in kdeplatformfiledialoghelper.cpp:118
> I'm not sure I understand the comment. Is it like a TODO?

It works without comment, i think these connections are not needed.

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: dfaure, anthonyfieroni, elvisangelaccio, plasma-devel, broulik, arichardson
Cc: apol, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, mart

D14822: Use QJSValue as method parameter type for the scripting interface

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> applet.cpp:139
>  
> -d->configGroup.writeEntry(key, value);
> +d->configGroup.writeEntry(key, value.toVariant());
>  d->configDirty = true;

So writeEntry gets QJSValue& now why you call again toVariant(), here 
everywhere else.

REPOSITORY
  R120 Plasma Workspace

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

To: fvogt, #plasma
Cc: anthonyfieroni, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14895: Plasmashell freezes when trying to get free space info from mounted remote filesystem after losing connection to it

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> soliddeviceengine.cpp:67
> +KDiskFreeSpaceInfo info = KDiskFreeSpaceInfo::freeSpaceInfo(m_path);
> +emit infoObtained(m_path, m_udi, info.isValid(), info.available(), 
> info.size());
> +}

Don't you forget to emit finished?

> soliddeviceengine.cpp:591-600
> +connect(thread, SIGNAL(started()), worker, SLOT(getFreeSpaceInfo()));
> +connect(worker, SIGNAL(finished()), thread, SLOT(quit()));
> +
> +// delete later thread and worker
> +connect(worker, SIGNAL(finished()), worker, SLOT(deleteLater()));
> +connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater()));
> +

Use new connect syntax .

> soliddeviceengine.cpp:622
> +
> +m_timers[path]->deleteLater();
> +m_timers.remove(path);

It does not need, you connect finished to deleteLater.

REPOSITORY
  R120 Plasma Workspace

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

To: McPain, broulik, ngraham
Cc: anthonyfieroni, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14895: Plasmashell freezes when trying to get free space info from mounted remote filesystem after losing connection to it

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> soliddeviceengine.cpp:564
> +
> +m_timers[path] = timer;
> +

You can remove it, see below.

> soliddeviceengine.cpp:574
> +connect(job, ::FileSystemFreeSpaceJob::result,
> +[this, path, udi](KIO::Job *job, KIO::filesize_t size, 
> KIO::filesize_t available) {
> +if (m_timers[path]->isActive())

Capture timer too

> soliddeviceengine.cpp:575-576
> +[this, path, udi](KIO::Job *job, KIO::filesize_t size, 
> KIO::filesize_t available) {
> +if (m_timers[path]->isActive())
> +m_timers[path]->stop();
> +

You can omit declaration of map, just

  timer->stop();

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

To: McPain, broulik, ngraham, davidedmundson
Cc: anthonyfieroni, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14895: Plasmashell freezes when trying to get free space info from mounted remote filesystem after losing connection to it

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> soliddeviceengine.cpp:588
> +// start timer: after 15 seconds we will get an error
> +timer->start(15000);
>  }

job->start();

as well.

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

To: McPain, broulik, ngraham, davidedmundson
Cc: anthonyfieroni, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14855: Add applet with screen layouts and presentation mode

[Anthony Fieroni]

anthonyfieroni added a comment.


  @broulik, i'm pretty sure that it has a review that enable accessing OSD by 
mouse, can you investigate and ship it. I'm not against this but accessing OSD 
by mouse is pretty good feature after all.

REPOSITORY
  R104 KScreen

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

To: broulik, #plasma, #vdg, fischbach, harmathy
Cc: anthonyfieroni, ngraham, mart, gladhorn, abetts, plasma-devel, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D14895: Plasmashell freezes when trying to get free space info from mounted remote filesystem after losing connection to it

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> McPain wrote in soliddeviceengine.cpp:560
> Any ideas how to do it correctly?

It has a default parameters just call event with 3 parameters not 5.

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

To: McPain, broulik, ngraham, davidedmundson
Cc: anthonyfieroni, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14437: Fix QFileDialog not remembering the last visited directory.

[Anthony Fieroni]

anthonyfieroni added a comment.


  +1

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: dfaure, anthonyfieroni, elvisangelaccio, plasma-devel, broulik, arichardson
Cc: apol, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, mart

D15093: Add WireGuard capability.

[Anthony Fieroni]

anthonyfieroni added a comment.


  When you use background-color you combine with QPalette, at least, because 
you break all other themes.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: andersonbruce, #plasma, jgrulich, pino
Cc: anthonyfieroni, pino, lbeltrame, ngraham, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15011: [Kickoff] Make the search field always look like a search field

[Anthony Fieroni]

anthonyfieroni added a comment.


  I'm -1 for this, it's not needed to click in just start typing witch is more 
intuitive.

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #plasma, #vdg, davidedmundson
Cc: anthonyfieroni, filipf, abetts, davidedmundson, michaeltunnell, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D14050: Fwupd Backend For Review and Improvement

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> CMakeLists.txt:16
> +
> +#find_package(GIO)
> +find_package(Soup)

Why you comment it when you use it, see below.

> FwupdBackend.cpp:68
> +if (!m_fetching)
> +   m_reviews->initialize();
> +populate(QStringLiteral("Releases"));

Indentations should be 4 spaces, if statement goes with { on same line and } 
even on one line  body.

> FwupdBackend.cpp:85
> +g_strdelimit (tmp, "/", '_');
> +return g_strdup_printf ("org.fwupd.%s.device", tmp);
> +}

It's memory leak.

> FwupdSourcesBackend.cpp:75-77
> +else if(value.toInt() == Qt::Unchecked)
> +{
> +if((value.toInt() == Qt::Checked) )

?

REPOSITORY
  R134 Discover Software Store

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

To: abhijeet2096, apol, davidedmundson
Cc: anthonyfieroni, plasma-devel, ragreen, ixoos, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14060: Clean up string casts

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> parser.cpp:145
> +const QByteArray type = 
> map[QStringLiteral("type")].toByteArray().toUpper();
>  if (type.contains("LVDS") || type.contains("EDP") || 
> type.contains("IDP") || type.contains("7")) {
>  output->setType(Output::Panel);

It should be QLatin1String on contains?

REPOSITORY
  R110 KScreen Library

BRANCH
  master

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

To: gladhorn, #plasma, davidedmundson
Cc: anthonyfieroni, broulik, davidedmundson, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10019: Port minimize all applet from it's own plugin to org.kde.TaskManager

[Anthony Fieroni]

anthonyfieroni added a comment.


  Thats what doubt me:
  
https://phabricator.kde.org/source/kdeplasma-addons/browse/master/applets/minimizeall/plugin/minimizeall.cpp;65fc80441388c5f3e70832c40e1d7afea6778e83$63-68

REPOSITORY
  R114 Plasma Addons

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

To: davidedmundson, #plasma
Cc: anthonyfieroni, sebas, broulik, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart

D11233: Ordered system tray

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> ConfigEntries.qml:464
> +  (shownAutoItems[0].taskId != modelData.taskId))
> +text: "▲"
> +width: 30

icon: "arrow-up"

> ConfigEntries.qml:478
> +   != modelData.taskId))
> +text: "▼"
> +width: 30

icon: "arrow-down"

> systemtray.cpp:589
> +for (auto  : itemIdTrimmingRegExpRules) {
> +result = itemId.remove(QRegExp(regExp));
> +if (!QString::compare(result, itemId)) {

Use QRegularExpression instead

REPOSITORY
  R120 Plasma Workspace

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

To: wsdfhjxc, #plasma_workspaces, #plasma, #vdg
Cc: anthonyfieroni, Pitel, alexeymin, plasma-devel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10835: [kicker] Never execute recent documents

[Anthony Fieroni]

anthonyfieroni added a reviewer: hein.

REPOSITORY
  R119 Plasma Desktop

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

To: aleksejshilin, #plasma, hein
Cc: ngraham, hein, broulik, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11292: [RFC] Ordered system tray v2

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> ConfigEntries.qml:110-119
> +  /*  for (var i = 0; i < statusNotifierModel_cfg.count; ++i) {
> +var item = statusNotifierModel.get(i);
> +itemInfo[item.Id] = {
>  "taskId": item.Id,
>  "name": item.Title,
>  "iconName": item.IconName,
> +"icon": item.Icon,

Commented code is not allowed to get in, you should decide it's needed or not.

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel
Cc: anthonyfieroni, wsdfhjxc, ngraham, plasma-devel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D12240: Save the dialog view settings when closing the dialog without hitting the cancel button

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> kdeplatformfiledialoghelper.cpp:108
>  m_buttons->addButton(m_fileWidget->cancelButton(), 
> QDialogButtonBox::RejectRole);
>  connect(m_buttons, SIGNAL(rejected()), m_fileWidget, SLOT(slotCancel()));
>  connect(m_fileWidget->okButton(), SIGNAL(clicked(bool)), m_fileWidget, 
> SLOT(slotOk()));

Even better to me should be

  connect(this, ::rejected, m_fileWidget, 
::slotCancel);

PS: You can make a different review to change old syntax (SIGNAL/SLOT) to new 
function pointers.

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

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

D12240: Save the dialog view settings when closing the dialog without hitting the cancel button

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> kdeplatformfiledialoghelper.cpp:243
> +{
> +m_fileWidget->writeViewConfig();
> +QDialog::reject();

You can see at line 108 on reject is called slotCancel, so for convenience just 
call it

  m_fileWidget->slotCancel();

Then you can discard D12239 

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

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

D11571: [Battery Monitor] Support "Gaming Input Battery type"

[Anthony Fieroni]

anthonyfieroni added a comment.


  @broulik, does it makes sens, when batteries isn't available (as your 
screenshot) to be shown, the icon of next device in the list, in systray ?

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, dollinger, lbeltrame
Cc: anthonyfieroni, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11590: Add mutex round static member used in a thread

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> backgroundlistmodel.cpp:532
>  
> -m_suffixes = suffixes.toList();
> +s_suffixes = suffixes.toList();
>  }

Potentially it still can produce a crash, the function returns a reference to 
list, simultaneously read/write on list is race condition. So function can 
return a copy to static list for this function, then global variables or 
mutexes doesn't needed.

REPOSITORY
  R120 Plasma Workspace

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

To: davidedmundson, #plasma
Cc: anthonyfieroni, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10890: Don't skip certain recent documents in kicker and taskmanager

[Anthony Fieroni]

anthonyfieroni added a comment.


  It looks good to me

REPOSITORY
  R119 Plasma Desktop

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

To: aleksejshilin, #plasma, broulik, hein
Cc: anthonyfieroni, ngraham, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10890: Prepare for KIO KRecentDocument fix (D10863)

[Anthony Fieroni]

anthonyfieroni added a comment.


  So why you called hack, why no just add conditional compilation against 5.44

REPOSITORY
  R119 Plasma Desktop

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

To: aleksejshilin, #plasma, broulik, hein
Cc: anthonyfieroni, ngraham, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni added a comment.


  Someone?

REPOSITORY
  R114 Plasma Addons

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

To: anthonyfieroni, davidedmundson, #plasma, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16231: [ksmserver] Rewrite Startup

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> startup.cpp:194
> +connect(phase1, ::finished, this, [=]() {
> +ksmserver->setupShortcuts(); // done only here, because it needs 
> kglobalaccel :-/
> +});

Can we delay it to finishStartup?

> startup.cpp:210
>  
> -void Startup::autoStart0()
> +void Startup::upAndRunning( const QString& msg )
>  {

It's unused, can we remove it?

> startup.h:39
> +void start() override;
> +private:
>  };

Remove ?

REPOSITORY
  R120 Plasma Workspace

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

To: davidedmundson, #plasma
Cc: anthonyfieroni, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni added a comment.


  In D15616#341840 , @davidedmundson 
wrote:
  
  > The DataEngine also has a timer for timeouts, so we now have two timeout 
timers. 
  >  Having two timeout timers doesn't seem right.
  
  
  Yep, i'm on same opinion, it looks like i some cases timer in DataEngine is 
not started. One possible case what i provide in 
ComicProvider::requestRedirectedUrl, i don't see other cases where dataUpdated 
isn't called. mRestartEngine timer is in "any case".

REPOSITORY
  R114 Plasma Addons

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

To: anthonyfieroni, davidedmundson, #plasma, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 43461.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15616?vs=42286=43461

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

AFFECTED FILES
  dataengines/comic/comicprovider.cpp

To: anthonyfieroni, davidedmundson, #plasma, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

This revision was automatically updated to reflect the committed changes.
Closed by commit R114:7c7c00798048: [comicprovider] Restart timer on redirect 
request (authored by anthonyfieroni).

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15616?vs=43461=44158

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

AFFECTED FILES
  dataengines/comic/comicprovider.cpp

To: anthonyfieroni, davidedmundson, #plasma, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni added a comment.


  @davidedmundson to commit or you have other objections?

REPOSITORY
  R114 Plasma Addons

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

To: anthonyfieroni, davidedmundson, #plasma, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16715: make global menu minimize state aware

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> appmenumodel.cpp:245-248
> +if (info.isMinimized()) {
> +setMenuIsHidden(true);
> +} else {
> +setMenuIsHidden(false);

setMenuHidden(info.isMinimized());

> appmenumodel.h:42
>  Q_PROPERTY(bool menuAvailable READ menuAvailable WRITE setMenuAvailable 
> NOTIFY menuAvailableChanged)
> +Q_PROPERTY(bool menuIsHidden READ menuIsHidden NOTIFY 
> menuIsHiddenChanged)
>  

Naming should be menuHidden, setMenuHidden if we follow rules.

REPOSITORY
  R120 Plasma Workspace

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

To: mvourlakos, #plasma, broulik
Cc: anthonyfieroni, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16842: [clipboard] Limit length of clipboard text item visualisation

[Anthony Fieroni]

This revision was automatically updated to reflect the committed changes.
anthonyfieroni marked an inline comment as done.
Closed by commit R120:d53622d7d9e0: [clipboard] Limit length of clipboard text 
item visualisation (authored by anthonyfieroni).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D16842?vs=45367=45395#toc

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16842?vs=45367=45395

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

AFFECTED FILES
  applets/clipboard/contents/ui/TextItemDelegate.qml

To: anthonyfieroni, #plasma, broulik, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16856: [comicproviderwrapper] Always finish on last request in pageRetrieved

[Anthony Fieroni]

anthonyfieroni created this revision.
anthonyfieroni added reviewers: Plasma, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
anthonyfieroni requested review of this revision.

REVISION SUMMARY
  Otherwise provider can be marked as failed

REPOSITORY
  R114 Plasma Addons

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

AFFECTED FILES
  dataengines/comic/comicproviderwrapper.cpp

To: anthonyfieroni, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16890: [systemtray] Keep config entries after modifying

[Anthony Fieroni]

anthonyfieroni added a comment.


  Weird. If configuration window stays open, clicks in systray work correct, 
when it's dismissed clicks are incorrect.

REPOSITORY
  R120 Plasma Workspace

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

To: anthonyfieroni, #plasma, broulik, davidedmundson
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16918: [brightnesscontrol] Do not change screen brightness on invalid values

[Anthony Fieroni]

anthonyfieroni created this revision.
anthonyfieroni added reviewers: Plasma, broulik.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
anthonyfieroni requested review of this revision.

REVISION SUMMARY
  The problem:
  
  1. Unplug laptop power supply
  2. On screen energy saving in settings
  3. Uncheck screen brightness (on all profiles or on that will be loaded)
  4. Wait minutes display to do off (not dimmed)
  5. Plug power supply without display on
  6. When it's on, screen brightness is 100%
  
  Probably it's depends of backend (intel in my case) on dpms it returns 
*garbage* values even on max brightness (it's still not tested)
  
  The solution:
  Trying to cache max brightness level and denied to change screen brightness 
on garbage values

TEST PLAN
  @broulik if you have better solution, i'll test it

REPOSITORY
  R122 Powerdevil

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

AFFECTED FILES
  daemon/actions/bundled/brightnesscontrol.cpp
  daemon/actions/bundled/brightnesscontrol.h

To: anthonyfieroni, #plasma, broulik
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16856: [comic] Sync configuration on write

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 45556.
anthonyfieroni added a comment.


  Better not change data engine

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16856?vs=45396=45556

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

AFFECTED FILES
  applets/comic/comic.cpp
  applets/comic/comicupdater.cpp

To: anthonyfieroni, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16873: [comic] Request current identifier on date changed

[Anthony Fieroni]

anthonyfieroni created this revision.
anthonyfieroni added reviewers: Plasma, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
anthonyfieroni requested review of this revision.

REVISION SUMMARY
  The problem is when date is changed we connect to source ex. 
dilbert:2018-11-14 but in other hand script (main.es) makes
  
  if (comic.identifier.toString() == date.currentDate().toString()) {
  
comic.requestPage("http://dilbert.com/;, comic.User, infos);
  
  so provider do its job but dataengine does not have a data for us, cause 
identifier is dilbert: and we are not connected to, so dataUpdated is not 
called which cause busy indicator to eats cpu power. Another fix can be to have 
permanently connection to comic_name: to enter dataUpdated in any case.

REPOSITORY
  R114 Plasma Addons

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

AFFECTED FILES
  applets/comic/comic.cpp

To: anthonyfieroni, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16890: [systemtray] Keep config entries after modifying

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 45502.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16890?vs=45500=45502

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

AFFECTED FILES
  applets/systemtray/package/contents/ui/main.qml

To: anthonyfieroni, #plasma, broulik, davidedmundson
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16890: [systemtray] Keep config entries after modifying

[Anthony Fieroni]

anthonyfieroni created this revision.
anthonyfieroni added reviewers: Plasma, broulik, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
anthonyfieroni requested review of this revision.

REVISION SUMMARY
  It still has one problem, after configuration is changed in Entries MouseArea 
index is invalid, i can't understand why, @broulik ?
  
  BUG: 390890

TEST PLAN
  1. Right button on systemtray
  2. Check / uncheck extra imtem
  3. Click on Entries
  4. Show / hidden some of them
  5. Return to General
  
  again 2
  again 4
  
  All good, but after close configuration dialog clicking on item in systray is 
an invalid index.

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  applets/systemtray/package/contents/ui/ConfigEntries.qml
  applets/systemtray/package/contents/ui/main.qml

To: anthonyfieroni, #plasma, broulik, davidedmundson
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16842: [clipboard] Limit length of clipboard text item visualisation

[Anthony Fieroni]

anthonyfieroni created this revision.
anthonyfieroni added reviewers: Plasma, broulik, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
anthonyfieroni requested review of this revision.

REVISION SUMMARY
  Applying regex to long text item (> 5000 symbols) is extremely slow and 
memory consuming.

TEST PLAN
  Make a config over 16 records in Clipboard, copy some json string or whatever 
you have as long texts > 5000 symbols. I hat slow plasmashell, high cpu usage, 
slow running and over 700 MB of RAM.

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  applets/clipboard/contents/ui/TextItemDelegate.qml

To: anthonyfieroni, #plasma, broulik, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16844: [comic] Correct usage of DataEngineConsumer

[Anthony Fieroni]

anthonyfieroni created this revision.
anthonyfieroni added reviewers: Plasma, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
anthonyfieroni requested review of this revision.

REVISION SUMMARY
  Comic applet is DataEngineConsumer which take care of loading / unloading 
engine.

REPOSITORY
  R114 Plasma Addons

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

AFFECTED FILES
  applets/comic/comic.cpp
  dataengines/comic/comicproviderwrapper.cpp

To: anthonyfieroni, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16844: [comic] Correct usage of DataEngineConsumer

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 45370.
anthonyfieroni added a comment.


  Fix diff

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16844?vs=45369=45370

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

AFFECTED FILES
  applets/comic/comic.cpp

To: anthonyfieroni, #plasma, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15840: [Folder View] Remember selected item when navigating in subfolders

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> FolderView.qml:1114
>  onListingCompleted: {
>  if (!gridView.model && plasmoid.expanded) {
>  gridView.model = positioner;

Do we have grodView.model after back clicked? If we do in expanded view (that's 
when Folder View is embedded in the panel) goingBack will not be performed.

REPOSITORY
  R119 Plasma Desktop

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

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

D15875: [Media Controller] Disallow seeking beyond end of track

[Anthony Fieroni]

anthonyfieroni added a comment.


  1s file is a corner case as well as 0, cause you have always 2 positions - 
start and stop. In 0/1s these 2 steps just match, if you are write 0 isn't 
valid in 0s length file, does it?
  +1 for the patch from me.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, fvogt
Cc: anthonyfieroni, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni added a comment.


  Ping

REPOSITORY
  R114 Plasma Addons

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

To: anthonyfieroni, davidedmundson, #plasma, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15840: [Folder View] Remember selected item when navigating in subfolders

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> thsurrel wrote in FolderView.qml:1114
> My understanding was that the gridView.model could be undefined only if the 
> FolderModel was not ready. But if we navigated into a subfolder and we are 
> asking to go back, than the FolderModel had to be up and running already. 
> Could be safe to move this 'if' outside the 'else' though.

Ok, then just embed Folder View in the panel (by right click on the panel, 
panel options -> add widgets or drag'n'drop it in) then navigate to check that 
should it goback to be applied as well.

REPOSITORY
  R119 Plasma Desktop

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

To: thsurrel, #plasma, broulik, hein
Cc: anthonyfieroni, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni added a comment.


  Ping

REPOSITORY
  R114 Plasma Addons

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

To: anthonyfieroni, davidedmundson, #plasma, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 42035.
anthonyfieroni added a comment.


  Another approach: If timer expires in 2 min without result disconnect it from 
source and starts again for next 2 min, when it expires if try to update again 
by activate slotCurrentDay

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15616?vs=41977=42035

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

AFFECTED FILES
  applets/comic/comic.cpp
  applets/comic/comic.h
  dataengines/comic/comicprovider.cpp

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni added a comment.


  Ping, @davidedmundson let's make some change on this

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

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 42148.
anthonyfieroni added a comment.


  Stop indicator in a minute, restart engine in 10 sec.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15616?vs=42035=42148

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

AFFECTED FILES
  applets/comic/comic.cpp
  applets/comic/comic.h
  dataengines/comic/comicprovider.cpp

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 42286.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15616?vs=42148=42286

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

AFFECTED FILES
  applets/comic/comic.cpp
  applets/comic/comic.h
  dataengines/comic/comicprovider.cpp

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D13745: Implement support for virtual desktops on Wayland

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> abstracttasksproxymodeliface.cpp:216
> +
> +const QModelIndex  = mapIfaceToSource(index);
> +const AbstractTasksModelIface *m = dynamic_cast AbstractTasksModelIface *>(sourceIndex.model());

mapIfaceToSource and mapToSource returns non-const index i've wonder way it's 
get as const ref then performed a const_cast?

REPOSITORY
  R120 Plasma Workspace

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

To: hein, mart, mvourlakos
Cc: anthonyfieroni, zzag, ngraham, abetts, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni created this revision.
anthonyfieroni added reviewers: davidedmundson, Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
anthonyfieroni requested review of this revision.

REVISION SUMMARY
  This happen really rare, i've used comic applet on my desktop about years, 
but in such cases dataUpdated is not called and busy indicator *burns* cpu. So 
i provide possible fix in comic provider, data engine, when redirection is 
needed error timer is not restarted in such case error is never handle 
properly. But *every* case i made a more aggressive solution, a timer in comic 
applet, if in 3 minutes dataUpdated isn't called it just force disconnect 
engine from source and stops indicator.
  
  CCBUG: 363292

REPOSITORY
  R114 Plasma Addons

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

AFFECTED FILES
  applets/comic/comic.cpp
  applets/comic/comic.h
  dataengines/comic/comicprovider.cpp

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 41977.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15616?vs=41976=41977

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

AFFECTED FILES
  applets/comic/comic.cpp
  applets/comic/comic.h
  dataengines/comic/comicprovider.cpp

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16890: [systemtray] Keep config entries after modifying

[Anthony Fieroni]

anthonyfieroni added a comment.


  But for some reason, on second Entries show warnings says: cannot call 
createObject on null object. Looks like it is treated as local object or 
something.

REPOSITORY
  R120 Plasma Workspace

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

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

D16873: [comic] Request current identifier on date changed

[Anthony Fieroni]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R114:fd19c36ec4e4: [comic] Request current identifier on data 
change (authored by anthonyfieroni).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D16873?vs=47418=48386#toc

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16873?vs=47418=48386

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

AFFECTED FILES
  applets/comic/comic.cpp

To: anthonyfieroni, #plasma, davidedmundson, mart
Cc: plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16856: [comicupdater] Fix last day check

[Anthony Fieroni]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R114:28493251921b: [comicupdater] Fix last day check (authored 
by anthonyfieroni).

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16856?vs=47416=48385

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

AFFECTED FILES
  applets/comic/comicupdater.cpp

To: anthonyfieroni, #plasma, davidedmundson
Cc: mart, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D16918: [dimdisplay] Do not change screen brightness on invalid values

[Anthony Fieroni]

This revision was automatically updated to reflect the committed changes.
Closed by commit R122:6f350690723a: [dimdisplay] Do not change screen 
brightness on invalid values (authored by anthonyfieroni).

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16918?vs=47017=48388

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

AFFECTED FILES
  daemon/actions/bundled/dimdisplay.cpp

To: anthonyfieroni, #plasma, broulik, mart
Cc: thsurrel, broulik, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17874: [dimdisplay] Effectively waits display to wake-up

[Anthony Fieroni]

anthonyfieroni added a comment.


  Does someone test it, @thsurrel ?

REPOSITORY
  R122 Powerdevil

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

To: anthonyfieroni, mart, broulik, thsurrel, #plasma
Cc: plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart