Attachment 66310 Details for Bug 185523 – Different approach v01 (draft)

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

[patch] Different approach v01 (draft)

logging-v01.txt (text/plain), 13.30 KB, created by Paul Webster

on 2007-05-08 11:46:51 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Paul Webster

Created: 2007-05-08 11:46:51 EDT

Size: 13.30 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#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	8 May 2007 15:43:52 -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,8 @@
> 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.dialogs.MessageDialog;
>+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 +466,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 +594,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 +648,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 +666,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);
> 	}
>@@ -758,7 +765,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 +775,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 +785,9 @@
> 			}
> 		}
> 
>+		KeySequence errorSequence = null;
>+		Collection errorMatch = null;
>+
> 		KeySequence sequenceBeforeKeyStroke = state.getCurrentSequence();
> 		for (Iterator iterator = potentialKeyStrokes.iterator(); iterator
> 				.hasNext();) {
>@@ -806,9 +818,35 @@
> 				// 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;
>+				}
> 			}
> 		}
> 
>+		if (sequenceBeforeKeyStroke.isEmpty() && errorSequence != null) {
>+			StringBuffer b = new StringBuffer();
>+			Iterator i = errorMatch.iterator();
>+			while (i.hasNext()) {
>+				if (b.length() > 0) {
>+					b.append(", "); //$NON-NLS-1$
>+				}
>+				try {
>+					b.append(((Binding) i.next()).getParameterizedCommand()
>+							.getName());
>+				} catch (NotDefinedException e) {
>+					// well, he won't play nice
>+				}
>+			}
>+			MessageDialog.openError(Util.getShellToParentOn(), "Key Conflict", //$NON-NLS-1$
>+					b.toString());
>+
>+		}
>+
> 		resetState(true);
> 		return !sequenceBeforeKeyStroke.isEmpty();
> 	}
>#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	8 May 2007 15:43:53 -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	8 May 2007 15:43:53 -0000
>@@ -10,9 +10,6 @@
>  *******************************************************************************/
> package org.eclipse.jface.bindings;
> 
>-import java.io.BufferedWriter;
>-import java.io.IOException;
>-import java.io.StringWriter;
> import java.util.ArrayList;
> import java.util.Arrays;
> import java.util.Collection;
>@@ -42,6 +39,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;
>@@ -272,8 +270,6 @@
> 	 */
> 	private Map prefixTable = null;
> 
>-	private Set triggerConflicts = new HashSet();
>-
> 	/**
> 	 * <p>
> 	 * Constructs a new instance of <code>BindingManager</code>.
>@@ -438,7 +434,7 @@
> 	 * This method completes in <code>O(1)</code>.
> 	 */
> 	private final void clearSolution() {
>-		setActiveBindings(null, null, null);
>+		setActiveBindings(null, null, null, null);
> 	}
> 
> 	/**
>@@ -511,7 +507,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.
> 		 */
>@@ -631,27 +628,7 @@
> 					final Binding winner = resolveConflicts((Collection) match,
> 							activeContextTree);
> 					if (winner == null) {
>-						// warn once ... so as not to flood the logs
>-						if (triggerConflicts.add(trigger)) {
>-							final StringWriter sw = new StringWriter();
>-							final BufferedWriter buffer = new BufferedWriter(sw);
>-							try {
>-								buffer.write("A conflict occurred for "); //$NON-NLS-1$
>-								buffer.write(trigger.toString());
>-								buffer.write(':');
>-								Iterator i = ((Collection) match).iterator();
>-								while (i.hasNext()) {
>-									buffer.newLine();
>-									buffer.write(i.next().toString());
>-								}
>-								buffer.flush();
>-							} catch (IOException e) {
>-								// we should not get this
>-							}
>-							conflicts.add(new Status(IStatus.WARNING,
>-									"org.eclipse.jface", //$NON-NLS-1$
>-									sw.toString()));
>-						}
>+						conflictsByTrigger.put(trigger, match);
> 						if (DEBUG) {
> 							Tracing.printTrace("BINDINGS", //$NON-NLS-1$
> 									"A conflict occurred for " + trigger); //$NON-NLS-1$
>@@ -997,10 +974,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 +1036,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 +1684,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 +1713,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 +1726,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 +2138,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	8 May 2007 15:43:53 -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();
> }

Actions: View | Diff

Attachments on bug 185523: 66310 | 66360 | 66535