Bug 109200

Summary: Web Inspector: implement smart braces functionality
Product: WebKit Reporter: Andrey Lushnikov <lushnikov>
Component: Web Inspector (Deprecated)Assignee: Andrey Lushnikov <lushnikov>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, gyuyoung.kim, jochen, keishi, loislo, pfeldman, pmuellr, rakuco, vsevik, web-inspector-bugs, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

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.