QCommandLineParser: pluck some low-hanging fruit re: exception safety

Make
  QCommandLineParser::add{Help,Version}Option()
  QCommandLineOption::setDefaultValue()
  QCommandLineOptionPrivate::setNames()
have transaction semantics: either they succeed, or they change nothing.
It's trivial to provide this guarantee, so do it.

Add a test for the surprising property that setDefaultValue("") resets
defaultValues() to an empty QStringList instead of one that contains
the empty string.

Change-Id: I61623019de3c7d2e52c24f42cc2e23ec5fddc4da
Reviewed-by: David Faure <david.faure@kdab.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Marc Mutz 2013-10-05 03:20:56 +02:00 committed by The Qt Project
parent 689152e7c1
commit 03affacaa3
3 changed files with 26 additions and 7 deletions

View File

@ -199,7 +199,8 @@ QStringList QCommandLineOption::names() const
void QCommandLineOptionPrivate::setNames(const QStringList &nameList)
{
names.clear();
QStringList newNames;
newNames.reserve(nameList.size());
if (nameList.isEmpty())
qWarning("QCommandLineOption: Options must have at least one name");
foreach (const QString &name, nameList) {
@ -214,9 +215,11 @@ void QCommandLineOptionPrivate::setNames(const QStringList &nameList)
else if (name.contains(QLatin1Char('=')))
qWarning("QCommandLineOption: Option names cannot contain a '='");
else
names.append(name);
newNames.append(name);
}
}
// commit
names.swap(newNames);
}
/*!
@ -288,9 +291,13 @@ QString QCommandLineOption::description() const
*/
void QCommandLineOption::setDefaultValue(const QString &defaultValue)
{
d->defaultValues.clear();
if (!defaultValue.isEmpty())
d->defaultValues << defaultValue;
QStringList newDefaultValues;
if (!defaultValue.isEmpty()) {
newDefaultValues.reserve(1);
newDefaultValues << defaultValue;
}
// commit:
d->defaultValues.swap(newDefaultValues);
}
/*!

View File

@ -280,9 +280,9 @@ bool QCommandLineParser::addOption(const QCommandLineOption &option)
*/
QCommandLineOption QCommandLineParser::addVersionOption()
{
d->builtinVersionOption = true;
QCommandLineOption opt(QStringList() << QStringLiteral("v") << QStringLiteral("version"), tr("Displays version information."));
addOption(opt);
d->builtinVersionOption = true;
return opt;
}
@ -300,7 +300,6 @@ QCommandLineOption QCommandLineParser::addVersionOption()
*/
QCommandLineOption QCommandLineParser::addHelpOption()
{
d->builtinHelpOption = true;
QCommandLineOption opt(QStringList()
#ifdef Q_OS_WIN
<< QStringLiteral("?")
@ -308,6 +307,7 @@ QCommandLineOption QCommandLineParser::addHelpOption()
<< QStringLiteral("h")
<< QStringLiteral("help"), tr("Displays this help."));
addOption(opt);
d->builtinHelpOption = true;
return opt;
}

View File

@ -69,6 +69,7 @@ private slots:
void testUnknownOptionErrorHandling();
void testDoubleDash_data();
void testDoubleDash();
void testDefaultValue();
void testProcessNotCalled();
void testEmptyArgsList();
void testMissingOptionValue();
@ -322,6 +323,17 @@ void tst_QCommandLineParser::testDoubleDash()
QCOMPARE(parser.unknownOptionNames(), QStringList());
}
void tst_QCommandLineParser::testDefaultValue()
{
QCommandLineOption opt(QStringLiteral("name"), QStringLiteral("desc"),
QStringLiteral("valueName"), QStringLiteral("default"));
QCOMPARE(opt.defaultValues(), QStringList(QStringLiteral("default")));
opt.setDefaultValue(QStringLiteral(""));
QCOMPARE(opt.defaultValues(), QStringList());
opt.setDefaultValue(QStringLiteral("default"));
QCOMPARE(opt.defaultValues(), QStringList(QStringLiteral("default")));
}
void tst_QCommandLineParser::testProcessNotCalled()
{
QCoreApplication app(empty_argc, empty_argv);