Git: Proliferate FilePath use in GerritModel a bit

Plus some code cosmetics.

Change-Id: Idc07e881c09cbc7347396ef1bb60cce00539250c
Reviewed-by: Orgad Shaneh <orgads@gmail.com>

src/libs/utils/pathchooser.cpp

@@ -25,6 +25,7 @@
 
 #include "pathchooser.h"
 
+#include "commandline.h"
 #include "environment.h"
 #include "hostosinfo.h"
 #include "macroexpander.h"
@@ -718,9 +719,9 @@ FancyLineEdit *PathChooser::lineEdit() const
     return d->m_lineEdit;
 }
 
-QString PathChooser::toolVersion(const QString &binary, const QStringList &arguments)
+QString PathChooser::toolVersion(const CommandLine &cmd)
 {
-    return BinaryVersionToolTipEventFilter::toolVersion({FilePath::fromString(binary), arguments});
+    return BinaryVersionToolTipEventFilter::toolVersion(cmd);
 }
 
 void PathChooser::installLineEditVersionToolTip(QLineEdit *le, const QStringList &arguments)

src/libs/utils/pathchooser.h

@@ -38,6 +38,7 @@ QT_END_NAMESPACE
 
 namespace Utils {
 
+class CommandLine;
 class FancyLineEdit;
 class MacroExpander;
 class Environment;
@@ -124,7 +125,7 @@ public:
     void setCommandVersionArguments(const QStringList &arguments);
 
     // Utility to run a tool and return its stdout.
-    static QString toolVersion(const QString &binary, const QStringList &arguments);
+    static QString toolVersion(const Utils::CommandLine &cmd);
     // Install a tooltip on lineedits used for binaries showing the version.
     static void installLineEditVersionToolTip(QLineEdit *le, const QStringList &arguments);
 

src/plugins/git/gerrit/gerritmodel.cpp

@@ -251,9 +251,9 @@ private:
 
     void errorTermination(const QString &msg);
 
-    Utils::QtcProcess m_process;
+    QtcProcess m_process;
     QTimer m_timer;
-    QString m_binary;
+    FilePath m_binary;
     QByteArray m_output;
     QString m_error;
     QFutureInterface<void> m_progress;
@@ -321,10 +321,9 @@ void QueryContext::start()
     fp->setKeepOnFinish(Core::FutureProgress::HideOnFinish);
     m_progress.reportStarted();
     // Order: synchronous call to error handling if something goes wrong.
-    VcsOutputWindow::appendCommand(m_process.workingDirectory(),
+    VcsOutputWindow::appendCommand(m_process.workingDirectory(), {m_binary, m_arguments});
-                                   {FilePath::fromString(m_binary), m_arguments});
     m_timer.start();
-    m_process.setCommand({FilePath::fromString(m_binary), m_arguments});
+    m_process.setCommand({m_binary, m_arguments});
     m_process.start();
 }
 
@@ -344,7 +343,7 @@ void QueryContext::terminate()
 
 void QueryContext::processError(QProcess::ProcessError e)
 {
-    const QString msg = tr("Error running %1: %2").arg(m_binary, m_process.errorString());
+    const QString msg = tr("Error running %1: %2").arg(m_binary.toUserOutput(), m_process.errorString());
     if (e == QProcess::FailedToStart)
         errorTermination(msg);
     else
@@ -357,10 +356,10 @@ void QueryContext::processFinished()
         m_timer.stop();
     emit errorText(m_error);
     if (m_process.exitStatus() != QProcess::NormalExit) {
-        errorTermination(tr("%1 crashed.").arg(m_binary));
+        errorTermination(tr("%1 crashed.").arg(m_binary.toUserOutput()));
         return;
     } else if (m_process.exitCode()) {
-        errorTermination(tr("%1 returned %2.").arg(m_binary).arg(m_process.exitCode()));
+        errorTermination(tr("%1 returned %2.").arg(m_binary.toUserOutput()).arg(m_process.exitCode()));
         return;
     }
     emit resultRetrieved(m_output);

src/plugins/git/gerrit/gerritoptionspage.cpp

@@ -124,8 +124,8 @@ GerritParameters GerritOptionsWidget::parameters() const
                                  static_cast<unsigned short>(m_portSpinBox->value()),
                                  m_userLineEdit->text().trimmed(),
                                  GerritServer::Ssh);
-    result.ssh = m_sshChooser->filePath().toString();
+    result.ssh = m_sshChooser->filePath();
-    result.curl = m_curlChooser->filePath().toString();
+    result.curl = m_curlChooser->filePath();
     result.https = m_httpsCheckBox->isChecked();
     return result;
 }
@@ -134,8 +134,8 @@ void GerritOptionsWidget::setParameters(const GerritParameters &p)
 {
     m_hostLineEdit->setText(p.server.host);
     m_userLineEdit->setText(p.server.user.userName);
-    m_sshChooser->setPath(p.ssh);
+    m_sshChooser->setFilePath(p.ssh);
-    m_curlChooser->setPath(p.curl);
+    m_curlChooser->setFilePath(p.curl);
     m_portSpinBox->setValue(p.server.port);
     m_httpsCheckBox->setChecked(p.https);
 }

src/plugins/git/gerrit/gerritparameters.cpp

@@ -26,6 +26,7 @@
 #include "gerritparameters.h"
 #include "gerritplugin.h"
 
+#include <utils/commandline.h>
 #include <utils/hostosinfo.h>
 #include <utils/pathchooser.h>
 
@@ -38,52 +39,51 @@ using namespace Utils;
 namespace Gerrit {
 namespace Internal {
 
-static const char settingsGroupC[] = "Gerrit";
+const char settingsGroupC[] = "Gerrit";
-static const char hostKeyC[] = "Host";
+const char hostKeyC[] = "Host";
-static const char userKeyC[] = "User";
+const char userKeyC[] = "User";
-static const char portKeyC[] = "Port";
+const char portKeyC[] = "Port";
-static const char portFlagKeyC[] = "PortFlag";
+const char portFlagKeyC[] = "PortFlag";
-static const char sshKeyC[] = "Ssh";
+const char sshKeyC[] = "Ssh";
-static const char curlKeyC[] = "Curl";
+const char curlKeyC[] = "Curl";
-static const char httpsKeyC[] = "Https";
+const char httpsKeyC[] = "Https";
-static const char savedQueriesKeyC[] = "SavedQueries";
+const char savedQueriesKeyC[] = "SavedQueries";
 
-static const char defaultPortFlag[] = "-p";
+const char defaultPortFlag[] = "-p";
 
-static inline QString detectApp(const char *defaultExe)
+static FilePath detectApp(const QString &defaultExe)
 {
-    const QString defaultApp = HostOsInfo::withExecutableSuffix(QLatin1String(defaultExe));
+    const QString defaultApp = HostOsInfo::withExecutableSuffix(defaultExe);
-    QString app = QStandardPaths::findExecutable(defaultApp);
+    const QString app = QStandardPaths::findExecutable(defaultApp);
     if (!app.isEmpty() || !HostOsInfo::isWindowsHost())
-        return app;
+        return FilePath::fromString(app);
     // Windows: Use app.exe from git if it cannot be found.
     const FilePath gitBinDir = GerritPlugin::gitBinDirectory();
     if (gitBinDir.isEmpty())
-        return QString();
+        return {};
     FilePath path = gitBinDir.pathAppended(defaultApp);
     if (path.exists())
-        return path.toString();
+        return path;
 
     // If app was not found, and git bin is Git/usr/bin (Git for Windows),
     // search also in Git/mingw{32,64}/bin
     if (!gitBinDir.endsWith("/usr/bin"))
-        return QString();
+        return {};
     path = gitBinDir.parentDir().parentDir();
-    QDir dir(path.toString());
+    const FilePaths entries = path.dirEntries({"mingw*"}, {});
-    const QStringList entries = dir.entryList({"mingw*"});
     if (entries.isEmpty())
-        return QString();
+        return {};
-    path = path / entries.first() / "bin" / defaultApp;
+    path = entries.first() / "bin" / defaultApp;
     if (path.exists())
-        return path.toString();
+        return path;
-    return QString();
+    return {};
 }
 
-static inline QString detectSsh()
+static FilePath detectSsh()
 {
-    const QByteArray gitSsh = qgetenv("GIT_SSH");
+    const QString gitSsh = qEnvironmentVariable("GIT_SSH");
     if (!gitSsh.isEmpty())
-        return QString::fromLocal8Bit(gitSsh);
+        return FilePath::fromString(gitSsh);
     return detectApp("ssh");
 }
 
@@ -91,7 +91,7 @@ void GerritParameters::setPortFlagBySshType()
 {
     bool isPlink = false;
     if (!ssh.isEmpty()) {
-        const QString version = PathChooser::toolVersion(ssh, {"-V"});
+        const QString version = PathChooser::toolVersion({ssh, {"-V"}});
         isPlink = version.contains("plink", Qt::CaseInsensitive);
     }
     portFlag = QLatin1String(isPlink ? "-P" : defaultPortFlag);
@@ -114,8 +114,8 @@ void GerritParameters::toSettings(QSettings *s) const
     s->setValue(userKeyC, server.user.userName);
     s->setValue(portKeyC, server.port);
     s->setValue(portFlagKeyC, portFlag);
-    s->setValue(sshKeyC, ssh);
+    s->setValue(sshKeyC, ssh.toVariant());
-    s->setValue(curlKeyC, curl);
+    s->setValue(curlKeyC, curl.toVariant());
     s->setValue(httpsKeyC, https);
     s->endGroup();
 }
@@ -132,16 +132,16 @@ void GerritParameters::fromSettings(const QSettings *s)
     const QString rootKey = QLatin1String(settingsGroupC) + '/';
     server.host = s->value(rootKey + hostKeyC, GerritServer::defaultHost()).toString();
     server.user.userName = s->value(rootKey + userKeyC, QString()).toString();
-    ssh = s->value(rootKey + sshKeyC, QString()).toString();
+    ssh = FilePath::fromVariant(s->value(rootKey + sshKeyC, QString()));
-    curl = s->value(rootKey + curlKeyC).toString();
+    curl = FilePath::fromVariant(s->value(rootKey + curlKeyC));
     server.port = ushort(s->value(rootKey + portKeyC, QVariant(GerritServer::defaultPort)).toInt());
     portFlag = s->value(rootKey + portFlagKeyC, defaultPortFlag).toString();
     savedQueries = s->value(rootKey + savedQueriesKeyC, QString()).toString()
             .split(',');
     https = s->value(rootKey + httpsKeyC, QVariant(true)).toBool();
-    if (ssh.isEmpty() || !QFile::exists(ssh))
+    if (ssh.isEmpty() || !ssh.exists())
         ssh = detectSsh();
-    if (curl.isEmpty() || !QFile::exists(curl))
+    if (curl.isEmpty() || !curl.exists())
         curl = detectApp("curl");
 }
 

src/plugins/git/gerrit/gerritparameters.h

@@ -27,7 +27,7 @@
 
 #include "gerritserver.h"
 
-#include <QStringList>
+#include <utils/filepath.h>
 
 QT_FORWARD_DECLARE_CLASS(QSettings)
 
@@ -47,8 +47,8 @@ public:
     void setPortFlagBySshType();
 
     GerritServer server;
-    QString ssh;
+    Utils::FilePath ssh;
-    QString curl;
+    Utils::FilePath curl;
     QStringList savedQueries;
     bool https = true;
     QString portFlag;

src/plugins/git/gerrit/gerritserver.cpp

@@ -161,7 +161,7 @@ bool GerritServer::fillFromRemote(const QString &remote,
         return true;
     }
     curlBinary = parameters.curl;
-    if (curlBinary.isEmpty() || !QFile::exists(curlBinary))
+    if (curlBinary.isEmpty() || !curlBinary.exists())
         return false;
     const StoredHostValidity validity = forceReload ? Invalid : loadSettings();
     switch (validity) {
@@ -245,7 +245,7 @@ int GerritServer::testConnection()
     static GitClient *const client = GitClient::instance();
     const QStringList arguments = curlArguments() << (url(RestUrl) + accountUrlC);
     QtcProcess proc;
-    client->vcsFullySynchronousExec(proc, {}, {FilePath::fromString(curlBinary), arguments},
+    client->vcsFullySynchronousExec(proc, {}, {curlBinary, arguments},
                                     Core::ShellCommand::NoOutput);
     if (proc.result() == QtcProcess::FinishedWithSuccess) {
         QString output = proc.stdOut();
@@ -346,15 +346,14 @@ void GerritServer::resolveVersion(const GerritParameters &p, bool forceReload)
         if (port)
             arguments << p.portFlag << QString::number(port);
         arguments << hostArgument() << "gerrit" << "version";
-        client->vcsFullySynchronousExec(proc, {}, {FilePath::fromString(p.ssh), arguments},
+        client->vcsFullySynchronousExec(proc, {}, {p.ssh, arguments}, Core::ShellCommand::NoOutput);
-                                        Core::ShellCommand::NoOutput);
         QString stdOut = proc.stdOut().trimmed();
         stdOut.remove("gerrit version ");
         version = stdOut;
     } else {
         const QStringList arguments = curlArguments() << (url(RestUrl) + versionUrlC);
         QtcProcess proc;
-        client->vcsFullySynchronousExec(proc, {}, {FilePath::fromString(curlBinary), arguments},
+        client->vcsFullySynchronousExec(proc, {}, {curlBinary, arguments},
                                         Core::ShellCommand::NoOutput);
         // REST endpoint for version is only available from 2.8 and up. Do not consider invalid
         // if it fails.

src/plugins/git/gerrit/gerritserver.h

@@ -25,7 +25,7 @@
 
 #pragma once
 
-#include <QStringList>
+#include <utils/filepath.h>
 
 namespace Gerrit {
 namespace Internal {
@@ -88,7 +88,7 @@ public:
     bool validateCert = true;
 
 private:
-    QString curlBinary;
+    Utils::FilePath curlBinary;
     StoredHostValidity loadSettings();
     void saveSettings(StoredHostValidity validity) const;
     bool setupAuthentication();