VcsBase: Introduce vcsExecWithHandler()

Before, vcsExec() returned already started VcsCommand.
Later, callers of vcsExec() were establishing connections
to the retured VcsCommand::done() signal. However, when
process fails to start (e.g. because of non-existing
executable), the done() signal may be emitted synchonously
from inside VcsCommand::start(). In this scenario
callers of VcsCommand could miss the emission of done()
signal and connect to already finished command.

Instead, provide a vcsExecWithHandler() function which
takes a handler to be called when command finished.
In addition it takes the context object, too.
Don't return VcsCommand from vcsExec() anymore.

Change-Id: I2fb5fbe5d27632ea039c650d37e5d7d1b60cebc0
Reviewed-by: Orgad Shaneh <orgads@gmail.com>
This commit is contained in:
Jarek Kobus
2022-12-08 18:50:54 +01:00
parent c08317b5a6
commit 287a7c9268
11 changed files with 193 additions and 167 deletions

View File

@@ -593,15 +593,17 @@ void BranchModel::removeTag(const QModelIndex &idx)
removeNode(idx);
}
VcsCommand *BranchModel::checkoutBranch(const QModelIndex &idx)
void BranchModel::checkoutBranch(const QModelIndex &idx, const QObject *context,
const CommandHandler &handler)
{
QString branch = fullName(idx, !isLocal(idx));
const QString branch = fullName(idx, !isLocal(idx));
if (branch.isEmpty())
return nullptr;
return;
// No StashGuard since this function for now is only used with clean working dir.
// If it is ever used from another place, please add StashGuard here
return d->client->checkout(d->workingDirectory, branch, GitClient::StashMode::NoStash);
d->client->checkout(d->workingDirectory, branch, GitClient::StashMode::NoStash,
context, handler);
}
bool BranchModel::branchIsMerged(const QModelIndex &idx)

View File

@@ -9,7 +9,9 @@
#include <optional>
namespace VcsBase { class VcsCommand; }
namespace VcsBase {
class CommandResult;
}
namespace Git::Internal {
@@ -51,7 +53,8 @@ public:
void removeBranch(const QModelIndex &idx);
void removeTag(const QModelIndex &idx);
VcsBase::VcsCommand *checkoutBranch(const QModelIndex &idx);
void checkoutBranch(const QModelIndex &idx, const QObject *context = nullptr,
const std::function<void(const VcsBase::CommandResult &)> &handler = {});
bool branchIsMerged(const QModelIndex &idx);
QModelIndex addBranch(const QString &name, bool track, const QModelIndex &trackedBranch);
void setRemoteTracking(const QModelIndex &trackingIndex);

View File

@@ -416,28 +416,25 @@ bool BranchView::checkout()
return false;
}
VcsCommand *command = m_model->checkoutBranch(selected);
const bool moveChanges = branchCheckoutDialog.moveLocalChangesToNextBranch();
const bool popStash = branchCheckoutDialog.popStashOfNextBranch();
if (command && (moveChanges || popStash)) {
connect(command, &VcsCommand::done,
this, [this, client, popMessageStart, moveChanges, popStash] {
if (moveChanges) {
client->endStashScope(m_repository);
} else if (popStash) {
QList<Stash> stashes;
QString stashName;
client->synchronousStashList(m_repository, &stashes);
for (const Stash &stash : std::as_const(stashes)) {
if (stash.message.startsWith(popMessageStart)) {
stashName = stash.name;
break;
}
const auto commandHandler = [=](const CommandResult &) {
if (moveChanges) {
client->endStashScope(m_repository);
} else if (popStash) {
QList<Stash> stashes;
QString stashName;
client->synchronousStashList(m_repository, &stashes);
for (const Stash &stash : std::as_const(stashes)) {
if (stash.message.startsWith(popMessageStart)) {
stashName = stash.name;
break;
}
client->synchronousStashRestore(m_repository, stashName, true);
}
});
}
client->synchronousStashRestore(m_repository, stashName, true);
}
};
m_model->checkoutBranch(selected, this, commandHandler);
}
if (QTC_GUARD(m_branchView))

View File

@@ -317,9 +317,7 @@ void GitBaseDiffEditorController::updateBranchList()
if (revision.isEmpty())
return;
VcsCommand *command = m_instance->vcsExec(baseDirectory(),
{"branch", noColorOption, "-a", "--contains", revision});
connect(command, &VcsCommand::done, this, [this, command] {
const auto commandHandler = [this](const CommandResult &result) {
const QString remotePrefix = "remotes/";
const QString localPrefix = "<Local>";
const int prefixLength = remotePrefix.length();
@@ -327,7 +325,7 @@ void GitBaseDiffEditorController::updateBranchList()
QStringList branches;
QString previousRemote = localPrefix;
bool first = true;
for (const QString &branch : command->cleanedStdOut().split('\n')) {
for (const QString &branch : result.cleanedStdOut().split('\n')) {
const QString b = branch.mid(2).trimmed();
if (b.isEmpty())
continue;
@@ -356,7 +354,10 @@ void GitBaseDiffEditorController::updateBranchList()
QString newDescription = description();
newDescription.replace(Constants::EXPAND_BRANCHES, branchList);
setDescription(newDescription);
});
};
m_instance->vcsExecWithHandler(baseDirectory(),
{"branch", noColorOption, "-a", "--contains", revision},
this, commandHandler, RunFlags::None, false);
}
///////////////////////////////
@@ -734,7 +735,7 @@ public:
static void handleResponse(const VcsBase::CommandResult &result,
const FilePath &workingDirectory,
const QString &abortCommand)
const QString &abortCommand = {})
{
const bool success = result.result() == ProcessResult::FinishedWithSuccess;
const QString stdOutData = success ? QString() : result.cleanedStdOut();
@@ -1107,9 +1108,9 @@ void GitClient::merge(const FilePath &workingDirectory, const QStringList &unmer
void GitClient::status(const FilePath &workingDirectory) const
{
VcsOutputWindow::setRepository(workingDirectory);
VcsCommand *command = vcsExec(workingDirectory, {"status", "-u"}, nullptr, true);
connect(command, &VcsCommand::done,
VcsOutputWindow::instance(), &VcsOutputWindow::clearRepository);
vcsExecWithHandler(workingDirectory, {"status", "-u"}, this, [](const CommandResult &) {
VcsOutputWindow::instance()->clearRepository();
});
}
static QStringList normalLogArguments()
@@ -1345,22 +1346,23 @@ VcsBaseEditorWidget *GitClient::annotate(
return editor;
}
VcsCommand *GitClient::checkout(const FilePath &workingDirectory, const QString &ref,
StashMode stashMode)
void GitClient::checkout(const FilePath &workingDirectory, const QString &ref, StashMode stashMode,
const QObject *context, const VcsBase::CommandHandler &handler)
{
if (stashMode == StashMode::TryStash && !beginStashScope(workingDirectory, "Checkout"))
return nullptr;
QStringList arguments = setupCheckoutArguments(workingDirectory, ref);
VcsCommand *command = vcsExec(
workingDirectory, arguments, nullptr, true,
RunFlags::ExpectRepoChanges | RunFlags::ShowSuccessMessage);
connect(command, &VcsCommand::done, this, [this, workingDirectory, stashMode, command] {
return;
const QStringList arguments = setupCheckoutArguments(workingDirectory, ref);
const auto commandHandler = [=](const CommandResult &result) {
if (stashMode == StashMode::TryStash)
endStashScope(workingDirectory);
if (command->result() == ProcessResult::FinishedWithSuccess)
if (result.result() == ProcessResult::FinishedWithSuccess)
updateSubmodulesIfNeeded(workingDirectory, true);
});
return command;
if (handler)
handler(result);
};
vcsExecWithHandler(workingDirectory, arguments, context, commandHandler,
RunFlags::ExpectRepoChanges | RunFlags::ShowSuccessMessage);
}
/* method used to setup arguments for checkout, in case user wants to create local branch */
@@ -1461,14 +1463,12 @@ void GitClient::reset(const FilePath &workingDirectory, const QString &argument,
void GitClient::removeStaleRemoteBranches(const FilePath &workingDirectory, const QString &remote)
{
const QStringList arguments = {"remote", "prune", remote};
VcsCommand *command = vcsExec(workingDirectory, arguments, nullptr, true,
RunFlags::ShowSuccessMessage);
connect(command, &VcsCommand::done, this, [workingDirectory, command] {
if (command->result() == ProcessResult::FinishedWithSuccess)
const auto commandHandler = [workingDirectory](const CommandResult &result) {
if (result.result() == ProcessResult::FinishedWithSuccess)
GitPlugin::updateBranches(workingDirectory);
});
};
vcsExecWithHandler(workingDirectory, arguments, this, commandHandler,
RunFlags::ShowSuccessMessage);
}
void GitClient::recoverDeletedFiles(const FilePath &workingDirectory)
@@ -2295,9 +2295,9 @@ void GitClient::updateSubmodulesIfNeeded(const FilePath &workingDirectory, bool
}
}
VcsCommand *cmd = vcsExec(workingDirectory, {"submodule", "update"}, nullptr, true,
RunFlags::ExpectRepoChanges);
connect(cmd, &VcsCommand::done, this, &GitClient::finishSubmoduleUpdate);
vcsExecWithHandler(workingDirectory, {"submodule", "update"},
this, [this](const CommandResult &) { finishSubmoduleUpdate(); },
RunFlags::ExpectRepoChanges);
}
void GitClient::finishSubmoduleUpdate()
@@ -3077,12 +3077,12 @@ void GitClient::revertFiles(const QStringList &files, bool revertStaging)
void GitClient::fetch(const FilePath &workingDirectory, const QString &remote)
{
QStringList const arguments = {"fetch", (remote.isEmpty() ? "--all" : remote)};
VcsCommand *command = vcsExec(workingDirectory, arguments, nullptr, true,
RunFlags::ShowSuccessMessage);
connect(command, &VcsCommand::done, this, [workingDirectory, command] {
if (command->result() == ProcessResult::FinishedWithSuccess)
const auto commandHandler = [workingDirectory](const CommandResult &result) {
if (result.result() == ProcessResult::FinishedWithSuccess)
GitPlugin::updateBranches(workingDirectory);
});
};
vcsExecWithHandler(workingDirectory, arguments, this, commandHandler,
RunFlags::ShowSuccessMessage);
}
bool GitClient::executeAndHandleConflicts(const FilePath &workingDirectory,
@@ -3237,101 +3237,85 @@ void GitClient::subversionDeltaCommit(const FilePath &workingDirectory) const
vcsExec(workingDirectory, {"svn", "dcommit"}, nullptr, true, RunFlags::ShowSuccessMessage);
}
class PushHandler : public QObject
enum class PushFailure { Unknown, NonFastForward, NoRemoteBranch };
static PushFailure handleError(const QString &text, QString *pushFallbackCommand)
{
public:
PushHandler(GitClient *gitClient, const FilePath &workingDir, const QStringList &pushArgs)
: m_gitClient(gitClient)
{
VcsCommand *command = gitClient->vcsExec(workingDir, QStringList({"push"}) + pushArgs,
nullptr, true, RunFlags::ShowSuccessMessage);
// Make command a parent of this in order to delete this when command is deleted
setParent(command);
connect(command, &VcsCommand::done, this, [=] {
QString pushFallbackCommand;
const PushFailure pushFailure = handleError(command->cleanedStdErr(),
&pushFallbackCommand);
if (command->result() == ProcessResult::FinishedWithSuccess) {
GitPlugin::updateCurrentBranch();
return;
}
if (pushFailure == Unknown || !m_gitClient)
return;
if (text.contains("non-fast-forward"))
return PushFailure::NonFastForward;
if (pushFailure == NonFastForward) {
const QColor warnColor = Utils::creatorTheme()->color(Theme::TextColorError);
if (QMessageBox::question(
Core::ICore::dialogParent(), Tr::tr("Force Push"),
Tr::tr("Push failed. Would you like to force-push <span style=\"color:#%1\">"
"(rewrites remote history)</span>?")
.arg(QString::number(warnColor.rgba(), 16)),
QMessageBox::Yes | QMessageBox::No,
QMessageBox::No) != QMessageBox::Yes) {
return;
}
VcsCommand *rePushCommand = m_gitClient->vcsExec(workingDir,
QStringList({"push", "--force-with-lease"}) + pushArgs,
nullptr, true, RunFlags::ShowSuccessMessage);
connect(rePushCommand, &VcsCommand::done, this, [rePushCommand] {
if (rePushCommand->result() == ProcessResult::FinishedWithSuccess)
GitPlugin::updateCurrentBranch();
});
return;
}
// NoRemoteBranch case
if (QMessageBox::question(
Core::ICore::dialogParent(), Tr::tr("No Upstream Branch"),
Tr::tr("Push failed because the local branch \"%1\" "
"does not have an upstream branch on the remote.\n\n"
"Would you like to create the branch \"%1\" on the "
"remote and set it as upstream?")
.arg(m_gitClient->synchronousCurrentLocalBranch(workingDir)),
QMessageBox::Yes | QMessageBox::No,
QMessageBox::No) != QMessageBox::Yes) {
return;
}
const QStringList fallbackCommandParts =
pushFallbackCommand.split(' ', Qt::SkipEmptyParts);
VcsCommand *rePushCommand = m_gitClient->vcsExec(workingDir,
fallbackCommandParts.mid(1), nullptr, true, RunFlags::ShowSuccessMessage);
connect(rePushCommand, &VcsCommand::done, this, [workingDir, rePushCommand] {
if (rePushCommand->result() == ProcessResult::FinishedWithSuccess)
GitPlugin::updateBranches(workingDir);
});
});
}
private:
enum PushFailure { Unknown, NonFastForward, NoRemoteBranch };
PushFailure handleError(const QString &text, QString *pushFallbackCommand) const {
if (text.contains("non-fast-forward"))
return NonFastForward;
if (text.contains("has no upstream branch")) {
const QStringList lines = text.split('\n', Qt::SkipEmptyParts);
for (const QString &line : lines) {
/* Extract the suggested command from the git output which
if (text.contains("has no upstream branch")) {
const QStringList lines = text.split('\n', Qt::SkipEmptyParts);
for (const QString &line : lines) {
/* Extract the suggested command from the git output which
* should be similar to the following:
*
* git push --set-upstream origin add_set_upstream_dialog
*/
const QString trimmedLine = line.trimmed();
if (trimmedLine.startsWith("git push")) {
*pushFallbackCommand = trimmedLine;
break;
}
const QString trimmedLine = line.trimmed();
if (trimmedLine.startsWith("git push")) {
*pushFallbackCommand = trimmedLine;
break;
}
return NoRemoteBranch;
}
return Unknown;
};
QPointer<GitClient> m_gitClient;
return PushFailure::NoRemoteBranch;
}
return PushFailure::Unknown;
};
void GitClient::push(const FilePath &workingDirectory, const QStringList &pushArgs)
{
new PushHandler(this, workingDirectory, pushArgs);
const auto commandHandler = [=](const CommandResult &result) {
QString pushFallbackCommand;
const PushFailure pushFailure = handleError(result.cleanedStdErr(),
&pushFallbackCommand);
if (result.result() == ProcessResult::FinishedWithSuccess) {
GitPlugin::updateCurrentBranch();
return;
}
if (pushFailure == PushFailure::Unknown)
return;
if (pushFailure == PushFailure::NonFastForward) {
const QColor warnColor = Utils::creatorTheme()->color(Theme::TextColorError);
if (QMessageBox::question(
Core::ICore::dialogParent(), Tr::tr("Force Push"),
Tr::tr("Push failed. Would you like to force-push <span style=\"color:#%1\">"
"(rewrites remote history)</span>?")
.arg(QString::number(warnColor.rgba(), 16)),
QMessageBox::Yes | QMessageBox::No,
QMessageBox::No) != QMessageBox::Yes) {
return;
}
const auto commandHandler = [](const CommandResult &result) {
if (result.result() == ProcessResult::FinishedWithSuccess)
GitPlugin::updateCurrentBranch();
};
vcsExecWithHandler(workingDirectory, QStringList{"push", "--force-with-lease"} + pushArgs,
this, commandHandler, RunFlags::ShowSuccessMessage);
return;
}
// NoRemoteBranch case
if (QMessageBox::question(
Core::ICore::dialogParent(), Tr::tr("No Upstream Branch"),
Tr::tr("Push failed because the local branch \"%1\" "
"does not have an upstream branch on the remote.\n\n"
"Would you like to create the branch \"%1\" on the "
"remote and set it as upstream?")
.arg(synchronousCurrentLocalBranch(workingDirectory)),
QMessageBox::Yes | QMessageBox::No, QMessageBox::No) != QMessageBox::Yes) {
return;
}
const QStringList fallbackCommandParts = pushFallbackCommand.split(' ', Qt::SkipEmptyParts);
const auto commandHandler = [workingDirectory](const CommandResult &result) {
if (result.result() == ProcessResult::FinishedWithSuccess)
GitPlugin::updateBranches(workingDirectory);
};
vcsExecWithHandler(workingDirectory, fallbackCommandParts.mid(1),
this, commandHandler, RunFlags::ShowSuccessMessage);
};
vcsExecWithHandler(workingDirectory, QStringList({"push"}) + pushArgs, this, commandHandler,
RunFlags::ShowSuccessMessage);
}
bool GitClient::synchronousMerge(const FilePath &workingDirectory, const QString &branch,
@@ -3393,7 +3377,7 @@ VcsCommand *GitClient::vcsExecAbortable(const FilePath &workingDirectory,
if (isRebase)
command->setProgressParser(GitProgressParser());
command->start();
// TODO: Don't return command, take handler arg
return command;
}
@@ -3450,9 +3434,11 @@ void GitClient::stashPop(const FilePath &workingDirectory, const QString &stash)
QStringList arguments = {"stash", "pop"};
if (!stash.isEmpty())
arguments << stash;
VcsCommand *cmd = vcsExec(workingDirectory, arguments, nullptr, true,
RunFlags::ExpectRepoChanges);
ConflictHandler::attachToCommand(cmd, workingDirectory);
const auto commandHandler = [workingDirectory](const CommandResult &result) {
ConflictHandler::handleResponse(result, workingDirectory);
};
vcsExecWithHandler(workingDirectory, arguments, this, commandHandler,
RunFlags::ExpectRepoChanges);
}
bool GitClient::synchronousStashRestore(const FilePath &workingDirectory,

View File

@@ -181,8 +181,9 @@ public:
QString revision = {}, QString *errorMessage = nullptr,
bool revertStaging = true);
enum class StashMode { NoStash, TryStash };
VcsBase::VcsCommand *checkout(const Utils::FilePath &workingDirectory, const QString &ref,
StashMode stashMode = StashMode::TryStash);
void checkout(const Utils::FilePath &workingDirectory, const QString &ref,
StashMode stashMode = StashMode::TryStash, const QObject *context = nullptr,
const VcsBase::CommandHandler &handler = {});
QStringList setupCheckoutArguments(const Utils::FilePath &workingDirectory, const QString &ref);
void updateSubmodulesIfNeeded(const Utils::FilePath &workingDirectory, bool prompt);

View File

@@ -1572,19 +1572,19 @@ void GitPluginPrivate::instantBlame()
const QFileInfo fi(filePath.toString());
const Utils::FilePath workingDirectory = Utils::FilePath::fromString(fi.path());
const QString lineString = QString("%1,%1").arg(line);
const VcsCommand *command = GitClient::instance()->vcsExec(
workingDirectory, {"blame", "-p", "-L", lineString, "--", filePath.toString()},
nullptr, false, RunFlags::NoOutput);
connect(command, &VcsCommand::done, this, [command, filePath, line, this] {
if (command->result() == ProcessResult::FinishedWithError &&
command->cleanedStdErr().contains("no such path")) {
const auto commandHandler = [this, filePath, line](const CommandResult &result) {
if (result.result() == ProcessResult::FinishedWithError &&
result.cleanedStdErr().contains("no such path")) {
disconnect(m_blameCursorPosConn);
return;
}
const QString output = command->cleanedStdOut();
const QString output = result.cleanedStdOut();
const CommitInfo info = parseBlameOutput(output.split('\n'), filePath, m_author);
m_blameMark.reset(new BlameMark(filePath, line, info));
});
};
GitClient::instance()->vcsExecWithHandler(workingDirectory,
{"blame", "-p", "-L", lineString, "--", filePath.toString()},
this, commandHandler, RunFlags::NoOutput, false);
}
void GitPluginPrivate::stopInstantBlame()

View File

@@ -203,6 +203,7 @@ void GitSubmitEditor::updateFileModel()
if (w->updateInProgress() || m_workingDirectory.isEmpty())
return;
w->setUpdateInProgress(true);
// TODO: Check if fetch works OK from separate thread, refactor otherwise
m_fetchWatcher.setFuture(Utils::runAsync(&CommitDataFetchResult::fetch,
m_commitType, m_workingDirectory));
Core::ProgressManager::addTask(m_fetchWatcher.future(), Tr::tr("Refreshing Commit Data"),

View File

@@ -159,18 +159,37 @@ void VcsBaseClientImpl::annotateRevisionRequested(const FilePath &workingDirecto
annotate(workingDirectory, file, changeCopy, line);
}
VcsCommand *VcsBaseClientImpl::vcsExec(const FilePath &workingDirectory,
const QStringList &arguments,
VcsBaseEditorWidget *editor, bool useOutputToWindow,
RunFlags additionalFlags) const
void VcsBaseClientImpl::vcsExecWithHandler(const FilePath &workingDirectory,
const QStringList &arguments,
const QObject *context,
const CommandHandler &handler,
RunFlags additionalFlags,
bool useOutputToWindow) const
{
VcsCommand *command = createCommand(workingDirectory, nullptr,
useOutputToWindow ? VcsWindowOutputBind : NoOutputBind);
command->addFlags(additionalFlags);
command->addJob({vcsBinary(), arguments}, vcsTimeoutS());
if (handler) {
connect(command, &VcsCommand::done, context, [command, handler] {
handler(CommandResult(*command));
});
}
command->start();
}
void VcsBaseClientImpl::vcsExec(const FilePath &workingDirectory,
const QStringList &arguments,
VcsBaseEditorWidget *editor, bool useOutputToWindow,
RunFlags additionalFlags) const
{
VcsCommand *command = createCommand(workingDirectory, editor,
useOutputToWindow ? VcsWindowOutputBind : NoOutputBind);
command->addFlags(additionalFlags);
if (editor)
command->setCodec(editor->codec());
enqueueJob(command, arguments);
return command;
command->addJob({vcsBinary(), arguments}, vcsTimeoutS());
command->start();
}
int VcsBaseClientImpl::vcsTimeoutS() const

View File

@@ -30,6 +30,8 @@ class VcsBaseEditorConfig;
class VcsBaseEditorWidget;
class VcsCommand;
using CommandHandler = std::function<void(const CommandResult &)>;
class VCSBASE_EXPORT VcsBaseClientImpl : public QObject
{
Q_OBJECT
@@ -85,12 +87,18 @@ public:
RunFlags flags = RunFlags::None,
int timeoutS = -1, QTextCodec *codec = nullptr) const;
void vcsExecWithHandler(const Utils::FilePath &workingDirectory,
const QStringList &arguments,
const QObject *context,
const CommandHandler &handler,
RunFlags additionalFlags = RunFlags::None,
bool useOutputToWindow = true) const;
// Simple helper to execute a single command using createCommand and enqueueJob.
VcsCommand *vcsExec(const Utils::FilePath &workingDirectory,
const QStringList &arguments,
VcsBaseEditorWidget *editor = nullptr,
bool useOutputToWindow = false,
RunFlags additionalFlags = RunFlags::None) const;
void vcsExec(const Utils::FilePath &workingDirectory,
const QStringList &arguments,
VcsBaseEditorWidget *editor = nullptr,
bool useOutputToWindow = false,
RunFlags additionalFlags = RunFlags::None) const;
protected:
void resetCachedVcsInfo(const Utils::FilePath &workingDir);

View File

@@ -333,4 +333,10 @@ CommandResult::CommandResult(const QtcProcess &process)
, m_rawStdOut(process.rawStdOut())
{}
CommandResult::CommandResult(const VcsCommand &command)
: m_result(command.result())
, m_cleanedStdOut(command.cleanedStdOut())
, m_cleanedStdErr(command.cleanedStdErr())
{}
} // namespace VcsBase

View File

@@ -27,11 +27,14 @@ namespace VcsBase {
namespace Internal { class VcsCommandPrivate; }
class VcsCommand;
class VCSBASE_EXPORT CommandResult
{
public:
CommandResult() = default;
CommandResult(const Utils::QtcProcess &process);
CommandResult(const VcsCommand &command);
CommandResult(Utils::ProcessResult result, const QString &exitMessage)
: m_result(result), m_exitMessage(exitMessage) {}