plasma-applets icon indicating copy to clipboard operation
plasma-applets copied to clipboard

Commandoutput with Indicator and Richtext Support

Open Shihira opened this issue 7 years ago • 7 comments

  • Added indicator. The original state was called expanded mode now.
  • Indicator icon option.
  • I really need my script to run every 50 ms. Please let users take their own risks.
  • Richtext support. Links are treated as shell commands.
  • Execution pauses when panel getting folded.

Shihira avatar Jan 08 '18 20:01 Shihira

PS: How I use this indicator mode

Imgur

Shihira avatar Jan 08 '18 20:01 Shihira

Hmmm, if you hadn't already written the code, I'd have recommended the kargos widget (https://store.kde.org/p/1173112/ | https://github.com/lipido/kargos).

Anyways, lets break down the PR.

Changing the min interval to 50ms. Not sure why I thought 1sec was the minimum. I do remember PlasmaCore.DataSource having a hardcoded minimum though. We can confirm the shorter period works with:

  • date +%S.%N
  • Test in terminal with: watch -n 0.1 date +%S.%N

Oh right, now I remember why.

While DataEngine.cpp does have a hardcoded minimum of 20 times a second (50ms).

  • https://github.com/KDE/plasma-framework/blob/master/src/plasma/dataengine.cpp#L535

DataEngine's also have the ability to set a minimum, which the executable dataengine sets as 1000ms.

  • https://github.com/KDE/plasma-workspace/blob/master/dataengines/executable/executable.cpp#L70
ExecutableEngine::ExecutableEngine(QObject* parent, const QVariantList& args)
    : Plasma::DataEngine(parent, args)
{
    setMinimumPollingInterval(1000);
}

However, my assumption was wrong. That 1000ms is how often it checks to see if the process is still running. It can "update" sooner if the process finishes in just ~10ms.

We can confirm this by using date +%S.%N in the widget.

So nice catch, I should have tested that.

Zren avatar Jan 08 '18 22:01 Zren

  • You are using spaces instead of tabs. It's really obvious when you check GitHub's "Changes" diff.
  • "Expanded" mode makes sense to you and me, but isn't very obvious to the casual user, so I'd change it to "popup mode".
  • I'll have to look into why your diff changed the file permission from 100644 → 100755 even on files you didn't change.
  • id: link_runner => id: linkRunner, though actually, you don't need it at all since you're using it to open a url in the browser. Qt.openUrlExternally(link) should work.
  • units.devicePixelRatio is necessary for HiDPI screens. Not sure why you removed it from font.pixelSize: 16 * units.devicePixelRatio. Try testing with:
QT_DEVICE_PIXEL_RATIO=2 QML_DISABLE_DISK_CACHE=true plasmoidviewer -a package`
  • Hmmm, should probably version all of my ConfigSection.qml as that's definitely neither the before and after of this diff are correct. I can fix that after the merge though. If you're wondering what's wrong, it's because it should be content.data in default property alias _contentChildren: content.data.
    • .data accepts both .children and .resources
      https://doc-snapshots.qt.io/qt5-dev/qml-qtquick-item.html#data-prop
    • .children only accepts visual objects which extend Item (Rectangle/etc). If you pass something like (QtObject/Connection/``DataSource) which doesn't have a item.parent` it'll break.
    • This is what it should be like, with the "default" property aliased to the .data property, but the "bugfix" workaround looping the .children since only the visual Items have a parent property.
      https://github.com/Zren/plasma-applets/blob/master/org.kde.plasma.eventcalendar/package/contents/ui/lib/ConfigSection.qml

Zren avatar Jan 08 '18 22:01 Zren

  • Apologize for being so careless. I didn't mean to change the permission, tabs, font size and the ConfigSection.qml. It was just...being too careless.
  • 'Expanded' has a contrary meaning to 'popup' I am afraid. Clearly I didn't choose a good word, bringing about huge misunderstanding. See, I am not from the English world, so...
  • I introduced linkRunner because I wanted to run some shell commands by clicking links (like launching ksysguard).

Shihira avatar Jan 09 '18 05:01 Shihira

I introduced linkRunner because I wanted to run some shell commands by clicking links (like launching ksysguard).

How are you formatting the "links"? <a href="ksysguard">KSysGuard</a> ?

Zren avatar Jan 09 '18 06:01 Zren

Yes. Is that weird?

Shihira avatar Jan 09 '18 06:01 Shihira

It's a hackish way to do it, but that's fine I guess. I'll probably be adding the following after the merge:

onLinkActivated: {
    var urlStartRegex = /^\w+:\/\//
    if (urlStartRegex.match(link)) {
        Qt.openUrlExternally(link)
    } else { // Assume it's a command
        linkRunner.exec(link)
    }
}

So that you don't need to prefix urls with xdg-open like <a href="xdg-open http://google.com">

Zren avatar Jan 09 '18 06:01 Zren