Bug 85586 – Web Inspector: "Goto Function" filtering should be less restrictive.

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

User is forced to use asterisks to find something that is not camel-cased, or if he can't remember what terms are camel cased.

This is not convenient.

My proposal:
  1) Avoid asterisks at all; new filtering should behave like old filtering with asterisk added after every input character;
  2) Optimization: do not create regexp for each item inside filtering loop;

Created attachment 140171 [details]
Patch

[Pavel Feldman]

Comment on attachment 140171 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140171&action=review

> Source/WebCore/inspector/front-end/FilteredItemSelectionDialog.js:129
> +            if (this._checkItemAt(i)) {

To make things simple, I would structure the code as follows:

var regex = this._createSearchRegex(this._promptElement.value);
for (var i = ...) {
    if (regex.test(this._delegate.itemKeyAt(i))
        ...
}

Then method below would not be needed.

> Source/WebCore/inspector/front-end/FilteredItemSelectionDialog.js:195
> +        if (!query) {

The WebKit style for thise would be:

if (!query) {
    delete this._checkItemAt;
    return;
}

rest of the code here.

> Source/WebCore/inspector/front-end/FilteredItemSelectionDialog.js:204
> +            this._checkItemAt = checker.bind(this);

You should  never overwrite methods on the prototype.

> Source/WebCore/inspector/front-end/FilteredItemSelectionDialog.js:231
> +                c = c + "\\";

Escaping via appending \\ looks strange. Having test cases suggesting the behavior pattern would help understanding the logic.

Comment on attachment 140171 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140171&action=review

>> Source/WebCore/inspector/front-end/FilteredItemSelectionDialog.js:129
>> +            if (this._checkItemAt(i)) {
> 
> To make things simple, I would structure the code as follows:
> 
> var regex = this._createSearchRegex(this._promptElement.value);
> for (var i = ...) {
>     if (regex.test(this._delegate.itemKeyAt(i))
>         ...
> }
> 
> Then method below would not be needed.

Right you are, but i've tried to preserve original code structure.
Fixed.

>> Source/WebCore/inspector/front-end/FilteredItemSelectionDialog.js:195
>> +        if (!query) {
> 
> The WebKit style for thise would be:
> 
> if (!query) {
>     delete this._checkItemAt;
>     return;
> }
> 
> rest of the code here.

Thanks.

>> Source/WebCore/inspector/front-end/FilteredItemSelectionDialog.js:204
>> +            this._checkItemAt = checker.bind(this);
> 
> You should  never overwrite methods on the prototype.

OK

>> Source/WebCore/inspector/front-end/FilteredItemSelectionDialog.js:231
>> +                c = c + "\\";
> 
> Escaping via appending \\ looks strange. Having test cases suggesting the behavior pattern would help understanding the logic.

Oops. Fixed. Tests added

Created attachment 140398 [details]
Patch

Created attachment 140402 [details]
Patch

[WebKit Review Bot]

Comment on attachment 140402 [details]
Patch

Clearing flags on attachment: 140402

Committed r116244: <http://trac.webkit.org/changeset/116244>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.