Attachment 66535 Details for Bug 185523 – Key Assist v03 (draft)

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

[patch] Key Assist v03 (draft)

logging-v03.txt (text/plain), 15.40 KB, created by Paul Webster

on 2007-05-09 15:09:03 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Paul Webster

Created: 2007-05-09 15:09:03 EDT

Size: 15.40 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jface
>Index: src/org/eclipse/jface/bindings/CachedBindingSet.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jface/src/org/eclipse/jface/bindings/CachedBindingSet.java,v
>retrieving revision 1.10
>diff -u -r1.10 CachedBindingSet.java
>--- src/org/eclipse/jface/bindings/CachedBindingSet.java	8 May 2006 20:57:00 -0000	1.10
>+++ src/org/eclipse/jface/bindings/CachedBindingSet.java	9 May 2007 19:06:47 -0000
>@@ -61,6 +61,8 @@
> 	 * This value may be <code>null</code> if it has not yet been initialized.
> 	 */
> 	private Map bindingsByTrigger = null;
>+	
>+	private Map conflictsByTrigger = null;
> 
> 	/**
> 	 * The hash code for this object. This value is computed lazily, and marked
>@@ -229,6 +231,10 @@
> 	final Map getBindingsByTrigger() {
> 		return bindingsByTrigger;
> 	}
>+	
>+	final Map getConflictsByTrigger() {
>+		return conflictsByTrigger;
>+	}
> 
> 	/**
> 	 * Returns the map of prefixes to a map of trigger sequence to command
>@@ -294,6 +300,14 @@
> 
> 		this.bindingsByTrigger = commandIdsByTrigger;
> 	}
>+	
>+	final void setConflictsByTrigger(final Map c) {
>+		if (c == null) {
>+			throw new NullPointerException(
>+					"Cannot set a null binding conflicts"); //$NON-NLS-1$
>+		}
>+		conflictsByTrigger = c;
>+	}
> 
> 	/**
> 	 * Sets the map of prefixes to a map of trigger sequence to command
>Index: src/org/eclipse/jface/bindings/BindingManager.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jface/src/org/eclipse/jface/bindings/BindingManager.java,v
>retrieving revision 1.44
>diff -u -r1.44 BindingManager.java
>--- src/org/eclipse/jface/bindings/BindingManager.java	2 May 2007 17:14:46 -0000	1.44
>+++ src/org/eclipse/jface/bindings/BindingManager.java	9 May 2007 19:06:47 -0000
>@@ -42,6 +42,7 @@
> import org.eclipse.jface.bindings.keys.KeyLookupFactory;
> import org.eclipse.jface.bindings.keys.KeyStroke;
> import org.eclipse.jface.contexts.IContextIds;
>+import org.eclipse.jface.internal.InternalPolicy;
> import org.eclipse.jface.util.Policy;
> import org.eclipse.jface.util.Util;
> import org.eclipse.swt.SWT;
>@@ -181,6 +182,8 @@
> 	 * Otherwise, this value may be empty.
> 	 */
> 	private Map activeBindingsByParameterizedCommand = null;
>+	
>+	private Set triggerConflicts = new HashSet();
> 
> 	/**
> 	 * The scheme that is currently active. An active scheme is the one that is
>@@ -272,8 +275,6 @@
> 	 */
> 	private Map prefixTable = null;
> 
>-	private Set triggerConflicts = new HashSet();
>-
> 	/**
> 	 * <p>
> 	 * Constructs a new instance of <code>BindingManager</code>.
>@@ -438,7 +439,7 @@
> 	 * This method completes in <code>O(1)</code>.
> 	 */
> 	private final void clearSolution() {
>-		setActiveBindings(null, null, null);
>+		setActiveBindings(null, null, null, null);
> 	}
> 
> 	/**
>@@ -511,7 +512,8 @@
> 	 *            computed).
> 	 */
> 	private final void computeBindings(final Map activeContextTree,
>-			final Map bindingsByTrigger, final Map triggersByCommandId) {
>+			final Map bindingsByTrigger, final Map triggersByCommandId, 
>+			final Map conflictsByTrigger) {
> 		/*
> 		 * FIRST PASS: Remove all of the bindings that are marking deletions.
> 		 */
>@@ -632,6 +634,7 @@
> 							activeContextTree);
> 					if (winner == null) {
> 						// warn once ... so as not to flood the logs
>+						conflictsByTrigger.put(trigger, match);
> 						if (triggerConflicts.add(trigger)) {
> 							final StringWriter sw = new StringWriter();
> 							final BufferedWriter buffer = new BufferedWriter(sw);
>@@ -997,10 +1000,12 @@
> 		// Compute the active bindings.
> 		commandIdsByTrigger = new HashMap();
> 		final Map triggersByParameterizedCommand = new HashMap();
>+		final Map conflictsByTrigger = new HashMap();
> 		computeBindings(null, commandIdsByTrigger,
>-				triggersByParameterizedCommand);
>+				triggersByParameterizedCommand, conflictsByTrigger);
> 		existingCache.setBindingsByTrigger(commandIdsByTrigger);
> 		existingCache.setTriggersByCommandId(triggersByParameterizedCommand);
>+		existingCache.setConflictsByTrigger(conflictsByTrigger);
> 		return Collections.unmodifiableMap(commandIdsByTrigger);
> 	}
> 
>@@ -1057,11 +1062,13 @@
> 
> 		// Compute the active bindings.
> 		final Map commandIdsByTrigger = new HashMap();
>+		final Map conflictsByTrigger = new HashMap();
> 		triggersByParameterizedCommand = new HashMap();
> 		computeBindings(null, commandIdsByTrigger,
>-				triggersByParameterizedCommand);
>+				triggersByParameterizedCommand, conflictsByTrigger);
> 		existingCache.setBindingsByTrigger(commandIdsByTrigger);
> 		existingCache.setTriggersByCommandId(triggersByParameterizedCommand);
>+		existingCache.setConflictsByTrigger(conflictsByTrigger);
> 
> 		return Collections.unmodifiableMap(triggersByParameterizedCommand);
> 	}
>@@ -1703,7 +1710,7 @@
> 		if (bindings == null) {
> 			// Not yet initialized. This is happening too early. Do nothing.
> 			setActiveBindings(Collections.EMPTY_MAP, Collections.EMPTY_MAP,
>-					Collections.EMPTY_MAP);
>+					Collections.EMPTY_MAP, Collections.EMPTY_MAP);
> 			return;
> 		}
> 
>@@ -1732,7 +1739,8 @@
> 				Tracing.printTrace("BINDINGS", "Cache hit"); //$NON-NLS-1$ //$NON-NLS-2$
> 			}
> 			setActiveBindings(commandIdsByTrigger, existingCache
>-					.getTriggersByCommandId(), existingCache.getPrefixTable());
>+					.getTriggersByCommandId(), existingCache.getPrefixTable(),
>+					existingCache.getConflictsByTrigger());
> 			return;
> 		}
> 
>@@ -1744,12 +1752,15 @@
> 		// Compute the active bindings.
> 		commandIdsByTrigger = new HashMap();
> 		final Map triggersByParameterizedCommand = new HashMap();
>+		final Map conflictsByTrigger = new HashMap();
> 		computeBindings(activeContextTree, commandIdsByTrigger,
>-				triggersByParameterizedCommand);
>+				triggersByParameterizedCommand, conflictsByTrigger);
> 		existingCache.setBindingsByTrigger(commandIdsByTrigger);
> 		existingCache.setTriggersByCommandId(triggersByParameterizedCommand);
>+		existingCache.setConflictsByTrigger(conflictsByTrigger);
> 		setActiveBindings(commandIdsByTrigger, triggersByParameterizedCommand,
>-				buildPrefixTable(commandIdsByTrigger));
>+				buildPrefixTable(commandIdsByTrigger),
>+				conflictsByTrigger);
> 		existingCache.setPrefixTable(prefixTable);
> 	}
> 
>@@ -2153,11 +2164,13 @@
> 	 *            solution.
> 	 */
> 	private final void setActiveBindings(final Map activeBindings,
>-			final Map activeBindingsByCommandId, final Map prefixTable) {
>+			final Map activeBindingsByCommandId, final Map prefixTable,
>+			final Map conflicts) {
> 		this.activeBindings = activeBindings;
> 		final Map previousBindingsByParameterizedCommand = this.activeBindingsByParameterizedCommand;
> 		this.activeBindingsByParameterizedCommand = activeBindingsByCommandId;
> 		this.prefixTable = prefixTable;
>+		InternalPolicy.currentConflicts = conflicts;
> 
> 		fireBindingManagerChanged(new BindingManagerEvent(this, true,
> 				previousBindingsByParameterizedCommand, false, null, false,
>Index: src/org/eclipse/jface/internal/InternalPolicy.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jface/src/org/eclipse/jface/internal/InternalPolicy.java,v
>retrieving revision 1.1
>diff -u -r1.1 InternalPolicy.java
>--- src/org/eclipse/jface/internal/InternalPolicy.java	3 May 2007 02:38:07 -0000	1.1
>+++ src/org/eclipse/jface/internal/InternalPolicy.java	9 May 2007 19:06:47 -0000
>@@ -10,6 +10,9 @@
>  ******************************************************************************/
> package org.eclipse.jface.internal;
> 
>+import java.util.HashMap;
>+import java.util.Map;
>+
> /**
>  * Internal class used for non-API debug flags.
>  * 
>@@ -25,4 +28,9 @@
> 	 */
> 	public static boolean DEBUG_LOG_REENTRANT_VIEWER_CALLS = false;
> 
>+	/**
>+	 * (NON-API) Instead of logging current conflicts they can be
>+	 * held here.  If there is a problem, they can be reported then.
>+	 */
>+	public static Map currentConflicts = new HashMap();
> }
>#P org.eclipse.ui.workbench
>Index: Eclipse UI/org/eclipse/ui/internal/keys/WorkbenchKeyboard.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/WorkbenchKeyboard.java,v
>retrieving revision 1.94
>diff -u -r1.94 WorkbenchKeyboard.java
>--- Eclipse UI/org/eclipse/ui/internal/keys/WorkbenchKeyboard.java	16 Mar 2007 18:00:35 -0000	1.94
>+++ Eclipse UI/org/eclipse/ui/internal/keys/WorkbenchKeyboard.java	9 May 2007 19:06:47 -0000
>@@ -11,6 +11,7 @@
> package org.eclipse.ui.internal.keys;
> 
> import java.util.ArrayList;
>+import java.util.Collection;
> import java.util.Iterator;
> import java.util.List;
> import java.util.ResourceBundle;
>@@ -29,6 +30,7 @@
> import org.eclipse.jface.bindings.keys.KeyStroke;
> import org.eclipse.jface.bindings.keys.ParseException;
> import org.eclipse.jface.bindings.keys.SWTKeySupport;
>+import org.eclipse.jface.internal.InternalPolicy;
> import org.eclipse.swt.SWT;
> import org.eclipse.swt.custom.StyledText;
> import org.eclipse.swt.widgets.Combo;
>@@ -463,7 +465,8 @@
> 		}
> 
> 		try {
>-			final IHandlerService handlerService = (IHandlerService) workbench.getService(IHandlerService.class);
>+			final IHandlerService handlerService = (IHandlerService) workbench
>+					.getService(IHandlerService.class);
> 			handlerService.executeCommand(parameterizedCommand, trigger);
> 		} catch (final NotDefinedException e) {
> 			// The command is not defined. Forwarded to the IExecutionListener.
>@@ -590,11 +593,12 @@
> 	 */
> 	private Binding getPerfectMatch(KeySequence keySequence) {
> 		if (bindingService == null) {
>-			bindingService = (IBindingService) workbench.getService(IBindingService.class);
>+			bindingService = (IBindingService) workbench
>+					.getService(IBindingService.class);
> 		}
> 		return bindingService.getPerfectMatch(keySequence);
> 	}
>-	
>+
> 	final KeySequence getBuffer() {
> 		return state.getCurrentSequence();
> 	}
>@@ -643,7 +647,8 @@
> 	 */
> 	private boolean isPartialMatch(KeySequence keySequence) {
> 		if (bindingService == null) {
>-			bindingService = (IBindingService) workbench.getService(IBindingService.class);
>+			bindingService = (IBindingService) workbench
>+					.getService(IBindingService.class);
> 		}
> 		return bindingService.isPartialMatch(keySequence);
> 	}
>@@ -660,7 +665,8 @@
> 	 */
> 	private boolean isPerfectMatch(KeySequence keySequence) {
> 		if (bindingService == null) {
>-			bindingService = (IBindingService) workbench.getService(IBindingService.class);
>+			bindingService = (IBindingService) workbench
>+					.getService(IBindingService.class);
> 		}
> 		return bindingService.isPerfectMatch(keySequence);
> 	}
>@@ -720,6 +726,16 @@
> 		}
> 		keyAssistDialog.open();
> 	}
>+	
>+	public final void openKeyAssistShell(Collection bindings) {
>+		if (keyAssistDialog == null) {
>+			keyAssistDialog = new KeyAssistDialog(workbench, this, state);
>+		}
>+		if (keyAssistDialog.getShell() == null) {
>+			keyAssistDialog.setParentShell(Util.getShellToParentOn());
>+		}
>+		keyAssistDialog.open(bindings);
>+	}
> 
> 	/**
> 	 * Processes a key press with respect to the key binding architecture. This
>@@ -758,7 +774,8 @@
> 			final Widget widget = event.widget;
> 
> 			// Update the contexts.
>-			final ContextService contextService = (ContextService) workbench.getService(IContextService.class);
>+			final ContextService contextService = (ContextService) workbench
>+					.getService(IContextService.class);
> 			if ((widget instanceof Control) && (!widget.isDisposed())) {
> 				final Shell shell = ((Control) widget).getShell();
> 				contextService.updateShellKludge(shell);
>@@ -767,7 +784,8 @@
> 			}
> 
> 			// Update the handlers.
>-			final HandlerService handlerService = (HandlerService) workbench.getService(IHandlerService.class);
>+			final HandlerService handlerService = (HandlerService) workbench
>+					.getService(IHandlerService.class);
> 			if ((widget instanceof Control) && (!widget.isDisposed())) {
> 				final Shell shell = ((Control) widget).getShell();
> 				handlerService.updateShellKludge(shell);
>@@ -776,6 +794,9 @@
> 			}
> 		}
> 
>+		KeySequence errorSequence = null;
>+		Collection errorMatch = null;
>+
> 		KeySequence sequenceBeforeKeyStroke = state.getCurrentSequence();
> 		for (Iterator iterator = potentialKeyStrokes.iterator(); iterator
> 				.hasNext();) {
>@@ -806,10 +827,20 @@
> 				// We don't want to swallow keyboard navigation keys.
> 				return false;
> 
>+			} else {
>+				Collection match = (InternalPolicy.currentConflicts==null?null:(Collection) InternalPolicy.currentConflicts
>+						.get(sequenceAfterKeyStroke));
>+				if (match != null) {
>+					errorSequence = sequenceAfterKeyStroke;
>+					errorMatch = match;
>+				}
> 			}
> 		}
> 
> 		resetState(true);
>+		if (sequenceBeforeKeyStroke.isEmpty() && errorSequence != null) {
>+			openKeyAssistShell(errorMatch);
>+		}
> 		return !sequenceBeforeKeyStroke.isEmpty();
> 	}
> 
>Index: Eclipse UI/org/eclipse/ui/internal/keys/KeyAssistDialog.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/KeyAssistDialog.java,v
>retrieving revision 1.28
>diff -u -r1.28 KeyAssistDialog.java
>--- Eclipse UI/org/eclipse/ui/internal/keys/KeyAssistDialog.java	7 May 2007 22:11:36 -0000	1.28
>+++ Eclipse UI/org/eclipse/ui/internal/keys/KeyAssistDialog.java	9 May 2007 19:06:47 -0000
>@@ -11,8 +11,8 @@
> 
> package org.eclipse.ui.internal.keys;
> 
>-import com.ibm.icu.text.MessageFormat;
> import java.util.ArrayList;
>+import java.util.Collection;
> import java.util.Comparator;
> import java.util.Iterator;
> import java.util.List;
>@@ -55,6 +55,8 @@
> import org.eclipse.ui.internal.util.Util;
> import org.eclipse.ui.keys.IBindingService;
> 
>+import com.ibm.icu.text.MessageFormat;
>+
> /**
>  * <p>
>  * A dialog displaying a list of key bindings. The dialog will execute a command
>@@ -142,6 +144,8 @@
> 	 */
> 	private final WorkbenchKeyboard workbenchKeyboard;
> 
>+	private SortedMap conflictMatches;
>+
> 	/**
> 	 * Constructs a new instance of <code>KeyAssistDialog</code>. When the
> 	 * dialog is first constructed, it contains no widgets. The dialog is first
>@@ -419,7 +423,14 @@
> 		composite.setBackground(parent.getBackground());
> 
> 		// Layout the partial matches.
>-		final SortedMap partialMatches = getPartialMatches();
>+		final SortedMap partialMatches;
>+		if (conflictMatches!=null) {
>+			partialMatches = conflictMatches;
>+			conflictMatches=null;
>+		} else {
>+			partialMatches = getPartialMatches();
>+		}
>+		
> 		if (partialMatches.isEmpty()) {
> 			createEmptyDialogArea(composite);
> 		} else {
>@@ -640,6 +651,44 @@
> 		// Call the super method.
> 		return super.open();
> 	}
>+	
>+	public final int open(Collection bindings) {
>+		conflictMatches  = new TreeMap(new Comparator() {
>+			public final int compare(final Object a, final Object b) {
>+				final Binding bindingA = (Binding) a;
>+				final Binding bindingB = (Binding) b;
>+				final ParameterizedCommand commandA = bindingA
>+						.getParameterizedCommand();
>+				final ParameterizedCommand commandB = bindingB
>+						.getParameterizedCommand();
>+				try {
>+					return commandA.getName().compareTo(commandB.getName());
>+				} catch (final NotDefinedException e) {
>+					// should not happen
>+					return 0;
>+				}
>+			}
>+		});
>+		Iterator i = bindings.iterator();
>+		while (i.hasNext()) {
>+			Binding b = (Binding) i.next();
>+			conflictMatches.put(b, b.getTriggerSequence());
>+		}
>+		
>+		// If the dialog is already open, dispose the shell and recreate it.
>+		final Shell shell = getShell();
>+		if (shell != null) {
>+			close(false, false);
>+		}
>+		create();
>+
>+		// Configure the size and location.
>+		final Point size = configureSize();
>+		configureLocation(size);
>+
>+		// Call the super method.
>+		return super.open();
>+	}
> 
> 	/**
> 	 * Registers the shell as the same type as its parent with the context

Actions: View | Diff

Attachments on bug 185523: 66310 | 66360 | 66535