From 72426e2d4ce521262756bdb360912c6477cce831 Mon Sep 17 00:00:00 2001 From: Martchus Date: Sat, 22 Oct 2016 19:32:16 +0200 Subject: [PATCH] Fix bash completion when dir/file contains single quote Also a few other improvements in bash completion code --- application/argumentparser.cpp | 98 +++++++++++++++++++-------------- application/argumentparser.h | 23 ++++---- io/path.h | 5 ++ testfiles/t.aac | 1 + testfiles/test 'with quote'.mkv | 1 + tests/argumentparsertests.cpp | 23 ++++---- 6 files changed, 88 insertions(+), 63 deletions(-) create mode 100644 testfiles/t.aac create mode 100644 testfiles/test 'with quote'.mkv diff --git a/application/argumentparser.cpp b/application/argumentparser.cpp index 24f81ea..d500fa0 100644 --- a/application/argumentparser.cpp +++ b/application/argumentparser.cpp @@ -691,18 +691,19 @@ void ArgumentParser::printBashCompletion(int argc, const char *const *argv, unsi lastDetectedArgPath = lastDetectedArg->path(lastDetectedArg->occurrences() - 1); } - bool nextArgumentOrValue; + // determine last arg, omitting trailing empty args const char *const *lastSpecifiedArg; unsigned int lastSpecifiedArgIndex; if(argc) { - // determine last arg omitting trailing empty args lastSpecifiedArgIndex = static_cast(argc) - 1; lastSpecifiedArg = argv + lastSpecifiedArgIndex; for(; lastSpecifiedArg >= argv && **lastSpecifiedArg == '\0'; --lastSpecifiedArg, --lastSpecifiedArgIndex); } + // determine arguments relevant for completion + bool nextArgumentOrValue; if(lastDetectedArg && lastDetectedArg->isPresent()) { - if((nextArgumentOrValue = currentWordIndex > lastDetectedArgIndex)) { + if((nextArgumentOrValue = (currentWordIndex > lastDetectedArgIndex))) { // parameter values of the last arg are possible completions auto currentValueCount = lastDetectedArg->values(lastDetectedArg->occurrences() - 1).size(); if(currentValueCount) { @@ -740,6 +741,7 @@ void ArgumentParser::printBashCompletion(int argc, const char *const *argv, unsi } } else { + // no arguments detected -> just use main arguments for completion nextArgumentOrValue = true; insertSiblings(m_mainArgs, relevantArgs); } @@ -790,27 +792,37 @@ void ArgumentParser::printBashCompletion(int argc, const char *const *argv, unsi if(argc && currentWordIndex <= lastSpecifiedArgIndex && opening) { if(openingDenotationType == Value) { bool wordStart = true, ok = false, equationSignAlreadyPresent = false; - int wordIndex = 0; + size_t wordIndex = 0; for(const char *i = arg->preDefinedCompletionValues(), *end = opening + openingLen; *i;) { if(wordStart) { const char *i1 = i, *i2 = opening; for(; *i1 && i2 != end && *i1 == *i2; ++i1, ++i2); - ok = (i2 == end); + if((ok = (i2 == end))) { + cout << '\''; + } wordStart = false; wordIndex = 0; } else if((wordStart = (*i == ' ') || (*i == '\n'))) { equationSignAlreadyPresent = false; + if(ok) { + cout << '\'' << ' '; + } + ++i; + continue; } else if(*i == '=') { equationSignAlreadyPresent = true; } if(ok) { if(!compoundOpeningStartLen || wordIndex >= compoundOpeningStartLen) { + if(*i == '\'') { + cout << "'\"'\"'"; + } cout << *i; } ++i, ++wordIndex; - if(appendEquationSign && !equationSignAlreadyPresent) { - switch(*i) { - case ' ': case '\n': case '\0': + switch(*i) { + case ' ': case '\n': case '\0': + if(appendEquationSign && !equationSignAlreadyPresent) { cout << '='; noWhitespace = true; equationSignAlreadyPresent = false; @@ -822,23 +834,33 @@ void ArgumentParser::printBashCompletion(int argc, const char *const *argv, unsi } cout << ' '; } - } else if(appendEquationSign) { + } else if(const char *i = arg->preDefinedCompletionValues()) { bool equationSignAlreadyPresent = false; - for(const char *i = arg->preDefinedCompletionValues(); *i;) { - cout << *i; + cout << '\''; + while(*i) { + if(*i == '\'') { + cout << "'\"'\"'"; + } else { + cout << *i; + } switch(*(++i)) { case '=': equationSignAlreadyPresent = true; break; case ' ': case '\n': case '\0': - if(!equationSignAlreadyPresent) { + if(appendEquationSign && !equationSignAlreadyPresent) { cout << '='; equationSignAlreadyPresent = false; } + if(*i != '\0') { + cout << '\''; + if(*(++i)) { + cout << ' ' << '\''; + } + } } } - } else { - cout << arg->preDefinedCompletionValues() << ' '; + cout << '\'' << ' '; } } } @@ -861,11 +883,11 @@ void ArgumentParser::printBashCompletion(int argc, const char *const *argv, unsi } if(openingDenotationType == Abbreviation && opening) { - cout << '-' << opening << arg->abbreviation() << ' '; + cout << '\'' << '-' << opening << arg->abbreviation() << '\'' << ' '; } else if(arg->denotesOperation() && (!actualArgumentCount() || (currentWordIndex == 0 && (!lastDetectedArg || (lastDetectedArg->isPresent() && lastDetectedArgIndex == 0))))) { - cout << arg->name() << ' '; + cout << '\'' << arg->name() << '\'' << ' '; } else { - cout << '-' << '-' << arg->name() << ' '; + cout << '\'' << '-' << '-' << arg->name() << '\'' << ' '; } } // -> completions for files and dirs @@ -873,7 +895,7 @@ void ArgumentParser::printBashCompletion(int argc, const char *const *argv, unsi string actualDir, actualFile; bool haveFileOrDirCompletions = false; if(argc && currentWordIndex == lastSpecifiedArgIndex && opening) { - // the "opening" might contain escaped characters which need to be unescaped first + // the "opening" might contain escaped characters which need to be unescaped first (let's hope this covers all possible escapings) string unescapedOpening(opening); findAndReplace(unescapedOpening, "\\ ", " "); findAndReplace(unescapedOpening, "\\,", ","); @@ -882,6 +904,9 @@ void ArgumentParser::printBashCompletion(int argc, const char *const *argv, unsi findAndReplace(unescapedOpening, "\\!", "!"); findAndReplace(unescapedOpening, "\\#", "#"); findAndReplace(unescapedOpening, "\\$", "$"); + findAndReplace(unescapedOpening, "\\'", "'"); + findAndReplace(unescapedOpening, "\\\"", "\""); + findAndReplace(unescapedOpening, "\\\\", "\\"); // determine the "directory" part string dir = directory(unescapedOpening); if(dir.empty()) { @@ -905,42 +930,35 @@ void ArgumentParser::printBashCompletion(int argc, const char *const *argv, unsi } actualFile = move(file); } - // -> completion for files + + // -> completion for files and dirs + DirectoryEntryType entryTypes = DirectoryEntryType::None; if(completeFiles) { - if(argc && currentWordIndex <= lastSpecifiedArgIndex && opening) { - for(const string &dirEntry : directoryEntries(actualDir.c_str(), DirectoryEntryType::File)) { - if(startsWith(dirEntry, actualFile)) { - cout << '\''; - if(actualDir != ".") { - cout << actualDir; - } - cout << dirEntry << '\'' << ' '; - haveFileOrDirCompletions = true; - } - } - } else { - for(const string &dirEntry : directoryEntries(".", DirectoryEntryType::File)) { - cout << dirEntry << ' '; - haveFileOrDirCompletions = true; - } - } + entryTypes |= DirectoryEntryType::File; } - // -> completion for dirs if(completeDirs) { + entryTypes |= DirectoryEntryType::Directory; + } + if(entryTypes != DirectoryEntryType::None) { + const string replace("'"), with("'\"'\"'"); if(argc && currentWordIndex <= lastSpecifiedArgIndex && opening) { - for(const string &dirEntry : directoryEntries(actualDir.c_str(), DirectoryEntryType::Directory)) { + list entries = directoryEntries(actualDir.c_str(), entryTypes); + findAndReplace(actualDir, replace, with); + for(string &dirEntry : entries) { if(startsWith(dirEntry, actualFile)) { cout << '\''; if(actualDir != ".") { cout << actualDir; } + findAndReplace(dirEntry, replace, with); cout << dirEntry << '\'' << ' '; haveFileOrDirCompletions = true; } } } else { - for(const string &dirEntry : directoryEntries(".", DirectoryEntryType::Directory)) { - cout << '\'' << dirEntry << '/' << '\'' << ' '; + for(string &dirEntry : directoryEntries(".", entryTypes)) { + findAndReplace(dirEntry, replace, with); + cout << '\'' << dirEntry << '\'' << ' '; haveFileOrDirCompletions = true; } } diff --git a/application/argumentparser.h b/application/argumentparser.h index 22f8205..1e6e386 100644 --- a/application/argumentparser.h +++ b/application/argumentparser.h @@ -238,8 +238,7 @@ private: /*! * \brief Returns the name of the argument. * - * The parser compares the name with the characters following a "--" prefix to - * identify arguments. + * The parser compares the name with the characters following a "--" prefix to identify arguments. */ inline const char *Argument::name() const { @@ -249,17 +248,17 @@ inline const char *Argument::name() const /*! * \brief Sets the name of the argument. * - * The name mustn't be empty or contain white spaces or equation chars. + * The name mustn't be empty, start with a minus or contain white spaces, equation chars, quotes and newlines. * - * The parser compares the name with the characters following a "--" prefix to - * identify arguments. + * The parser compares the name with the characters following a "--" prefix to identify arguments. */ inline void Argument::setName(const char *name) { #ifdef DEBUG_BUILD if(name && *name) { + assert(*name != '-'); for(const char *c = name; *c; ++c) { - assert(*c != ' ' && *c != '='); + assert(*c != ' ' && *c != '=' && *c != '\'' && *c != '\"' && *c != '\n' && *c != '\r'); } } #endif @@ -269,8 +268,7 @@ inline void Argument::setName(const char *name) /*! * \brief Returns the abbreviation of the argument. * - * The parser compares the abbreviation with the characters following a "-" prefix to - * identify arguments. + * The parser compares the abbreviation with the characters following a "-" prefix to identify arguments. */ inline char Argument::abbreviation() const { @@ -280,15 +278,14 @@ inline char Argument::abbreviation() const /*! * \brief Sets the abbreviation of the argument. * - * The abbreviation might be empty but mustn't contain any white spaces or - * equation chars when provided. + * The abbreviation might be empty but mustn't be white spaces, equation char, single quote, double quote or newline. * - * The parser compares the abbreviation with the characters following a "-" prefix to - * identify arguments. + * The parser compares the abbreviation with the characters following a "-" prefix to identify arguments. */ inline void Argument::setAbbreviation(char abbreviation) { - IF_DEBUG_BUILD(assert(abbreviation != ' ' && abbreviation != '=')); + IF_DEBUG_BUILD(assert(abbreviation != ' ' && abbreviation != '=' && abbreviation != '-' + && abbreviation != '\'' && abbreviation != '"' && abbreviation != '\n' && abbreviation != '\r')); m_abbreviation = abbreviation; } diff --git a/io/path.h b/io/path.h index 8bde847..1c71191 100644 --- a/io/path.h +++ b/io/path.h @@ -38,6 +38,11 @@ constexpr DirectoryEntryType operator|(DirectoryEntryType lhs, DirectoryEntryTyp return static_cast(static_cast(lhs) | static_cast(rhs)); } +constexpr DirectoryEntryType &operator|=(DirectoryEntryType &lhs, DirectoryEntryType rhs) +{ + return (lhs = static_cast(static_cast(lhs) | static_cast(rhs))); +} + constexpr DirectoryEntryType operator&(DirectoryEntryType lhs, DirectoryEntryType rhs) { return static_cast(static_cast(lhs) & static_cast(rhs)); diff --git a/testfiles/t.aac b/testfiles/t.aac new file mode 100644 index 0000000..8d1c8b6 --- /dev/null +++ b/testfiles/t.aac @@ -0,0 +1 @@ + diff --git a/testfiles/test 'with quote'.mkv b/testfiles/test 'with quote'.mkv new file mode 100644 index 0000000..8d1c8b6 --- /dev/null +++ b/testfiles/test 'with quote'.mkv @@ -0,0 +1 @@ + diff --git a/tests/argumentparsertests.cpp b/tests/argumentparsertests.cpp index 81dd79c..b5db976 100644 --- a/tests/argumentparsertests.cpp +++ b/tests/argumentparsertests.cpp @@ -399,7 +399,7 @@ void ArgumentParserTests::testBashCompletion() parser.readSpecifiedArgs(parser.m_mainArgs, index, argv, argv1 + 1, lastDetectedArg, true); parser.printBashCompletion(1, argv1, 0, lastDetectedArg); cout.rdbuf(regularCoutBuffer); - CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=(set )\n"), buffer.str()); + CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=('set' )\n"), buffer.str()); // argument is already specified const char *const argv2[] = {"set"}; @@ -409,7 +409,7 @@ void ArgumentParserTests::testBashCompletion() parser.readSpecifiedArgs(parser.m_mainArgs, index, argv, argv2 + 1, lastDetectedArg, true); parser.printBashCompletion(1, argv2, 0, lastDetectedArg); cout.rdbuf(regularCoutBuffer); - CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=(set )\n"), buffer.str()); + CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=('set' )\n"), buffer.str()); // advance the cursor position -> the completion should propose the next argument index = 0, lastDetectedArg = nullptr, buffer.str(string()), setArg.reset(); @@ -418,7 +418,7 @@ void ArgumentParserTests::testBashCompletion() parser.readSpecifiedArgs(parser.m_mainArgs, index, argv, argv2 + 1, lastDetectedArg, true); parser.printBashCompletion(1, argv2, 1, lastDetectedArg); cout.rdbuf(regularCoutBuffer); - CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=(--files --values )\n"), buffer.str()); + CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=('--files' '--values' )\n"), buffer.str()); // specifying no args should propose all main arguments index = 0, lastDetectedArg = nullptr, buffer.str(string()), getArg.reset(), setArg.reset(); @@ -427,7 +427,7 @@ void ArgumentParserTests::testBashCompletion() parser.readSpecifiedArgs(parser.m_mainArgs, index, argv, nullptr, lastDetectedArg, true); parser.printBashCompletion(0, nullptr, 0, lastDetectedArg); cout.rdbuf(regularCoutBuffer); - CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=(display-file-info get set --help )\n"), buffer.str()); + CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=('display-file-info' 'get' 'set' '--help' )\n"), buffer.str()); // values const char *const argv3[] = {"get", "--fields"}; @@ -437,7 +437,7 @@ void ArgumentParserTests::testBashCompletion() parser.readSpecifiedArgs(parser.m_mainArgs, index, argv, argv3 + 2, lastDetectedArg, true); parser.printBashCompletion(2, argv3, 2, lastDetectedArg); cout.rdbuf(regularCoutBuffer); - CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=(title album artist trackpos --files )\n"), buffer.str()); + CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=('title' 'album' 'artist' 'trackpos' '--files' )\n"), buffer.str()); // values with equation sign, one letter already present const char *const argv4[] = {"set", "--values", "a"}; @@ -447,11 +447,14 @@ void ArgumentParserTests::testBashCompletion() parser.readSpecifiedArgs(parser.m_mainArgs, index, argv, argv4 + 3, lastDetectedArg, true); parser.printBashCompletion(3, argv4, 2, lastDetectedArg); cout.rdbuf(regularCoutBuffer); - CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=(album= artist= ); compopt -o nospace\n"), buffer.str()); + CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=('album=' 'artist=' ); compopt -o nospace\n"), buffer.str()); // file names string iniFilePath = TestUtilities::testFilePath("test.ini"); - iniFilePath.resize(iniFilePath.size() - 3); + iniFilePath.resize(iniFilePath.size() - 4); + string mkvFilePath = TestUtilities::testFilePath("test 'with quote'.mkv"); + mkvFilePath.resize(mkvFilePath.size() - 17); + TestUtilities::testFilePath("t.aac"); const char *const argv5[] = {"get", "--files", iniFilePath.c_str()}; index = 0, lastDetectedArg = nullptr, buffer.str(string()), getArg.reset(), setArg.reset(); cout.rdbuf(buffer.rdbuf()); @@ -459,7 +462,7 @@ void ArgumentParserTests::testBashCompletion() parser.readSpecifiedArgs(parser.m_mainArgs, index, argv, argv5 + 3, lastDetectedArg, true); parser.printBashCompletion(3, argv5, 2, lastDetectedArg); cout.rdbuf(regularCoutBuffer); - CPPUNIT_ASSERT_EQUAL("COMPREPLY=('" + iniFilePath + "ini' ); compopt -o filenames\n", buffer.str()); + CPPUNIT_ASSERT_EQUAL("COMPREPLY=('" + mkvFilePath + " '\"'\"'with quote'\"'\"'.mkv' '" + iniFilePath + ".ini' ); compopt -o filenames\n", buffer.str()); // sub arguments const char *const argv6[] = {"set", "--"}; @@ -469,7 +472,7 @@ void ArgumentParserTests::testBashCompletion() parser.readSpecifiedArgs(parser.m_mainArgs, index, argv, argv6 + 2, lastDetectedArg, true); parser.printBashCompletion(2, argv6, 1, lastDetectedArg); cout.rdbuf(regularCoutBuffer); - CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=(--files --values )\n"), buffer.str()); + CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=('--files' '--values' )\n"), buffer.str()); // nested sub arguments const char *const argv7[] = {"-i", "--sub", "--"}; @@ -479,7 +482,7 @@ void ArgumentParserTests::testBashCompletion() parser.readSpecifiedArgs(parser.m_mainArgs, index, argv, argv7 + 3, lastDetectedArg, true); parser.printBashCompletion(3, argv7, 2, lastDetectedArg); cout.rdbuf(regularCoutBuffer); - CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=(--files --nested-sub --verbose )\n"), buffer.str()); + CPPUNIT_ASSERT_EQUAL(string("COMPREPLY=('--files' '--nested-sub' '--verbose' )\n"), buffer.str()); } catch(...) { cout.rdbuf(regularCoutBuffer);