Bug 109200 - Web Inspector: implement smart braces functionality
Summary: Web Inspector: implement smart braces functionality
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andrey Lushnikov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-07 08:56 PST by Andrey Lushnikov
Modified: 2013-02-15 04:32 PST (History)
12 users (show)

See Also:


Attachments
Patch (12.97 KB, patch)
2013-02-07 09:02 PST, Andrey Lushnikov
no flags Details | Formatted Diff | Diff
Patch (13.48 KB, patch)
2013-02-07 10:27 PST, Andrey Lushnikov
no flags Details | Formatted Diff | Diff
Patch (15.54 KB, patch)
2013-02-12 01:47 PST, Andrey Lushnikov
no flags Details | Formatted Diff | Diff
Patch (15.69 KB, patch)
2013-02-12 01:55 PST, Andrey Lushnikov
no flags Details | Formatted Diff | Diff
Patch (15.72 KB, patch)
2013-02-12 02:52 PST, Andrey Lushnikov
no flags Details | Formatted Diff | Diff
Patch (18.65 KB, patch)
2013-02-14 05:37 PST, Andrey Lushnikov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Lushnikov 2013-02-07 08:56:57 PST
By "smart braces" I mean:
- When I type "(", a closing brace should be added immediately and the cursor should be located inside "()" pair.
- When then I hit "Backspace", both braces should be deleted
- If, instead, I type ")" - cursor should go over closing brace without adding a new one.
Comment 1 Andrey Lushnikov 2013-02-07 09:02:51 PST
Created attachment 187125 [details]
Patch
Comment 2 Andrey Lushnikov 2013-02-07 10:27:22 PST
Created attachment 187133 [details]
Patch
Comment 3 Andrey Lushnikov 2013-02-12 01:47:57 PST
Created attachment 187809 [details]
Patch
Comment 4 Andrey Lushnikov 2013-02-12 01:48:50 PST
Comment on attachment 187809 [details]
Patch

test expectations not updated
Comment 5 Andrey Lushnikov 2013-02-12 01:55:56 PST
Created attachment 187811 [details]
Patch
Comment 6 Andrey Lushnikov 2013-02-12 02:52:34 PST
Created attachment 187819 [details]
Patch
Comment 7 Pavel Feldman 2013-02-12 06:45:05 PST
Comment on attachment 187819 [details]
Patch

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

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1398
> +            this._textInputInterceptors["("] = this._handleBracePairInsertion.bind(this, "()");

registerInterceptor("(", "()", this._handleBracePairInsertion); would look more user-friendly

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:2829
> +        var interceptor = this._textInputInterceptors[event.data];

textInput will trigger for pasting text as well and you don't want interceptors to work there. Instad of text input interceptors, you want keyDown handlers.
Comment 8 Andrey Lushnikov 2013-02-14 05:37:15 PST
Created attachment 188329 [details]
Patch
Comment 9 Andrey Lushnikov 2013-02-14 06:21:46 PST
Comment on attachment 187819 [details]
Patch

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

>> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1398
>> +            this._textInputInterceptors["("] = this._handleBracePairInsertion.bind(this, "()");
> 
> registerInterceptor("(", "()", this._handleBracePairInsertion); would look more user-friendly

not sure I got your idea.. Do you mean we should create a public "registerInterceptor" method?

>> Source/WebCore/inspector/front-end/DefaultTextEditor.js:2829
>> +        var interceptor = this._textInputInterceptors[event.data];
> 
> textInput will trigger for pasting text as well and you don't want interceptors to work there. Instad of text input interceptors, you want keyDown handlers.

fixed to handling in keyPress
Comment 10 Pavel Feldman 2013-02-15 04:02:54 PST
Comment on attachment 188329 [details]
Patch

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

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:3549
> +        } else

return false

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:3595
> +        } else

return false;
Comment 11 WebKit Review Bot 2013-02-15 04:32:07 PST
Comment on attachment 188329 [details]
Patch

Clearing flags on attachment: 188329

Committed r142983: <http://trac.webkit.org/changeset/142983>
Comment 12 WebKit Review Bot 2013-02-15 04:32:12 PST
All reviewed patches have been landed.  Closing bug.