KeyBindingManager methods should take a command

[njx]

Most API functions that take a commandId can also take a command instead, but the KeyBindingManager methods only take a commandId.

[pthiess]

Reviewed assigned to @redmunds

[redmunds]

I added the Starter Bug tag. Even though this is assigned to me, anyone is welcome to take this one.

[lkcampbell]

I'm working on a fix for this bug.

[lkcampbell]

Do you want me to try and write some new unit tests as well?

Right now, I am just focusing on not breaking the existing tests :).

[redmunds]

You don't need to duplicate all of the other tests, but we should have at least 1 test for passing a Command.

[lkcampbell]

One quick question on any of these fixes I do. I am using the Whitespace Normalizer extension on Brackets which automatically cleans up whitespace on any file I touch.

Do you guys want me to do this? Is it helpful or does it made the diffs and code more difficult to maintain?

[peterflynn]

@lkcampbell My 2c is that it's better not to do that cleanup. If definitely adds to the diffs, and whitespace on blank lines is permitted by our style guide -- maybe even desired, since it makes the cursor jump around less as you navigate across lines. We actually did a bunch of work to turn off JSLint warnings for such lines so we could have them in our code.

Trailing whitespace on non-blank lines is a minor annoyance though, so it's ok by me to clean up cases like that.

[lkcampbell]

@peterflynn Sounds good to me. I'm not fond of the jumpy cursor either.

I will throw the empty line white space back in and avoid cleanups in the future.

[redmunds]

Confirmed. Closing.