Consider brackets within wildcard as regular characters

[vlakoff]

For context, in #4170 we added support for « ? » and « * » glob wildcards in search patterns. But a side-effect is that « [ » and « ] » also become special characters, which is rather unexcepted and undesirable.

This PR is a new attempt at #16965, which was broken and had to be reverted in #17820. I still don't know why #16965 was failing…

Here I'm taking a new approach: we fully regex-escape the provided pattern, then we replace \? and \* with . and .* respectively. This approach should be more robust, and is even simpler actually. See this question on SO that led me to this approach.

I'm taking the opportunity to point out that at the time of #4170 (2015), only plaintext search was available, hence the need for these wildcards. In 2018 the powerful regex search has been added: #9255. Ideally, we should be able to choose between plaintext, wildcard and regex searches, see this draft.

[vlakoff]

I had this PR in draft for a few months, and I had the following in sandbox, see the comment:

// The order of the replacements below is important <-- this comment

// Replace "\?" (which has been regex-escaped above) with "."
wildcard.replace(u"\\?"_s, u"."_s);

// Replace "\*" (which has been regex-escaped above) with ".*"
// Consecutive wildcards are merged to avoid catastrophic backtracking
wildcard.replace(QRegularExpression(u"(?:\\\\*)+"_s), u".*"_s);

On second thought, I cannot figure out any case where switching the replacement order would cause issues.

But maybe I'm overlooking something I had thought of a few months ago. Does anyone think of any edge case?

[vlakoff]

Refs:

• QRegularExpression::wildcardToRegularExpression() definition and its documentation
• QRegularExpression::escape() definition, and its documentation

[glassez]

So, no one has anything to say about it?

[vlakoff]

Mmmm, not sure about the number of backslashes here:

wildcard.replace(QRegularExpression(u"(?:\\\\*)+"_s), u".*"_s);

1. We want to match literally \*, so the regex would be \\\* (escape both the backslash and the star).
2. Then, we have to escape backslashes when typing the string, so the result would be \\\\\\* (double each backslash).

[vlakoff]

Force-pushed to (probably) fix the number of backslashes, as described above.

[vlakoff]

There is a build failure, but it is unrelated to this PR, see here:

WARNING: Plugin "libqsqlodbc.dylib" uses private API and is not Mac App store compliant.
WARNING: Plugin "libqsqlpsql.dylib" uses private API and is not Mac App store compliant.
ERROR: no file at "/usr/local/opt/libiodbc/lib/libiodbc.2.dylib"
ERROR: no file at "/Applications/Postgres.app/Contents/Versions/14/lib/libpq.5.dylib"

[github-actions]

This PR is stale because it has been 60 days with no activity. This PR will be automatically closed within 7 days if there is no further activity.

[vlakoff]

Coincidentally, I had seen the 5.0 beta release, and I was about to "bump" this PR.

[vlakoff]

By the way, someone might be interested to submit a PR for Qt's wildcardToRegularExpression, to prevent regex catastrophic backtracking, if there are multiple consecutive * wildcards in the glob pattern. (I have confirmed that indeed, that can cause the application to freeze)

It is as simple as adding the following in the '*' branch of the main loop:

        case '*':
            rx += settings.starEscape;
+++         while (i < wclen && wc[i] == u'*') {
+++             ++i;
+++         }
            break;