Bug 289486 - Breakpoint hits unexpectedly
diff --git a/bundles/org.eclipse.e4.languages.javascript.debug.connect/src/org/eclipse/e4/languages/javascript/debug/connect/JSONConstants.java b/bundles/org.eclipse.e4.languages.javascript.debug.connect/src/org/eclipse/e4/languages/javascript/debug/connect/JSONConstants.java index cb7fc8a..723a054 100644 --- a/bundles/org.eclipse.e4.languages.javascript.debug.connect/src/org/eclipse/e4/languages/javascript/debug/connect/JSONConstants.java +++ b/bundles/org.eclipse.e4.languages.javascript.debug.connect/src/org/eclipse/e4/languages/javascript/debug/connect/JSONConstants.java
@@ -124,4 +124,9 @@ public static final String SOURCE = "source"; //$NON-NLS-1$ public static final String LOCATION = "location"; //$NON-NLS-1$ public static final String BASE = "base"; //$NON-NLS-1$ + + /** + * + */ + public static final String PATH = "path"; }
diff --git a/bundles/org.eclipse.e4.languages.javascript.debug.model/src/org/eclipse/e4/languages/internal/javascript/debug/launching/JavascriptSourceLookupParticipant.java b/bundles/org.eclipse.e4.languages.javascript.debug.model/src/org/eclipse/e4/languages/internal/javascript/debug/launching/JavascriptSourceLookupParticipant.java index 79b1276..39a0574 100644 --- a/bundles/org.eclipse.e4.languages.javascript.debug.model/src/org/eclipse/e4/languages/internal/javascript/debug/launching/JavascriptSourceLookupParticipant.java +++ b/bundles/org.eclipse.e4.languages.javascript.debug.model/src/org/eclipse/e4/languages/internal/javascript/debug/launching/JavascriptSourceLookupParticipant.java
@@ -68,7 +68,7 @@ Map props = script.sourceProperties(); IProject project = getProject(props); if (project != null) { - String pathname = (String) props.get("path"); + String pathname = (String) props.get(JSONConstants.PATH); String name = (String) props.get(JSONConstants.NAME); if (pathname != null && name != null) { IPath path = new Path(pathname).append(name);
diff --git a/bundles/org.eclipse.e4.languages.javascript.debug.model/src/org/eclipse/e4/languages/javascript/debug/model/JSDIBreakpoint.java b/bundles/org.eclipse.e4.languages.javascript.debug.model/src/org/eclipse/e4/languages/javascript/debug/model/JSDIBreakpoint.java index 6b1f8f8..f088b5f 100644 --- a/bundles/org.eclipse.e4.languages.javascript.debug.model/src/org/eclipse/e4/languages/javascript/debug/model/JSDIBreakpoint.java +++ b/bundles/org.eclipse.e4.languages.javascript.debug.model/src/org/eclipse/e4/languages/javascript/debug/model/JSDIBreakpoint.java
@@ -15,13 +15,17 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import org.eclipse.core.resources.IMarker; import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IPath; +import org.eclipse.core.runtime.Path; import org.eclipse.debug.core.DebugPlugin; import org.eclipse.debug.core.model.Breakpoint; import org.eclipse.debug.core.model.IDebugTarget; import org.eclipse.debug.core.model.ILineBreakpoint; +import org.eclipse.e4.languages.javascript.debug.connect.JSONConstants; import org.eclipse.e4.languages.javascript.jsdi.ScriptReference; import org.eclipse.e4.languages.javascript.jsdi.event.BreakpointEvent; import org.eclipse.e4.languages.javascript.jsdi.event.Event; @@ -29,6 +33,7 @@ import org.eclipse.e4.languages.javascript.jsdi.event.ScriptLoadEvent; import org.eclipse.e4.languages.javascript.jsdi.request.EventRequest; import org.eclipse.e4.languages.javascript.jsdi.request.ScriptLoadRequest; +import org.osgi.framework.Constants; /** * Abstract representation of a JSDI breakpoint @@ -37,12 +42,19 @@ */ public abstract class JSDIBreakpoint extends Breakpoint implements ILineBreakpoint, IJSDIEventListener { + static final String OSGI_SYMBOLIC_NAME = "OSGi." + Constants.BUNDLE_SYMBOLICNAME; //$NON-NLS-1$ + /** * Breakpoint attribute for the path of the script */ public static final String SCRIPT_PATH = "SCRIPT_PATH"; //$NON-NLS-1$ /** + * The type name within the script + */ + public static final String TYPE_NAME = "TYPE_NAME"; //$NON-NLS-1$ + + /** * JSDT member handle */ public static final String HANDLE = "HANDLE"; //$NON-NLS-1$ @@ -121,6 +133,7 @@ if (script == null) { return; } + script = new Path(script).lastSegment(); List/* ScriptReference */scripts = target.allScriptsByName(script); boolean success = true; for (Iterator iter = scripts.iterator(); iter.hasNext();) { @@ -252,6 +265,15 @@ } /** + * Returns the type name that the breakpoint is set within. When <code>null</code> is returned the breakpoint is set in the top level type i.e. the root source + * + * @return the typeName + */ + public String getTypeName() throws CoreException { + return ensureMarker().getAttribute(TYPE_NAME, null); + } + + /** * Registers the given request for this breakpoint for the given target * * @param target @@ -303,6 +325,9 @@ public boolean handleEvent(Event event, JSDIDebugTarget target, boolean suspendVote, EventSet eventSet) { // get the thread and suspend it if (event instanceof BreakpointEvent) { + if (skipEvent((BreakpointEvent) event)) { + return true; + } JSDIThread thread = target.findThread(((BreakpointEvent) event).thread()); if (thread != null) { return !thread.suspendForBreakpoint(this, suspendVote); @@ -313,7 +338,7 @@ ScriptReference script = sevent.script(); try { // TODO need something fancier in the future - if (getScriptPath().equals(script.sourcePath())) { + if (new Path(getScriptPath()).lastSegment().equals(script.sourcePath())) { createRequest(target, sevent.script()); } } catch (CoreException ce) { @@ -324,6 +349,67 @@ return true; } + /** + * If the breakpoint event should be skipped - i.e. if the thread should resume and ignore the event + * + * @param event + * @return true if the event should be skipped, false otherwise + */ + boolean skipEvent(BreakpointEvent event) { + try { + Path root = new Path(getScriptPath()); + if (root.segmentCount() == 1) { + // TODO hack to support external + virtual breakpoints + // that have no path + return false; + } + ScriptReference script = event.location().scriptReference(); + Map attribs = script.sourceProperties(); + if (attribs != null) { + IPath path = new Path((String) attribs.get(OSGI_SYMBOLIC_NAME)); + path = path.append((String) attribs.get(JSONConstants.PATH)); + path = path.append((String) attribs.get(JSONConstants.NAME)); + return !pathsEqual(root, path); + } + } catch (CoreException ce) { + // TODO log this + } + return true; + } + + /** + * Returns if the type names for the breakpoint are equal or not. Two <code>null</code> type names are considered to be equal. + * + * @param tname1 + * @param tname2 + * @return true if the type names are equal, false otherwise + */ + boolean typeNamesEqual(String tname1, String tname2) { + if (tname1 == null && tname2 == null) { + return true; + } + return tname1 != null && tname1.equals(tname2); + } + + /** + * Custom comparison to avoid the leading separator issue from saying the paths are not equal + * + * @param p1 + * @param p2 + * @return + */ + boolean pathsEqual(IPath p1, IPath p2) { + if (p1.segmentCount() == p2.segmentCount()) { + String[] segments = p1.segments(); + for (int i = 0; i < segments.length; i++) { + if (!segments[i].equals(p2.segment(i))) { + return false; + } + } + } + return true; + } + /* * (non-Javadoc) *
diff --git a/bundles/org.eclipse.e4.languages.javascript.debug.ui/src/org/eclipse/e4/languages/internal/javascript/debug/ui/JSDIModelPresentation.java b/bundles/org.eclipse.e4.languages.javascript.debug.ui/src/org/eclipse/e4/languages/internal/javascript/debug/ui/JSDIModelPresentation.java index 6365157..708151f 100644 --- a/bundles/org.eclipse.e4.languages.javascript.debug.ui/src/org/eclipse/e4/languages/internal/javascript/debug/ui/JSDIModelPresentation.java +++ b/bundles/org.eclipse.e4.languages.javascript.debug.ui/src/org/eclipse/e4/languages/internal/javascript/debug/ui/JSDIModelPresentation.java
@@ -11,10 +11,14 @@ package org.eclipse.e4.languages.internal.javascript.debug.ui; import java.io.File; +import java.util.HashMap; import org.eclipse.core.filesystem.EFS; import org.eclipse.core.resources.IFile; +import org.eclipse.core.resources.IResource; import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IPath; +import org.eclipse.core.runtime.Path; import org.eclipse.debug.core.DebugException; import org.eclipse.debug.core.model.IDebugTarget; import org.eclipse.debug.core.model.IStackFrame; @@ -24,8 +28,9 @@ import org.eclipse.debug.ui.IDebugModelPresentationExtension; import org.eclipse.debug.ui.IValueDetailListener; import org.eclipse.e4.languages.javascript.debug.connect.JSONConstants; -import org.eclipse.e4.languages.javascript.debug.model.JSDILineBreakpoint; +import org.eclipse.e4.languages.javascript.debug.model.JSDIBreakpoint; import org.eclipse.e4.languages.javascript.debug.model.JSDIFunctionBreakpoint; +import org.eclipse.e4.languages.javascript.debug.model.JSDILineBreakpoint; import org.eclipse.e4.languages.javascript.debug.model.JSDIProperty; import org.eclipse.e4.languages.javascript.debug.model.JSDIValue; import org.eclipse.e4.languages.javascript.debug.model.JSDIVariable; @@ -42,10 +47,25 @@ /** * Default model presentation for JSDI model elements * - * @since 1.0 + * @since 0.9 */ public class JSDIModelPresentation extends LabelProvider implements IDebugModelPresentationExtension { + /** + * Qualified names presentation property (value <code>"DISPLAY_QUALIFIED_NAMES"</code>). + * When <code>DISPLAY_QUALIFIED_NAMES</code> is set to <code>True</code>, + * this label provider should use fully qualified type names when rendering elements. + * When set to <code>False</code>, this label provider should use simple + * names when rendering elements. + * @see #setAttribute(String, Object) + */ + static final String DISPLAY_QUALIFIED_NAMES = "DISPLAY_QUALIFIED_NAMES"; //$NON-NLS-1$ + + /** + * Map of attributes set from the debug model - i.e. things like if qualified names are being shown or not + */ + HashMap attributes = null; + /* (non-Javadoc) * @see org.eclipse.debug.ui.IDebugModelPresentationExtension#requiresUIThread(java.lang.Object) */ @@ -64,9 +84,39 @@ /* (non-Javadoc) * @see org.eclipse.debug.ui.IDebugModelPresentation#setAttribute(java.lang.String, java.lang.Object) */ - public void setAttribute(String attribute, Object value) {} + public void setAttribute(String attribute, Object value) { + if(this.attributes == null) { + this.attributes = new HashMap(); + } + this.attributes.put(attribute, value); + } /* (non-Javadoc) + * @see org.eclipse.jface.viewers.BaseLabelProvider#dispose() + */ + public void dispose() { + if(this.attributes != null) { + this.attributes.clear(); + this.attributes = null; + } + super.dispose(); + } + + /** + * @return true is qualified names are being shown in the various debug views + */ + boolean showQualifiedNames() { + if(this.attributes != null) { + Boolean show = (Boolean) this.attributes.get(DISPLAY_QUALIFIED_NAMES); + if(show != null) { + return show.booleanValue(); + } + } + //TODO hack to always return qualified names until the toggle action is platform available + return true; + } + + /* (non-Javadoc) * @see org.eclipse.jface.viewers.LabelProvider#getText(java.lang.Object) */ public String getText(Object element) { @@ -87,12 +137,10 @@ return ((IValue)element).getValueString(); } if(element instanceof JSDILineBreakpoint) { - JSDILineBreakpoint breakpoint = (JSDILineBreakpoint) element; - return NLS.bind("{0} [line: {1}]", new String[] {breakpoint.getScriptPath(), Integer.toString(breakpoint.getLineNumber())}); + return getLineBreakpointText((JSDILineBreakpoint) element); } if(element instanceof JSDIFunctionBreakpoint) { - JSDIFunctionBreakpoint breakpoint = (JSDIFunctionBreakpoint) element; - return NLS.bind("{0} - {1}", new String[] {breakpoint.getScriptPath(), breakpoint.getFunctionName()}); + return getFunctionBreakpointText((JSDIFunctionBreakpoint) element); } } catch(CoreException ce) { @@ -101,6 +149,45 @@ return element.toString(); } + /** + * Returns the text for a line breakpoint + * @param breakpoint + * @return the breakpoint text + * @throws CoreException + */ + String getLineBreakpointText(JSDILineBreakpoint breakpoint) throws CoreException { + String path = getElementPath(breakpoint.getScriptPath()); + StringBuffer buffer = new StringBuffer(); + buffer.append(path).append(" [line: ").append(breakpoint.getLineNumber()).append("]"); + return buffer.toString(); + } + + /** + * Returns the text for a method breakpoint + * @param breakpoint + * @return the breakpoint text + * @throws CoreException + */ + String getFunctionBreakpointText(JSDIFunctionBreakpoint breakpoint) throws CoreException { + String path = getElementPath(breakpoint.getScriptPath()); + StringBuffer buffer = new StringBuffer(); + buffer.append(path).append(" - ").append(breakpoint.getFunctionName()); + return buffer.toString(); + } + + /** + * Returns the path of the element based on if qualified names are being shown + * @param path + * @return + */ + String getElementPath(String path) { + IPath epath = new Path(path); + if(showQualifiedNames()) { + return epath.toOSString(); + } + return epath.lastSegment(); + } + /* (non-Javadoc) * @see org.eclipse.jface.viewers.LabelProvider#getImage(java.lang.Object) */ @@ -163,6 +250,12 @@ if(element instanceof IFile) { return new FileEditorInput((IFile) element); } + if(element instanceof JSDIBreakpoint) { + IResource resource = ((JSDIBreakpoint)element).getMarker().getResource(); + if(resource.getType() == IResource.FILE) { + return new FileEditorInput((IFile) resource); + } + } return null; }
diff --git a/bundles/org.eclipse.e4.languages.javascript.debug.ui/src/org/eclipse/e4/languages/internal/javascript/debug/ui/breakpoints/ToggleBreakpointAdapter.java b/bundles/org.eclipse.e4.languages.javascript.debug.ui/src/org/eclipse/e4/languages/internal/javascript/debug/ui/breakpoints/ToggleBreakpointAdapter.java index 990d3de..f5316f0 100644 --- a/bundles/org.eclipse.e4.languages.javascript.debug.ui/src/org/eclipse/e4/languages/internal/javascript/debug/ui/breakpoints/ToggleBreakpointAdapter.java +++ b/bundles/org.eclipse.e4.languages.javascript.debug.ui/src/org/eclipse/e4/languages/internal/javascript/debug/ui/breakpoints/ToggleBreakpointAdapter.java
@@ -16,6 +16,7 @@ import org.eclipse.core.resources.IResource; import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; @@ -110,10 +111,8 @@ } catch (BadLocationException ble) {} HashMap attributes = new HashMap(); - //hack if the type is null the member is declared in a top-level type - //nothing else we can do - String name = getTypeName(element); - attributes.put(JSDIBreakpoint.SCRIPT_PATH, name); + attributes.put(JSDIBreakpoint.TYPE_NAME, getTypeName(element)); + attributes.put(JSDIBreakpoint.SCRIPT_PATH, getScriptPath(element)); attributes.put(JSDIBreakpoint.HANDLE, element.getHandleIdentifier()); new JSDILineBreakpoint(resource, linenumber, charstart, charend, attributes, true); return Status.OK_STATUS; @@ -133,6 +132,42 @@ } /** + * Returns the path to the script in the workspace or the name of the script in the event it is + * and external or virtual script + * @param element + * @return the path to the script + */ + String getScriptPath(IJavaScriptElement element) { + switch(element.getElementType()) { + case IJavaScriptElement.TYPE: { + return ((IType)element).getPath().toOSString(); + } + case IJavaScriptElement.METHOD: + case IJavaScriptElement.FIELD: { + IMember member = (IMember) element; + IType type = member.getDeclaringType(); + if(type == null) { + IJavaScriptElement parent = element.getParent(); + switch(parent.getElementType()) { + case IJavaScriptElement.TYPE: { + return ((IType)parent).getPath().toOSString(); + } + case IJavaScriptElement.JAVASCRIPT_UNIT: + case IJavaScriptElement.CLASS_FILE: { + return ((ITypeRoot)parent).getPath().toOSString(); + } + } + return element.getParent().getElementName(); + } + return type.getPath().toOSString(); + } + default: { + return element.getElementName(); + } + } + } + + /** * Resolves the type name from the given element * @param element * @return @@ -146,13 +181,12 @@ case IJavaScriptElement.FIELD: { IMember member = (IMember) element; IType type = member.getDeclaringType(); - if(type == null) { - return element.getParent().getElementName(); + if(type != null) { + return type.getFullyQualifiedName(); } - return type.getFullyQualifiedName(); } default: { - return element.getElementName(); + return null; } } }