Couzteau issue1241

[couzteau]

Hello Brackets people,
Happy to make a stab at my first contribution. :)

All keycode references in src should be updated. All tests are passing.
I have not updated all tests to use the new module yet, but would be happy to update them soon.
Got to get to my day job ;)

Not sure if you like that I added some KeyCode definitions that are not defined here:
http://www.javascripter.net/faq/keyeventconstantsfirefox.htm

Those listed below are not defined in the , as IE deviates from other browsers. Since brackets uses them I added them following the same naming convention as other key codes. We may want to use a different prefix rather than DOM_VK? to stay consisten Happy to change if you prefer.
DOM_VK_SEMICOLON: 186,
DOM_VK_EQUALS: 187,
DOM_VK_COMMA: 188,
DOM_VK_DASH: 189

Adding them in required to remove definition defined the reference doc above:
DOM_VK_EQUALS: 61,
DOM_VK_SEMICOLON: 59,

I have not updated any key code references in sources in 3rd party or extensions.

Jochen

[jasonsanjose]

fix comment

[couzteau]

fixed

[jasonsanjose]

Reviewing

[jasonsanjose]

Looks like there are some additional references that use e.which instead of e.keyCode in widgets/Dialogs. Would you update those as well?

Initial review complete.

[couzteau]

Sure - will make the changes.

[couzteau]

Thanks for the through review - i was rushing it a bit this morning

[couzteau]

@RaymondLim : I did think about that, but I looked it up. Turns out the original definition
per also defines the code as VK_EQUALS
http://www.javascripter.net/faq/keyeventconstantsfirefox.htm

Note - I redefined the code as the brackets uses 187.
KeyEvent.DOM_VK_EQUALS = 61

•    DOM_VK_EQUALS: 187,
  

Also, why plural here? should be VK_EQUAL.

[RaymondLim]

@couzteau

I googled on VK_EQUAL and found out that there are two camps. So I'm okay to keep it as is.

BTW, there are some hard-coded key codes in CodeHintManager.js that you need to replace with your changes too.

[couzteau]

Thanks Ray - I'll take a look at Code hinte manger as well as the tests and update the pull request.

[jasonsanjose]

Hi Jochen. Looks like you need to sync up with master before we can merge.

[couzteau]

Thanks for the info - won't get ti it before tonight.

[jasonsanjose]

Removing $ was ok here, but you should be seeing a jslint error for window.

[couzteau]

Yes - JSLint did complain. - I put it back in

[couzteau]

It was complaining about both - I put both back in.

[jasonsanjose]

@couzteau please also sign the CLA before I merge this pull request http://dev.brackets.io/brackets-contributor-license-agreement.html

[couzteau]

done

[jasonsanjose]

Thanks. Merging.

[couzteau]

Yay! Thanks -my pleasure.