D14162: Figure out the escaped path list on kconfig

David Faure noreply at phabricator.kde.org
Mon Aug 6 13:01:28 BST 2018


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180806/38751df8/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list