Changeset View
Standalone View
processui/ksysguardprocesslist.cpp
Show All 38 Lines | |||||
39 | #include <QStyle> | 39 | #include <QStyle> | ||
40 | #include <QStyledItemDelegate> | 40 | #include <QStyledItemDelegate> | ||
41 | #include <QPainter> | 41 | #include <QPainter> | ||
42 | #include <QLineEdit> | 42 | #include <QLineEdit> | ||
43 | #include <QSignalMapper> | 43 | #include <QSignalMapper> | ||
44 | #include <QToolTip> | 44 | #include <QToolTip> | ||
45 | #include <QAbstractItemModel> | 45 | #include <QAbstractItemModel> | ||
46 | #include <QDBusMetaType> | 46 | #include <QDBusMetaType> | ||
47 | #include <QtDBus> | ||||
47 | #include <QPointer> | 48 | #include <QPointer> | ||
48 | #include <QDialog> | 49 | #include <QDialog> | ||
49 | #include <QIcon> | 50 | #include <QIcon> | ||
50 | 51 | | |||
51 | #include <signal.h> //For SIGTERM | 52 | #include <signal.h> //For SIGTERM | ||
52 | 53 | | |||
53 | #include <KAuth> | 54 | #include <KAuth> | ||
54 | #include <KAuthAction> | 55 | #include <KAuthAction> | ||
55 | #include <KAuthActionReply> | 56 | #include <KAuthActionReply> | ||
56 | #include <KAuthObjectDecorator> | 57 | #include <KAuthObjectDecorator> | ||
57 | #include <klocalizedstring.h> | 58 | #include <klocalizedstring.h> | ||
58 | #include <kmessagebox.h> | 59 | #include <kmessagebox.h> | ||
59 | #include <KWindowSystem> | 60 | #include <KWindowSystem> | ||
61 | #include <KService> | ||||
62 | #include <KRun> | ||||
63 | #include <KGlobalAccel> | ||||
60 | 64 | | |||
61 | #include "ReniceDlg.h" | 65 | #include "ReniceDlg.h" | ||
62 | #include "ui_ProcessWidgetUI.h" | 66 | #include "ui_ProcessWidgetUI.h" | ||
63 | #include "scripting.h" | 67 | #include "scripting.h" | ||
64 | 68 | | |||
65 | #include <sys/types.h> | 69 | #include <sys/types.h> | ||
66 | #include <unistd.h> | 70 | #include <unistd.h> | ||
67 | 71 | | |||
▲ Show 20 Lines • Show All 120 Lines • ▼ Show 20 Line(s) | 182 | if (option.state & QStyle::State_HasFocus) { | |||
188 | style->drawPrimitive(QStyle::PE_FrameFocusRect, &o, painter, option.widget); | 192 | style->drawPrimitive(QStyle::PE_FrameFocusRect, &o, painter, option.widget); | ||
189 | } | 193 | } | ||
190 | } | 194 | } | ||
191 | }; | 195 | }; | ||
192 | 196 | | |||
193 | struct KSysGuardProcessListPrivate { | 197 | struct KSysGuardProcessListPrivate { | ||
194 | 198 | | |||
195 | KSysGuardProcessListPrivate(KSysGuardProcessList* q, const QString &hostName) | 199 | KSysGuardProcessListPrivate(KSysGuardProcessList* q, const QString &hostName) | ||
196 | : mModel(q, hostName), mFilterModel(q), mUi(new Ui::ProcessWidget()), mProcessContextMenu(nullptr), mUpdateTimer(nullptr) | 200 | : mModel(q, hostName), mFilterModel(q), mUi(new Ui::ProcessWidget()), mProcessContextMenu(nullptr), mUpdateTimer(nullptr), mToolsMenu(new QMenu(q)) | ||
anthonyfieroni: new QMenu(q) or delete in destructor. | |||||
197 | { | 201 | { | ||
198 | mScripting = nullptr; | 202 | mScripting = nullptr; | ||
199 | mNeedToExpandInit = false; | 203 | mNeedToExpandInit = false; | ||
200 | mNumItemsSelected = -1; | 204 | mNumItemsSelected = -1; | ||
201 | mResortCountDown = 2; //The items added initially will be already sorted, but without CPU info. On the second refresh we will have CPU usage, so /then/ we can resort | 205 | mResortCountDown = 2; //The items added initially will be already sorted, but without CPU info. On the second refresh we will have CPU usage, so /then/ we can resort | ||
202 | renice = new QAction(i18np("Set Priority...", "Set Priority...", 1), q); | 206 | renice = new QAction(i18np("Set Priority...", "Set Priority...", 1), q); | ||
203 | renice->setShortcut(Qt::Key_F8); | 207 | renice->setShortcut(Qt::Key_F8); | ||
204 | selectParent = new QAction(i18n("Jump to Parent Process"), q); | 208 | selectParent = new QAction(i18n("Jump to Parent Process"), q); | ||
▲ Show 20 Lines • Show All 76 Lines • ▼ Show 20 Line(s) | |||||
281 | QAction *sigStop; | 285 | QAction *sigStop; | ||
282 | QAction *sigCont; | 286 | QAction *sigCont; | ||
283 | QAction *sigHup; | 287 | QAction *sigHup; | ||
284 | QAction *sigInt; | 288 | QAction *sigInt; | ||
285 | QAction *sigTerm; | 289 | QAction *sigTerm; | ||
286 | QAction *sigKill; | 290 | QAction *sigKill; | ||
287 | QAction *sigUsr1; | 291 | QAction *sigUsr1; | ||
288 | QAction *sigUsr2; | 292 | QAction *sigUsr2; | ||
293 | | ||||
294 | QMenu* mToolsMenu; | ||||
289 | }; | 295 | }; | ||
290 | 296 | | |||
291 | KSysGuardProcessList::KSysGuardProcessList(QWidget* parent, const QString &hostName) | 297 | KSysGuardProcessList::KSysGuardProcessList(QWidget* parent, const QString &hostName) | ||
292 | : QWidget(parent), d(new KSysGuardProcessListPrivate(this, hostName)) | 298 | : QWidget(parent), d(new KSysGuardProcessListPrivate(this, hostName)) | ||
293 | { | 299 | { | ||
294 | qRegisterMetaType<QList<long long> >(); | 300 | qRegisterMetaType<QList<long long> >(); | ||
295 | qDBusRegisterMetaType<QList<long long> >(); | 301 | qDBusRegisterMetaType<QList<long long> >(); | ||
296 | 302 | | |||
▲ Show 20 Lines • Show All 56 Lines • ▼ Show 20 Line(s) | 309 | #endif | |||
353 | d->mFilterModel.setSortCaseSensitivity(Qt::CaseInsensitive); | 359 | d->mFilterModel.setSortCaseSensitivity(Qt::CaseInsensitive); | ||
354 | 360 | | |||
355 | d->mUi->txtFilter->installEventFilter(this); | 361 | d->mUi->txtFilter->installEventFilter(this); | ||
356 | d->mUi->treeView->installEventFilter(this); | 362 | d->mUi->treeView->installEventFilter(this); | ||
357 | 363 | | |||
358 | d->mUi->treeView->setDragEnabled(true); | 364 | d->mUi->treeView->setDragEnabled(true); | ||
359 | d->mUi->treeView->setDragDropMode(QAbstractItemView::DragOnly); | 365 | d->mUi->treeView->setDragDropMode(QAbstractItemView::DragOnly); | ||
360 | 366 | | |||
361 | 367 | | |||
ngraham: Let's do unrelated whitespace/style cleanup in another patch. | |||||
gregormi: does this comment still apply? | |||||
rkflx: Yes, it still shows up as a whitespace change on Phabricator. | |||||
362 | //Sort by username by default | 368 | //Sort by username by default | ||
363 | d->mUi->treeView->sortByColumn(ProcessModel::HeadingUser, Qt::AscendingOrder); | 369 | d->mUi->treeView->sortByColumn(ProcessModel::HeadingUser, Qt::AscendingOrder); | ||
364 | 370 | | |||
365 | // Add all the actions to the main widget, and get all the actions to call actionTriggered when clicked | 371 | // Add all the actions to the main widget, and get all the actions to call actionTriggered when clicked | ||
366 | QSignalMapper *signalMapper = new QSignalMapper(this); | 372 | QSignalMapper *signalMapper = new QSignalMapper(this); | ||
367 | QList<QAction *> actions; | 373 | QList<QAction *> actions; | ||
368 | actions << d->renice << d->kill << d->terminate << d->selectParent << d->selectTracer << d->window << d->jumpToSearchFilter; | 374 | actions << d->renice << d->kill << d->terminate << d->selectParent << d->selectTracer << d->window << d->jumpToSearchFilter; | ||
369 | actions << d->resume << d->sigStop << d->sigCont << d->sigHup << d->sigInt << d->sigTerm << d->sigKill << d->sigUsr1 << d->sigUsr2; | 375 | actions << d->resume << d->sigStop << d->sigCont << d->sigHup << d->sigInt << d->sigTerm << d->sigKill << d->sigUsr1 << d->sigUsr2; | ||
370 | 376 | | |||
371 | foreach(QAction *action, actions) { | 377 | foreach(QAction *action, actions) { | ||
372 | addAction(action); | 378 | addAction(action); | ||
373 | connect(action, SIGNAL(triggered(bool)), signalMapper, SLOT(map())); | 379 | connect(action, SIGNAL(triggered(bool)), signalMapper, SLOT(map())); | ||
374 | signalMapper->setMapping(action, action); | 380 | signalMapper->setMapping(action, action); | ||
375 | } | 381 | } | ||
376 | connect(signalMapper, SIGNAL(mapped(QObject*)), SLOT(actionTriggered(QObject*))); | 382 | connect(signalMapper, SIGNAL(mapped(QObject*)), SLOT(actionTriggered(QObject*))); | ||
377 | 383 | | |||
378 | retranslateUi(); | 384 | retranslateUi(); | ||
379 | 385 | | |||
380 | d->mUi->btnKillProcess->setIcon(QIcon::fromTheme(QStringLiteral("process-stop"))); | 386 | d->mUi->btnKillProcess->setIcon(QIcon::fromTheme(QStringLiteral("process-stop"))); | ||
Do you really need this? Maybe it's enough if you don't change this property to true in processui/ProcessWidgetUI.ui:31. rkflx: Do you really need this? Maybe it's enough if you don't change this property to `true` in… | |||||
381 | d->mUi->btnKillProcess->setToolTip(i18n("<qt>End the selected process. Warning - you may lose unsaved work.<br>Right click on a process to send other signals.<br>See What's This for technical information.<br>To target a specific window to kill, press Ctrl+Alt+Esc at any time.")); | 387 | d->mUi->btnKillProcess->setToolTip(i18n("<qt>End the selected process. Warning - you may lose unsaved work.<br>Right click on a process to send other signals.<br>See What's This for technical information.")); | ||
388 | | ||||
389 | auto addByDesktopName = [this](const QString& desktopName) | ||||
What does d1 mean? Is it necessary to duplicate the d-pointer? As far as I can see, you are already capturing this in the lambda, so you won't need to capture (this->)d. rkflx: What does `d1` mean? Is it necessary to duplicate the d-pointer?
As far as I can see, you are… | |||||
390 | { | ||||
391 | auto kService = KService::serviceByDesktopName(desktopName); | ||||
392 | if (kService) { | ||||
393 | auto action = new QAction(QIcon::fromTheme(kService->icon()), | ||||
394 | kService->name(), nullptr); | ||||
rkflx: Drop `!= nullptr`. | |||||
395 | | ||||
396 | connect(action, &QAction::triggered, this, | ||||
397 | [kService](bool) { | ||||
398 | KRun::runService(*kService, { }, nullptr); | ||||
399 | }); | ||||
400 | d->mToolsMenu->addAction(action); | ||||
401 | } | ||||
402 | }; | ||||
403 | | ||||
404 | addByDesktopName(QStringLiteral("org.kde.konsole")); | ||||
405 | | ||||
406 | const QString ksysguardDesktopName = QStringLiteral("org.kde.ksysguard"); | ||||
407 | // The following expression is true when the libksysguard process list is _not_ embedded in KSysGuard. | ||||
408 | // Only then we add KSysGuard to the menu | ||||
409 | if (qApp->desktopFileName() != ksysguardDesktopName) { | ||||
410 | addByDesktopName(ksysguardDesktopName); | ||||
411 | } | ||||
Could you do an exact match on the filename, i.e. only the last part of the full path? There might be situations where "System Monitor" is developed or installed in a directory containing this string by chance. rkflx: Could you do an exact match on the filename, i.e. only the last part of the full path? There… | |||||
Recently I learned comparing with qApp->desktopFileName() might be even better than looking at the path of the executable… rkflx: Recently I learned comparing with `qApp->desktopFileName()` might be even better than looking… | |||||
Interesting. This indeed returns "org.kde.ksysguard". Do you have an idea, how Qt knows this desktop name? The only occurrences of this string in the ksysguard project are:
gregormi: Interesting. This indeed returns "org.kde.ksysguard". Do you have an idea, how Qt knows this… | |||||
Interesting question ;) After looking into Qt's sources for desktopFileName, I set a breakpoint on QGuiApplication::setDesktopFileName, and ended up in KAboutData::setApplicationData. In ksysguard.cpp you then have KAboutData aboutData( QStringLiteral("ksysguard"), … … QStringLiteral("ksysguard")aboutData.setOrganizationDomain(QByteArray("kde.org")); …which looks related (did not check in detail, though). rkflx: Interesting question ;)
After looking into Qt's sources for [`desktopFileName`](https://code. | |||||
412 | | ||||
413 | addByDesktopName(QStringLiteral("org.kde.ksystemlog")); | ||||
414 | addByDesktopName(QStringLiteral("org.kde.kinfocenter")); | ||||
415 | addByDesktopName(QStringLiteral("org.kde.filelight")); | ||||
416 | addByDesktopName(QStringLiteral("org.kde.sweeper")); | ||||
417 | addByDesktopName(QStringLiteral("org.kde.kmag")); | ||||
418 | addByDesktopName(QStringLiteral("htop")); | ||||
419 | | ||||
Sweeper and KMag do not show up for me, even though I have both of them installed. Is your naming correct? (Sidenote: This smells like it should be handled by your KMoreTools, although that would probably require some changes there first… :) rkflx: Sweeper and KMag do not show up for me, even though I have both of them installed. Is your… | |||||
gregormi: I fixed the desktop names. | |||||
420 | // Add Run Command... | ||||
421 | // | ||||
422 | auto runCommandAction = new QAction(i18nc("@action:inmenu", "Run Command"), this); | ||||
This looks a bit odd. I would organize this with linebreaks alone, no need for separate code blocks. rkflx: This looks a bit odd. I would organize this with linebreaks alone, no need for separate code… | |||||
gregormi: Done. | |||||
423 | // //INFO: This is one way of how to find out the two required parameters for the globalShortcut method: | ||||
rkflx: Please give your variable a more descriptive name. | |||||
424 | // auto list = KGlobalAccel::getGlobalShortcutsByKey(QKeySequence(QStringLiteral("Alt+Space"))); | ||||
425 | // foreach (auto item, list) { | ||||
426 | // qDebug() << item.componentUniqueName() << item.uniqueName(); | ||||
427 | // //prints: 'krunner', 'run command' | ||||
rkflx: Left-over debug code? | |||||
It was supposed to serve as reminder of how the parameters for the globalShortcut method were determined. To help debugging later. Should it be removed? gregormi: It was supposed to serve as reminder of how the parameters for the globalShortcut method were… | |||||
At least elsewhere there is no such comment and I doubt the object's name will change in the future, but if you want to keep it, that's also fine with me. rkflx: At least [elsewhere](https://lxr.kde.org/source/kde/workspace/plasma… | |||||
428 | // } | ||||
429 | const auto runCommandShortcutList = KGlobalAccel::self()->globalShortcut(QStringLiteral("krunner"), QStringLiteral("run command")); | ||||
Do you think it is really necessary to display not set? For me, an empty string would also work just fine, now that you are using \t instead of (…). Also I found a way to omit the string and \t entirely and let Qt do all the work: auto runCommandAction = new QAction(i18nc("@action:inmenu", "Run Command"), this); const auto runCommandShortcutList = KGlobalAccel::self()->globalShortcut(QStringLiteral("krunner"), QStringLiteral("run command")); if (runCommandShortcutList.size() > 0) { runCommandAction->setShortcut(runCommandShortcutList[0]); } rkflx: Do you think it is really necessary to display `not set`? For me, an empty string would also… | |||||
Yes, is this much better. Might there be a chance of a some kind of keyboard shortcut conflict because we now set it also locally? gregormi: Yes, is this much better.
Might there be a chance of a some kind of keyboard shortcut conflict… | |||||
Not sure whether there are any guarantees, but at least swapping the shortcuts between Run Command and Kill a Window, we can observe that in case of (silent) conflicts the global shortcut has precedence, i.e. "it works"™. Perhaps only setting the string is cleaner, but then due to \t we'd have RTL problems again like in your other patch. I'd go the shortcut way, unless anybody from Plasma comes up with good reasons not to. rkflx: > Might there be a chance of a some kind of keyboard shortcut conflict because we now set it… | |||||
430 | runCommandAction->setShortcuts(runCommandShortcutList); | ||||
431 | runCommandAction->setIcon(QIcon::fromTheme(QStringLiteral("system-run"))); | ||||
432 | connect(runCommandAction, &QAction::triggered, this, [](){ | ||||
Meanwhile I learned (while checking why you kept toString(QKeySequence::NativeText) compared to my suggestion) that this can be written even simpler, no need for the if at all: runCommandAction->setShortcuts(runCommandShortcutList); rkflx: Meanwhile I learned (while checking why you kept `toString(QKeySequence::NativeText)` compared… | |||||
433 | KRun::runCommand(QStringLiteral("krunner"), nullptr); | ||||
434 | }); | ||||
435 | d->mToolsMenu->addAction(runCommandAction); | ||||
436 | | ||||
437 | // Add the xkill functionality... | ||||
438 | auto killWindowAction = new QAction(QIcon::fromTheme(QStringLiteral("document-close")), | ||||
439 | i18nc("@action:inmenu", "Kill a Window"), this); | ||||
440 | // Find shortcut of xkill functionality which is defined in KWin | ||||
441 | const auto killWindowShortcutList = KGlobalAccel::self()->globalShortcut(QStringLiteral("kwin"), QStringLiteral("Kill Window")); | ||||
442 | killWindowAction->setShortcuts(killWindowShortcutList); | ||||
443 | // We don't use xkill directly but the method in KWin which allows to press Esc to abort. | ||||
444 | auto killWindowKwinMethod = new QDBusInterface(QStringLiteral("org.kde.KWin"), QStringLiteral("/KWin"), QStringLiteral("org.kde.KWin")); | ||||
445 | // If KWin is not the window manager, then we disable the entry: | ||||
446 | if (!killWindowKwinMethod->isValid()) { | ||||
447 | killWindowAction->setEnabled(false); | ||||
448 | } | ||||
449 | connect(killWindowAction, &QAction::triggered, this, [this, killWindowKwinMethod](bool) { | ||||
450 | // with DBus call, always use the async method. | ||||
451 | // Otherwise it could wait up to 30 seconds in certain situations. | ||||
452 | killWindowKwinMethod->asyncCall(QStringLiteral("killWindow")); | ||||
453 | }); | ||||
454 | d->mToolsMenu->addAction(killWindowAction); | ||||
455 | | ||||
456 | d->mUi->btnTools->setMenu(d->mToolsMenu); | ||||
382 | } | 457 | } | ||
383 | 458 | | |||
384 | KSysGuardProcessList::~KSysGuardProcessList() | 459 | KSysGuardProcessList::~KSysGuardProcessList() | ||
385 | { | 460 | { | ||
386 | delete d; | 461 | delete d; | ||
387 | } | 462 | } | ||
388 | 463 | | |||
389 | QTreeView *KSysGuardProcessList::treeView() const { | 464 | QTreeView *KSysGuardProcessList::treeView() const { | ||
Show All 21 Lines | 485 | if(isVisible()) | |||
411 | expandInit(); | 486 | expandInit(); | ||
412 | } | 487 | } | ||
413 | void KSysGuardProcessList::filterTextChanged(const QString &newText) { | 488 | void KSysGuardProcessList::filterTextChanged(const QString &newText) { | ||
414 | d->mFilterModel.setFilterRegExp(newText.trimmed()); | 489 | d->mFilterModel.setFilterRegExp(newText.trimmed()); | ||
415 | if(isVisible()) | 490 | if(isVisible()) | ||
416 | expandInit(); | 491 | expandInit(); | ||
417 | d->mUi->btnKillProcess->setEnabled(d->mUi->treeView->selectionModel()->hasSelection()); | 492 | d->mUi->btnKillProcess->setEnabled(d->mUi->treeView->selectionModel()->hasSelection()); | ||
418 | d->mUi->treeView->scrollTo(d->mUi->treeView->currentIndex()); | 493 | d->mUi->treeView->scrollTo(d->mUi->treeView->currentIndex()); | ||
419 | } | 494 | } | ||
ngraham: Let's do unrelated whitespace/style cleanup in another patch. | |||||
420 | 495 | | |||
421 | int KSysGuardProcessList::visibleProcessesCount() const { | 496 | int KSysGuardProcessList::visibleProcessesCount() const { | ||
422 | //This assumes that all the visible rows are processes. This is true currently, but might not be | 497 | //This assumes that all the visible rows are processes. This is true currently, but might not be | ||
423 | //true if we add support for showing threads etc | 498 | //true if we add support for showing threads etc | ||
424 | if(d->mModel.isSimpleMode()) | 499 | if(d->mModel.isSimpleMode()) | ||
425 | return d->mFilterModel.rowCount(); | 500 | return d->mFilterModel.rowCount(); | ||
426 | return d->totalRowCount(QModelIndex()); | 501 | return d->totalRowCount(QModelIndex()); | ||
427 | } | 502 | } | ||
Show All 21 Lines | 517 | { | |||
449 | action.addArgument(QStringLiteral("pidcount"), processCount); | 524 | action.addArgument(QStringLiteral("pidcount"), processCount); | ||
450 | } | 525 | } | ||
451 | void KSysGuardProcessList::selectionChanged() | 526 | void KSysGuardProcessList::selectionChanged() | ||
452 | { | 527 | { | ||
453 | int numSelected = d->mUi->treeView->selectionModel()->selectedRows().size(); | 528 | int numSelected = d->mUi->treeView->selectionModel()->selectedRows().size(); | ||
454 | if(numSelected == d->mNumItemsSelected) | 529 | if(numSelected == d->mNumItemsSelected) | ||
455 | return; | 530 | return; | ||
456 | d->mNumItemsSelected = numSelected; | 531 | d->mNumItemsSelected = numSelected; | ||
457 | d->mUi->btnKillProcess->setEnabled(numSelected != 0); | 532 | d->mUi->btnKillProcess->setEnabled(numSelected != 0); | ||
ngraham: Let's do unrelated whitespace/style cleanup in another patch. | |||||
458 | 533 | | |||
459 | d->renice->setText(i18np("Set Priority...", "Set Priority...", numSelected)); | 534 | d->renice->setText(i18np("Set Priority...", "Set Priority...", numSelected)); | ||
460 | d->kill->setText(i18np("Forcibly Kill Process", "Forcibly Kill Processes", numSelected)); | 535 | d->kill->setText(i18np("Forcibly Kill Process", "Forcibly Kill Processes", numSelected)); | ||
461 | d->terminate->setText(i18ncp("Context menu", "End Process", "End Processes", numSelected)); | 536 | d->terminate->setText(i18ncp("Context menu", "End Process", "End Processes", numSelected)); | ||
462 | } | 537 | } | ||
463 | void KSysGuardProcessList::showProcessContextMenu(const QModelIndex &index) { | 538 | void KSysGuardProcessList::showProcessContextMenu(const QModelIndex &index) { | ||
464 | if(!index.isValid()) return; | 539 | if(!index.isValid()) return; | ||
465 | QRect rect = d->mUi->treeView->visualRect(index); | 540 | QRect rect = d->mUi->treeView->visualRect(index); | ||
▲ Show 20 Lines • Show All 1051 Lines • Show Last 20 Lines |
new QMenu(q) or delete in destructor.