Changeset View
Standalone View
processui/ksysguardprocesslist.cpp
Show First 20 Lines • Show All 50 Lines • ▼ Show 20 Line(s) | |||||
51 | 51 | | |||
52 | #include <KAuth> | 52 | #include <KAuth> | ||
53 | #include <KAuthAction> | 53 | #include <KAuthAction> | ||
54 | #include <KAuthActionReply> | 54 | #include <KAuthActionReply> | ||
55 | #include <KAuthObjectDecorator> | 55 | #include <KAuthObjectDecorator> | ||
56 | #include <klocalizedstring.h> | 56 | #include <klocalizedstring.h> | ||
57 | #include <kmessagebox.h> | 57 | #include <kmessagebox.h> | ||
58 | #include <KWindowSystem> | 58 | #include <KWindowSystem> | ||
59 | #include <KService> | ||||
60 | #include <KRun> | ||||
61 | #include <KGlobalAccel> | ||||
59 | 62 | | |||
60 | #include "ReniceDlg.h" | 63 | #include "ReniceDlg.h" | ||
61 | #include "ui_ProcessWidgetUI.h" | 64 | #include "ui_ProcessWidgetUI.h" | ||
62 | #include "scripting.h" | 65 | #include "scripting.h" | ||
63 | 66 | | |||
64 | #include <sys/types.h> | 67 | #include <sys/types.h> | ||
65 | #include <unistd.h> | 68 | #include <unistd.h> | ||
66 | 69 | | |||
67 | //Trolltech have a testing class for classes that inherit QAbstractItemModel. If you want to run with this run-time testing enabled, put the modeltest.* files in this directory and uncomment the next line | 70 | //Trolltech have a testing class for classes that inherit QAbstractItemModel. If you want to run with this run-time testing enabled, put the modeltest.* files in this directory and uncomment the next line | ||
68 | //#define DO_MODELCHECK | 71 | //#define DO_MODELCHECK | ||
69 | #ifdef DO_MODELCHECK | 72 | #ifdef DO_MODELCHECK | ||
70 | #include "modeltest.h" | 73 | #include "modeltest.h" | ||
74 | class KGlobalAccel; | ||||
71 | #endif | 75 | #endif | ||
72 | class ProgressBarItemDelegate : public QStyledItemDelegate | 76 | class ProgressBarItemDelegate : public QStyledItemDelegate | ||
73 | { | 77 | { | ||
74 | public: | 78 | public: | ||
75 | ProgressBarItemDelegate(QObject *parent) : QStyledItemDelegate(parent) { | 79 | ProgressBarItemDelegate(QObject *parent) : QStyledItemDelegate(parent) { | ||
76 | } | 80 | } | ||
77 | 81 | | |||
78 | void paint(QPainter *painter, const QStyleOptionViewItem &opt, const QModelIndex &index) const override | 82 | void paint(QPainter *painter, const QStyleOptionViewItem &opt, const QModelIndex &index) const override | ||
▲ Show 20 Lines • Show All 108 Lines • ▼ Show 20 Line(s) | 181 | if (option.state & QStyle::State_HasFocus) { | |||
187 | style->drawPrimitive(QStyle::PE_FrameFocusRect, &o, painter, option.widget); | 191 | style->drawPrimitive(QStyle::PE_FrameFocusRect, &o, painter, option.widget); | ||
188 | } | 192 | } | ||
189 | } | 193 | } | ||
190 | }; | 194 | }; | ||
191 | 195 | | |||
192 | struct KSysGuardProcessListPrivate { | 196 | struct KSysGuardProcessListPrivate { | ||
193 | 197 | | |||
194 | KSysGuardProcessListPrivate(KSysGuardProcessList* q, const QString &hostName) | 198 | KSysGuardProcessListPrivate(KSysGuardProcessList* q, const QString &hostName) | ||
195 | : mModel(q, hostName), mFilterModel(q), mUi(new Ui::ProcessWidget()), mProcessContextMenu(nullptr), mUpdateTimer(nullptr) | 199 | : mModel(q, hostName), mFilterModel(q), mUi(new Ui::ProcessWidget()), mProcessContextMenu(nullptr), mUpdateTimer(nullptr), mToolsMenu(new QMenu()) | ||
anthonyfieroni: new QMenu(q) or delete in destructor. | |||||
196 | { | 200 | { | ||
197 | mScripting = nullptr; | 201 | mScripting = nullptr; | ||
198 | mNeedToExpandInit = false; | 202 | mNeedToExpandInit = false; | ||
199 | mNumItemsSelected = -1; | 203 | mNumItemsSelected = -1; | ||
200 | 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 | 204 | 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 | ||
201 | renice = new QAction(i18np("Set Priority...", "Set Priority...", 1), q); | 205 | renice = new QAction(i18np("Set Priority...", "Set Priority...", 1), q); | ||
202 | renice->setShortcut(Qt::Key_F8); | 206 | renice->setShortcut(Qt::Key_F8); | ||
203 | selectParent = new QAction(i18n("Jump to Parent Process"), q); | 207 | selectParent = new QAction(i18n("Jump to Parent Process"), q); | ||
▲ Show 20 Lines • Show All 76 Lines • ▼ Show 20 Line(s) | |||||
280 | QAction *sigStop; | 284 | QAction *sigStop; | ||
281 | QAction *sigCont; | 285 | QAction *sigCont; | ||
282 | QAction *sigHup; | 286 | QAction *sigHup; | ||
283 | QAction *sigInt; | 287 | QAction *sigInt; | ||
284 | QAction *sigTerm; | 288 | QAction *sigTerm; | ||
285 | QAction *sigKill; | 289 | QAction *sigKill; | ||
286 | QAction *sigUsr1; | 290 | QAction *sigUsr1; | ||
287 | QAction *sigUsr2; | 291 | QAction *sigUsr2; | ||
292 | | ||||
293 | QMenu* mToolsMenu; | ||||
288 | }; | 294 | }; | ||
289 | 295 | | |||
290 | KSysGuardProcessList::KSysGuardProcessList(QWidget* parent, const QString &hostName) | 296 | KSysGuardProcessList::KSysGuardProcessList(QWidget* parent, const QString &hostName) | ||
291 | : QWidget(parent), d(new KSysGuardProcessListPrivate(this, hostName)) | 297 | : QWidget(parent), d(new KSysGuardProcessListPrivate(this, hostName)) | ||
292 | { | 298 | { | ||
293 | qRegisterMetaType<QList<long long> >(); | 299 | qRegisterMetaType<QList<long long> >(); | ||
294 | qDBusRegisterMetaType<QList<long long> >(); | 300 | qDBusRegisterMetaType<QList<long long> >(); | ||
295 | 301 | | |||
▲ Show 20 Lines • Show All 56 Lines • ▼ Show 20 Line(s) | 308 | #endif | |||
352 | d->mFilterModel.setSortCaseSensitivity(Qt::CaseInsensitive); | 358 | d->mFilterModel.setSortCaseSensitivity(Qt::CaseInsensitive); | ||
353 | 359 | | |||
354 | d->mUi->txtFilter->installEventFilter(this); | 360 | d->mUi->txtFilter->installEventFilter(this); | ||
355 | d->mUi->treeView->installEventFilter(this); | 361 | d->mUi->treeView->installEventFilter(this); | ||
356 | 362 | | |||
357 | d->mUi->treeView->setDragEnabled(true); | 363 | d->mUi->treeView->setDragEnabled(true); | ||
358 | d->mUi->treeView->setDragDropMode(QAbstractItemView::DragOnly); | 364 | d->mUi->treeView->setDragDropMode(QAbstractItemView::DragOnly); | ||
359 | 365 | | |||
360 | | ||||
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. | |||||
361 | //Sort by username by default | 366 | //Sort by username by default | ||
362 | d->mUi->treeView->sortByColumn(ProcessModel::HeadingUser, Qt::AscendingOrder); | 367 | d->mUi->treeView->sortByColumn(ProcessModel::HeadingUser, Qt::AscendingOrder); | ||
363 | 368 | | |||
364 | // Add all the actions to the main widget, and get all the actions to call actionTriggered when clicked | 369 | // Add all the actions to the main widget, and get all the actions to call actionTriggered when clicked | ||
365 | QSignalMapper *signalMapper = new QSignalMapper(this); | 370 | QSignalMapper *signalMapper = new QSignalMapper(this); | ||
366 | QList<QAction *> actions; | 371 | QList<QAction *> actions; | ||
367 | actions << d->renice << d->kill << d->terminate << d->selectParent << d->selectTracer << d->window << d->jumpToSearchFilter; | 372 | actions << d->renice << d->kill << d->terminate << d->selectParent << d->selectTracer << d->window << d->jumpToSearchFilter; | ||
368 | actions << d->resume << d->sigStop << d->sigCont << d->sigHup << d->sigInt << d->sigTerm << d->sigKill << d->sigUsr1 << d->sigUsr2; | 373 | actions << d->resume << d->sigStop << d->sigCont << d->sigHup << d->sigInt << d->sigTerm << d->sigKill << d->sigUsr1 << d->sigUsr2; | ||
369 | 374 | | |||
370 | foreach(QAction *action, actions) { | 375 | foreach(QAction *action, actions) { | ||
371 | addAction(action); | 376 | addAction(action); | ||
372 | connect(action, SIGNAL(triggered(bool)), signalMapper, SLOT(map())); | 377 | connect(action, SIGNAL(triggered(bool)), signalMapper, SLOT(map())); | ||
373 | signalMapper->setMapping(action, action); | 378 | signalMapper->setMapping(action, action); | ||
374 | } | 379 | } | ||
375 | connect(signalMapper, SIGNAL(mapped(QObject*)), SLOT(actionTriggered(QObject*))); | 380 | connect(signalMapper, SIGNAL(mapped(QObject*)), SLOT(actionTriggered(QObject*))); | ||
376 | 381 | | |||
377 | retranslateUi(); | 382 | retranslateUi(); | ||
378 | 383 | | |||
384 | d->mUi->btnKillProcess->setEnabled(false); | ||||
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… | |||||
379 | d->mUi->btnKillProcess->setIcon(QIcon::fromTheme(QStringLiteral("process-stop"))); | 385 | d->mUi->btnKillProcess->setIcon(QIcon::fromTheme(QStringLiteral("process-stop"))); | ||
380 | 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.")); | 386 | 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.")); | ||
387 | | ||||
388 | auto d1 = d; | ||||
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… | |||||
389 | auto addByDesktopName = [this, d1](const QString& desktopName) | ||||
390 | { | ||||
391 | auto d = d1; | ||||
392 | auto kService = KService::serviceByDesktopName(desktopName); | ||||
393 | if (kService != nullptr) { | ||||
rkflx: Drop `!= nullptr`. | |||||
394 | auto action = new QAction(QIcon::fromTheme(kService->icon()), | ||||
395 | kService->name(), nullptr); | ||||
396 | | ||||
397 | connect(action, &QAction::triggered, this, | ||||
398 | [kService](bool) { | ||||
399 | KRun::runService(*kService, { }, nullptr); | ||||
400 | }); | ||||
401 | d->mToolsMenu->addAction(action); | ||||
402 | } | ||||
403 | }; | ||||
404 | | ||||
405 | addByDesktopName(QStringLiteral("org.kde.konsole")); | ||||
406 | | ||||
407 | // QCoreApplication::applicationFilePath() returns something like "/usr/bin/ksysguard" | ||||
408 | // when this view is embedded in KSysGuard. | ||||
409 | // And in this case we do not add the KSysGuard item to the menu. | ||||
410 | if (!QCoreApplication::applicationFilePath().contains(QStringLiteral("ksysguard"))) { | ||||
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. | |||||
411 | addByDesktopName(QStringLiteral("org.kde.ksysguard")); | ||||
412 | } | ||||
413 | | ||||
414 | addByDesktopName(QStringLiteral("org.kde.ksystemlog")); | ||||
415 | addByDesktopName(QStringLiteral("org.kde.kinfocenter")); | ||||
416 | addByDesktopName(QStringLiteral("org.kde.filelight")); | ||||
417 | addByDesktopName(QStringLiteral("sweeper")); | ||||
418 | addByDesktopName(QStringLiteral("kmag")); | ||||
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. | |||||
419 | addByDesktopName(QStringLiteral("htop")); | ||||
420 | | ||||
421 | { | ||||
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. | |||||
422 | auto action = new QAction(i18n("Run Command"), this); | ||||
rkflx: Please give your variable a more descriptive name. | |||||
423 | action->setIcon(QIcon::fromTheme(QStringLiteral("system-run"))); | ||||
424 | connect(action, &QAction::triggered, this, [](){ | ||||
425 | KRun::runCommand(QStringLiteral("krunner"), nullptr); | ||||
426 | }); | ||||
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… | |||||
427 | d->mToolsMenu->addAction(action); | ||||
428 | } | ||||
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… | |||||
429 | | ||||
430 | { | ||||
431 | // find shortcut of xkill functionality which is defined in KWin | ||||
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… | |||||
432 | auto killWindowKeys = KGlobalAccel::self()->globalShortcut(QStringLiteral("kwin"), QStringLiteral("Kill Window")); | ||||
433 | QString killWindowShortcut = i18nc("the keyboard shortcut of the Kill Window function", "not set"); | ||||
434 | if (killWindowKeys.size() > 0) { | ||||
435 | killWindowShortcut = killWindowKeys[0].toString(); | ||||
436 | } | ||||
437 | auto killWindowAction = new QAction(QIcon::fromTheme(QStringLiteral("document-close")), | ||||
438 | i18nc("%1 is a keyboard shortcut", "Kill a window (%1)", killWindowShortcut), this); | ||||
439 | | ||||
440 | // Alternative to using xkill directly. The KWin method also allows to press Esc to abort. | ||||
441 | auto killWindowKwinMethod = new QDBusInterface(QStringLiteral("org.kde.KWin"), QStringLiteral("/KWin"), QStringLiteral("org.kde.KWin")); | ||||
442 | // If KWin is not the window manager, then we disable the entry: | ||||
443 | if (!killWindowKwinMethod->isValid()) { | ||||
444 | killWindowAction->setEnabled(false); | ||||
445 | } | ||||
446 | | ||||
447 | connect(killWindowAction, &QAction::triggered, this, [this, killWindowKwinMethod](bool) { | ||||
448 | // with DBus call, always use the async method. | ||||
449 | // Otherwise it could wait up to 30 seconds in certain situations. | ||||
450 | killWindowKwinMethod->asyncCall(QStringLiteral("killWindow")); | ||||
451 | }); | ||||
452 | | ||||
453 | d->mToolsMenu->addAction(killWindowAction); | ||||
454 | } | ||||
455 | | ||||
456 | d->mUi->btnTools->setMenu(d->mToolsMenu); | ||||
381 | } | 457 | } | ||
382 | 458 | | |||
383 | KSysGuardProcessList::~KSysGuardProcessList() | 459 | KSysGuardProcessList::~KSysGuardProcessList() | ||
384 | { | 460 | { | ||
385 | delete d; | 461 | delete d; | ||
386 | } | 462 | } | ||
387 | 463 | | |||
388 | QTreeView *KSysGuardProcessList::treeView() const { | 464 | QTreeView *KSysGuardProcessList::treeView() const { | ||
Show All 19 Lines | 481 | { //index is the item the user selected in the combo box | |||
408 | d->mUi->cmbFilter->setCurrentIndex( (int)state); | 484 | d->mUi->cmbFilter->setCurrentIndex( (int)state); | ||
409 | if(isVisible()) | 485 | if(isVisible()) | ||
410 | expandInit(); | 486 | expandInit(); | ||
411 | } | 487 | } | ||
412 | void KSysGuardProcessList::filterTextChanged(const QString &newText) { | 488 | void KSysGuardProcessList::filterTextChanged(const QString &newText) { | ||
413 | d->mFilterModel.setFilterRegExp(newText.trimmed()); | 489 | d->mFilterModel.setFilterRegExp(newText.trimmed()); | ||
414 | if(isVisible()) | 490 | if(isVisible()) | ||
415 | expandInit(); | 491 | expandInit(); | ||
416 | d->mUi->btnKillProcess->setEnabled( d->mUi->treeView->selectionModel()->hasSelection() ); | 492 | d->mUi->btnKillProcess->setEnabled(d->mUi->treeView->selectionModel()->hasSelection()); | ||
417 | d->mUi->treeView->scrollTo( d->mUi->treeView->currentIndex()); | 493 | d->mUi->treeView->scrollTo(d->mUi->treeView->currentIndex()); | ||
ngraham: Let's do unrelated whitespace/style cleanup in another patch. | |||||
418 | } | 494 | } | ||
419 | 495 | | |||
420 | int KSysGuardProcessList::visibleProcessesCount() const { | 496 | int KSysGuardProcessList::visibleProcessesCount() const { | ||
421 | //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 | ||
422 | //true if we add support for showing threads etc | 498 | //true if we add support for showing threads etc | ||
423 | if(d->mModel.isSimpleMode()) | 499 | if(d->mModel.isSimpleMode()) | ||
424 | return d->mFilterModel.rowCount(); | 500 | return d->mFilterModel.rowCount(); | ||
425 | return d->totalRowCount(QModelIndex()); | 501 | return d->totalRowCount(QModelIndex()); | ||
Show All 22 Lines | 517 | { | |||
448 | action.addArgument(QStringLiteral("pidcount"), processCount); | 524 | action.addArgument(QStringLiteral("pidcount"), processCount); | ||
449 | } | 525 | } | ||
450 | void KSysGuardProcessList::selectionChanged() | 526 | void KSysGuardProcessList::selectionChanged() | ||
451 | { | 527 | { | ||
452 | int numSelected = d->mUi->treeView->selectionModel()->selectedRows().size(); | 528 | int numSelected = d->mUi->treeView->selectionModel()->selectedRows().size(); | ||
453 | if(numSelected == d->mNumItemsSelected) | 529 | if(numSelected == d->mNumItemsSelected) | ||
454 | return; | 530 | return; | ||
455 | d->mNumItemsSelected = numSelected; | 531 | d->mNumItemsSelected = numSelected; | ||
456 | d->mUi->btnKillProcess->setEnabled( numSelected != 0 ); | 532 | d->mUi->btnKillProcess->setEnabled(numSelected != 0); | ||
ngraham: Let's do unrelated whitespace/style cleanup in another patch. | |||||
457 | 533 | | |||
458 | d->renice->setText(i18np("Set Priority...", "Set Priority...", numSelected)); | 534 | d->renice->setText(i18np("Set Priority...", "Set Priority...", numSelected)); | ||
459 | d->kill->setText(i18np("Forcibly Kill Process", "Forcibly Kill Processes", numSelected)); | 535 | d->kill->setText(i18np("Forcibly Kill Process", "Forcibly Kill Processes", numSelected)); | ||
460 | d->terminate->setText(i18ncp("Context menu", "End Process", "End Processes", numSelected)); | 536 | d->terminate->setText(i18ncp("Context menu", "End Process", "End Processes", numSelected)); | ||
461 | } | 537 | } | ||
462 | void KSysGuardProcessList::showProcessContextMenu(const QModelIndex &index) { | 538 | void KSysGuardProcessList::showProcessContextMenu(const QModelIndex &index) { | ||
463 | if(!index.isValid()) return; | 539 | if(!index.isValid()) return; | ||
464 | 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.