[PATCH] Add completion to the query parser, and hook it up to clang-query.
Manuel Klimek
klimek at google.com
Mon Jan 27 11:12:19 PST 2014
================
Comment at: clang-query/QueryParser.h:23
@@ +22,3 @@
+
+class QueryParser {
+ public:
----------------
While mostly self-explanatory, I still think the public interface might benefit from some comments...
================
Comment at: clang-query/QueryParser.cpp:64-71
@@ +63,10 @@
+ LexOrCompleteWord &Case(const char (&S)[N], const T &Value) {
+ if (WordCompletionPos == ~0u)
+ SS.Case(S, Value);
+ else if (N != 1 && SeenCases.insert(Value).second &&
+ std::memcmp(S, Str.data(), WordCompletionPos) == 0)
+ P->Completions.push_back(LineEditor::Completion(
+ std::string(S + WordCompletionPos, N - 1 - WordCompletionPos) + " ",
+ std::string(S, N - 1)));
+ return *this;
+ }
----------------
I find this somewhat hard to read.
Things that might make it easier:
- pull out a constant for ~0u, or use one of the already existing constants
- s/SS/Switch/, or find an even more descriptive name what we're switching on here
- is there a particular reason you're using memcmp instead of using StringRefs and substr and operator==?
================
Comment at: clang-query/QueryParser.cpp:51
@@ -48,3 +50,3 @@
-static QueryRef ParseSetBool(bool QuerySession::*Var, StringRef ValStr) {
- unsigned Value = StringSwitch<unsigned>(ValStr)
+template <typename T> struct QueryParser::LexOrCompleteWord {
+ StringSwitch<T> SS;
----------------
When reading this without the context of how it's used it's hard to know why this is templated, and what the template parameters are supposed to be.
================
Comment at: clang-query/QueryParser.h:53
@@ +52,3 @@
+
+ const char *CompletionPos;
+ std::vector<llvm::LineEditor::Completion> Completions;
----------------
I think some of the code later might be simpler if this was an offset from Begin. I might be missing something, though.
http://llvm-reviews.chandlerc.com/D2263
More information about the cfe-commits
mailing list