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
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
Comment on attachment 140402 [details] Patch Clearing flags on attachment: 140402 Committed r116244: <http://trac.webkit.org/changeset/116244>
All reviewed patches have been landed. Closing bug.