Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChristian Walther2018-12-10 10:42:44 +0000
committerChristian Walther2018-12-10 10:42:44 +0000
commitf2f92ab4045802da83d2efeae9548a4fd544becb (patch)
treea97d92a724a61e54aeb53fee816d64da9a581b26
parent2fc42590fb8e9169941ce822ef7123e8b4f63690 (diff)
downloadorg.eclipse.cdt-f2f92ab4045802da83d2efeae9548a4fd544becb.tar.gz
org.eclipse.cdt-f2f92ab4045802da83d2efeae9548a4fd544becb.tar.xz
org.eclipse.cdt-f2f92ab4045802da83d2efeae9548a4fd544becb.zip
Bug 335344 - External settings lost after changing language IDs, take 2
The original fix (a733900) only fixed part of the problem: It worked for the case where a complete CExternalSetting was removed and replaced by a different one, but not in the case where individual entries from a CExternalSetting were moved to a different one, but others remained (and, in both cases, the two CExternalSettings applied to the same ICLanguageSetting). This commit - adds a test for the additional condition, which would previously fail - reverts the previous fix, which is made redundant by the new one - fixes both cases by applying removals before additions with ICSettingEntry granularity per ICLanguageSetting rather than for whole CExternalSettings. Change-Id: I1b1ee7443b83189c29e458eef12be9cad6b3965d Signed-off-by: Christian Walther <walther@indel.ch>
-rw-r--r--core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/ExternalSettingsProviderTests.java80
-rw-r--r--core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/TestExtSettingsProvider.java16
-rw-r--r--core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsDeltaProcessor.java28
-rw-r--r--core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettinsDeltaCalculator.java15
4 files changed, 113 insertions, 26 deletions
diff --git a/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/ExternalSettingsProviderTests.java b/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/ExternalSettingsProviderTests.java
index 2a352d69877..dd9e1fc7a95 100644
--- a/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/ExternalSettingsProviderTests.java
+++ b/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/ExternalSettingsProviderTests.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2007, 2015 Intel Corporation and others.
+ * Copyright (c) 2007, 2018 Intel Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -20,6 +20,8 @@ package org.eclipse.cdt.core.settings.model;
import java.util.Arrays;
import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Set;
import org.eclipse.cdt.core.dom.IPDOMManager;
import org.eclipse.cdt.core.model.CoreModel;
@@ -408,6 +410,82 @@ public class ExternalSettingsProviderTests extends BaseTestCase {
}
/**
+ * Test if moving an entry to an external setting with different language IDs works
+ */
+ public void testChangeLanguageSetMove() throws CoreException {
+ TestExtSettingsProvider.setVariantNum(5);
+
+ CoreModel model = CoreModel.getDefault();
+ ICProjectDescriptionManager mngr = model.getProjectDescriptionManager();
+ IProject project = p5.getProject();
+
+ // add external settings provider
+ ICProjectDescription des = model.getProjectDescription(project);
+ ICConfigurationDescription cfgDes = des.getConfigurations()[0];
+ String[] extPIds = new String[] { TestExtSettingsProvider.TEST_EXTERNAL_PROVIDER_ID };
+ cfgDes.setExternalSettingsProviderIds(extPIds);
+ model.setProjectDescription(project, des);
+
+ // read out the settings it caused
+ des = model.getProjectDescription(project, false);
+ cfgDes = des.getConfigurations()[0];
+ ICFolderDescription root = cfgDes.getRootFolderDescription();
+ HashMap<String, ICLanguageSetting> languageSettingsById = new HashMap<>();
+ for (ICLanguageSetting s : root.getLanguageSettings()) {
+ languageSettingsById.put(s.getLanguageId(), s);
+ }
+
+ ICLanguageSetting ls;
+ ICLanguageSettingEntry[] entries;
+ Set<ICLanguageSettingEntry> expectedEntries1 = new HashSet<>(
+ Arrays.asList(new CMacroEntry("MACRO_1", "Value1", 0)));
+ Set<ICLanguageSettingEntry> expectedEntries12 = new HashSet<>(
+ Arrays.asList(new CMacroEntry("MACRO_1", "Value1", 0), new CMacroEntry("MACRO_2", "Value2", 0)));
+ Set<ICLanguageSettingEntry> expectedEntries123 = new HashSet<>(
+ Arrays.asList(new CMacroEntry("MACRO_1", "Value1", 0), new CMacroEntry("MACRO_2", "Value2", 0),
+ new CMacroEntry("MACRO_3", "Value3", 0)));
+
+ // MACRO_2 should be present for assembly but not for C
+ ls = languageSettingsById.get("org.eclipse.cdt.core.assembly");
+ assertNotNull(ls);
+ entries = ls.getSettingEntries(ICSettingEntry.MACRO);
+ assertEquals(3, entries.length);
+ assertEquals(expectedEntries123, new HashSet<>(Arrays.asList(entries)));
+
+ ls = languageSettingsById.get("org.eclipse.cdt.core.gcc");
+ assertNotNull(ls);
+ entries = ls.getSettingEntries(ICSettingEntry.MACRO);
+ assertEquals(1, entries.length);
+ assertEquals(expectedEntries1, new HashSet<>(Arrays.asList(entries)));
+
+ // update settings provider
+ TestExtSettingsProvider.setVariantNum(6);
+ mngr.updateExternalSettingsProviders(extPIds, null);
+
+ // read out the settings it caused
+ des = model.getProjectDescription(project, false);
+ cfgDes = des.getConfigurations()[0];
+ root = cfgDes.getRootFolderDescription();
+ languageSettingsById = new HashMap<>();
+ for (ICLanguageSetting s : root.getLanguageSettings()) {
+ languageSettingsById.put(s.getLanguageId(), s);
+ }
+
+ // MACRO_2 should be present for both now
+ ls = languageSettingsById.get("org.eclipse.cdt.core.gcc");
+ assertNotNull(ls);
+ entries = ls.getSettingEntries(ICSettingEntry.MACRO);
+ assertEquals(2, entries.length);
+ assertEquals(expectedEntries12, new HashSet<>(Arrays.asList(entries)));
+
+ ls = languageSettingsById.get("org.eclipse.cdt.core.assembly");
+ assertNotNull(ls);
+ entries = ls.getSettingEntries(ICSettingEntry.MACRO);
+ assertEquals(3, entries.length);
+ assertEquals(expectedEntries123, new HashSet<>(Arrays.asList(entries)));
+ }
+
+ /**
* Test if macros with the same name but different values can coexist when
* they belong to different language ids
*/
diff --git a/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/TestExtSettingsProvider.java b/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/TestExtSettingsProvider.java
index 7fa6afa9d09..a09b539efbd 100644
--- a/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/TestExtSettingsProvider.java
+++ b/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/TestExtSettingsProvider.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2007, 2012 Intel Corporation and others.
+ * Copyright (c) 2007, 2018 Intel Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -59,8 +59,20 @@ public class TestExtSettingsProvider extends CExternalSettingProvider {
new CExternalSetting(new String[] { "org.eclipse.cdt.core.assembly" }, null, null,
new ICSettingEntry[] { new CMacroEntry("THE_MACRO", "TheValue", 0) }) };
+ private static CExternalSetting[] SETTINGS_6 = new CExternalSetting[] {
+ new CExternalSetting(null, null, null, new ICSettingEntry[] { new CMacroEntry("MACRO_1", "Value1", 0) }),
+ new CExternalSetting(new String[] { "org.eclipse.cdt.core.assembly" }, null, null, new ICSettingEntry[] {
+ new CMacroEntry("MACRO_2", "Value2", 0), new CMacroEntry("MACRO_3", "Value3", 0) }) };
+
+ private static CExternalSetting[] SETTINGS_7 = new CExternalSetting[] {
+ new CExternalSetting(null, null, null,
+ new ICSettingEntry[] { new CMacroEntry("MACRO_1", "Value1", 0),
+ new CMacroEntry("MACRO_2", "Value2", 0) }),
+ new CExternalSetting(new String[] { "org.eclipse.cdt.core.assembly" }, null, null,
+ new ICSettingEntry[] { new CMacroEntry("MACRO_3", "Value3", 0) }) };
+
public static final CExternalSetting[][] SETTINGS_VARIANTS = new CExternalSetting[][] { SETTINGS_1, SETTINGS_2,
- SETTINGS_3, SETTINGS_4, SETTINGS_5 };
+ SETTINGS_3, SETTINGS_4, SETTINGS_5, SETTINGS_6, SETTINGS_7 };
private static int variantNum;
diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsDeltaProcessor.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsDeltaProcessor.java
index 62a77b63470..7420d51faa9 100644
--- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsDeltaProcessor.java
+++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettingsDeltaProcessor.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2007, 2010 Intel Corporation and others.
+ * Copyright (c) 2007, 2018 Intel Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -10,6 +10,7 @@
*
* Contributors:
* Intel Corporation - Initial API and implementation
+ * Christian Walther (Indel AG) - [335344] changing language IDs
*******************************************************************************/
package org.eclipse.cdt.internal.core.settings.model;
@@ -147,15 +148,7 @@ public class CExternalSettingsDeltaProcessor {
ICLanguageSetting setting = des.getLanguageSetting();
if (setting == null)
return false;
-
- boolean changed = false;
- for (ExtSettingsDelta delta : deltas) {
- if (isSettingCompatible(setting, delta.fSetting)) {
- if (applyDelta(setting, delta, kindMask))
- changed = true;
- }
- }
- return changed;
+ return applyDelta(setting, deltas, kindMask);
}
static boolean applyDelta(ICFolderDescription des, ExtSettingsDelta deltas[], int kindMask) {
@@ -173,16 +166,24 @@ public class CExternalSettingsDeltaProcessor {
static boolean applyDelta(ICLanguageSetting setting, ExtSettingsDelta[] deltas, int kindMask) {
boolean changed = false;
+ // apply removals before additions in case several deltas apply to the same setting
+ for (ExtSettingsDelta delta : deltas) {
+ if (isSettingCompatible(setting, delta.fSetting)) {
+ if (applyDelta(setting, delta, kindMask, false, true))
+ changed = true;
+ }
+ }
for (ExtSettingsDelta delta : deltas) {
if (isSettingCompatible(setting, delta.fSetting)) {
- if (applyDelta(setting, delta, kindMask))
+ if (applyDelta(setting, delta, kindMask, true, false))
changed = true;
}
}
return changed;
}
- static boolean applyDelta(ICLanguageSetting setting, ExtSettingsDelta delta, int kindMask) {
+ static boolean applyDelta(ICLanguageSetting setting, ExtSettingsDelta delta, int kindMask, boolean additions,
+ boolean removals) {
int kinds[] = KindBasedStore.getLanguageEntryKinds();
ICLanguageSettingEntry entries[];
ICSettingEntry diff[][];
@@ -196,7 +197,8 @@ public class CExternalSettingsDeltaProcessor {
continue;
entries = setting.getSettingEntries(kind);
- List<ICLanguageSettingEntry> list = calculateUpdatedEntries(entries, diff[0], diff[1]);
+ List<ICLanguageSettingEntry> list = calculateUpdatedEntries(entries, additions ? diff[0] : null,
+ removals ? diff[1] : null);
if (list != null) {
setting.setSettingEntries(kind, list);
diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettinsDeltaCalculator.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettinsDeltaCalculator.java
index ef4681a7ec7..9f297793f45 100644
--- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettinsDeltaCalculator.java
+++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettinsDeltaCalculator.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2007, 2011 Intel Corporation and others.
+ * Copyright (c) 2007, 2018 Intel Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -19,7 +19,6 @@ import java.util.Arrays;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -235,7 +234,7 @@ class CExternalSettinsDeltaCalculator {
if (oldSettings == null || oldSettings.length == 0)
return createDeltas(newSettings, true);
- LinkedList<ExtSettingsDelta> deltaList = new LinkedList<>();
+ List<ExtSettingsDelta> deltaList = new ArrayList<>();
Map<ExtSettingMapKey, ICExternalSetting> newMap = toSettingsKeyMap(newSettings);
Map<ExtSettingMapKey, ICExternalSetting> oldMap = toSettingsKeyMap(oldSettings);
@@ -243,20 +242,16 @@ class CExternalSettinsDeltaCalculator {
CExternalSetting newSetting = (CExternalSetting) entry.getValue();
CExternalSetting oldSetting = (CExternalSetting) oldMap.remove(entry.getKey());
if (oldSetting == null) {
- deltaList.addLast(new ExtSettingsDelta(newSetting, true));
+ deltaList.add(new ExtSettingsDelta(newSetting, true));
} else {
ExtSettingsDelta delta = createDelta(newSetting, oldSetting);
if (delta != null)
- deltaList.addLast(delta);
+ deltaList.add(delta);
}
}
for (ICExternalSetting oldSettng : oldMap.values()) {
- // removals must be prepended to the list so that they are applied before additions,
- // otherwise a setting that was just added might be immediately removed again in
- // CExternalSettingsDeltaProcessor.applyDelta(ICLanguageSetting, ExtSettingsDelta[], int)
- // if the old and new setting only differ in their language sets and these overlap
- deltaList.addFirst(new ExtSettingsDelta((CExternalSetting) oldSettng, false));
+ deltaList.add(new ExtSettingsDelta((CExternalSetting) oldSettng, false));
}
if (deltaList.size() == 0)

Back to the top