Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Key Binding Scheme Selector #3857

Merged
merged 3 commits into from
Feb 10, 2017
Merged

Conversation

akervern
Copy link
Contributor

@akervern akervern commented Jan 23, 2017

What does this PR do?

Here is a primarily work before trying to contribute an Eclipse based Key Binding:

This PR add the ability to switch between registered Key Bindings Scheme.

  • Refactor the way org.eclipse.che.ide.keybinding.KeyBindingManager worked. Using a Map of registered schemes instead of manager's attributes.
  • Expose accessor / helper methods on org.eclipse.che.ide.keybinding.KeyBindingManager
  • Fallback selected scheme missing bindings to the global one

In the key binding popup:

  • Add a listbox listing to select any registered scheme
  • Show witch action is fallback-ing on the global one

https://github.com/codenvy/qa/pull/581

Changelog

org.eclipse.che.ide.api.keybinding.KeyBindingAgent#getEclipse has been deprecated: use KeyBindingAgent#getActive() or KeyBindingAgent#getScheme(String) if you want to access a Scheme

Release Notes

We have deprecated org.eclipse.che.ide.api.keybinding.KeyBindingAgent#getEclipse. You can access a scheme via KeyBindingAgent#getActive() or KeyBindingAgent#getScheme(String).

Signed-off-by: Arnaud Kervern <[email protected]>
@codenvy-ci
Copy link

Can one of the admins verify this patch?

@benoitf
Copy link
Contributor

benoitf commented Jan 23, 2017

ci-build

@benoitf benoitf added kind/enhancement A feature request - must adhere to the feature request template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Jan 23, 2017
@codenvy-ci
Copy link

Build # 1742 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/1742/ to view the results.

@akervern
Copy link
Contributor Author

Any news?


String isGlobal();

String selectionLabel();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akervern
Add some javadoc for selection* fields, please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments added.

@vparfonov vparfonov added status/pending-merge and removed status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Feb 10, 2017
@vparfonov vparfonov added this to the 5.3.0 milestone Feb 10, 2017
@bmicklea
Copy link

Two questions about this:

  • Do we know how widely people use org.eclipse.che.ide.api.keybinding.KeyBindingAgent#getEclipse today?
  • Is there any urgency to removing it?

@akervern
Copy link
Contributor Author

From my point of view; org.eclipse.che.ide.api.keybinding.KeyBindingAgent#getEclipse seems to not be called internally, neither in any plugin in the main repository.
I kept it in case some external plugins / proprietary code use it.

Copy link

@bmicklea bmicklea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification

@vparfonov vparfonov merged commit e4117f9 into eclipse-che:master Feb 10, 2017
@JamesDrummond JamesDrummond mentioned this pull request Feb 17, 2017
9 tasks
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
* Add Key Binding Scheme Selector

Signed-off-by: Arnaud Kervern <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants