Differential D16484 Diff 44770 plugins/scratchpad/scratchpadjob.cpp

Changeset View

Standalone View

View Options

plugins/scratchpad/scratchpadjob.cpp

  • This file was added.
1​/* This file is part of KDevelop
2​ *
3​ * Copyright 2018 Amish K. Naidu <amhndu@gmail.com>
4​ *
5​ * This program is free software; you can redistribute it and/or
6​ * modify it under the terms of the GNU General Public License
7​ * as published by the Free Software Foundation; either version 2
8​ * of the License, or (at your option) any later version.
9​ *
10​ * This program is distributed in the hope that it will be useful,
11​ * but WITHOUT ANY WARRANTY; without even the implied warranty of
12​ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13​ * GNU General Public License for more details.
14​ *
15​ * You should have received a copy of the GNU General Public License
16​ * along with this program; if not, write to the Free Software
17​ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
18​ * 02110-1301, USA.
19​ */
20​
21​#include "scratchpadjob.h"
22​#include "scratchpad.h"
23​
24​#include <debug.h>
25​
26​#include <outputview/outputmodel.h>
27​#include <util/processlinemaker.h>
28​
29​#include <KProcess>
30​#include <KLocalizedString>
31​
32​#include <QMetaEnum>
33​
34​ScratchpadJob::ScratchpadJob(const QString& command, const QString& title, QObject* parent)
35​ : KDevelop::OutputJob(parent)
36​ , m_process(new KProcess(this))
37​ , m_lineMaker(new KDevelop::ProcessLineMaker(m_process, this))
38​{
39​ qCDebug(PLUGIN_SCRATCHPAD) << "Creating job for" << title;
40​
41​ setCapabilities(Killable);
42​
43​ if (!command.isEmpty()) {
44​ m_process->setShellCommand(command);
45​
46​ setStandardToolView(KDevelop::IOutputView::RunView);
47​ setTitle(i18nc("prefix to distinguish scratch tabs", "scratch:%1", title));
48​
49​ auto* model = new KDevelop::OutputModel(this);
brauchUnsubmitted
Done

You need to check whether the command is non-empty, otherwise you create unkillable zombie processes.

brauch: You need to check whether the command is non-empty, otherwise you create unkillable zombie…
50​ setModel(model);
51​
52​ connect(m_lineMaker, &KDevelop::ProcessLineMaker::receivedStdoutLines,
53​ model, &KDevelop::OutputModel::appendLines);
brauchUnsubmitted
Done

I think the i18n calls with the word "scratch" need a context, otherwise translators will not know what's going on here

brauch: I think the i18n calls with the word "scratch" need a context, otherwise translators will not…
amhnduAuthorUnsubmitted
Done

I'm not sure how to do that. Does this even require i18n ?

amhndu: I'm not sure how to do that. Does this even require i18n ?
brauchUnsubmitted
Done

Look at "i18nc". I think it does, yes, all user-visible strings should be wrapped in i18n calls.

brauch: Look at "i18nc". I think it does, yes, all user-visible strings should be wrapped in i18n calls.
54​ connect(m_lineMaker, &KDevelop::ProcessLineMaker::receivedStderrLines,
55​ model, &KDevelop::OutputModel::appendLines);
56​ m_process->setOutputChannelMode(KProcess::MergedChannels);
57​ connect(m_process, QOverload<int, QProcess::ExitStatus>::of(&KProcess::finished),
58​ this, &ScratchpadJob::processFinished);
59​ connect(m_process, &KProcess::errorOccurred, this, &ScratchpadJob::processError);
60​ } else {
61​ qCCritical(PLUGIN_SCRATCHPAD) << "Empty command in scratch job.";
62​ deleteLater();
63​ }
brauchUnsubmitted
Done
QOverload<int, QProcess::ExitStatus>::of(&KProcess::finished)
brauch: ``` QOverload<int, QProcess::ExitStatus>::of(&KProcess::finished) ```
64​}
65​
66​void ScratchpadJob::start()
67​{
68​ const auto program = m_process->program().join(QLatin1Char(' '));
69​ if (!program.trimmed().isEmpty()) {
70​ startOutput();
71​ outputModel()->appendLine(i18n("Running %1...", program));
72​ m_process->start();
73​ }
74​}
75​
76​bool ScratchpadJob::doKill()
77​{
78​ qCDebug(PLUGIN_SCRATCHPAD) << "killing process";
79​ m_process->kill();
80​ return true;
81​}
82​
83​void ScratchpadJob::processFinished(int exitCode, QProcess::ExitStatus)
84​{
85​ qCDebug(PLUGIN_SCRATCHPAD) << "finished process";
86​ m_lineMaker->flushBuffers();
87​ outputModel()->appendLine(i18n("Process finished with exit code %1.", exitCode));
88​ emitResult();
89​}
90​
91​void ScratchpadJob::processError(QProcess::ProcessError error)
92​{
93​ qCDebug(PLUGIN_SCRATCHPAD) << "process encountered error" << error;
brauchUnsubmitted
Done

This is always an internal error, i.e. this plugin doing something wrong. I think it should be presented to the user as such.

brauch: This is always an internal error, i.e. this plugin doing something wrong. I think it should be…
amhnduAuthorUnsubmitted
Done

"Failed to run scratch: %1" ?

amhndu: "Failed to run scratch: %1" ?
brauchUnsubmitted
Done

Something like that, yeah. It's an error which occured with the process, not an error the process itself encountered.

brauch: Something like that, yeah. It's an error which occured with the process, not an error the…
94​ outputModel()->appendLine(i18n("Failed to run scratch: %1",
95​ QLatin1String(QMetaEnum::fromType<QProcess::ProcessError>().valueToKey(error))));
96​ emitResult();
97​}
98​
99​KDevelop::OutputModel* ScratchpadJob::outputModel()
kfunkUnsubmitted
Done

const

kfunk: `const`
amhnduAuthorUnsubmitted
Not Done

see line 94 right above, can't be const

amhndu: see line 94 right above, can't be const
kfunkUnsubmitted
Not Done

Dito

kfunk: Dito
100​{
101​ return static_cast<KDevelop::OutputModel*>(model());
102​}