Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 380322

Summary:	Scheme is ignored while selecting the best shortcut to display
Product:	[Eclipse Project] Platform	Reporter:	Paul Webster <pwebster>
Component:	UI	Assignee:	Paul Webster <pwebster>
Status:	VERIFIED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	bsd, daniel_megert, emoffatt, gheorghe, john.arthorne, ob1.eclipse
Version:	4.2	Flags:	ob1.eclipse: review+ bsd: review+
Target Milestone:	4.2 RC2
Hardware:	PC
OS:	Linux
Whiteboard:
Bug Depends on:	380301
Bug Blocks:	375762

Description Paul Webster

2012-05-22 15:39:25 EDT

When you switch to the emacs scheme, Edit>Find/Replace still shows CTRL+F (although that's next word in emacs).  It should be ALT+R.

That's because getBestSequenceFor(*) ignores the scheme when sorting the bindings.

It should sort by scheme first, and then pick best values out of that.

PW

Comment 1 Paul Webster

2012-05-22 15:45:33 EDT

I've pushed with the fix, but before I get it reviewed we should decide if we need to clean up the code.

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=pwebster/bug380322&id=4b2a72f8f1934f4c1e3dfd4ea24bc52cc5972933

As is we end up with replication of the same compareSchemes in a couple of locations.  BindingService, BindingTables, BindingTableManager.

This could be reduced to one location and reused, possibly a utility method.

PW

Comment 2 Paul Webster

2012-05-22 18:01:06 EDT

This is the fix that includes refactoring compareSchemes(*) into a utility method:

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=pwebster/bug380322&id=42ba5e21e99a9fc8f7f7b5542fc60f54418077f5

PW

Comment 3 Paul Webster

2012-05-22 18:01:42 EDT

It depends on the fix for bug 380301

Oleg, Eric, could I get a review?

PW

Comment 4 Oleg Besedin

2012-05-23 11:48:41 EDT

+1 for the fix from the comment 1 (without refactoring).

I'd prefer to have minimal code changes at this point. Probably open a bug for 4.3 as a reminder to clean up the code when we are in a normal development cycle.

Comment 5 Brian de Alwis

2012-05-23 13:23:54 EDT

Looks safe to me.

Comment 6 Paul Webster

2012-05-23 14:47:09 EDT

Fixed with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=4b2a72f8f1934f4c1e3dfd4ea24bc52cc5972933

I opened bug 380463  for the refactoring.

PW

Comment 7 Paul Webster

2012-05-23 14:47:42 EDT

Comment 8 Dani Megert

2012-05-24 02:46:46 EDT

Verified in 4.2-I20120523-1900.