diff options
author | Philip Langer | 2017-11-14 13:32:16 +0000 |
---|---|---|
committer | Laurent Goubet | 2018-05-01 10:23:11 +0000 |
commit | e991effe0dcdf2ac503cc1c8afc9d58a76afec6e (patch) | |
tree | 3dcef50cc45f7725cb04d6569cbb05d392b5affe /plugins | |
parent | 1f4885375768a4512fb4c5fb988705baf0aa0901 (diff) | |
download | org.eclipse.emf.compare-e991effe0dcdf2ac503cc1c8afc9d58a76afec6e.tar.gz org.eclipse.emf.compare-e991effe0dcdf2ac503cc1c8afc9d58a76afec6e.tar.xz org.eclipse.emf.compare-e991effe0dcdf2ac503cc1c8afc9d58a76afec6e.zip |
The show merge consequences control is confusing when disabled
With the preference to disable the showing of consequences, the tool bar
button does nothing in any case because consequences are never shown.
It's not clear how to enable it again and one might easily conclude
there are no consequences.
Better would be to allow the preview merge mode to be null to indicate
that no consequences should be shown, and better to make this an
explicit choice in the drop down menu. A new image is needed for that.
Better would be if the menu item style where radio button.
This approach allows the user to disable it on a per-view basis, with
the preference controlling only the initial default for the preview mode
when opening a new editor. Of course there is a listener to respect the
mode preferences when it changes.
Also, mirroring does not show the expected (correct) results. It seems
best to invert the mode and of course the computation needs to take into
account the mirroring state as well to show the right inverse things.
The idea being that switching the mirroring state will not show modified
highlighted difference consequence change because the radio choice
changes as well, so the double inverse leads to the same highlighted
results.
Change-Id: I2a72bc9aa465b3e624ab98539a511db259cd3369
Signed-off-by: Philip Langer <planger@eclipsesource.com>
Diffstat (limited to 'plugins')
6 files changed, 111 insertions, 60 deletions
diff --git a/plugins/org.eclipse.emf.compare.ide.ui.tests/src/org/eclipse/emf/compare/ide/ui/tests/compareconfiguration/EMFCompareConfigurationTest.java b/plugins/org.eclipse.emf.compare.ide.ui.tests/src/org/eclipse/emf/compare/ide/ui/tests/compareconfiguration/EMFCompareConfigurationTest.java index bf58c9a1a..d871b9ac8 100644 --- a/plugins/org.eclipse.emf.compare.ide.ui.tests/src/org/eclipse/emf/compare/ide/ui/tests/compareconfiguration/EMFCompareConfigurationTest.java +++ b/plugins/org.eclipse.emf.compare.ide.ui.tests/src/org/eclipse/emf/compare/ide/ui/tests/compareconfiguration/EMFCompareConfigurationTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2014 Obeo. + * Copyright (c) 2014, 2017 Obeo and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -12,10 +12,12 @@ package org.eclipse.emf.compare.ide.ui.tests.compareconfiguration; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import org.eclipse.compare.CompareConfiguration; import org.eclipse.emf.compare.ide.ui.internal.EMFCompareIDEUIPlugin; import org.eclipse.emf.compare.ide.ui.internal.configuration.EMFCompareConfiguration; +import org.eclipse.emf.compare.internal.merge.MergeMode; import org.junit.Test; @SuppressWarnings({"restriction", "nls" }) @@ -27,10 +29,15 @@ public class EMFCompareConfigurationTest { cc.setLeftEditable(true); cc.setRightEditable(true); EMFCompareConfiguration emfCC = new EMFCompareConfiguration(cc); - // From now, the following properties are not null: - // PREVIEW_MERGE_MODE, SMV_FILTERS, SMV_GROUP_PROVIDERS, EVENT_BUS. + // This property is null until it's been configured. Object preview_mode = emfCC.getProperty(EMFCompareIDEUIPlugin.PLUGIN_ID + ".PREVIEW_MERGE_MODE"); - assertNotNull(preview_mode); + assertNull(preview_mode); + // Configure it and then it should have that value. + emfCC.setMergePreviewMode(MergeMode.ACCEPT); + preview_mode = emfCC.getProperty(EMFCompareIDEUIPlugin.PLUGIN_ID + ".PREVIEW_MERGE_MODE"); + assertEquals(MergeMode.ACCEPT, preview_mode); + // From now, the following properties are not null: + // SMV_FILTERS, SMV_GROUP_PROVIDERS, EVENT_BUS. Object smv_filters = emfCC.getProperty(EMFCompareIDEUIPlugin.PLUGIN_ID + ".SMV_FILTERS"); assertNotNull(smv_filters); Object smv_group_providers = emfCC diff --git a/plugins/org.eclipse.emf.compare.ide.ui/icons/full/toolb16/disabled_highlight_related_changes.gif b/plugins/org.eclipse.emf.compare.ide.ui/icons/full/toolb16/disabled_highlight_related_changes.gif Binary files differnew file mode 100644 index 000000000..30c569ad0 --- /dev/null +++ b/plugins/org.eclipse.emf.compare.ide.ui/icons/full/toolb16/disabled_highlight_related_changes.gif diff --git a/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/configuration/EMFCompareConfiguration.java b/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/configuration/EMFCompareConfiguration.java index aa4facee7..e58a36565 100644 --- a/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/configuration/EMFCompareConfiguration.java +++ b/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/configuration/EMFCompareConfiguration.java @@ -14,7 +14,6 @@ *******************************************************************************/ package org.eclipse.emf.compare.ide.ui.internal.configuration; -import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.eventbus.EventBus; @@ -105,14 +104,6 @@ public class EMFCompareConfiguration extends ForwardingCompareConfiguration impl } private void setDefaultValues() { - if (getProperty(PREVIEW_MERGE_MODE) == null) { - if (isLeftEditable() && isRightEditable()) { - setProperty(PREVIEW_MERGE_MODE, MergeMode.RIGHT_TO_LEFT); - } else { - setProperty(PREVIEW_MERGE_MODE, MergeMode.ACCEPT); - } - } - EventBus eventBus = new EventBus(); if (getProperty(SMV_FILTERS) == null) { setProperty(SMV_FILTERS, new StructureMergeViewerFilter(eventBus)); @@ -308,7 +299,6 @@ public class EMFCompareConfiguration extends ForwardingCompareConfiguration impl } public void setMergePreviewMode(MergeMode previewMergeMode) { - Preconditions.checkNotNull(previewMergeMode); MergeMode oldValue = getMergePreviewMode(); setProperty(PREVIEW_MERGE_MODE, previewMergeMode); getEventBus().post(new MergePreviewModeChange(oldValue, previewMergeMode)); diff --git a/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/ide_ui_messages.properties b/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/ide_ui_messages.properties index 433aa7084..a0a2cd098 100644 --- a/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/ide_ui_messages.properties +++ b/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/ide_ui_messages.properties @@ -63,6 +63,7 @@ dropdown.left.to.right.text = Show consequences of merging from left to right dropdown.right.to.left.text = Show consequences of merging from right to left dropdown.accept.text = Show consequences of accepting change dropdown.reject.text = Show consequences of rejecting change +dropdown.disabled = Do not show consequences of changes undo.menu.item.text = Merge redo.menu.item.text = Merge diff --git a/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/structuremergeviewer/EMFCompareStructureMergeViewer.java b/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/structuremergeviewer/EMFCompareStructureMergeViewer.java index a84e44e78..2e984af6b 100644 --- a/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/structuremergeviewer/EMFCompareStructureMergeViewer.java +++ b/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/structuremergeviewer/EMFCompareStructureMergeViewer.java @@ -950,7 +950,11 @@ public class EMFCompareStructureMergeViewer extends AbstractStructuredViewerWrap if (!inChange) { SWTUtil.safeAsyncExec(new Runnable() { public void run() { - updateHighlightRelatedChanges(getSelection()); + if (getCompareConfiguration().getMergePreviewMode() == null) { + clearHighlightRelatedChanges(); + } else { + updateHighlightRelatedChanges(getSelection()); + } } }); } @@ -1228,10 +1232,14 @@ public class EMFCompareStructureMergeViewer extends AbstractStructuredViewerWrap compareConfiguration.setLeftEditable(input.isLeftEditable()); compareConfiguration.setRightEditable(input.isRightEditable()); - if (input.isLeftEditable() && input.isRightEditable()) { - compareConfiguration.setMergePreviewMode(MergeMode.RIGHT_TO_LEFT); + if (isHighlightRelatedChanges()) { + if (input.isLeftEditable() && input.isRightEditable()) { + compareConfiguration.setMergePreviewMode(MergeMode.RIGHT_TO_LEFT); + } else { + compareConfiguration.setMergePreviewMode(MergeMode.ACCEPT); + } } else { - compareConfiguration.setMergePreviewMode(MergeMode.ACCEPT); + compareConfiguration.setMergePreviewMode(null); } // setup defaults @@ -1418,10 +1426,14 @@ public class EMFCompareStructureMergeViewer extends AbstractStructuredViewerWrap compareConfiguration.setLeftEditable(leftEditable); compareConfiguration.setRightEditable(rightEditable); - if (leftEditable && rightEditable) { - compareConfiguration.setMergePreviewMode(MergeMode.RIGHT_TO_LEFT); + if (isHighlightRelatedChanges()) { + if (leftEditable && rightEditable) { + compareConfiguration.setMergePreviewMode(MergeMode.RIGHT_TO_LEFT); + } else { + compareConfiguration.setMergePreviewMode(MergeMode.ACCEPT); + } } else { - compareConfiguration.setMergePreviewMode(MergeMode.ACCEPT); + compareConfiguration.setMergePreviewMode(null); } final BasicDiagnostic diagnostic = new BasicDiagnostic(Diagnostic.OK, @@ -1636,11 +1648,11 @@ public class EMFCompareStructureMergeViewer extends AbstractStructuredViewerWrap * selection */ protected void updateHighlightRelatedChanges(ISelection selection) { - if (!isHighlightRelatedChanges()) { - return; + if (getCompareConfiguration().getMergePreviewMode() != null) { + dependencyData.updateDependencies(selection, + EMFCompareRCPPlugin.getDefault().getMergerRegistry()); + internalRedraw(); } - dependencyData.updateDependencies(selection, EMFCompareRCPPlugin.getDefault().getMergerRegistry()); - internalRedraw(); } /** @@ -1989,10 +2001,17 @@ public class EMFCompareStructureMergeViewer extends AbstractStructuredViewerWrap protected void handlePreferenceChangedEvent(PropertyChangeEvent event) { if (event.getProperty() == EMFCompareUIPreferences.EDITOR_TREE_HIGHLIGHT_RELATED_CHANGES) { boolean highlightRelatedChanges = Boolean.parseBoolean(event.getNewValue().toString()); + EMFCompareConfiguration compareConfiguration = getCompareConfiguration(); if (highlightRelatedChanges) { - updateHighlightRelatedChanges(getSelection()); + if (compareConfiguration.getMergePreviewMode() == null) { + if (compareConfiguration.isLeftEditable() && compareConfiguration.isRightEditable()) { + compareConfiguration.setMergePreviewMode(MergeMode.RIGHT_TO_LEFT); + } else { + compareConfiguration.setMergePreviewMode(MergeMode.ACCEPT); + } + } } else { - clearHighlightRelatedChanges(); + compareConfiguration.setMergePreviewMode(null); } } } diff --git a/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/structuremergeviewer/actions/DropDownMergeMenuAction.java b/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/structuremergeviewer/actions/DropDownMergeMenuAction.java index 7d3263b3b..c38284dc4 100644 --- a/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/structuremergeviewer/actions/DropDownMergeMenuAction.java +++ b/plugins/org.eclipse.emf.compare.ide.ui/src/org/eclipse/emf/compare/ide/ui/internal/structuremergeviewer/actions/DropDownMergeMenuAction.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2013, 2014 Obeo. + * Copyright (c) 2013, 2017 Obeo and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -45,7 +45,7 @@ public class DropDownMergeMenuAction extends Action implements IMenuCreator { private Menu fMenu; /** The accept menu item. */ - private final List<Action> actions; + private final List<DropDownAction> actions; /** * Constructor. @@ -57,10 +57,22 @@ public class DropDownMergeMenuAction extends Action implements IMenuCreator { this.configuration = configuration; actions = newArrayList(); + MergeMode currentPreviewMode = configuration.getMergePreviewMode(); for (MergeMode mergePreviewMode : previewModes) { - actions.add(new DropDownAction(configuration, mergePreviewMode)); + DropDownAction action = new DropDownAction(configuration, mergePreviewMode); + actions.add(action); + if (mergePreviewMode == currentPreviewMode) { + action.setChecked(true); + } } - setTextAndImage(this, configuration.getMergePreviewMode()); + + DropDownAction action = new DropDownAction(configuration, null); + actions.add(action); + if (currentPreviewMode == null) { + action.setChecked(true); + } + + setTextAndImage(this, currentPreviewMode); setMenuCreator(this); configuration.getEventBus().register(this); @@ -68,7 +80,11 @@ public class DropDownMergeMenuAction extends Action implements IMenuCreator { @Subscribe public void mergePreviewModeChange(IMergePreviewModeChange event) { - setTextAndImage(this, event.getNewValue()); + MergeMode mergeMode = event.getNewValue(); + for (DropDownAction dropDownAction : actions) { + dropDownAction.setChecked(mergeMode == dropDownAction.getMergeMode()); + } + setTextAndImage(this, mergeMode); } /** @@ -79,7 +95,11 @@ public class DropDownMergeMenuAction extends Action implements IMenuCreator { @Override public void run() { MergeMode mergeWay = configuration.getMergePreviewMode(); - configuration.setMergePreviewMode(mergeWay.inverse()); + if (mergeWay == null) { + actions.get(0).run(); + } else { + configuration.setMergePreviewMode(mergeWay.inverse()); + } } /** @@ -135,33 +155,42 @@ public class DropDownMergeMenuAction extends Action implements IMenuCreator { } static void setTextAndImage(IAction action, MergeMode mergeMode) { - switch (mergeMode) { - case LEFT_TO_RIGHT: - action.setImageDescriptor(AbstractUIPlugin.imageDescriptorFromPlugin( - EMFCompareIDEUIPlugin.PLUGIN_ID, "icons/full/toolb16/left_to_right.gif")); //$NON-NLS-1$ - action.setToolTipText(EMFCompareIDEUIMessages.getString("dropdown.left.to.right.text")); //$NON-NLS-1$ - action.setText(EMFCompareIDEUIMessages.getString("dropdown.left.to.right.text")); //$NON-NLS-1$ - break; - case RIGHT_TO_LEFT: - action.setImageDescriptor(AbstractUIPlugin.imageDescriptorFromPlugin( - EMFCompareIDEUIPlugin.PLUGIN_ID, "icons/full/toolb16/right_to_left.gif")); //$NON-NLS-1$ - action.setToolTipText(EMFCompareIDEUIMessages.getString("dropdown.right.to.left.text")); //$NON-NLS-1$ - action.setText(EMFCompareIDEUIMessages.getString("dropdown.right.to.left.text")); //$NON-NLS-1$ - break; - case ACCEPT: - action.setImageDescriptor(AbstractUIPlugin.imageDescriptorFromPlugin( - EMFCompareIDEUIPlugin.PLUGIN_ID, "icons/full/toolb16/accept.gif")); //$NON-NLS-1$ - action.setToolTipText(EMFCompareIDEUIMessages.getString("dropdown.accept.text")); //$NON-NLS-1$ - action.setText(EMFCompareIDEUIMessages.getString("dropdown.accept.text")); //$NON-NLS-1$ - break; - case REJECT: - action.setImageDescriptor(AbstractUIPlugin.imageDescriptorFromPlugin( - EMFCompareIDEUIPlugin.PLUGIN_ID, "icons/full/toolb16/reject.gif")); //$NON-NLS-1$ - action.setToolTipText(EMFCompareIDEUIMessages.getString("dropdown.reject.text")); //$NON-NLS-1$ - action.setText(EMFCompareIDEUIMessages.getString("dropdown.reject.text")); //$NON-NLS-1$ - break; - default: - throw new IllegalStateException(); + if (mergeMode == null) { + action.setImageDescriptor( + AbstractUIPlugin.imageDescriptorFromPlugin(EMFCompareIDEUIPlugin.PLUGIN_ID, + "icons/full/toolb16/disabled_highlight_related_changes.gif")); //$NON-NLS-1$ + action.setToolTipText(EMFCompareIDEUIMessages.getString("dropdown.disabled")); //$NON-NLS-1$ + action.setText(EMFCompareIDEUIMessages.getString("dropdown.disabled")); //$NON-NLS-1$ + + } else { + switch (mergeMode) { + case LEFT_TO_RIGHT: + action.setImageDescriptor(AbstractUIPlugin.imageDescriptorFromPlugin( + EMFCompareIDEUIPlugin.PLUGIN_ID, "icons/full/toolb16/left_to_right.gif")); //$NON-NLS-1$ + action.setToolTipText(EMFCompareIDEUIMessages.getString("dropdown.left.to.right.text")); //$NON-NLS-1$ + action.setText(EMFCompareIDEUIMessages.getString("dropdown.left.to.right.text")); //$NON-NLS-1$ + break; + case RIGHT_TO_LEFT: + action.setImageDescriptor(AbstractUIPlugin.imageDescriptorFromPlugin( + EMFCompareIDEUIPlugin.PLUGIN_ID, "icons/full/toolb16/right_to_left.gif")); //$NON-NLS-1$ + action.setToolTipText(EMFCompareIDEUIMessages.getString("dropdown.right.to.left.text")); //$NON-NLS-1$ + action.setText(EMFCompareIDEUIMessages.getString("dropdown.right.to.left.text")); //$NON-NLS-1$ + break; + case ACCEPT: + action.setImageDescriptor(AbstractUIPlugin.imageDescriptorFromPlugin( + EMFCompareIDEUIPlugin.PLUGIN_ID, "icons/full/toolb16/accept.gif")); //$NON-NLS-1$ + action.setToolTipText(EMFCompareIDEUIMessages.getString("dropdown.accept.text")); //$NON-NLS-1$ + action.setText(EMFCompareIDEUIMessages.getString("dropdown.accept.text")); //$NON-NLS-1$ + break; + case REJECT: + action.setImageDescriptor(AbstractUIPlugin.imageDescriptorFromPlugin( + EMFCompareIDEUIPlugin.PLUGIN_ID, "icons/full/toolb16/reject.gif")); //$NON-NLS-1$ + action.setToolTipText(EMFCompareIDEUIMessages.getString("dropdown.reject.text")); //$NON-NLS-1$ + action.setText(EMFCompareIDEUIMessages.getString("dropdown.reject.text")); //$NON-NLS-1$ + break; + default: + throw new IllegalStateException(); + } } } @@ -179,11 +208,16 @@ public class DropDownMergeMenuAction extends Action implements IMenuCreator { * The compare configuration object. */ public DropDownAction(IEMFCompareConfiguration configuration, MergeMode mergeMode) { + super("", IAction.AS_RADIO_BUTTON); //$NON-NLS-1$ this.configuration = configuration; this.mergeMode = mergeMode; setTextAndImage(this, mergeMode); } + public MergeMode getMergeMode() { + return mergeMode; + } + /** * {@inheritDoc} * |