Browse Source

Prevent use of static variables in ArgumentParser

Have --no-color and --help added by default rather
so argument parser is in control over them and don't
has to use static functions.
sendfile
Martchus 5 years ago
parent
commit
bc8ea407bc
  1. 24
      application/argumentparser.cpp
  2. 117
      application/argumentparser.h
  3. 37
      tests/argumentparsertests.cpp
  4. 7
      tests/testutils.cpp
  5. 1
      tests/testutils.h

24
application/argumentparser.cpp

@ -714,6 +714,7 @@ ArgumentParser::ArgumentParser()
, m_executable(nullptr)
, m_unknownArgBehavior(UnknownArgumentBehavior::Fail)
, m_defaultArg(nullptr)
, m_helpArg(*this)
{
}
@ -963,7 +964,7 @@ void ArgumentParser::readArgs(int argc, const char *const *argv)
// read specified arguments
ArgumentReader reader(*this, argv, argv + argcForReader, completionMode);
const bool allArgsProcessed(reader.read());
NoColorArgument::apply();
m_noColorArg.apply();
// fail when not all arguments could be processed, except when in completion mode
if (!completionMode && !allArgsProcessed) {
@ -1658,8 +1659,6 @@ HelpArgument::HelpArgument(ArgumentParser &parser)
* \sa NoColorArgument::NoColorArgument(), EscapeCodes::enabled
*/
NoColorArgument *NoColorArgument::s_instance = nullptr;
/*!
* \brief Constructs a new NoColorArgument argument.
* \remarks This will also set EscapeCodes::enabled to the value of the environment variable ENABLE_ESCAPE_CODES.
@ -1673,11 +1672,6 @@ NoColorArgument::NoColorArgument()
{
setCombinable(true);
if (s_instance) {
return;
}
s_instance = this;
// set the environmentvariable: note that this is not directly used and just assigned for printing help
setEnvironmentVariable("ENABLE_ESCAPE_CODES");
@ -1701,22 +1695,12 @@ NoColorArgument::NoColorArgument()
EscapeCodes::enabled = false;
}
/*!
* \brief Destroys the object.
*/
NoColorArgument::~NoColorArgument()
{
if (s_instance == this) {
s_instance = nullptr;
}
}
/*!
* \brief Sets EscapeCodes::enabled according to the presense of the first instantiation of NoColorArgument.
*/
void NoColorArgument::apply()
void NoColorArgument::apply() const
{
if (NoColorArgument::s_instance && NoColorArgument::s_instance->isPresent()) {
if (isPresent()) {
#ifdef CPP_UTILITIES_ESCAPE_CODES_ENABLED_BY_DEFAULT
EscapeCodes::enabled = false;
#else

117
application/argumentparser.h

@ -48,8 +48,6 @@ CPP_UTILITIES_EXPORT extern std::vector<const char *> dependencyVersions2;
::ApplicationUtilities::applicationUrl = APP_URL; \
SET_DEPENDENCY_INFO
CPP_UTILITIES_EXPORT extern void (*exitFunction)(int);
class Argument;
class ArgumentParser;
class ArgumentReader;
@ -389,6 +387,30 @@ template <typename... TargetType> std::vector<std::tuple<TargetType...>> Argumen
return res;
}
class CPP_UTILITIES_EXPORT HelpArgument : public Argument {
public:
HelpArgument(ArgumentParser &parser);
};
class CPP_UTILITIES_EXPORT OperationArgument : public Argument {
public:
OperationArgument(const char *name, char abbreviation = '\0', const char *description = nullptr, const char *example = nullptr);
};
class CPP_UTILITIES_EXPORT ConfigValueArgument : public Argument {
public:
ConfigValueArgument(const char *name, char abbreviation = '\0', const char *description = nullptr,
std::initializer_list<const char *> valueNames = std::initializer_list<const char *>());
};
class CPP_UTILITIES_EXPORT NoColorArgument : public Argument {
friend ArgumentParserTests;
public:
NoColorArgument();
void apply() const;
};
struct ArgumentCompletionInfo;
class CPP_UTILITIES_EXPORT ArgumentParser {
@ -424,6 +446,11 @@ public:
void setDefaultArgument(Argument *argument);
Argument *specifiedOperation() const;
bool isUncombinableMainArgPresent() const;
void setExitFunction(std::function<void(int)> exitFunction);
const HelpArgument &helpArg() const;
HelpArgument &helpArg();
const NoColorArgument &noColorArg() const;
NoColorArgument &noColorArg();
private:
// declare internal operations
@ -440,6 +467,9 @@ private:
const char *m_executable;
UnknownArgumentBehavior m_unknownArgBehavior;
Argument *m_defaultArg;
HelpArgument m_helpArg;
NoColorArgument m_noColorArg;
std::function<void(int)> m_exitFunction;
};
/*!
@ -932,6 +962,24 @@ inline void Argument::reset()
m_occurrences.clear();
}
inline OperationArgument::OperationArgument(const char *name, char abbreviation, const char *description, const char *example)
: Argument(name, abbreviation, description, example)
{
setDenotesOperation(true);
}
/*!
* \brief Constructs a new ConfigValueArgument with the specified parameter. The initial value of requiredValueCount() is set to size of specified \a valueNames.
*/
inline ConfigValueArgument::ConfigValueArgument(
const char *name, char abbreviation, const char *description, std::initializer_list<const char *> valueNames)
: Argument(name, abbreviation, description)
{
setCombinable(true);
setRequiredValueCount(valueNames.size());
setValueNames(valueNames);
}
/*!
* \brief Returns information about all occurrences of the argument which have been detected when parsing.
* \remarks The convenience methods isPresent(), values() and path() provide direct access to these information for a particular occurrence.
@ -1034,51 +1082,46 @@ inline void ArgumentParser::invokeCallbacks()
invokeCallbacks(m_mainArgs);
}
class CPP_UTILITIES_EXPORT HelpArgument : public Argument {
public:
HelpArgument(ArgumentParser &parser);
};
class CPP_UTILITIES_EXPORT OperationArgument : public Argument {
public:
OperationArgument(const char *name, char abbreviation = '\0', const char *description = nullptr, const char *example = nullptr);
};
inline OperationArgument::OperationArgument(const char *name, char abbreviation, const char *description, const char *example)
: Argument(name, abbreviation, description, example)
/*!
* \brief Specifies a function quit the application.
* \remarks Currently only used after printing Bash completion. Default is std::exit().
*/
inline void ArgumentParser::setExitFunction(std::function<void(int)> exitFunction)
{
setDenotesOperation(true);
m_exitFunction = exitFunction;
}
class CPP_UTILITIES_EXPORT ConfigValueArgument : public Argument {
public:
ConfigValueArgument(const char *name, char abbreviation = '\0', const char *description = nullptr,
std::initializer_list<const char *> valueNames = std::initializer_list<const char *>());
};
/*!
* \brief Constructs a new ConfigValueArgument with the specified parameter. The initial value of requiredValueCount() is set to size of specified \a valueNames.
* \brief Returns the `--help` argument (which is always implicitely added to the main arguments).
*/
inline ConfigValueArgument::ConfigValueArgument(
const char *name, char abbreviation, const char *description, std::initializer_list<const char *> valueNames)
: Argument(name, abbreviation, description)
inline const HelpArgument &ArgumentParser::helpArg() const
{
setCombinable(true);
setRequiredValueCount(valueNames.size());
setValueNames(valueNames);
return m_helpArg;
}
class CPP_UTILITIES_EXPORT NoColorArgument : public Argument {
friend ArgumentParserTests;
/*!
* \brief Returns the `--help` argument (which is always implicitely added to the main arguments).
*/
inline HelpArgument &ArgumentParser::helpArg()
{
return m_helpArg;
}
public:
NoColorArgument();
~NoColorArgument();
static void apply();
/*!
* \brief Returns the `--no-color` argument (which is always implicitely added to the main arguments).
*/
inline const NoColorArgument &ArgumentParser::noColorArg() const
{
return m_noColorArg;
}
private:
static NoColorArgument *s_instance;
};
/*!
* \brief Returns the `--no-color` argument (which is always implicitely added to the main arguments).
*/
inline NoColorArgument &ArgumentParser::noColorArg()
{
return m_noColorArg;
}
} // namespace ApplicationUtilities

37
tests/argumentparsertests.cpp

@ -118,7 +118,6 @@ void ArgumentParserTests::testParsing()
ArgumentParser parser;
SET_APPLICATION_INFO;
QT_CONFIG_ARGUMENTS qtConfigArgs;
HelpArgument helpArg(parser);
Argument verboseArg("verbose", 'v', "be verbose");
verboseArg.setCombinable(true);
Argument fileArg("file", 'f', "specifies the path of the file to be opened");
@ -145,8 +144,7 @@ void ArgumentParserTests::testParsing()
Argument displayTagInfoArg("get", 'p', "displays the values of all specified tag fields (displays all fields if none specified)");
displayTagInfoArg.setDenotesOperation(true);
displayTagInfoArg.setSubArguments({ &fieldsArg, &filesArg, &verboseArg, &notAlbumArg });
NoColorArgument noColorArg;
parser.setMainArguments({ &qtConfigArgs.qtWidgetsGuiArg(), &printFieldNamesArg, &displayTagInfoArg, &displayFileInfoArg, &helpArg, &noColorArg });
parser.setMainArguments({ &qtConfigArgs.qtWidgetsGuiArg(), &printFieldNamesArg, &displayTagInfoArg, &displayFileInfoArg });
// no args present
parser.parseArgs(0, nullptr);
@ -361,7 +359,7 @@ void ArgumentParserTests::testParsing()
CPPUNIT_ASSERT(!filesArg.isPresent());
CPPUNIT_ASSERT(fileArg.isPresent());
CPPUNIT_ASSERT(!verboseArg.isPresent());
CPPUNIT_ASSERT(noColorArg.isPresent());
CPPUNIT_ASSERT(parser.noColorArg().isPresent());
CPPUNIT_ASSERT_EQUAL(1_st, fileArg.values(0).size());
CPPUNIT_ASSERT_EQUAL("test-v"s, string(fileArg.values(0).front()));
@ -528,7 +526,7 @@ void ArgumentParserTests::testBashCompletion()
Argument setArg("set", 's', "sets tag values");
setArg.setSubArguments({ &valuesArg, &filesArg, &selectorsArg });
parser.setMainArguments({ &helpArg, &displayFileInfoArg, &getArg, &setArg });
parser.setMainArguments({ &displayFileInfoArg, &getArg, &setArg });
// fail due to operation flags not set
const char *const argv1[] = { "se" };
@ -560,7 +558,7 @@ void ArgumentParserTests::testBashCompletion()
// advance the cursor position -> the completion should propose the next argument
parser.resetArgs();
{
const OutputCheck c("COMPREPLY=('--files' '--selectors' '--values' )\n");
const OutputCheck c("COMPREPLY=('--files' '--selectors' '--no-color' '--values' )\n");
reader.reset(argv2, argv2 + 1).read();
parser.printBashCompletion(1, argv2, 1, reader);
}
@ -569,7 +567,7 @@ void ArgumentParserTests::testBashCompletion()
parser.resetArgs();
filesArg.setDenotesOperation(true);
{
const OutputCheck c("COMPREPLY=('files' '--selectors' '--values' )\n");
const OutputCheck c("COMPREPLY=('files' '--selectors' '--no-color' '--values' )\n");
reader.reset(argv2, argv2 + 1).read();
parser.printBashCompletion(1, argv2, 1, reader);
}
@ -578,7 +576,7 @@ void ArgumentParserTests::testBashCompletion()
parser.resetArgs();
filesArg.setDenotesOperation(false);
{
const OutputCheck c("COMPREPLY=('display-file-info' 'get' 'set' '--help' )\n");
const OutputCheck c("COMPREPLY=('display-file-info' 'get' 'set' '--help' '--no-color' )\n");
reader.reset(nullptr, nullptr).read();
parser.printBashCompletion(0, nullptr, 0, reader);
}
@ -587,7 +585,7 @@ void ArgumentParserTests::testBashCompletion()
const char *const argv3[] = { "get", "--fields" };
parser.resetArgs();
{
const OutputCheck c("COMPREPLY=('title' 'album' 'artist' 'trackpos' '--files' )\n");
const OutputCheck c("COMPREPLY=('title' 'album' 'artist' 'trackpos' '--files' '--no-color' )\n");
reader.reset(argv3, argv3 + 2).read();
parser.printBashCompletion(2, argv3, 2, reader);
}
@ -637,7 +635,7 @@ void ArgumentParserTests::testBashCompletion()
// pre-defined values for implicit argument
parser.resetArgs();
{
const OutputCheck c("COMPREPLY=('title' 'album' 'artist' 'trackpos' '--fields' '--files' )\n");
const OutputCheck c("COMPREPLY=('title' 'album' 'artist' 'trackpos' '--fields' '--files' '--no-color' )\n");
reader.reset(argv3, argv3 + 1).read();
parser.printBashCompletion(1, argv3, 2, reader);
}
@ -661,7 +659,7 @@ void ArgumentParserTests::testBashCompletion()
const char *const argv6[] = { "set", "--" };
parser.resetArgs();
{
const OutputCheck c("COMPREPLY=('--files' '--selectors' '--values' )\n");
const OutputCheck c("COMPREPLY=('--files' '--selectors' '--no-color' '--values' )\n");
reader.reset(argv6, argv6 + 2).read();
parser.printBashCompletion(2, argv6, 1, reader);
}
@ -670,7 +668,7 @@ void ArgumentParserTests::testBashCompletion()
const char *const argv7[] = { "-i", "--sub", "--" };
parser.resetArgs();
{
const OutputCheck c("COMPREPLY=('--files' '--nested-sub' '--verbose' )\n");
const OutputCheck c("COMPREPLY=('--files' '--nested-sub' '--no-color' '--verbose' )\n");
reader.reset(argv7, argv7 + 3).read();
parser.printBashCompletion(3, argv7, 2, reader);
}
@ -700,13 +698,13 @@ void ArgumentParserTests::testBashCompletion()
}
// override exit function to prevent readArgs() from terminating the test run
exitFunction = [](int) { exitCalled = true; };
parser.setExitFunction([](int) { exitCalled = true; });
// call completion via readArgs() with current word index
const char *const argv10[] = { "/some/path/tageditor", "--bash-completion-for", "0" };
parser.resetArgs();
{
const OutputCheck c("COMPREPLY=('display-file-info' 'get' 'set' '--help' )\n");
const OutputCheck c("COMPREPLY=('display-file-info' 'get' 'set' '--help' '--no-color' )\n");
parser.readArgs(3, argv10);
}
CPPUNIT_ASSERT(!strcmp("/some/path/tageditor", parser.executable()));
@ -757,7 +755,7 @@ void ArgumentParserTests::testHelp()
envArg.setEnvironmentVariable("FILES");
envArg.setRequiredValueCount(2);
envArg.appendValueName("file");
parser.addMainArgument(&helpArg);
parser.helpArg().setRequired(true);
parser.addMainArgument(&verboseArg);
parser.addMainArgument(&filesArg);
parser.addMainArgument(&envArg);
@ -788,6 +786,10 @@ void ArgumentParserTests::testHelp()
" env\n"
" default environment variable: FILES\n"
"\n"
"\e[1m--no-color\e[0m\n"
" disables formatted/colorized output\n"
" default environment variable: ENABLE_ESCAPE_CODES\n"
"\n"
"Project website: " APP_URL "\n");
EscapeCodes::enabled = true;
parser.parseArgs(2, argv);
@ -857,9 +859,6 @@ void ArgumentParserTests::testNoColorArgument()
// assume escape codes are enabled by default
EscapeCodes::enabled = true;
// ensure the initialization is not skipped
NoColorArgument::s_instance = nullptr;
{
unsetenv("ENABLE_ESCAPE_CODES");
NoColorArgument noColorArg;
@ -868,8 +867,6 @@ void ArgumentParserTests::testNoColorArgument()
noColorArg.m_occurrences.emplace_back(0);
noColorArg.apply();
CPPUNIT_ASSERT_MESSAGE("default negated if present", !EscapeCodes::enabled);
const NoColorArgument secondInstance;
CPPUNIT_ASSERT_EQUAL_MESSAGE("s_instance not altered by 2nd instance", &noColorArg, NoColorArgument::s_instance);
}
{
setenv("ENABLE_ESCAPE_CODES", " 0 ", 1);

7
tests/testutils.cpp

@ -110,8 +110,7 @@ TestApplication *TestApplication::m_instance = nullptr;
* \throws Throws std::runtime_error if an instance has already been created.
*/
TestApplication::TestApplication(int argc, char **argv)
: m_helpArg(m_parser)
, m_testFilesPathArg("test-files-path", 'p', "specifies the path of the directory with test files")
: m_testFilesPathArg("test-files-path", 'p', "specifies the path of the directory with test files")
, m_applicationPathArg("app-path", 'a', "specifies the path of the application to be tested")
, m_workingDirArg("working-dir", 'w', "specifies the directory to store working copies of test files")
, m_unitsArg("units", 'u', "specifies the units to test; omit to test all units")
@ -142,7 +141,7 @@ TestApplication::TestApplication(int argc, char **argv)
m_unitsArg.setRequiredValueCount(Argument::varValueCount);
m_unitsArg.setValueNames({ "unit1", "unit2", "unit3" });
m_unitsArg.setCombinable(true);
m_parser.setMainArguments({ &m_testFilesPathArg, &m_applicationPathArg, &m_workingDirArg, &m_unitsArg, &m_helpArg });
m_parser.setMainArguments({ &m_testFilesPathArg, &m_applicationPathArg, &m_workingDirArg, &m_unitsArg });
// parse arguments
try {
@ -154,7 +153,7 @@ TestApplication::TestApplication(int argc, char **argv)
}
// print help
if (m_helpArg.isPresent()) {
if (m_parser.helpArg().isPresent()) {
exit(0);
}
}

1
tests/testutils.h

@ -44,7 +44,6 @@ private:
static std::string readTestfilePathFromSrcRef();
ApplicationUtilities::ArgumentParser m_parser;
ApplicationUtilities::HelpArgument m_helpArg;
ApplicationUtilities::Argument m_testFilesPathArg;
ApplicationUtilities::Argument m_applicationPathArg;
ApplicationUtilities::Argument m_workingDirArg;

Loading…
Cancel
Save