Bug 87654 - [EFL] Refactoring. Get rid of unnecessary singleCharacterString() function
Summary: [EFL] Refactoring. Get rid of unnecessary singleCharacterString() function
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mikhail Pozdnyakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-28 06:10 PDT by Mikhail Pozdnyakov
Modified: 2012-05-29 05:13 PDT (History)
4 users (show)

See Also:

Attachments
patch (12.41 KB, patch)
2012-05-28 06:25 PDT, Mikhail Pozdnyakov 		no flags Details | Formatted Diff | Diff
patch v2 (Review remarks are met) (12.86 KB, patch)
2012-05-28 07:05 PDT, Mikhail Pozdnyakov 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Pozdnyakov 2012-05-28 06:10:33 PDT 
Evas key events already provide key string, for instance if the User presses 'Return' key the generated Evas_Event_Key_Down(_Up) events will contain following fields:
keyname = "Return", key = "Return", string = "\r", compose = "\r"
So no practical need for singleCharacterString() evaluating the event string from event keyname.
The only place where this function is currently used is DRT, however DRT EventSender also can provide event string itself.
Comment 1 Mikhail Pozdnyakov 2012-05-28 06:25:25 PDT 
Created attachment 144345 [details]
patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2012-05-28 06:34:30 PDT 
Comment on attachment 144345 [details]
patch

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

> Tools/DumpRenderTree/efl/EventSender.cpp:124
> +    KeyEventInfo(const CString& keyName, unsigned modifiers)
> +        : keyName(keyName)
> +        , modifiers(modifiers)

Instead of adding a new constructor, I'd change the order of parameters in the other one and make keyString an optional last parameter whose default value is an empty string.
Comment 3 Mikhail Pozdnyakov 2012-05-28 07:05:29 PDT 
Created attachment 144353 [details]
patch v2 (Review remarks are met)
Comment 4 Mikhail Pozdnyakov 2012-05-28 07:06:40 PDT 
(In reply to comment #2)
> (From update of attachment 144345 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=144345&action=review
> 
> > Tools/DumpRenderTree/efl/EventSender.cpp:124
> > +    KeyEventInfo(const CString& keyName, unsigned modifiers)
> > +        : keyName(keyName)
> > +        , modifiers(modifiers)
> 
> Instead of adding a new constructor, I'd change the order of parameters in the other one and make keyString an optional last parameter whose default value is an empty string.

done.
Comment 5 Raphael Kubo da Costa (:rakuco) 2012-05-28 07:19:52 PDT 
Comment on attachment 144353 [details]
patch v2 (Review remarks are met)

Looks good, thanks.
Comment 6 Gyuyoung Kim 2012-05-28 18:14:30 PDT 
Comment on attachment 144353 [details]
patch v2 (Review remarks are met)

Looks good to me too.
Comment 7 Csaba Osztrogonác 2012-05-29 04:48:34 PDT 
Comment on attachment 144353 [details]
patch v2 (Review remarks are met)

rs=me
Comment 8 WebKit Review Bot 2012-05-29 05:13:18 PDT 
Comment on attachment 144353 [details]
patch v2 (Review remarks are met)

Clearing flags on attachment: 144353

Committed r118760: <http://trac.webkit.org/changeset/118760>
Comment 9 WebKit Review Bot 2012-05-29 05:13:22 PDT 
All reviewed patches have been landed.  Closing bug.