diff options
author | Robin Stocker | 2013-07-18 21:21:37 +0000 |
---|---|---|
committer | Gerrit Code Review @ Eclipse.org | 2013-09-12 10:28:52 +0000 |
commit | 6c1705dcf55ac49e4c24030b097185b28a1447ac (patch) | |
tree | 0275cc2cb3472862b9794497cebb635a43adcc66 | |
parent | 826e224bff38b0f19f2f86079802951b37f15077 (diff) | |
download | egit-6c1705dcf55ac49e4c24030b097185b28a1447ac.tar.gz egit-6c1705dcf55ac49e4c24030b097185b28a1447ac.tar.xz egit-6c1705dcf55ac49e4c24030b097185b28a1447ac.zip |
[stagingView] Fix spurious warning if view was hidden when input set
See bug for how to reproduce. The fix consists of two parts:
1. Change isVisible to getVisible.
For isVisible to be true, the parents of also have to be visible. In the
case of the warning label, we also want to correctly toggle it even if
e.g. the staging view is not currently visible.
2. Rework modify handlers
Instead of requiring users of CommitMessageComponent to handle modify
events of author/committer texts themselves, handle it in the component.
The advantage of this is that the message does not have to be updated
when the listeners in the component are disabled. Before this, a warning
would theoretically be shown for a brief moment when authorText.setText
was called inside updateUI but the committer was not yet set.
The first part alone would have been enough to fix the problem. The
second part improves the code by making it simpler.
Bug: 413286
Change-Id: Ic46f1a5b0627fdca71cc0a83c28c3121ce519685
Signed-off-by: Robin Stocker <robin@nibor.org>
5 files changed, 33 insertions, 34 deletions
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/ToggleableWarningLabel.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/ToggleableWarningLabel.java index 404e8911f6..3deb4b2582 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/ToggleableWarningLabel.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/ToggleableWarningLabel.java @@ -64,7 +64,7 @@ public class ToggleableWarningLabel extends Composite { * Hide the warning label. */ public void hideMessage() { - if (isVisible()) + if (getVisible()) changeVisibility(false); } diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitDialog.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitDialog.java index 3f88843c8a..ef262c0a6e 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitDialog.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitDialog.java @@ -6,7 +6,7 @@ * Copyright (C) 2007, Shawn O. Pearce <spearce@spearce.org> * Copyright (C) 2011, Mathias Kinzler <mathias.kinzler@sap.com> * Copyright (C) 2012, Daniel Megert <daniel_megert@ch.ibm.com> - * Copyright (C) 2012, Robin Stocker <robin@nibor.org> + * Copyright (C) 2012, 2013 Robin Stocker <robin@nibor.org> * Copyright (C) 2012, IBM Corporation (Markus Keller <markus_keller@ch.ibm.com>) * Copyright (C) 2013, François Rey <eclipse.org_@_francois_._rey_._name> * @@ -64,8 +64,6 @@ import org.eclipse.jface.resource.ImageDescriptor; import org.eclipse.jface.resource.JFaceResources; import org.eclipse.jface.resource.LocalResourceManager; import org.eclipse.jface.resource.ResourceManager; -import org.eclipse.jface.text.DocumentEvent; -import org.eclipse.jface.text.IDocumentListener; import org.eclipse.jface.viewers.BaseLabelProvider; import org.eclipse.jface.viewers.CellLabelProvider; import org.eclipse.jface.viewers.CheckStateChangedEvent; @@ -93,8 +91,6 @@ import org.eclipse.swt.SWT; import org.eclipse.swt.custom.SashForm; import org.eclipse.swt.events.DisposeEvent; import org.eclipse.swt.events.DisposeListener; -import org.eclipse.swt.events.ModifyEvent; -import org.eclipse.swt.events.ModifyListener; import org.eclipse.swt.events.SelectionAdapter; import org.eclipse.swt.events.SelectionEvent; import org.eclipse.swt.graphics.Image; @@ -681,24 +677,6 @@ public class CommitDialog extends TitleAreaDialog { setTitle(UIText.CommitDialog_Title); setMessage(UIText.CommitDialog_Message, IMessageProvider.INFORMATION); - ModifyListener validator = new ModifyListener() { - - public void modifyText(ModifyEvent e) { - updateMessage(); - } - }; - commitText.getDocument().addDocumentListener(new IDocumentListener() { - - public void documentChanged(DocumentEvent event) { - updateMessage(); - } - - public void documentAboutToBeChanged(DocumentEvent event) { - // Intentionally empty - } - }); - authorText.addModifyListener(validator); - committerText.addModifyListener(validator); filesViewer.addCheckStateListener(new ICheckStateListener() { public void checkStateChanged(CheckStateChangedEvent event) { @@ -990,6 +968,10 @@ public class CommitDialog extends TitleAreaDialog { public void updateChangeIdToggleSelection(boolean selection) { changeIdItem.setSelection(selection); } + + public void statusUpdated() { + updateMessage(); + } }; commitMessageComponent = new CommitMessageComponent(repository, @@ -1044,6 +1026,10 @@ public class CommitDialog extends TitleAreaDialog { } private void updateMessage() { + if (commitButton == null) + // Not yet fully initialized. + return; + String message = null; int type = IMessageProvider.NONE; diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitMessageComponent.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitMessageComponent.java index 78239294b7..c2a408c05d 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitMessageComponent.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CommitMessageComponent.java @@ -395,6 +395,8 @@ public class CommitMessageComponent { */ public void enableListeners(boolean enable) { this.listenersEnabled = enable; + if (enable) + listener.statusUpdated(); } /** @@ -490,6 +492,13 @@ public class CommitMessageComponent { private void addListeners() { authorHandler = UIUtils.addPreviousValuesContentProposalToText( authorText, AUTHOR_VALUES_PREF); + authorText.addModifyListener(new ModifyListener() { + public void modifyText(ModifyEvent e) { + if (!listenersEnabled) + return; + listener.statusUpdated(); + } + }); committerText.addModifyListener(new ModifyListener() { String oldCommitter = committerText.getText(); @@ -506,6 +515,7 @@ public class CommitMessageComponent { oldSignOff, newSignOff)); oldCommitter = newCommitter; } + listener.statusUpdated(); } }); committerHandler = UIUtils.addPreviousValuesContentProposalToText( @@ -516,6 +526,7 @@ public class CommitMessageComponent { return; updateSignedOffButton(); updateChangeIdButton(); + listener.statusUpdated(); } }); } diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/ICommitMessageComponentNotifications.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/ICommitMessageComponentNotifications.java index 7957203a2b..a680ab51f2 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/ICommitMessageComponentNotifications.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/ICommitMessageComponentNotifications.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (C) 2010, Jens Baumgart <jens.baumgart@sap.com> + * Copyright (C) 2010, 2013 Jens Baumgart <jens.baumgart@sap.com> and others. * * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -28,4 +28,10 @@ public interface ICommitMessageComponentNotifications { * @param selection */ void updateChangeIdToggleSelection(boolean selection); + + /** + * The component host may have to update its status message (e.g. + * author/committer text changed). + */ + void statusUpdated(); } diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java index dad84f98be..1c9d6fef30 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java @@ -759,6 +759,10 @@ public class StagingView extends ViewPart implements IShowInSource { public void updateChangeIdToggleSelection(boolean selection) { addChangeIdAction.setChecked(selection); } + + public void statusUpdated() { + updateMessage(); + } }; commitMessageComponent = new CommitMessageComponent(listener); commitMessageComponent.attachControls(commitMessageText, authorText, @@ -788,14 +792,6 @@ public class StagingView extends ViewPart implements IShowInSource { } }); - ModifyListener modifyListener = new ModifyListener() { - public void modifyText(ModifyEvent e) { - updateMessage(); - } - }; - authorText.addModifyListener(modifyListener); - committerText.addModifyListener(modifyListener); - // react on selection changes IWorkbenchPartSite site = getSite(); ISelectionService srv = (ISelectionService) site @@ -1240,7 +1236,7 @@ public class StagingView extends ViewPart implements IShowInSource { warningLabel.showMessage(message); needsRedraw = true; } else { - needsRedraw = warningLabel.isVisible(); + needsRedraw = warningLabel.getVisible(); warningLabel.hideMessage(); } // Without this explicit redraw, the ControlDecoration of the |