Differential D11888 Diff 31550 src/lib/advancedqueryparser.cpp

Changeset View

Standalone View

src/lib/advancedqueryparser.cpp

Show All 24 Lines
25	#include <QDate>	25		#include <QDate>
26		26
27	using namespace Baloo;	27		using namespace Baloo;
28		28
29	AdvancedQueryParser::AdvancedQueryParser()	29		AdvancedQueryParser::AdvancedQueryParser()
30	{	30		{
31	}	31		}
32		32
33	static bool isOperator(const QChar& c)
34	{
35	switch (c.toLatin1()) {
36	case ':':
37	case '=':
38	case '>':
39	case '<':
40	case '(':
41	case ')':
42	return true;
43
44	default:
45	return false;
46	}
47	}
48
49	static QStringList lex(const QString& text)	33		static QStringList lex(const QString& text)
50	{	34		{
51	QStringList tokens;	35		QStringList tokenList;
			michaelhUnsubmitted Done Suggestion: Rename to `tokenList`. It's less likely to confuse it with `token` that way. michaelh: Suggestion: Rename to `tokenList`. It's less likely to confuse it with `token` that way.
52	QString token;	36		QString token;
53	bool inQuotes = false;	37		bool inQuotes = false;
54		38
55	for (int i=0, end=text.size(); i!=end; ++i) {	39		for (int i = 0, end = text.size(); i != end; ++i) {
56	QChar c = text.at(i);	40		QChar c = text.at(i);
57		41
58	if (c == QLatin1Char('"')) {	42		if (c == QLatin1Char('"')) {
59	// Quotes start or end string literals	43		// Quotes start or end string literals
60	inQuotes = !inQuotes;	44		inQuotes = !inQuotes;
61	} else if (inQuotes) {	45		} else if (inQuotes) {
62	// Don't do any processing in strings	46		// Don't do any processing in strings
63	token.append(c);	47		token.append(c);
64	} else if (c.isSpace() \|\| isOperator(c)) {	48		} else if (c.isSpace()) {
			michaelhUnsubmitted Done Please use `!token.isEmpty()` (Don't exactly know why, but have myself been told so several times. Maybe it's faster. :-) michaelh: Please use `!token.isEmpty()` (Don't exactly know why, but have myself been told so several…
65	// Spaces and operators end tokens	49		// Spaces end tokens
66	if (token.size() > 0) {	50		if (!token.isEmpty()) {
67	tokens.append(token);	51		tokenList.append(token);
68	token.clear();	52		token.clear();
69	}	53		}
70		54		} else if (c == '(' \|\| c == ')') {
71	// Operators are tokens themselves	55		// Parentheses end tokens, and are tokens by themselves
			michaelhUnsubmitted Done // Parentheses delimit tokens, and are tokens by themselves michaelh: // Parentheses delimit tokens, and are tokens by themselves
			brunsAuthorUnsubmitted Done Why choose a different wording here? Also, the important aspect here is the "end", which commits the current token to the list. bruns: Why choose a different wording here? Also, the important aspect here is the "end", which…
			michaelhUnsubmitted Done Because of opening and closing parentheses, but you're right 'end' is the important aspect here. michaelh: Because of opening and closing parentheses, but you're right 'end' is the important aspect here.
72	if (isOperator(c)) {	56		if (!token.isEmpty()) {
73	if (tokens.size() > 1) {	57		tokenList.append(token);
74	QString last = tokens.last();	58		token.clear();
75	if (last.size() == 1 && isOperator(last[0])) {
76	last.append(c);
77	tokens[tokens.size() - 1] = last;
78	continue;
79	}	59		}
		60		tokenList.append(c);
		61		} else if (c == '>' \|\| c == '<' \|\| c == ':' \|\| c == '=') {
			michaelhUnsubmitted Done Please use QLatin1Char michaelh: Please use QLatin1Char
		62		// Operators end tokens
		63		if (!token.isEmpty()) {
			michaelhUnsubmitted Done Please use `!token.isEmpty()` michaelh: Please use `!token.isEmpty()`
		64		tokenList.append(token);
		65		token.clear();
80	}	66		}
81	tokens.append(QString(c));	67		// accept '=' after any of the above
		68		if (text.at(i + 1) == '=') {
			michaelhUnsubmitted Done Please use QLatin1Char michaelh: Please use QLatin1Char
		69		tokenList.append(text.mid(i, 2));
			michaelhUnsubmitted Done ':' means `Term::Contains` and '=' means `Term::Equal` so ':=' is a little ambigous. Unless you have a particular reason to interpret ':=' as '=' we should take it as ':'. It may be better to have and extra `If` for '=' and ':' and simply drop any second "=" so we come out with ':' or '=' as token. The `parse()` function checks the second char only in case of '<' or '>'. So ':=' will become ':'. The 2-char tokens should to be added to the `switch` in `parse()` which then also could be simplified. The distinction between '>' and '>=' is the lexer's job, right? michaelh: ':' means `Term::Contains` and '=' means `Term::Equal` so ':=' is a little ambigous. Unless…
			brunsAuthorUnsubmitted Done ":=" and "==" are added as is, and the parser interprets both dependent on the first character only, i.e. ":" and "=". The behaviour for these two combinations is unchanged with this patch. The lexer should not handle ">" and ">=" differently, both are (valid) tokens, and should be returned as such. Adding e.g. ">=" to the switch statement is not possible, as it works on QChar's. bruns: ":=" and "==" are added as is, and the parser interprets both dependent on the first character…
			michaelhUnsubmitted Done I misunderstood your description as ':=' should become '='. The lexer should not handle ">" and ">=" differently, both are (valid) tokens, and should be returned as such. That is what I meant. I was confused by `parse()` lexing again when encountering a '>'. Adding e.g. ">=" to the switch statement is not possible, as it works on QChar's. I don't understand. token is QString. Why not instead of `switch (token.at(0).toLatin1())` switch (token) { case '>'`: comparator = Term::Greater; break; case '>='`: comparator = Term::GreaterEqual; break; What am I missing? michaelh: # I misunderstood your description as ':=' should become '='. > The lexer should not handle…
			brunsAuthorUnsubmitted Done I don't understand. token is QString. Why not instead of switch (token.at(0).toLatin1()) switch (token) { case '>'`: comparator = Term::Greater; break; case '>='`: comparator = Term::GreaterEqual; break; What am I missing? You can only `switch` on integral statements (ints, chars, ...). With C++11, you can do some `constexpr` hacks, but your code won't work as is. bruns: > I don't understand. token is QString. Why not instead of switch (token.at(0).toLatin1()) > >…
			michaelhUnsubmitted Done You can only switch on integral statements (ints, chars, ...). Heh? That's surprising![1] Anyway, thanks for the lesson a keep this code as it is now. [1] Coming from javascript `switch` definitely is a "false friend". Can I become a beef steak? michaelh: > You can only switch on integral statements (ints, chars, ...). Heh? That's surprising![1]…
		70		i++;
		71		} else {
		72		tokenList.append(c);
			michaelhUnsubmitted Done Please use QLatin1Char michaelh: Please use QLatin1Char
82	}	73		}
83
84	continue;
85	} else {	74		} else {
86	// Simply extend the current token	75		// Simply extend the current token
87	token.append(c);	76		token.append(c);
88	}	77		}
89	}	78		}
90		79
91	if (token.size() > 0) {	80		if (!token.isEmpty()) {
92	tokens.append(token);	81		tokenList.append(token);
93	}	82		}
94		83
95	return tokens;	84		return tokenList;
96	}	85		}
97		86
98	static void addTermToStack(QStack<Term>& stack, const Term& termInConstruction, Term::Operation op)	87		static void addTermToStack(QStack<Term>& stack, const Term& termInConstruction, Term::Operation op)
99	{	88		{
100	Term &tos = stack.top();	89		Term &tos = stack.top();
101		90
102	if (tos.isEmpty()) {	91		if (tos.isEmpty()) {
103	// Empty top of stack, just assign termInConstruction to it	92		// Empty top of stack, just assign termInConstruction to it
▲ Show 20 Lines • Show All 161 Lines • Show Last 20 Lines