Skip to content

Commit

Permalink
nvim-qt --help/version should work without $DISPLAY
Browse files Browse the repository at this point in the history 
The intent is to split command line handling from the App instance, so
that we can do it using a QCoreApplication and QCommandLineParser. To
achieve this i made multiple functions in the App class static.

Added a small test for #707 i.e. the following CLI invocations should
work without a $DISPLAY variable:

    --version
    --help

I've left --help-all out of this, since I cant get it to behave
reliably.
  • Loading branch information
equalsraf committed Jun 20, 2020
1 parent f561863 commit fab5ba3
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 43 deletions.
68 changes: 34 additions & 34 deletions src/gui/app.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ App::App(int &argc, char ** argv) noexcept
}
}

processCommandlineOptions();
processCommandlineOptions(m_parser, arguments());
}

bool App::event(QEvent *event) noexcept
Expand Down Expand Up @@ -149,97 +149,97 @@ void App::showUi() noexcept
///
/// When appropriate this function will call QCommandLineParser::showHelp()
/// terminating the program.
void App::processCommandlineOptions() noexcept
void App::processCommandlineOptions(QCommandLineParser& parser, QStringList arguments) noexcept
{
m_parser.addOption(QCommandLineOption("nvim",
parser.addOption(QCommandLineOption("nvim",
QCoreApplication::translate("main", "nvim executable path"),
QCoreApplication::translate("main", "nvim_path"),
"nvim"));
m_parser.addOption(QCommandLineOption("timeout",
parser.addOption(QCommandLineOption("timeout",
QCoreApplication::translate("main", "Error if nvim does not responde after count milliseconds"),
QCoreApplication::translate("main", "ms"),
"20000"));

// Some platforms use --qwindowgeometry, while other platforms use the --geometry.
// Make the correct help message is displayed.
if (hasGeometryArg()) {
m_parser.addOption(QCommandLineOption("geometry",
parser.addOption(QCommandLineOption("geometry",
QCoreApplication::translate("main", "Set initial window geometry"),
QCoreApplication::translate("main", "width>x<height")));
}
if (hasQWindowGeometryArg()) {
m_parser.addOption(QCommandLineOption("qwindowgeometry",
parser.addOption(QCommandLineOption("qwindowgeometry",
QCoreApplication::translate("main", "Set initial window geometry"),
QCoreApplication::translate("main", "width>x<height")));
}

m_parser.addOption(QCommandLineOption("stylesheet",
parser.addOption(QCommandLineOption("stylesheet",
QCoreApplication::translate("main", "Apply qss stylesheet from file"),
QCoreApplication::translate("main", "stylesheet")));
m_parser.addOption(QCommandLineOption("maximized",
parser.addOption(QCommandLineOption("maximized",
QCoreApplication::translate("main", "Maximize the window on startup")));
m_parser.addOption(QCommandLineOption("no-ext-tabline",
parser.addOption(QCommandLineOption("no-ext-tabline",
QCoreApplication::translate("main", "Disable the external GUI tabline")));
m_parser.addOption(QCommandLineOption("no-ext-popupmenu",
parser.addOption(QCommandLineOption("no-ext-popupmenu",
QCoreApplication::translate("main", "Disable the external GUI popup menu")));
m_parser.addOption(QCommandLineOption("fullscreen",
parser.addOption(QCommandLineOption("fullscreen",
QCoreApplication::translate("main", "Open the window in fullscreen on startup")));
m_parser.addOption(QCommandLineOption("embed",
parser.addOption(QCommandLineOption("embed",
QCoreApplication::translate("main", "Communicate with Neovim over stdin/out")));
m_parser.addOption(QCommandLineOption("server",
parser.addOption(QCommandLineOption("server",
QCoreApplication::translate("main", "Connect to existing Neovim instance"),
QCoreApplication::translate("main", "addr")));
m_parser.addOption(QCommandLineOption("spawn",
parser.addOption(QCommandLineOption("spawn",
QCoreApplication::translate("main", "Treat positional arguments as the nvim argv")));
m_parser.addOption(QCommandLineOption({ "v", "version" },
parser.addOption(QCommandLineOption({ "v", "version" },
QCoreApplication::translate("main", "Displays version information.")));

m_parser.addHelpOption();
parser.addHelpOption();

#ifdef Q_OS_UNIX
m_parser.addOption(QCommandLineOption("nofork",
parser.addOption(QCommandLineOption("nofork",
QCoreApplication::translate("main", "Run in foreground")));
#endif

m_parser.addPositionalArgument("file",
parser.addPositionalArgument("file",
QCoreApplication::translate("main", "Edit specified file(s)"), "[file...]");
m_parser.addPositionalArgument("...", "Additional arguments are forwarded to Neovim", "[-- ...]");
parser.addPositionalArgument("...", "Additional arguments are forwarded to Neovim", "[-- ...]");

m_parser.process(arguments());
parser.process(arguments);
}

void App::checkArgumentsMayTerminate() noexcept
void App::checkArgumentsMayTerminate(QCommandLineParser& parser) noexcept
{
if (m_parser.isSet("help")) {
m_parser.showHelp();
if (parser.isSet("help")) {
parser.showHelp();
}

if (m_parser.isSet("version")) {
showVersionInfo();
if (parser.isSet("version")) {
showVersionInfo(parser);
::exit(0);
}

int exclusive = m_parser.isSet("server") + m_parser.isSet("embed") + m_parser.isSet("spawn");
int exclusive = parser.isSet("server") + parser.isSet("embed") + parser.isSet("spawn");
if (exclusive > 1) {
qWarning() << "Options --server, --spawn and --embed are mutually exclusive\n";
::exit(-1);
}

if (!m_parser.positionalArguments().isEmpty() &&
(m_parser.isSet("embed") || m_parser.isSet("server"))) {
if (!parser.positionalArguments().isEmpty() &&
(parser.isSet("embed") || parser.isSet("server"))) {
qWarning() << "--embed and --server do not accept positional arguments\n";
::exit(-1);
}

if (m_parser.positionalArguments().isEmpty() && m_parser.isSet("spawn")) {
if (parser.positionalArguments().isEmpty() && parser.isSet("spawn")) {
qWarning() << "--spawn requires at least one positional argument\n";
::exit(-1);
}

bool valid_timeout;
int timeout_opt = m_parser.value("timeout").toInt(&valid_timeout);
int timeout_opt = parser.value("timeout").toInt(&valid_timeout);
if (!valid_timeout || timeout_opt <= 0) {
qWarning() << "Invalid argument for --timeout" << m_parser.value("timeout");
qWarning() << "Invalid argument for --timeout" << parser.value("timeout");
::exit(-1);
}
}
Expand Down Expand Up @@ -337,13 +337,13 @@ static QString GetNeovimVersionInfo(const QString& nvim) noexcept
return nvimproc.readAllStandardOutput();
}

void App::showVersionInfo() noexcept
void App::showVersionInfo(QCommandLineParser& parser) noexcept
{
QString versionInfo;
QTextStream out{ &versionInfo };

const QString nvimExecutable { (m_parser.isSet("nvim")) ?
m_parser.value("nvim") : "nvim" };
const QString nvimExecutable { (parser.isSet("nvim")) ?
parser.value("nvim") : "nvim" };

out << "NVIM-QT v" << PROJECT_VERSION << endl;
out << "Build type: " << CMAKE_BUILD_TYPE << endl;
Expand Down
11 changes: 6 additions & 5 deletions src/gui/app.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ class App: public QApplication
bool event(QEvent *event) noexcept;
void showUi() noexcept;
void connectToRemoteNeovim() noexcept;
void checkArgumentsMayTerminate() noexcept;
QCommandLineParser& commandLineParser() { return m_parser; }
static void checkArgumentsMayTerminate(QCommandLineParser&) noexcept;
static void processCommandlineOptions(QCommandLineParser&, QStringList) noexcept;

private:
QString getRuntimePath() noexcept;
QStringList getNeovimArgs() noexcept;
void processCommandlineOptions() noexcept;
static QString getRuntimePath() noexcept;
static QStringList getNeovimArgs() noexcept;
void setupRequestTimeout() noexcept;
void showVersionInfo() noexcept;
static void showVersionInfo(QCommandLineParser&) noexcept;

QCommandLineParser m_parser;
std::shared_ptr<NeovimConnector> m_connector;
Expand Down
9 changes: 5 additions & 4 deletions src/gui/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ int ui_main(int argc, char **argv)

NeovimQt::App app(argc, argv);

app.checkArgumentsMayTerminate();
app.checkArgumentsMayTerminate(app.commandLineParser());

app.connectToRemoteNeovim();

Expand All @@ -51,9 +51,10 @@ int cli_main(int argc, char **argv)
argsNoFork << argv[i];
}

NeovimQt::App app(argc, argv);

app.checkArgumentsMayTerminate();
QCoreApplication app(argc, argv);
QCommandLineParser p;
NeovimQt::App::processCommandlineOptions(p, app.arguments());
NeovimQt::App::checkArgumentsMayTerminate(p);

if (!QProcess::startDetached(app.applicationFilePath(), argsNoFork)) {
qWarning() << "Unable to fork into background";
Expand Down
2 changes: 2 additions & 0 deletions test/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ set(QTLIBS Qt5::Network Qt5::Svg Qt5::Test)

include_directories(${CMAKE_SOURCE_DIR}/src)
add_definitions(-DCMAKE_SOURCE_DIR=\"${CMAKE_SOURCE_DIR}\")
add_definitions(-DNVIM_QT_BINARY=\"$<TARGET_FILE:nvim-qt>\")
if (WIN32 AND USE_STATIC_QT)
add_definitions(-DUSE_STATIC_QT)
endif ()
Expand Down Expand Up @@ -37,6 +38,7 @@ add_xtest(tst_callallmethods)
add_xtest(tst_encoding)
add_xtest(tst_msgpackiodevice)
add_xtest_gui(tst_shell)
add_xtest_gui(tst_main)

# Platform Specific Input Tests
add_xtest(tst_input_mac
Expand Down
46 changes: 46 additions & 0 deletions test/tst_main.cpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#include <QtGlobal>
#include <QProcess>
#include <QtTest/QtTest>

namespace NeovimQt {

class Test: public QObject
{
Q_OBJECT
private slots:

void runsWithoutDISPLAY_data() {
QTest::addColumn<QStringList>("arguments");
QTest::addColumn<int>("expected_exitcode");

QTest::newRow("--help") << QStringList({NVIM_QT_BINARY, "--help"}) << 0;
QTest::newRow("--version") << QStringList({NVIM_QT_BINARY, "--help"}) << 0;
}

void runsWithoutDISPLAY() {
QFETCH(QStringList, arguments);
QFETCH(int, expected_exitcode);

QProcess p;
QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
env.remove("DISPLAY");
p.setProcessEnvironment(env);
p.setProgram(NVIM_QT_BINARY);
p.setArguments(arguments);
p.start();
bool finished = p.waitForFinished();
auto status = p.exitStatus();
int exit_code = p.exitCode();

qDebug() << finished << status << exit_code;
QCOMPARE(finished, true);
QCOMPARE(status, QProcess::NormalExit);
QCOMPARE(exit_code, expected_exitcode);
}

protected:
};

} // Namespace NeovimQt
QTEST_MAIN(NeovimQt::Test)
#include "tst_main.moc"

0 comments on commit fab5ba3

Please sign in to comment.