dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kconfigtest.cpp:524
> << "hostname[$e]=$(hostname)" << endl
> - << "noeol=foo"; // no EOL
> + << "noeol=foo" << endl // no EOL
> + << "escapes=aaa,bb/b,ccc\\,ccc" << endl
You broke the "no EOL" testcase.
Please add yours before it.
> kconfiggroup.cpp:163
> QStringList value;
> - QString val;
> - val.reserve(data.size());
> - bool quoted = false;
> - for (int p = 0; p < data.length(); p++) {
> - if (quoted) {
> - val += data[p];
> - quoted = false;
> - } else if (data[p].unicode() == '\\') {
> - quoted = true;
> - } else if (data[p].unicode() == ',') {
> - val.squeeze(); // release any unused memory
> - value.append(val);
> - val.clear();
> - val.reserve(data.size() - p);
> - } else {
> - val += data[p];
> + QVector<int> escapedAt;
> + bool escapedLast = false;
I don't understand why this needs a vector. Isn't this only about consecutive
backslashes?
Wouldn't a counter be enough?
> kconfigini.cpp:797
> + case ',':
> + // not really an escape sequence, but allowed in .desktop
> files, don't strip '\;' from the string
> + *r = '\\';
Did you mean '\,' here?
REPOSITORY
R237 KConfig
REVISION DETAIL
https://phabricator.kde.org/D14162
To: apol, #frameworks, dfaure
Cc: dfaure, anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns
