Fixed: #3032 KeyBindingManager no longer warns / prevents binding collisions

[WebsiteDeveloper]

Fix for #3032

[jasonsanjose]

@WebsiteDeveloper are you on win or mac? On your branch, I get the following console error on mac:

Cannot assign Cmd-G to navigate.gotoLine. It is already assigned to edit.findNext 

This works out to be okay. Cmd-G is assigned to findNext, it isn't removed, and gotoLine is assigned Cmd-G.

[jasonsanjose]

In #3032, what happens is that the extension tries to replace a cross-platform Ctrl-E/Cmd-E binding with another cross-platform Ctrl-E/Cmd-# binding. I think a more correct fix might be to check the new binding for an explicit platform and only override it in that case:

if (!existing.explicitPlatform && existingPlatform) {
  // remove the the generic binding to replace with this new platform-specific binding
  removeBinding(normalized)
} else { 
  console.error(...);
  return null;
}

[jasonsanjose]

Initial review complete

[WebsiteDeveloper]

@jasonsanjose i adressed your remark

[jasonsanjose]

Merging

[lkcampbell]

FYI, I just double checked the KeyBindingManager unit tests because we had two merges on KeyBindingManager.js fairly close to each other. I wasn't getting an error in my branch but I am getting an error now. I don't know if it is because of my change or @WebsiteDeveloper 's change or a combination of both, but it is there now.

[WebsiteDeveloper]

@lkcampbell it happens actually because of my changes, but jasonsanjose already set up a pull to fix this