Feature decorate disabled pkg keymap

[bobjunga]

Description of the Change

In PackageKeymapView::updateKeyBindingView in the loop that creates the table element with rows for each of the packages keybings, I added a block that checks to see if the keybinding is currently in atom.keymaps.keyBindings[] and adds decorators to the row elements if it is not.

Decorators on Disabled Keybinding rows:

• text-sublte class to the keybinding row
• 'icon icon-circle-slash' classes to the first column
• tooltip on the first column that reads "Disabled. This binding it not in the list of active bindings".

The result is ...

I added two specs -- one to test that enabled keybindings do not have the class decorators and another to test that a disabled keybinding does. I did two specs so that I could reuse an existing fixture which seemed less intrusive.

Alternate Designs

This just makes the package keymap section of settings view tell the current truth about the state of the package's keybindings. I can not think of any alternatives.

Benefits

There are already packages like 'disable-keybindings' (8k downloads) that allow the user selectively disable just some of the keybindings that a package provides but the settings view does not reflect the correct state when this happens. It decreases confusion when the user has a more accurate picture of the system state.

Possible Drawbacks

I think that this change is safe because it does not introduce any new behavior itself (you may support or be against the idea of disabling keybindings selectively at runtime) but if for any reason a keybinding is not active, this allows the user to see that its not active.

The user may be left wondering why the keybinding is disabled but addressing that would be a step further. Compared to the settings view making it look like all the package's keybindings are enabled when they are not, the user is better off.

We could go further and add a KeymapManager::remove(reason, filterCallback) API. Then the PackageKeymapView::updateKeyBindingView code could display the reason a keybinding is disabled. That would provide a path to good citizenship for the disable-keybindings and similar packages that modify atom.keymap.keyBindings directly when really they should not be doing that.

LMK if you would like me to add that to this PR.

Applicable Issues

None known. (BTW, I was not sure if I should go straight to a PR or if I should have created a feature request issue first. LMK)

[bobjunga]

Sorry about all the extraneous commits. It took me a while to find 'npm run lint' (I did try a lot of 'apm ..' commands)

I would clean it up but I dont see a way to change the branch once a PR is created and I would think rebasing the existing branch would be bad.

If you point me to instructions I would be happy to clean it up.

[bobjunga]

I created a new branch "feature-decorate-disabled-pkg-keymap-clean" that has the same code as this branch (feature-decorate-disabled-pkg-keymap) but only one commit.

[waynepaulpace]

Ur a unbelievable man
May God forgive u Wayne Paul pace cause u r a bad husband and father to ur wife )real one) every needs to no you r a

• A malignant narcissist@