Attachment 161445 Details for Bug 273450 – Patch proposal

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

[patch] Patch proposal

patch_3.7_20100309.txt (text/plain), 13.26 KB, created by Szymon Ptaszkiewicz

on 2010-03-09 07:11:48 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Szymon Ptaszkiewicz

Created: 2010-03-09 07:11:48 EST

Size: 13.26 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.compare
>Index: compare/org/eclipse/compare/CompareEditorInput.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.compare/plugins/org.eclipse.compare/compare/org/eclipse/compare/CompareEditorInput.java,v
>retrieving revision 1.116
>diff -u -r1.116 CompareEditorInput.java
>--- compare/org/eclipse/compare/CompareEditorInput.java	4 Mar 2010 16:26:05 -0000	1.116
>+++ compare/org/eclipse/compare/CompareEditorInput.java	9 Mar 2010 11:25:06 -0000
>@@ -14,6 +14,7 @@
> import java.util.ArrayList;
> import java.util.ResourceBundle;
> 
>+import org.eclipse.compare.contentmergeviewer.ContentMergeViewer;
> import org.eclipse.compare.contentmergeviewer.IFlushable;
> import org.eclipse.compare.internal.BinaryCompareViewer;
> import org.eclipse.compare.internal.ChangePropertyAction;
>@@ -192,8 +193,8 @@
> 	private String fTitle;
> 	private ListenerList fListenerList= new ListenerList();
> 	private CompareNavigator fNavigator;
>-	private boolean fDirty= false;
>-	private ArrayList fDirtyViewers= new ArrayList();
>+	private ArrayList fLeftDirtyViewers = new ArrayList();
>+	private ArrayList fRightDirtyViewers = new ArrayList();
> 	private IPropertyChangeListener fDirtyStateListener;
> 	
> 	boolean fStructureCompareOnSingleClick= true;
>@@ -1015,18 +1016,36 @@
> 	}
> 	
> 	/**
>-	 * Returns <code>true</code> if there are unsaved changes.
>-	 * The value returned is the value of the <code>DIRTY_STATE</code> property of this input object.
>-	 
>-	 * Returns <code>true</code> if this input has unsaved changes,
>-	 * that is if <code>setDirty(true)</code> has been called.
>-	 * Subclasses don't have to override if the functionality provided by <code>setDirty</code>
>-	 * is sufficient.
>-	 *
>+	 * Returns <code>true</code> if there are unsaved changes in either left or
>+	 * right side. The value returned is the value of the
>+	 * <code>DIRTY_STATE</code> property of this input object.
>+	 * 
>+	 * Returns <code>true</code> if left or right side has unsaved changes
>+	 * Subclasses don't have to override if the functionality provided by
>+	 * <code>setDirty</code> is sufficient.
>+	 * 
> 	 * @return <code>true</code> if there are changes that need to be saved
> 	 */
> 	public boolean isSaveNeeded() {
>-		return fDirty || fDirtyViewers.size() > 0;
>+		return isLeftSaveNeeded() || isRightSaveNeeded();
>+	}
>+
>+	/**
>+	 * Returns <code>true</code> if there are unsaved changes for left side.
>+	 * 
>+	 * @return <code>true</code> if there are changes that need to be saved
>+	 */
>+	protected boolean isLeftSaveNeeded() {
>+		return fLeftDirtyViewers.size() > 0;
>+	}
>+
>+	/**
>+	 * Returns <code>true</code> if there are unsaved changes for right side.
>+	 * 
>+	 * @return <code>true</code> if there are changes that need to be saved
>+	 */
>+	protected boolean isRightSaveNeeded() {
>+		return fRightDirtyViewers.size() > 0;
> 	}
> 	
> 	/**
>@@ -1043,31 +1062,70 @@
> 	}
> 		
> 	/**
>-	 * Sets the dirty state of this input to the given
>-	 * value and sends out a <code>PropertyChangeEvent</code> if the new value differs from the old value.
>-	 *
>-	 * @param dirty the dirty state for this compare input
>+	 * Sets the dirty state of this input to the given value and sends out a
>+	 * <code>PropertyChangeEvent</code> if the new value differs from the old
>+	 * value. Direct calling this method with parameter dirty equal to
>+	 * <code>false</code> when there are unsaved changes in viewers, results in
>+	 * inconsistent state. The dirty state of compare input should be based only
>+	 * on the information if there are changes in viewers for left or right
>+	 * side.
>+	 * 
>+	 * @param dirty
>+	 *            the dirty state for this compare input
> 	 */
> 	public void setDirty(boolean dirty) {
>-		boolean oldDirty = fDirty || fDirtyViewers.size() > 0;
>-		fDirty= dirty;
>-		if (!fDirty)
>-			fDirtyViewers.clear();
>-		if (oldDirty != dirty)
>-			Utilities.firePropertyChange(fListenerList, this, DIRTY_STATE, Boolean.valueOf(oldDirty), Boolean.valueOf(dirty));
>+		boolean oldDirty = isSaveNeeded();
>+		boolean newDirty = dirty || isSaveNeeded();
>+		if (!newDirty) {
>+			fLeftDirtyViewers.clear();
>+			fRightDirtyViewers.clear();
>+		}
>+		if (oldDirty != isSaveNeeded()) {
>+			Utilities.firePropertyChange(fListenerList, this, DIRTY_STATE, Boolean.valueOf(oldDirty), Boolean.valueOf(isSaveNeeded()));
>+		}
> 	}
> 	
>+	/**
>+	 * Method adds or removes viewers that changed left or right side of this
>+	 * compare input. Any modification of any of the list of viewers may result
>+	 * in dirty state change.
>+	 * 
>+	 * @param source
>+	 *            the object that fired <code>PropertyChangeEvent</code>
>+	 *            modifying the dirty state
>+	 * @param dirty
>+	 *            value that describes if the changes were added or removed
>+	 */
> 	private void setDirty(Object source, boolean dirty) {
> 		Assert.isNotNull(source);
>-		boolean oldDirty= fDirty || fDirtyViewers.size() > 0;
>-		if (dirty)
>-			fDirtyViewers.add(source);
>-		else
>-			fDirtyViewers.remove(source);
>-		boolean newDirty= fDirty || fDirtyViewers.size() > 0;
>-		if (DEBUG) System.out.println("setDirty("+source+", "+dirty+"): " + newDirty); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
>-		if (oldDirty != newDirty)
>+		boolean oldDirty = isSaveNeeded();
>+		ContentMergeViewer cmv = (ContentMergeViewer) source;
>+		if (dirty) {
>+			if (cmv.isLeftDirty()) {
>+				if (!fLeftDirtyViewers.contains(source)) {
>+					fLeftDirtyViewers.add(source);
>+				}
>+			}
>+			if (cmv.isRightDirty()) {
>+				if (!fRightDirtyViewers.contains(source)) {
>+					fRightDirtyViewers.add(source);
>+				}
>+			}
>+		} else {
>+			if (!cmv.isLeftDirty()) {
>+				fLeftDirtyViewers.remove(source);
>+			}
>+			if (!cmv.isRightDirty()) {
>+				fRightDirtyViewers.remove(source);
>+			}
>+		}
>+		boolean newDirty = isSaveNeeded();
>+		if (DEBUG) {
>+			System.out.println("setDirty(" + source + ", " + dirty + "): " + newDirty); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
>+		}
>+		if (oldDirty != newDirty) {
> 			Utilities.firePropertyChange(fListenerList, this, DIRTY_STATE, Boolean.valueOf(oldDirty), Boolean.valueOf(newDirty));
>+		}
> 	}
> 	
> 	/* (non Javadoc)
>Index: compare/org/eclipse/compare/contentmergeviewer/ContentMergeViewer.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.compare/plugins/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/ContentMergeViewer.java,v
>retrieving revision 1.78
>diff -u -r1.78 ContentMergeViewer.java
>--- compare/org/eclipse/compare/contentmergeviewer/ContentMergeViewer.java	13 May 2009 13:16:20 -0000	1.78
>+++ compare/org/eclipse/compare/contentmergeviewer/ContentMergeViewer.java	9 Mar 2010 11:25:06 -0000
>@@ -1168,9 +1168,8 @@
> 	protected void setLeftDirty(boolean dirty) {
> 		if (isLeftDirty() != dirty) {
> 			fIsLeftDirty = dirty;
>-			// Only fire the event if the combined dirty state has changed
>-			if (!isRightDirty())
>-				fireDirtyState(dirty);
>+			// Always fire the event if the dirty state has changed
>+			fireDirtyState(dirty);
> 		}
> 	}
> 	
>@@ -1186,9 +1185,8 @@
> 	protected void setRightDirty(boolean dirty) {
> 		if (isRightDirty() != dirty) {
> 			fIsRightDirty = dirty;
>-			// Only fire the event if the combined dirty state has changed
>-			if (!isLeftDirty())
>-				fireDirtyState(dirty);
>+			// Always fire the event if the dirty state has changed
>+			fireDirtyState(dirty);
> 		}
> 	}
> 	
>@@ -1256,7 +1254,7 @@
> 	 * @return the dirty state of the right side of this viewer
> 	 * @since 3.3
> 	 */
>-	protected boolean isRightDirty() {
>+	public boolean isRightDirty() {
> 		return fIsRightDirty;
> 	}
> 
>@@ -1265,7 +1263,7 @@
> 	 * @return the dirty state of the left side of this viewer
> 	 * @since 3.3
> 	 */
>-	protected boolean isLeftDirty() {
>+	public boolean isLeftDirty() {
> 		return fIsLeftDirty;
> 	}
> 
>#P org.eclipse.compare.tests
>Index: src/org/eclipse/compare/tests/ContentMergeViewerTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.compare.tests/src/org/eclipse/compare/tests/ContentMergeViewerTest.java,v
>retrieving revision 1.1
>diff -u -r1.1 ContentMergeViewerTest.java
>--- src/org/eclipse/compare/tests/ContentMergeViewerTest.java	16 Apr 2008 14:32:29 -0000	1.1
>+++ src/org/eclipse/compare/tests/ContentMergeViewerTest.java	9 Mar 2010 11:25:07 -0000
>@@ -21,6 +21,9 @@
> public class ContentMergeViewerTest extends TestCase {
> 
> 	private MyContentMergeViewer myContentMergeViewer;
>+	/**
>+	 * result[0]-event occurred or not; result[1]-new state that was set
>+	 */
> 	boolean[] result = new boolean[] { false, false };
> 
> 	public ContentMergeViewerTest() {
>@@ -40,11 +43,11 @@
> 		public boolean leftDirty = false;
> 		public boolean rightDirty = false;
> 
>-		protected boolean isLeftDirty() {
>+		public boolean isLeftDirty() {
> 			return leftDirty;
> 		}
> 
>-		protected boolean isRightDirty() {
>+		public boolean isRightDirty() {
> 			return rightDirty;
> 		}
> 
>@@ -109,7 +112,8 @@
> 		myContentMergeViewer.rightDirty = true;
> 		myContentMergeViewer.setLeftDirty(true);
> 
>-		Assert.assertEquals(false, result[0]);
>+		Assert.assertEquals(true, result[0]);
>+		Assert.assertEquals(true, result[1]);
> 	}
> 
> 	public void testTFTX() {
>@@ -160,7 +164,8 @@
> 		myContentMergeViewer.rightDirty = true;
> 		myContentMergeViewer.setLeftDirty(false);
> 
>-		Assert.assertEquals(false, result[0]);
>+		Assert.assertEquals(true, result[0]);
>+		Assert.assertEquals(false, result[1]);
> 	}
> 
> 	// set right to true
>@@ -187,7 +192,8 @@
> 		myContentMergeViewer.rightDirty = false;
> 		myContentMergeViewer.setRightDirty(true);
> 
>-		Assert.assertEquals(false, result[0]);
>+		Assert.assertEquals(true, result[0]);
>+		Assert.assertEquals(true, result[1]);
> 	}
> 
> 	public void testTTXT() {
>@@ -230,6 +236,7 @@
> 		myContentMergeViewer.rightDirty = true;
> 		myContentMergeViewer.setRightDirty(false);
> 
>-		Assert.assertEquals(false, result[0]);
>+		Assert.assertEquals(true, result[0]);
>+		Assert.assertEquals(false, result[1]);
> 	}
> }
>#P org.eclipse.team.ui
>Index: src/org/eclipse/team/internal/ui/synchronize/LocalResourceSaveableComparison.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.team.ui/src/org/eclipse/team/internal/ui/synchronize/LocalResourceSaveableComparison.java,v
>retrieving revision 1.9
>diff -u -r1.9 LocalResourceSaveableComparison.java
>--- src/org/eclipse/team/internal/ui/synchronize/LocalResourceSaveableComparison.java	12 Sep 2007 19:40:21 -0000	1.9
>+++ src/org/eclipse/team/internal/ui/synchronize/LocalResourceSaveableComparison.java	9 Mar 2010 11:25:08 -0000
>@@ -206,6 +206,9 @@
> 	public boolean isDirty() {
> 		// We need to get the dirty state from the compare editor input
> 		// since it is our only connection to the merge viewer
>+		if (editorInput instanceof SaveablesCompareEditorInput) {
>+			return ((SaveablesCompareEditorInput) editorInput).isSaveNeeded(this);
>+		}
> 		return editorInput.isSaveNeeded();
> 	}
> 	
>@@ -232,6 +235,18 @@
> 	 * @see org.eclipse.ui.Saveable#getName()
> 	 */
> 	public String getName() {
>+		// When we compare two files with each other we need to know the exact
>+		// name of each of the files, not the name taken from the input. Input
>+		// gives the name based on the main element of the input. The way how
>+		// the main element is calculated is described in method
>+		// org.eclipse.team.internal.ui.mapping.AbstractCompareInput#getMainElement()
>+		// See bug 273450.
>+		if (fileElement.equals(input.getLeft())) {
>+			return input.getLeft().getName();
>+		}
>+		if (fileElement.equals(input.getRight())) {
>+			return input.getRight().getName();
>+		}
> 		return input.getName();
> 	}
> 
>Index: src/org/eclipse/team/internal/ui/synchronize/SaveablesCompareEditorInput.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.team.ui/src/org/eclipse/team/internal/ui/synchronize/SaveablesCompareEditorInput.java,v
>retrieving revision 1.4
>diff -u -r1.4 SaveablesCompareEditorInput.java
>--- src/org/eclipse/team/internal/ui/synchronize/SaveablesCompareEditorInput.java	18 Feb 2010 14:48:02 -0000	1.4
>+++ src/org/eclipse/team/internal/ui/synchronize/SaveablesCompareEditorInput.java	9 Mar 2010 11:25:09 -0000
>@@ -357,6 +357,27 @@
> 		return super.isDirty();
> 	}
> 
>+	public boolean isSaveNeeded(Saveable arg) {
>+		// Method gives information if the Saveable given as a parameter needs
>+		// to be saved. Parameter arg can be either equal to fLeftSaveable or to
>+		// fRightSaveable. If the parameter is null or differs from both left
>+		// and right Saveable then default isSaveNeeded() is called. Default
>+		// method isSaveNeeded() is not aware which Saveable we wish to check
>+		// therefore it returns the state of the whole CompareEditorInput.
>+		// Whenever possible this method should be called instead of default
>+		// isSaveNeeded(). See bug 273450.
>+		if (arg == null) {
>+			return isSaveNeeded();
>+		}
>+		if (arg.equals(fLeftSaveable)) {
>+			return isLeftSaveNeeded();
>+		}
>+		if (arg.equals(fRightSaveable)) {
>+			return isRightSaveNeeded();
>+		}
>+		return isSaveNeeded();
>+	}
>+	
> 	/**
> 	 * Close the editor if it is not dirty. If it is still dirty, let the
> 	 * content merge viewer handle the compare input change.

Actions: View | Diff

Attachments on bug 273450: 161445 | 161585 | 168134 | 168536 | 168537 | 168897 | 176219